Re: [FFmpeg-devel] [PATCH v3 1/6] avcodec/mpeg12dec: extract only one type of CC substream

From: Stefano Sabatini <stefasab@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Marth64 <marth64@proxyid.net>
Subject: Re: [FFmpeg-devel] [PATCH v3 1/6] avcodec/mpeg12dec: extract only one type of CC substream
Date: Tue, 12 Mar 2024 12:00:17 +0100
Message-ID: <ZfA1wbRcRgl1FbT2@mariano> (raw)
In-Reply-To: <20240312060005.2111135-2-marth64@proxyid.net>

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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".