| Message ID | 20180306222512.20124-1-ffmpeg@tmm1.net |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote: > From: Aman Gupta <aman@tmm1.net> > > Some streams include both a53 and scte20 data. Before this commit, > the scte20 data would be used instead of the a53 data. > > See https://s3.amazonaws.com/tmm1/ccaptions/scte20plusa53.ts, > which produced garbage captions since 3f1a540204a8c187f77b3805d. > --- > libavcodec/mpeg12dec.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) why does the combination produce garbage ? why should not both be exported or it be user selectable? also turning one off for ever seems problematic for concatenated sequences. not every sequence would need to contain both i guess [...]
> On Mar 7, 2018, at 12:49 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote: >> From: Aman Gupta <aman@tmm1.net> >> >> Some streams include both a53 and scte20 data. Before this commit, >> the scte20 data would be used instead of the a53 data. >> >> See https://s3.amazonaws.com/tmm1/ccaptions/scte20plusa53.ts, >> which produced garbage captions since 3f1a540204a8c187f77b3805d. >> --- >> libavcodec/mpeg12dec.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) > > why does the combination produce garbage ? > why should not both be exported or it be user selectable? I suspect part of the issue is that the SCTE-20 data gets jammed into the a53_caption member. It may make sense to have it put into it’s own field, so that if both are present the encoder can decide which to insert into the final stream. But having both of them putting data into the same field is likely to produce indeterministic output. I’ve got basically the same issue in the decklink input where both EIA-608 and EIA-708 VANC can arrive in the same frame, and will need to either implement a similar change (i.e. prefer 708 if both are present), or be prepared to add a new side_data type for 608 so both can be sent and the downstream can choose which to include in the output. Devin > also turning one off for ever seems problematic for concatenated > sequences. not every sequence would need to contain both I guess It would certainly be good if it were configurable. I am certain there are users who will say “I know the X format is broken and I want to always send through the Y format", and that may not correspond to the heuristic implemented in the decoder (in this case, to always prefer 708 data over SCTE-20). Devin
On Wed, Mar 7, 2018 at 9:49 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote: > > From: Aman Gupta <aman@tmm1.net> > > > > Some streams include both a53 and scte20 data. Before this commit, > > the scte20 data would be used instead of the a53 data. > > > > See https://s3.amazonaws.com/tmm1/ccaptions/scte20plusa53.ts, > > which produced garbage captions since 3f1a540204a8c187f77b3805d. > > --- > > libavcodec/mpeg12dec.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > why does the combination produce garbage ? In my sample, the scte20 data by itself produces garbage captions. I'm not sure why. > why should not both be exported or it be user selectable? From what I've seen in US broadcast television, scte20 is only used on standard-def content and everything else uses normal a53. I'm not sure how we would export both since there's only one type of side data. > > also turning one off for ever seems problematic for concatenated > sequences. not every sequence would need to contain both i guess Yea that's theoreticaly possible, but I'd rather wait to add support until someone actually sees it in the wild. Before I added scte20 support a few months ago no one even noticed it was missing. It doesn't seem to have wide spread use. Aman > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The greatest way to live with honor in this world is to be what we pretend > to be. -- Socrates > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
> From what I've seen in US broadcast television, scte20 is only used on > standard-def content and everything else uses normal a53. A53 is definitely the more popular standard, and all that is approved for distribution in digital over the air broadcasts. SCTE-20 would only be found further up the distribution chain, and perhaps in distribution to some cable boxes (although it’s becoming less and less common that it can be decoded since most of the content is encrypted nowadays). > > I'm not sure how we would export both since there's only one type of side > data. We would have to add a new side data type, and encoders would have to be changed to look for both. >> >> also turning one off for ever seems problematic for concatenated >> sequences. not every sequence would need to contain both I guess Funny enough, I spent my entire morning debugging some issues with playing concatenated TS streams. If anyone thinks that’s supposed to be working today in ffmpeg, there’s a ton of work to be done in that area. > > > Yea that's theoreticaly possible, but I'd rather wait to add support until > someone actually sees it in the wild. > > Before I added scte20 support a few months ago no one even noticed it was > missing. It doesn't seem to have wide spread use. It’s not really that nobody noticed - it’s that most people in the broadcast space until recently had ruled out the ability to use ffmpeg for production because of it’s lack of good support for ancillary data such as captions. That situation is improving of course, but it’s not so much that “no one even noticed it was missing”. If changing the framework to support the extra side data format isn’t viable, then certainly prefering A53 over SCTE-20 would be the right way to go. I would make it configurable though. Devin
On Wed, Mar 7, 2018 at 10:35 AM Devin Heitmueller < dheitmueller@ltnglobal.com> wrote: > > > From what I've seen in US broadcast television, scte20 is only used on > > standard-def content and everything else uses normal a53. > > A53 is definitely the more popular standard, and all that is approved for > distribution in digital over the air broadcasts. SCTE-20 would only be > found further up the distribution chain, and perhaps in distribution to > some cable boxes (although it’s becoming less and less common that it can > be decoded since most of the content is encrypted nowadays). > > > > > I'm not sure how we would export both since there's only one type of side > > data. > > We would have to add a new side data type, and encoders would have to be > changed to look for both. > > >> > >> also turning one off for ever seems problematic for concatenated > >> sequences. not every sequence would need to contain both I guess > > Funny enough, I spent my entire morning debugging some issues with playing > concatenated TS streams. If anyone thinks that’s supposed to be working > today in ffmpeg, there’s a ton of work to be done in that area. > > > > > > > Yea that's theoreticaly possible, but I'd rather wait to add support > until > > someone actually sees it in the wild. > > > > Before I added scte20 support a few months ago no one even noticed it was > > missing. It doesn't seem to have wide spread use. > > It’s not really that nobody noticed - it’s that most people in the > broadcast space until recently had ruled out the ability to use ffmpeg for > production because of it’s lack of good support for ancillary data such as > captions. That situation is improving of course, but it’s not so much that > “no one even noticed it was missing”. > > If changing the framework to support the extra side data format isn’t > viable, then certainly prefering A53 over SCTE-20 would be the right way to > go. I would make it configurable though. Thanks for the background on SCTE-20. I don't really know much about it. I'm not opposed to adding new side data, but it doesn't sound like it's worth it in this particular case. Atleast to me; if someone else wants to pursue that approach I will happily help review and test any patches. > To make my patch configurable, I could change the ignore flag I added into a new option called parse_scte20: default to "prefer a53" like I have now, but can be set explicitly to "always" or "never". Wdyt? Aman > > Devin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Thu, Mar 08, 2018 at 07:13:57AM +0000, Aman Gupta wrote: > On Wed, Mar 7, 2018 at 10:35 AM Devin Heitmueller < > dheitmueller at ltnglobal.com> wrote: > > > > > > From what I've seen in US broadcast television, scte20 is only used on > > > standard-def content and everything else uses normal a53. > > > > A53 is definitely the more popular standard, and all that is approved for > > distribution in digital over the air broadcasts. SCTE-20 would only be > > found further up the distribution chain, and perhaps in distribution to > > some cable boxes (although it’s becoming less and less common that it can > > be decoded since most of the content is encrypted nowadays). > > > > > > > > > > I'm not sure how we would export both since there's only one type of side > > > data. > > > > We would have to add a new side data type, and encoders would have to be > > changed to look for both. > > > > > > >> > > >> also turning one off for ever seems problematic for concatenated > > >> sequences. not every sequence would need to contain both I guess > > > > Funny enough, I spent my entire morning debugging some issues with playing > > concatenated TS streams. If anyone thinks that’s supposed to be working > > today in ffmpeg, there’s a ton of work to be done in that area. > > > > > > > > > > > > > Yea that's theoreticaly possible, but I'd rather wait to add support > > until > > > someone actually sees it in the wild. > > > > > > Before I added scte20 support a few months ago no one even noticed it was > > > missing. It doesn't seem to have wide spread use. > > > > It’s not really that nobody noticed - it’s that most people in the > > broadcast space until recently had ruled out the ability to use ffmpeg for > > production because of it’s lack of good support for ancillary data such as > > captions. That situation is improving of course, but it’s not so much that > > “no one even noticed it was missing”. > > > > > > If changing the framework to support the extra side data format isn’t > > viable, then certainly prefering A53 over SCTE-20 would be the right way to > > go. I would make it configurable though. > > > Thanks for the background on SCTE-20. I don't really know much about it. > > I'm not opposed to adding new side data, but it doesn't sound like it's > worth it in this particular case. Atleast to me; if someone else wants to > pursue that approach I will happily help review and test any patches. > > > > To make my patch configurable, I could change the ignore flag I added into > a new option called parse_scte20: default to "prefer a53" like I have now, > but can be set explicitly to "always" or "never". Wdyt? I tried to use ffmpeg to transcode some TV recordings I have where I need to keep the closed captions. When I finally got the ffmpeg options right to copy the captions, the captions were a garbled, unrecognizable mess. I eventually worked around the problem with ffmpeg be using ccextractor to first extract the captions separately. I then muxed them back in later with the transcoded video and audio using ffmpeg. With ccextractor, I initially got garbled captions too until I used the --noscte20 option. After that, I got perfectly clean captions. When I looked to see if ffmpeg had a comparable switch, I found this thread. I build ffmpeg myself with this patch and sure enough I got perfectly clean captions too without having to use ccextractor. I see this patch hasn't been included in ffmpeg yet. Please reconsider including it in some form as it is definitely needed out here in the real world. Please note that I am not subscribed to the ffmpeg-devel list. Please copy me on any replies that you need me to see. David
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index 9e076e89da..45d0a9d737 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -59,6 +59,7 @@ typedef struct Mpeg1Context { int has_stereo3d; uint8_t *a53_caption; int a53_caption_size; + int ignore_scte20; uint8_t afd; int has_afd; int slice_count; @@ -2241,6 +2242,7 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx, /* extract A53 Part 4 CC data */ int cc_count = p[5] & 0x1f; if (cc_count > 0 && buf_size >= 7 + cc_count * 3) { + s1->ignore_scte20 = 1; av_freep(&s1->a53_caption); s1->a53_caption_size = cc_count * 3; s1->a53_caption = av_malloc(s1->a53_caption_size); @@ -2253,7 +2255,8 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx, } return 1; } else if (buf_size >= 2 && - p[0] == 0x03 && (p[1]&0x7f) == 0x01) { + p[0] == 0x03 && (p[1]&0x7f) == 0x01 && + !s1->ignore_scte20) { /* extract SCTE-20 CC data */ GetBitContext gb; int cc_count = 0;
From: Aman Gupta <aman@tmm1.net> Some streams include both a53 and scte20 data. Before this commit, the scte20 data would be used instead of the a53 data. See https://s3.amazonaws.com/tmm1/ccaptions/scte20plusa53.ts, which produced garbage captions since 3f1a540204a8c187f77b3805d. --- libavcodec/mpeg12dec.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)