diff mbox

[FFmpeg-devel,v2,1/2] avcodec/mpeg12dec: parse A53 caption data embedded in SCTE-20 user data

Message ID 20170315195304.94885-1-ffmpeg@tmm1.net
State Superseded
Headers show

Commit Message

Aman Karmani March 15, 2017, 7:53 p.m. UTC 
From: Aman Gupta <aman@tmm1.net>

includes a fate test, which requires
https://s3.amazonaws.com/tmm1/ccaptions/scte20.ts
to be uploaded as sub/scte20.ts
---
 libavcodec/mpeg12dec.c       | 39 +++++++++++++++++++++++++++++++++++++++
 tests/fate/subtitles.mak     |  3 +++
 tests/ref/fate/sub-cc-scte20 | 15 +++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 tests/ref/fate/sub-cc-scte20

Comments

Michael Niedermayer March 15, 2017, 9:06 p.m. UTC | #1 
On Wed, Mar 15, 2017 at 12:53:03PM -0700, Aman Gupta wrote:
> From: Aman Gupta <aman@tmm1.net>
> 
> includes a fate test, which requires
> https://s3.amazonaws.com/tmm1/ccaptions/scte20.ts
> to be uploaded as sub/scte20.ts

uploaded



> ---
>  libavcodec/mpeg12dec.c       | 39 +++++++++++++++++++++++++++++++++++++++
>  tests/fate/subtitles.mak     |  3 +++
>  tests/ref/fate/sub-cc-scte20 | 15 +++++++++++++++
>  3 files changed, 57 insertions(+)
>  create mode 100644 tests/ref/fate/sub-cc-scte20
> 
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index e49167f..cea8963 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -2260,6 +2260,45 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx,
>              avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>          }
>          return 1;
> +    } else if (buf_size >= 2 &&
> +               p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
> +        /* extract SCTE-20 CC data */
> +        GetBitContext gb;
> +        int cc_count = 0;
> +        int i;
> +
> +        init_get_bits(&gb, p + 2, buf_size - 2);
> +        cc_count = get_bits(&gb, 5);
> +        if (cc_count > 0) {

remainig bits or buf_size should be checked to be large enough for
cc_count

[...]
Aman Karmani March 15, 2017, 10:32 p.m. UTC | #2 
On Wed, Mar 15, 2017 at 2:06 PM, Michael Niedermayer <michael@niedermayer.cc
> wrote:

> On Wed, Mar 15, 2017 at 12:53:03PM -0700, Aman Gupta wrote:
> > From: Aman Gupta <aman@tmm1.net>
> >
> > includes a fate test, which requires
> > https://s3.amazonaws.com/tmm1/ccaptions/scte20.ts
> > to be uploaded as sub/scte20.ts
>
> uploaded
>
>
>
> > ---
> >  libavcodec/mpeg12dec.c       | 39 ++++++++++++++++++++++++++++++
> +++++++++
> >  tests/fate/subtitles.mak     |  3 +++
> >  tests/ref/fate/sub-cc-scte20 | 15 +++++++++++++++
> >  3 files changed, 57 insertions(+)
> >  create mode 100644 tests/ref/fate/sub-cc-scte20
> >
> > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > index e49167f..cea8963 100644
> > --- a/libavcodec/mpeg12dec.c
> > +++ b/libavcodec/mpeg12dec.c
> > @@ -2260,6 +2260,45 @@ static int mpeg_decode_a53_cc(AVCodecContext
> *avctx,
> >              avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> >          }
> >          return 1;
> > +    } else if (buf_size >= 2 &&
> > +               p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
> > +        /* extract SCTE-20 CC data */
> > +        GetBitContext gb;
> > +        int cc_count = 0;
> > +        int i;
> > +
> > +        init_get_bits(&gb, p + 2, buf_size - 2);
> > +        cc_count = get_bits(&gb, 5);
> > +        if (cc_count > 0) {
>
> remainig bits or buf_size should be checked to be large enough for
> cc_count
>

Good catch. Changing to buf_size>=3 in the outer conditional should be
sufficient, right?


>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Complexity theory is the science of finding the exact solution to an
> approximation. Benchmarking OTOH is finding an approximation of the exact
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
Michael Niedermayer March 16, 2017, 3:31 p.m. UTC | #3 
On Wed, Mar 15, 2017 at 03:32:22PM -0700, Aman Gupta wrote:
> On Wed, Mar 15, 2017 at 2:06 PM, Michael Niedermayer <michael@niedermayer.cc
> > wrote:
> 
> > On Wed, Mar 15, 2017 at 12:53:03PM -0700, Aman Gupta wrote:
> > > From: Aman Gupta <aman@tmm1.net>
> > >
> > > includes a fate test, which requires
> > > https://s3.amazonaws.com/tmm1/ccaptions/scte20.ts
> > > to be uploaded as sub/scte20.ts
> >
> > uploaded
> >
> >
> >
> > > ---
> > >  libavcodec/mpeg12dec.c       | 39 ++++++++++++++++++++++++++++++
> > +++++++++
> > >  tests/fate/subtitles.mak     |  3 +++
> > >  tests/ref/fate/sub-cc-scte20 | 15 +++++++++++++++
> > >  3 files changed, 57 insertions(+)
> > >  create mode 100644 tests/ref/fate/sub-cc-scte20
> > >
> > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > > index e49167f..cea8963 100644
> > > --- a/libavcodec/mpeg12dec.c
> > > +++ b/libavcodec/mpeg12dec.c
> > > @@ -2260,6 +2260,45 @@ static int mpeg_decode_a53_cc(AVCodecContext
> > *avctx,
> > >              avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> > >          }
> > >          return 1;
> > > +    } else if (buf_size >= 2 &&
> > > +               p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
> > > +        /* extract SCTE-20 CC data */
> > > +        GetBitContext gb;
> > > +        int cc_count = 0;
> > > +        int i;
> > > +
> > > +        init_get_bits(&gb, p + 2, buf_size - 2);
> > > +        cc_count = get_bits(&gb, 5);
> > > +        if (cc_count > 0) {
> >
> > remainig bits or buf_size should be checked to be large enough for
> > cc_count
> >
> 
> Good catch. Changing to buf_size>=3 in the outer conditional should be
> sufficient, right?

i think its not
the bitstream is initialized with buf_size - 2 so a buf_size  of 3
doesnt seem enough for al that can be read
also see get_bits_left()

[...]
diff mbox

Patch

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index e49167f..cea8963 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2260,6 +2260,45 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
         }
         return 1;
+    } else if (buf_size >= 2 &&
+               p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
+        /* extract SCTE-20 CC data */
+        GetBitContext gb;
+        int cc_count = 0;
+        int i;
+
+        init_get_bits(&gb, p + 2, buf_size - 2);
+        cc_count = get_bits(&gb, 5);
+        if (cc_count > 0) {
+            av_freep(&s1->a53_caption);
+            s1->a53_caption_size = cc_count * 3;
+            s1->a53_caption      = av_malloc(s1->a53_caption_size);
+            if (s1->a53_caption) {
+                uint8_t field, cc1, cc2;
+                uint8_t *cap = s1->a53_caption;
+                for (i = 0; i < cc_count; i++) {
+                    skip_bits(&gb, 2); // priority
+                    field = get_bits(&gb, 2);
+                    skip_bits(&gb, 5); // line_offset
+                    cc1 = get_bits(&gb, 8);
+                    cc2 = get_bits(&gb, 8);
+                    skip_bits(&gb, 1); // marker
+
+                    if (!field) { // forbidden
+                        cap[0] = cap[1] = cap[2] = 0x00;
+                    } else {
+                        field = (field == 2 ? 1 : 0);
+                        if (!s1->mpeg_enc_ctx.top_field_first) field = !field;
+                        cap[0] = 0x04 | field;
+                        cap[1] = ff_reverse[cc1];
+                        cap[2] = ff_reverse[cc2];
+                    }
+                    cap += 3;
+                }
+            }
+            avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
+        }
+        return 1;
     } else if (buf_size >= 11 &&
                p[0] == 'C' && p[1] == 'C' && p[2] == 0x01 && p[3] == 0xf8) {
         /* extract DVD CC data
diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
index fc2b192..0963479 100644
--- a/tests/fate/subtitles.mak
+++ b/tests/fate/subtitles.mak
@@ -7,6 +7,9 @@  fate-sub-cc: CMD = fmtstdout ass -f lavfi -i "movie=$(TARGET_SAMPLES)/sub/Closed
 FATE_SUBTITLES_ASS-$(call ALLYES, AVDEVICE LAVFI_INDEV CCAPTION_DECODER MOVIE_FILTER MPEGTS_DEMUXER) += fate-sub-cc-realtime
 fate-sub-cc-realtime: CMD = fmtstdout ass -real_time 1 -f lavfi -i "movie=$(TARGET_SAMPLES)/sub/Closedcaption_rollup.m2v[out0+subcc]"
 
+FATE_SUBTITLES_ASS-$(call ALLYES, AVDEVICE LAVFI_INDEV CCAPTION_DECODER MOVIE_FILTER MPEGTS_DEMUXER) += fate-sub-cc-scte20
+fate-sub-cc-scte20: CMD = fmtstdout ass -f lavfi -i "movie=$(TARGET_SAMPLES)/sub/scte20.ts[out0+subcc]"
+
 FATE_SUBTITLES_ASS-$(call DEMDEC, ASS, ASS) += fate-sub-ass-to-ass-transcode
 fate-sub-ass-to-ass-transcode: CMD = fmtstdout ass -i $(TARGET_SAMPLES)/sub/1ededcbd7b.ass
 
diff --git a/tests/ref/fate/sub-cc-scte20 b/tests/ref/fate/sub-cc-scte20
new file mode 100644
index 0000000..71fc92b
--- /dev/null
+++ b/tests/ref/fate/sub-cc-scte20
@@ -0,0 +1,15 @@ 
+[Script Info]
+; Script generated by FFmpeg/Lavc
+ScriptType: v4.00+
+PlayResX: 384
+PlayResY: 288
+
+[V4+ Styles]
+Format: Name, Fontname, Fontsize, PrimaryColour, SecondaryColour, OutlineColour, BackColour, Bold, Italic, Underline, StrikeOut, ScaleX, ScaleY, Spacing, Angle, BorderStyle, Outline, Shadow, Alignment, MarginL, MarginR, MarginV, Encoding
+Style: Default,Monospace,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,3,1,0,2,10,10,10,0
+
+[Events]
+Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text
+Dialogue: 0,0:00:00.00,0:00:01.44,Default,,0,0,0,,{\an7}{\pos(48,182)}BESIDES THE 
+Dialogue: 0,0:00:01.43,0:00:03.93,Default,,0,0,0,,{\an7}{\pos(38,166)}\hBESIDES THE \N{\an7}{\pos(38,197)}SPENDING AND THIS, IS THAT CAR 
+Dialogue: 0,0:00:03.94,0:00:06.31,Default,,0,0,0,,{\an7}{\pos(38,182)}SPENDING AND THIS, IS THAT CAR \N{\an7}{\pos(38,197)}MANUFACTURERS ARE ABOUT AS