From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id D5C9049DB8 for ; Mon, 11 Mar 2024 16:27:29 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A0C9468D097; Mon, 11 Mar 2024 18:27:26 +0200 (EET) Received: from mail-ej1-f45.google.com (mail-ej1-f45.google.com [209.85.218.45]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id BC26168C9EE for ; Mon, 11 Mar 2024 18:27:20 +0200 (EET) Received: by mail-ej1-f45.google.com with SMTP id a640c23a62f3a-a461e09ddddso168681666b.2 for ; Mon, 11 Mar 2024 09:27:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1710174439; x=1710779239; darn=ffmpeg.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=HO0/l8snBkXb/rRlEP+LMIC3UIA0z0fXFRpI2xOE6M0=; b=OAROhA1hiMtQ//3ukLUbNy7AlTX/0S9ZLBtZEI5oI8KqRHLUtQcxJuH3eErYHm1VfG opyNc6QUO+XwpSbpoStA3uhIz384VqEdzeo3PNgB+TsTTtoyfeziBP667KAGoOvAS329 2/15i2q8kyZXvGGgfrZ3ZEI9XHluNzx1oRQ79/h+/biA8x41OxdHV1t97WNINSG/t02g iBj3ZmZsN6mYqCbVX7Lqlt0b5KQxG6WbmiwMCVrxHXeAVmsn1NcKqiFYsnrdek/vop50 5ijg1TK3sqlysMYwa0Q6kIiWwQv0GqwkGURqC+iUTCVFiLWYVWzIIxgOPOTaYi+/gXiL LvsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710174439; x=1710779239; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HO0/l8snBkXb/rRlEP+LMIC3UIA0z0fXFRpI2xOE6M0=; b=iBQXfQEEvfLu4tQ9/BiDbVtKpDTVVlMaR1DrxsKzlEFNXVrYWji+R/doOxtYsgEI74 RCiBNh+PRbalMhbSRnEcmN/RvHwIkBG5FDkBi2jx+cIPFjtjkslrJ4058V+jxW0ufyw4 f2EEvkpR3xO3LaB6HgaLDuhAYax1Y1q1mzbQg2cLs3lGvzsKtaU3zM+1zuTO4zN6oWIc enCEPjmW7hsQnEqUJ/bZWEwDVYYyVPzGjx8YEplDvrwk+2PX2aPufp/CiKrYu6++GF9m oBX5/ZoWWHqMz3VMdQoEshIK0KT98RZCHwJdBYWwdmxRE6B8rscPgH7Cb1Gf3T0Uc1k9 l1Zg== X-Gm-Message-State: AOJu0YxpZTTWu1BgyqH6+zybDbpHhyF4ZAQL1dY0BP8tCK41RkdW0yx7 KpxVrXAwZKd1yRuTlaJpS5DX/D17zQbieG+cqzOPMqHUWqyMRScsBideQzOk X-Google-Smtp-Source: AGHT+IFymt2zyO54jvvu5T5MKYeJ1lGgYG59qZSlcU83xADy9kx+7ia9uysReK5PFQe7rp0tOKUILg== X-Received: by 2002:a17:907:c246:b0:a46:2ba9:68c5 with SMTP id tj6-20020a170907c24600b00a462ba968c5mr1868080ejc.27.1710174439312; Mon, 11 Mar 2024 09:27:19 -0700 (PDT) Received: from mariano ([188.210.239.79]) by smtp.gmail.com with ESMTPSA id f12-20020a17090624cc00b00a449f43a7afsm2992366ejb.113.2024.03.11.09.27.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Mar 2024 09:27:18 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id 6A7EBBFCDC; Mon, 11 Mar 2024 17:27:17 +0100 (CET) Date: Mon, 11 Mar 2024 17:27:17 +0100 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: Mail-Followup-To: FFmpeg development discussions and patches , Marth64 References: <20240311001856.128390-1-marth64@proxyid.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240311001856.128390-1-marth64@proxyid.net> User-Agent: Mutt/2.1.4 (2021-12-11) Subject: Re: [FFmpeg-devel] [PATCH] avcodec/mpeg12dec: extract only one type of CC substream X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Marth64 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On date Sunday 2024-03-10 19:18:56 -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. > > In tandem, I am working on an improvement to src_movie to allow passing decoder > options (as src_movie via lavfi is the "de facto" way to extract CCs right now). > This way, users can realize the newly added option. > > Signed-off-by: Marth64 > --- > libavcodec/mpeg12dec.c | 49 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 46 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c > index 3a2f17e508..a42e1c661f 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; > @@ -1908,7 +1916,8 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx, > { > 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 +1936,15 @@ 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; > + > + if (!s1->cc_format) { > + s1->cc_format = CC_FORMAT_A53_PART4; > + av_log(avctx, AV_LOG_DEBUG, "CC: first seen substream is A53 format\n"); > + } > } > 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; > @@ -1974,9 +1989,15 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx, > } > } > avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS; > + > + if (!s1->cc_format) { > + s1->cc_format = CC_FORMAT_SCTE20; > + av_log(avctx, AV_LOG_DEBUG, "CC: first seen substream is SCTE-20 format\n"); > + } > } > 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 > * > @@ -2034,6 +2055,11 @@ static int mpeg_decode_a53_cc(AVCodecContext *avctx, > } > } > avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS; > + > + if (!s1->cc_format) { > + s1->cc_format = CC_FORMAT_DVD; > + av_log(avctx, AV_LOG_DEBUG, "CC: first seen substream is DVD format\n"); > + } nit: probably this might be factorized, either with a function or a macro > } > return 1; > } > @@ -2598,11 +2624,28 @@ 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 (0=auto)", M2V_OFFSET(cc_format), AV_OPT_TYPE_INT, { .i64 = 0 }, CC_FORMAT_AUTO, CC_FORMAT_DVD, M2V_PARAM }, > + { NULL } ideally this should be documented in the doc, but it is missing for this one so probably it's not needed [...] Patch looks good to me, but let's wait a few days in case there are more comments. _______________________________________________ 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".