Message ID | 20190322232205.30420-1-mathieu@centricular.com |
---|---|
State | New |
Headers | show |
Anyone? :) On 3/23/19 12:22 AM, Mathieu Duponchelle wrote: > It is perfectly valid to have multiple CC Picture User Data > for the same frame. Instead of using the first side_data > potentially present with the A53_CC type, iterate over all > side_data. > --- > libavcodec/mpeg12enc.c | 56 +++++++++++++++++++++++------------------- > 1 file changed, 31 insertions(+), 25 deletions(-) > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c > index 2bc5289d63..0162939399 100644 > --- a/libavcodec/mpeg12enc.c > +++ b/libavcodec/mpeg12enc.c > @@ -547,31 +547,37 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) > } > > if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) { > - side_data = av_frame_get_side_data(s->current_picture_ptr->f, > - AV_FRAME_DATA_A53_CC); > - if (side_data) { > - if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { > - int i = 0; > - > - put_header (s, USER_START_CODE); > - > - put_bits(&s->pb, 8, 'G'); // user_identifier > - put_bits(&s->pb, 8, 'A'); > - put_bits(&s->pb, 8, '9'); > - put_bits(&s->pb, 8, '4'); > - put_bits(&s->pb, 8, 3); // user_data_type_code > - put_bits(&s->pb, 8, > - (side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // flags, cc_count > - put_bits(&s->pb, 8, 0xff); // em_data > - > - for (i = 0; i < side_data->size; i++) > - put_bits(&s->pb, 8, side_data->data[i]); > - > - put_bits(&s->pb, 8, 0xff); // marker_bits > - } else { > - av_log(s->avctx, AV_LOG_WARNING, > - "Warning Closed Caption size (%d) can not exceed 93 bytes " > - "and must be a multiple of 3\n", side_data->size); > + int i; > + > + for (i = 0; i < s->current_picture_ptr->f->nb_side_data; i++) { > + side_data = s->current_picture_ptr->f->side_data[i]; > + if (side_data->type != AV_FRAME_DATA_A53_CC) > + continue; > + > + if (side_data) { > + if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { > + int i = 0; > + > + put_header (s, USER_START_CODE); > + > + put_bits(&s->pb, 8, 'G'); // user_identifier > + put_bits(&s->pb, 8, 'A'); > + put_bits(&s->pb, 8, '9'); > + put_bits(&s->pb, 8, '4'); > + put_bits(&s->pb, 8, 3); // user_data_type_code > + put_bits(&s->pb, 8, > + (side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // flags, cc_count > + put_bits(&s->pb, 8, 0xff); // em_data > + > + for (i = 0; i < side_data->size; i++) > + put_bits(&s->pb, 8, side_data->data[i]); > + > + put_bits(&s->pb, 8, 0xff); // marker_bits > + } else { > + av_log(s->avctx, AV_LOG_WARNING, > + "Warning Closed Caption size (%d) can not exceed 93 bytes " > + "and must be a multiple of 3\n", side_data->size); > + } > } > } > }
2019-03-23 0:22 GMT+01:00, Mathieu Duponchelle <mathieu@centricular.com>: > It is perfectly valid to have multiple CC Picture User Data > for the same frame. Instead of using the first side_data > potentially present with the A53_CC type, iterate over all > side_data. Please separate the functional changes in your patch from the cosmetics, ie do not re-indent in the same patch to make reviews easier. Sorry for the late reply, Carl Eugen
No problem, note that the added indentation is because a loop was added :) On 4/10/19 1:14 AM, Carl Eugen Hoyos wrote: > 2019-03-23 0:22 GMT+01:00, Mathieu Duponchelle <mathieu@centricular.com>: >> It is perfectly valid to have multiple CC Picture User Data >> for the same frame. Instead of using the first side_data >> potentially present with the A53_CC type, iterate over all >> side_data. > Please separate the functional changes in your patch from > the cosmetics, ie do not re-indent in the same patch to make > reviews easier. > > Sorry for the late reply, Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hello ? :) On 4/10/19 1:26 PM, Mathieu Duponchelle wrote: > No problem, note that the added indentation is because a loop > was added :) > > On 4/10/19 1:14 AM, Carl Eugen Hoyos wrote: >> 2019-03-23 0:22 GMT+01:00, Mathieu Duponchelle <mathieu@centricular.com>: >>> It is perfectly valid to have multiple CC Picture User Data >>> for the same frame. Instead of using the first side_data >>> potentially present with the A53_CC type, iterate over all >>> side_data. >> Please separate the functional changes in your patch from >> the cosmetics, ie do not re-indent in the same patch to make >> reviews easier. >> >> Sorry for the late reply, Carl Eugen >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Ping! On 4/19/19 2:46 PM, Mathieu Duponchelle wrote: > Hello ? :) > > On 4/10/19 1:26 PM, Mathieu Duponchelle wrote: >> No problem, note that the added indentation is because a loop >> was added :) >> >> On 4/10/19 1:14 AM, Carl Eugen Hoyos wrote: >>> 2019-03-23 0:22 GMT+01:00, Mathieu Duponchelle <mathieu@centricular.com>: >>>> It is perfectly valid to have multiple CC Picture User Data >>>> for the same frame. Instead of using the first side_data >>>> potentially present with the A53_CC type, iterate over all >>>> side_data. >>> Please separate the functional changes in your patch from >>> the cosmetics, ie do not re-indent in the same patch to make >>> reviews easier. >>> >>> Sorry for the late reply, Carl Eugen >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Am Mi., 10. Apr. 2019 um 13:26 Uhr schrieb Mathieu Duponchelle
<mathieu@centricular.com>:
> No problem
I don't see an updated patch.
Carl Eugen
On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: > There isn't one, as I said the added indentation is because of the new loop! To get this committed to tree you need to comply to review requests. > > On 5/13/19 3:39 PM, Carl Eugen Hoyos wrote: >> Am Mi., 10. Apr. 2019 um 13:26 Uhr schrieb Mathieu Duponchelle >> <mathieu@centricular.com>: >> >>> No problem >> I don't see an updated patch. >> >> Carl Eugen >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hello Paul, On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <onemda@gmail.com> wrote: > > On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: > > There isn't one, as I said the added indentation is because of the new loop! > > To get this committed to tree you need to comply to review requests. I think Mathieu's point is that the code indentation change was not cosmetic - it's because the code in question is now inside a for loop, and thus it needed to be indented another level. Are you suggesting he should make a patch which results in the indentation being wrong, and then submit a second patch which fixes the incorrect indentation introduced by the first patch? I'm confident that Mathieu is trying to comply with review requests so he can get his code merged. In this particular case, Carl raised his concern about the indentation, Mathieu responded by suggesting that given there was a functional change the re-indent was correct, and then there was silence (i.e. neither agreement nor disagreement). I'm also asking because I have work which I'm hoping to get upstreamed as well at some point, and I'm sure I've got similar things in my patches. And having spent 20+ years reviewing patches on lots of other open source projects, I wouldn't have thought twice about accepting the patch as-is (given the change in indentation is a result of a functional change and is not cosmetic). In my experience, this particular patch isn't an example of what maintainers mean when they say "don't mix cosmetic whitespace changes with functional changes". Regards, Devin
Hello, Have you read the patch? The request is to split out cosmetic indentation to a separate commit, however the added indentation is not cosmetic, but simply due to a new inner loop .. Best, Mathieu On 5/17/19 10:44 AM, Paul B Mahol wrote: > On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: >> There isn't one, as I said the added indentation is because of the new loop! > To get this committed to tree you need to comply to review requests. > >> On 5/13/19 3:39 PM, Carl Eugen Hoyos wrote: >>> Am Mi., 10. Apr. 2019 um 13:26 Uhr schrieb Mathieu Duponchelle >>> <mathieu@centricular.com>: >>> >>>> No problem >>> I don't see an updated patch. >>> >>> Carl Eugen >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> To unsubscribe, visit link above, or email >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Thanks :) On 5/19/19 7:00 PM, Devin Heitmueller wrote: > Hello Paul, > > On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <onemda@gmail.com> wrote: >> On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: >>> There isn't one, as I said the added indentation is because of the new loop! >> To get this committed to tree you need to comply to review requests. > I think Mathieu's point is that the code indentation change was not > cosmetic - it's because the code in question is now inside a for loop, > and thus it needed to be indented another level. > > Are you suggesting he should make a patch which results in the > indentation being wrong, and then submit a second patch which fixes > the incorrect indentation introduced by the first patch? > > I'm confident that Mathieu is trying to comply with review requests so > he can get his code merged. In this particular case, Carl raised his > concern about the indentation, Mathieu responded by suggesting that > given there was a functional change the re-indent was correct, and > then there was silence (i.e. neither agreement nor disagreement). > > I'm also asking because I have work which I'm hoping to get upstreamed > as well at some point, and I'm sure I've got similar things in my > patches. And having spent 20+ years reviewing patches on lots of > other open source projects, I wouldn't have thought twice about > accepting the patch as-is (given the change in indentation is a result > of a functional change and is not cosmetic). In my experience, this > particular patch isn't an example of what maintainers mean when they > say "don't mix cosmetic whitespace changes with functional changes". > > Regards, > > Devin >
On 20.05.2019, at 09:23, Mathieu Duponchelle <mathieu@centricular.com> wrote: > Thanks :) > > On 5/19/19 7:00 PM, Devin Heitmueller wrote: >> Hello Paul, >> >> On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <onemda@gmail.com> wrote: >>> On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: >>>> There isn't one, as I said the added indentation is because of the new loop! >>> To get this committed to tree you need to comply to review requests. >> I think Mathieu's point is that the code indentation change was not >> cosmetic - it's because the code in question is now inside a for loop, >> and thus it needed to be indented another level. >> >> Are you suggesting he should make a patch which results in the >> indentation being wrong, and then submit a second patch which fixes >> the incorrect indentation introduced by the first patch? I think it should probably be up to the maintainer, but possibly yes. A lot of review still happens primarily by email, and if you re-indent code it becomes impossible to see what, if anything, you changed in that block. Enabling reviews of the patch via email means not re-indenting even if it becomes "wrong". Not everyone does reviews that way though, so some maintainers nowadays might prefer it differently?
On Mon, May 20, 2019 at 9:44 AM Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote: > > > > On 20.05.2019, at 09:23, Mathieu Duponchelle <mathieu@centricular.com> wrote: > > > Thanks :) > > > > On 5/19/19 7:00 PM, Devin Heitmueller wrote: > >> Hello Paul, > >> > >> On Fri, May 17, 2019 at 4:44 AM Paul B Mahol <onemda@gmail.com> wrote: > >>> On 5/17/19, Mathieu Duponchelle <mathieu@centricular.com> wrote: > >>>> There isn't one, as I said the added indentation is because of the new loop! > >>> To get this committed to tree you need to comply to review requests. > >> I think Mathieu's point is that the code indentation change was not > >> cosmetic - it's because the code in question is now inside a for loop, > >> and thus it needed to be indented another level. > >> > >> Are you suggesting he should make a patch which results in the > >> indentation being wrong, and then submit a second patch which fixes > >> the incorrect indentation introduced by the first patch? > > I think it should probably be up to the maintainer, but possibly yes. > A lot of review still happens primarily by email, and if you re-indent code > it becomes impossible to see what, if anything, you changed in that block. > Enabling reviews of the patch via email means not re-indenting even if > it becomes "wrong". > Not everyone does reviews that way though, so some maintainers nowadays might prefer it differently? My main concern with such requests is that its added effort to even make these patches, since many developers will just automatically indent where appropriate (as it should be), or even have tooling to do it for them. Personally, I would have to use another editor to "un-indent" the code, since my main editor automatically corrects this stuff for me in newly written code. As such, I'll personally never comply with such requests, because I think its silly, and it would be extraordinary effort to even do it, first restoring original indent with a second editor. In so many cases the "cosmetic" second commit was also quite simply forgotten, resulting in an ugly codebase. Nevermind the added noise in the commit log. I don't think its that much to ask for a reviewer to also use a tool that can ignore whitespace. Maybe we can teach patchwork to offer a button to view diffs without whitespace, if people don't want to use a simple tool. - Hendrik
On Mon, May 20, 2019 at 10:05:49AM +0200, Hendrik Leppkes wrote: > On Mon, May 20, 2019 at 9:44 AM Reimar Döffinger > <Reimar.Doeffinger@gmx.de> wrote: > > On 20.05.2019, at 09:23, Mathieu Duponchelle <mathieu@centricular.com> wrote: > > > On 5/19/19 7:00 PM, Devin Heitmueller wrote: > > >> Are you suggesting he should make a patch which results in the > > >> indentation being wrong, and then submit a second patch which fixes > > >> the incorrect indentation introduced by the first patch? > > > > I think it should probably be up to the maintainer, but possibly yes. > > A lot of review still happens primarily by email, and if you re-indent code > > it becomes impossible to see what, if anything, you changed in that block. > > Enabling reviews of the patch via email means not re-indenting even if > > it becomes "wrong". > > Not everyone does reviews that way though, so some maintainers nowadays might prefer it differently? > > My main concern with such requests is that its added effort to even > make these patches, since many developers will just automatically > indent where appropriate (as it should be), or even have tooling to do > it for them. > Personally, I would have to use another editor to "un-indent" the > code, since my main editor automatically corrects this stuff for me in > newly written code. As such, I'll personally never comply with such > requests, because I think its silly, and it would be extraordinary > effort to even do it, first restoring original indent with a second > editor. There's many ways to do it, but the reviewability issue is already solved by e.g. sending the patch as generated by diff -w (have not tried, but git send-email seems to accept that option), so I think you are overstating the effort a bit :) There is also the point that this should not be an issue normally: - It doesn't matter if it's just a small block of code. - If it's a large block of code, many times it means the code should have been refactored long ago and put into a separate function. So there often is the option of "volunteering" for some code cleanup first :) > I don't think its that much to ask for a reviewer to also use a tool > that can ignore whitespace. Maybe we can teach patchwork to offer a > button to view diffs without whitespace, if people don't want to use a > simple tool. FFmpeg used to have far too few reviewers. If that is still the case, I think it's reasonable to put reviewer convenience over submitter convenience. I don't see how patchwork is really useful: finding the patch there is already a pain, and we don't use it for reviews AT ALL, we use email. Now there might be an argument if we should have alternative/additional review tools to email, but it's a bit of a separate question. Personally, my suggestion would be that if you re-indented a lot and unless you know the most likely potential reviewers do not care, send a patch without whitespace changes at least in addition. I mean, I don't have a super-strong opinion, but if I have to dig through whitespace changes I'll probably just not review a patch, because far too often all I have at hand to do them is an email client... Best regards, Reimar Döffinger
As suggested elsewhere, here's the output of git diff -w, I hope that helps :) diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c index 2bc5289d63..0162939399 100644 --- a/libavcodec/mpeg12enc.c +++ b/libavcodec/mpeg12enc.c @@ -547,8 +547,13 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) } if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) { - side_data = av_frame_get_side_data(s->current_picture_ptr->f, - AV_FRAME_DATA_A53_CC); + int i; + + for (i = 0; i < s->current_picture_ptr->f->nb_side_data; i++) { + side_data = s->current_picture_ptr->f->side_data[i]; + if (side_data->type != AV_FRAME_DATA_A53_CC) + continue; + if (side_data) { if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { int i = 0; @@ -575,6 +580,7 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) } } } + } s->mb_y = 0; ff_mpeg1_encode_slice_header(s); On 5/13/19 3:39 PM, Carl Eugen Hoyos wrote: > Am Mi., 10. Apr. 2019 um 13:26 Uhr schrieb Mathieu Duponchelle > <mathieu@centricular.com>: > >> No problem > I don't see an updated patch. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Sorry for pinging again, but at this point I don't know what more I can do to get this (necessary) fix upstream :) On 5/29/19 3:44 PM, Mathieu Duponchelle wrote: > As suggested elsewhere, here's the output of git diff -w, I hope that helps :) > > diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c > > index 2bc5289d63..0162939399 100644 > > --- a/libavcodec/mpeg12enc.c > > +++ b/libavcodec/mpeg12enc.c > > @@ -547,8 +547,13 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) > > } > > > > if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) { > > - side_data = av_frame_get_side_data(s->current_picture_ptr->f, > > - AV_FRAME_DATA_A53_CC); > > + int i; > > + > > + for (i = 0; i < s->current_picture_ptr->f->nb_side_data; i++) { > > + side_data = s->current_picture_ptr->f->side_data[i]; > > + if (side_data->type != AV_FRAME_DATA_A53_CC) > > + continue; > > + > > if (side_data) { > > if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { > > int i = 0; > > @@ -575,6 +580,7 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) > > } > > } > > } > > + } > > > > s->mb_y = 0; > > ff_mpeg1_encode_slice_header(s); > > > On 5/13/19 3:39 PM, Carl Eugen Hoyos wrote: >> Am Mi., 10. Apr. 2019 um 13:26 Uhr schrieb Mathieu Duponchelle >> <mathieu@centricular.com>: >> >>> No problem >> I don't see an updated patch. >> >> Carl Eugen >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/mpeg12enc.c b/libavcodec/mpeg12enc.c index 2bc5289d63..0162939399 100644 --- a/libavcodec/mpeg12enc.c +++ b/libavcodec/mpeg12enc.c @@ -547,31 +547,37 @@ void ff_mpeg1_encode_picture_header(MpegEncContext *s, int picture_number) } if (s->codec_id == AV_CODEC_ID_MPEG2VIDEO && s->a53_cc) { - side_data = av_frame_get_side_data(s->current_picture_ptr->f, - AV_FRAME_DATA_A53_CC); - if (side_data) { - if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { - int i = 0; - - put_header (s, USER_START_CODE); - - put_bits(&s->pb, 8, 'G'); // user_identifier - put_bits(&s->pb, 8, 'A'); - put_bits(&s->pb, 8, '9'); - put_bits(&s->pb, 8, '4'); - put_bits(&s->pb, 8, 3); // user_data_type_code - put_bits(&s->pb, 8, - (side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // flags, cc_count - put_bits(&s->pb, 8, 0xff); // em_data - - for (i = 0; i < side_data->size; i++) - put_bits(&s->pb, 8, side_data->data[i]); - - put_bits(&s->pb, 8, 0xff); // marker_bits - } else { - av_log(s->avctx, AV_LOG_WARNING, - "Warning Closed Caption size (%d) can not exceed 93 bytes " - "and must be a multiple of 3\n", side_data->size); + int i; + + for (i = 0; i < s->current_picture_ptr->f->nb_side_data; i++) { + side_data = s->current_picture_ptr->f->side_data[i]; + if (side_data->type != AV_FRAME_DATA_A53_CC) + continue; + + if (side_data) { + if (side_data->size <= A53_MAX_CC_COUNT * 3 && side_data->size % 3 == 0) { + int i = 0; + + put_header (s, USER_START_CODE); + + put_bits(&s->pb, 8, 'G'); // user_identifier + put_bits(&s->pb, 8, 'A'); + put_bits(&s->pb, 8, '9'); + put_bits(&s->pb, 8, '4'); + put_bits(&s->pb, 8, 3); // user_data_type_code + put_bits(&s->pb, 8, + (side_data->size / 3 & A53_MAX_CC_COUNT) | 0x40); // flags, cc_count + put_bits(&s->pb, 8, 0xff); // em_data + + for (i = 0; i < side_data->size; i++) + put_bits(&s->pb, 8, side_data->data[i]); + + put_bits(&s->pb, 8, 0xff); // marker_bits + } else { + av_log(s->avctx, AV_LOG_WARNING, + "Warning Closed Caption size (%d) can not exceed 93 bytes " + "and must be a multiple of 3\n", side_data->size); + } } } }