diff mbox series

[FFmpeg-devel,v4] avcodec/mpeg12dec: extract only one type of CC substream

Message ID 20240312154311.2345769-1-marth64@proxyid.net
State Accepted
Commit 0b342a2f156a5eb8ebbb2c07f1b887ab6fda8fc5
Headers show
Series [FFmpeg-devel,v4] avcodec/mpeg12dec: extract only one type of CC substream | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Marth64 March 12, 2024, 3:43 p.m. UTC 
In MPEG-2 user data, there can be different types of Closed Captions
formats embedded (A53, SCTE-20, or DVD). The current behavior of the
CC extraction code in the MPEG-2 decoder is to not be aware of
multiple formats if multiple exist, therefore allowing one format
to overwrite the other during the extraction process since the CC
extraction shares one output buffer for the normalized bytes.

This causes sources that have two CC formats to produce flawed output.
There exist real-world samples which contain both A53 and SCTE-20 captions
in the same MPEG-2 stream, and that manifest this problem. Example of symptom:
THANK YOU (expected) --> THTHANANK K YOYOUU (actual)

The solution is to pick only the first CC substream observed with valid bytes,
and ignore the other types. Additionally, provide an option for users
to manually "force" a type in the event that this matters for a particular
source.

Signed-off-by: Marth64 <marth64@proxyid.net>
---
 libavcodec/mpeg12dec.c | 64 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 61 insertions(+), 3 deletions(-)

Comments

Stefano Sabatini March 12, 2024, 5:10 p.m. UTC | #1 
On date Tuesday 2024-03-12 10:43:11 -0500, Marth64 wrote:
> In MPEG-2 user data, there can be different types of Closed Captions
> formats embedded (A53, SCTE-20, or DVD). The current behavior of the
> CC extraction code in the MPEG-2 decoder is to not be aware of
> multiple formats if multiple exist, therefore allowing one format
> to overwrite the other during the extraction process since the CC
> extraction shares one output buffer for the normalized bytes.
> 
> This causes sources that have two CC formats to produce flawed output.
> There exist real-world samples which contain both A53 and SCTE-20 captions
> in the same MPEG-2 stream, and that manifest this problem. Example of symptom:
> THANK YOU (expected) --> THTHANANK K YOYOUU (actual)
> 
> The solution is to pick only the first CC substream observed with valid bytes,
> and ignore the other types. Additionally, provide an option for users
> to manually "force" a type in the event that this matters for a particular
> source.
> 
> Signed-off-by: Marth64 <marth64@proxyid.net>
> ---
>  libavcodec/mpeg12dec.c | 64 ++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 61 insertions(+), 3 deletions(-)

LGTM.
Stefano Sabatini March 19, 2024, 4:30 p.m. UTC | #2 
On date Tuesday 2024-03-12 18:10:09 +0100, Stefano Sabatini wrote:
> On date Tuesday 2024-03-12 10:43:11 -0500, Marth64 wrote:
> > In MPEG-2 user data, there can be different types of Closed Captions
> > formats embedded (A53, SCTE-20, or DVD). The current behavior of the
> > CC extraction code in the MPEG-2 decoder is to not be aware of
> > multiple formats if multiple exist, therefore allowing one format
> > to overwrite the other during the extraction process since the CC
> > extraction shares one output buffer for the normalized bytes.
> > 
> > This causes sources that have two CC formats to produce flawed output.
> > There exist real-world samples which contain both A53 and SCTE-20 captions
> > in the same MPEG-2 stream, and that manifest this problem. Example of symptom:
> > THANK YOU (expected) --> THTHANANK K YOYOUU (actual)
> > 
> > The solution is to pick only the first CC substream observed with valid bytes,
> > and ignore the other types. Additionally, provide an option for users
> > to manually "force" a type in the event that this matters for a particular
> > source.
> > 
> > Signed-off-by: Marth64 <marth64@proxyid.net>
> > ---
> >  libavcodec/mpeg12dec.c | 64 ++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 61 insertions(+), 3 deletions(-)
> 
> LGTM.

Applied.
diff mbox series

Patch

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 3a2f17e508..4ad1eb6572 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -62,6 +62,13 @@ 
 
 #define A53_MAX_CC_COUNT 2000
 
+enum Mpeg2ClosedCaptionsFormat {
+    CC_FORMAT_AUTO,
+    CC_FORMAT_A53_PART4,
+    CC_FORMAT_SCTE20,
+    CC_FORMAT_DVD
+};
+
 typedef struct Mpeg1Context {
     MpegEncContext mpeg_enc_ctx;
     int mpeg_enc_ctx_allocated; /* true if decoding context allocated */
@@ -70,6 +77,7 @@  typedef struct Mpeg1Context {
     AVStereo3D stereo3d;
     int has_stereo3d;
     AVBufferRef *a53_buf_ref;
+    enum Mpeg2ClosedCaptionsFormat cc_format;
     uint8_t afd;
     int has_afd;
     int slice_count;
@@ -1903,12 +1911,27 @@  static int vcr2_init_sequence(AVCodecContext *avctx)
     return 0;
 }
 
+static void mpeg_set_cc_format(AVCodecContext *avctx, enum Mpeg2ClosedCaptionsFormat format,
+                               const char *label)
+{
+    Mpeg1Context *s1 = avctx->priv_data;
+
+    av_assert2(format != CC_FORMAT_AUTO);
+
+    if (!s1->cc_format) {
+        s1->cc_format = format;
+
+        av_log(avctx, AV_LOG_DEBUG, "CC: first seen substream is %s format\n", label);
+    }
+}
+
 static int mpeg_decode_a53_cc(AVCodecContext *avctx,
                               const uint8_t *p, int buf_size)
 {
     Mpeg1Context *s1 = avctx->priv_data;
 
-    if (buf_size >= 6 &&
+    if ((!s1->cc_format || s1->cc_format == CC_FORMAT_A53_PART4) &&
+        buf_size >= 6 &&
         p[0] == 'G' && p[1] == 'A' && p[2] == '9' && p[3] == '4' &&
         p[4] == 3 && (p[5] & 0x40)) {
         /* extract A53 Part 4 CC data */
@@ -1927,9 +1950,11 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
                 memcpy(s1->a53_buf_ref->data + old_size, p + 7, cc_count * UINT64_C(3));
 
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
+            mpeg_set_cc_format(avctx, CC_FORMAT_A53_PART4, "A/53 Part 4");
         }
         return 1;
-    } else if (buf_size >= 2 &&
+    } else if ((!s1->cc_format || s1->cc_format == CC_FORMAT_SCTE20) &&
+               buf_size >= 2 &&
                p[0] == 0x03 && (p[1]&0x7f) == 0x01) {
         /* extract SCTE-20 CC data */
         GetBitContext gb;
@@ -1973,10 +1998,13 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
                     cap += 3;
                 }
             }
+
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
+            mpeg_set_cc_format(avctx, CC_FORMAT_SCTE20, "SCTE-20");
         }
         return 1;
-    } else if (buf_size >= 11 &&
+    } else if ((!s1->cc_format || s1->cc_format == CC_FORMAT_DVD) &&
+               buf_size >= 11 &&
                p[0] == 'C' && p[1] == 'C' && p[2] == 0x01 && p[3] == 0xf8) {
         /* extract DVD CC data
          *
@@ -2033,7 +2061,9 @@  static int mpeg_decode_a53_cc(AVCodecContext *avctx,
                     p += 6;
                 }
             }
+
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
+            mpeg_set_cc_format(avctx, CC_FORMAT_DVD, "DVD");
         }
         return 1;
     }
@@ -2598,11 +2628,39 @@  const FFCodec ff_mpeg1video_decoder = {
                            },
 };
 
+#define M2V_OFFSET(x) offsetof(Mpeg1Context, x)
+#define M2V_PARAM     AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
+
+static const AVOption mpeg2video_options[] = {
+    { "cc_format", "extract a specific Closed Captions format",
+       M2V_OFFSET(cc_format), AV_OPT_TYPE_INT, { .i64 = CC_FORMAT_AUTO },
+        CC_FORMAT_AUTO, CC_FORMAT_DVD, M2V_PARAM, .unit = "cc_format" },
+
+       { "auto",   "pick first seen CC substream",  0, AV_OPT_TYPE_CONST,
+        { .i64 =   CC_FORMAT_AUTO },                .flags = M2V_PARAM, .unit = "cc_format" },
+       { "a53",    "pick A/53 Part 4 CC substream", 0, AV_OPT_TYPE_CONST,
+        { .i64 =   CC_FORMAT_A53_PART4 },           .flags = M2V_PARAM, .unit = "cc_format" },
+       { "scte20", "pick SCTE-20 CC substream",     0, AV_OPT_TYPE_CONST,
+        { .i64 =   CC_FORMAT_SCTE20 },              .flags = M2V_PARAM, .unit = "cc_format" },
+       { "dvd",    "pick DVD CC substream",         0, AV_OPT_TYPE_CONST,
+        { .i64 =   CC_FORMAT_DVD },                 .flags = M2V_PARAM, .unit = "cc_format" },
+    { NULL }
+};
+
+static const AVClass mpeg2video_class = {
+    .class_name = "MPEG-2 video",
+    .item_name  = av_default_item_name,
+    .option     = mpeg2video_options,
+    .version    = LIBAVUTIL_VERSION_INT,
+    .category   = AV_CLASS_CATEGORY_DECODER,
+};
+
 const FFCodec ff_mpeg2video_decoder = {
     .p.name         = "mpeg2video",
     CODEC_LONG_NAME("MPEG-2 video"),
     .p.type         = AVMEDIA_TYPE_VIDEO,
     .p.id           = AV_CODEC_ID_MPEG2VIDEO,
+    .p.priv_class   = &mpeg2video_class,
     .priv_data_size = sizeof(Mpeg1Context),
     .init           = mpeg_decode_init,
     .close          = mpeg_decode_end,