Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2019-01-05 Thread David Engel
On Thu, Mar 08, 2018 at 07:13:57AM +, 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
-- 
David Engel
da...@istwok.net
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-07 Thread Aman Gupta
On Wed, Mar 7, 2018 at 10:35 AM Devin Heitmueller <
dheitmuel...@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
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-07 Thread Devin Heitmueller

> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-07 Thread Aman Gupta
On Wed, Mar 7, 2018 at 9:49 AM Michael Niedermayer 
wrote:

> On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote:
> > From: Aman Gupta 
> >
> > 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
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-07 Thread Devin Heitmueller

> On Mar 7, 2018, at 12:49 PM, Michael Niedermayer  
> wrote:
> 
> On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote:
>> From: Aman Gupta 
>> 
>> 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


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-07 Thread Michael Niedermayer
On Tue, Mar 06, 2018 at 02:25:12PM -0800, Aman Gupta wrote:
> From: Aman Gupta 
> 
> 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

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avcodec/mpeg12dec: ignore scte20 captions when a53 data is also present

2018-03-06 Thread Aman Gupta
From: Aman Gupta 

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(-)

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;
-- 
2.14.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel