| Message ID | 20240312060005.2111135-2-marth64@proxyid.net |
|---|---|
| State | New |
| Headers | show |
| Series | Closed Captions improvements (phase 1) | expand |
| 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 |
On date Tuesday 2024-03-12 01:00:00 -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 | 67 ++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 64 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c > index 3a2f17e508..8961a290a3 100644 > --- a/libavcodec/mpeg12dec.c > +++ b/libavcodec/mpeg12dec.c > @@ -62,6 +62,16 @@ > > #define A53_MAX_CC_COUNT 2000 > > +enum Mpeg2ClosedCaptionsFormat { > + CC_FORMAT_AUTO, > + CC_FORMAT_A53_PART4, > + CC_FORMAT_SCTE20, > + CC_FORMAT_DVD > +}; > +static const char mpeg2_cc_format_labels[4][12] = { nit: this might be static const char *mpeg2_cc_format_labels[4] = { to avoid unnecessary constraints on the string length, or you might pass the CC name in the function directly to avoid to maintain the array (as it is not shared at the moment) > + "Unknown", "A/53 Part 4", "SCTE-20", "DVD" > +}; > + [...] LGTM otherwise.
Stefano Sabatini: > On date Tuesday 2024-03-12 01:00:00 -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 | 67 ++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 64 insertions(+), 3 deletions(-) >> >> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c >> index 3a2f17e508..8961a290a3 100644 >> --- a/libavcodec/mpeg12dec.c >> +++ b/libavcodec/mpeg12dec.c >> @@ -62,6 +62,16 @@ >> >> #define A53_MAX_CC_COUNT 2000 >> >> +enum Mpeg2ClosedCaptionsFormat { >> + CC_FORMAT_AUTO, >> + CC_FORMAT_A53_PART4, >> + CC_FORMAT_SCTE20, >> + CC_FORMAT_DVD >> +}; > >> +static const char mpeg2_cc_format_labels[4][12] = { > > nit: this might be > static const char *mpeg2_cc_format_labels[4] = { > This would add relocations and put this into .data.rel.ro. > to avoid unnecessary constraints on the string length, or you might > pass the CC name in the function directly to avoid to maintain the > array (as it is not shared at the moment) > That sound like a good idea. >> + "Unknown", "A/53 Part 4", "SCTE-20", "DVD" >> +}; >> + > > [...] > > LGTM otherwise.
Quoting Marth64 (2024-03-12 07:00:00) > 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. Is it not possible to extract all of them simultaneously?
>Is it not possible to extract all of them simultaneously?
Each of the extractable CC bitstreams would need to be exposed as an
independent eia608 AVStream, because each represents a distinct stream.
They can’t be commingled (which is what this patch addressed). I’m not sure
of a way to sanely present multiple eia608 streams to the end user.
AVStreamGroup gives me hope for the future, but I can’t think of a
technique in present day.
This is because: as it stands now the only way I know of to open a context
on an mpeg2/h26x source with an eia608 AVStream exposed is via the lavfi
movie=in[out+subcc] technique - but that only exposes one stream.
I guess the subcc keyword in the filter can be expanded to expose multiple
bitstreams, even via AVStreamGroup, but user would have to be aware and
opt-in. Otherwise it could get confusing. How would the user easily know
which bitstreams are in the source file? We can probe this to some extent,
but what if the eia608 bytes don’t show until halfway through the source?
Open to ideas, thoughts, as I’m gathering my own on this in a notebook over
the past few weeks.
Respectfully,
Marth64
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index 3a2f17e508..8961a290a3 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -62,6 +62,16 @@ #define A53_MAX_CC_COUNT 2000 +enum Mpeg2ClosedCaptionsFormat { + CC_FORMAT_AUTO, + CC_FORMAT_A53_PART4, + CC_FORMAT_SCTE20, + CC_FORMAT_DVD +}; +static const char mpeg2_cc_format_labels[4][12] = { + "Unknown", "A/53 Part 4", "SCTE-20", "DVD" +}; + typedef struct Mpeg1Context { MpegEncContext mpeg_enc_ctx; int mpeg_enc_ctx_allocated; /* true if decoding context allocated */ @@ -70,6 +80,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 +1914,27 @@ static int vcr2_init_sequence(AVCodecContext *avctx) return 0; } +static void mpeg_set_cc_format(AVCodecContext *avctx, enum Mpeg2ClosedCaptionsFormat format) +{ + 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", + mpeg2_cc_format_labels[format]); + } +} + 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 +1953,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); } 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 +2001,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); } 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 +2064,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); } return 1; } @@ -2598,11 +2631,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,
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 | 67 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 3 deletions(-)