PR #25588 opened by jayaimzzz URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25588 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25588.patch
Follow-up to #25586. As @dheitmueller pointed out there, the real bug is in the decoder. mpeg12dec collects caption user data in `a53_buf_ref` and only attaches it to a frame in `mpeg_field_start()`. When a picture gets skipped instead (an open-GOP B-frame, a P-frame before the first keyframe, or anything dropped by `skip_frame`), its captions stay in the buffer and get merged into the next decoded frame. Tuning in to a live ATSC broadcast skips a run of P and B pictures before the first I-frame, so the first frame ends up with several hundred cc tuples (up to ~600 in my captures). That's far over what cc_count can describe, and encoders write broken SEIs from it. This drops the captions of a picture that was never decoded when the next picture, GOP or sequence header comes in. Captions in GOP user data (DVD format) are left alone for the pictures that follow. Testing: - `make fate` with the full samples: all pass except `fate-sub-mcc`, whose reference I updated. `scte20.ts` starts with a skipped open-GOP B-frame, and its caption data used to be merged into the first decoded frame. Only the `00:00:00:03` line of the MCC changes, and the caption text decoded from it (`[out0+subcc]` to SRT) is identical before and after. - `Closedcaption_rollup.m2v` decoded from the start gives identical caption output. The same file cut mid-stream no longer produces a stray garbled ")" caption at the start. - Live OTA MPEG-2 (720p59.94 and 1080i, HDHomeRun), starting mid-stream and encoding with the unmodified `h264_qsv -a53cc 1`: no caption SEI over 255 bytes any more (the old decoder produced many), 0 `trace_headers` errors, and the same number of caption SEIs. One behavior change: with `-skip_frame nonref` (or higher), the captions of the discarded pictures are now dropped instead of being merged into the next frame. Code assistance: Claude Code (Claude Opus) helped trace this, write the patch and run the tests; I reviewed the change and the results. 🤖 Generated with [Claude Code](https://claude.com/claude-code) From e5a71d4603749f54619330244fa8d11297640d9d Mon Sep 17 00:00:00 2001 From: James Horton <[email protected]> Date: Thu, 29 Oct 2026 14:07:01 -0400 Subject: [PATCH] avcodec/mpeg12dec: drop closed captions of skipped pictures Closed caption user data is collected in a53_buf_ref and only attached to a frame when a picture is decoded. When a picture is skipped instead (B-frames of an open GOP, P-frames before the first keyframe, or pictures discarded by skip_frame), its captions stayed in the buffer and were all attached to the next decoded frame. When decoding starts in the middle of a broadcast, this puts several hundred caption tuples on the first frame, far more than cc_count allows, and encoders then write malformed caption SEIs (e.g. h264_qsv truncates the SEI size). Drop the captions of a picture that was not decoded when the next picture, GOP or sequence header is parsed. Captions carried in GOP user data (DVD format) are kept for the pictures that follow. fate-sub-mcc changes because scte20.ts starts with a skipped open-GOP B-frame whose caption data used to be merged into the first decoded frame; the decoded caption text is unchanged. Signed-off-by: James Horton <[email protected]> --- libavcodec/mpeg12dec.c | 21 +++++++++++++++++++++ tests/fate/subtitles.mak | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index e30fa843cca..61bf64b45d4 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -83,6 +83,7 @@ typedef struct Mpeg1Context { enum AVStereo3DType stereo3d_type; int has_stereo3d; AVBufferRef *a53_buf_ref; + int cc_pending; /* picture header seen, its CC not yet attached to a frame */ enum Mpeg2ClosedCaptionsFormat cc_format; uint8_t afd; int has_afd; @@ -1233,6 +1234,8 @@ static int mpeg_field_start(Mpeg1Context *s1, const uint8_t *buf, int buf_size) int second_field = 0; int ret; + s1->cc_pending = 0; + if (!(avctx->flags2 & AV_CODEC_FLAG2_CHUNKS)) { if (s->mb_width * s->mb_height * 11LL / (33 * 2 * 8) > buf_size) return AVERROR_INVALIDDATA; @@ -1898,6 +1901,18 @@ static void mpeg_set_cc_format(AVCodecContext *avctx, enum Mpeg2ClosedCaptionsFo } } +/* Closed captions are collected from the user data of each picture and + * attached to the frame when the picture is decoded. Drop those of a picture + * that was skipped instead (e.g. B-frames of an open GOP or P-frames before + * the first keyframe when starting mid-stream), so they don't pile up on the + * next decoded frame. */ +static void mpeg_drop_skipped_cc(Mpeg1Context *s1) +{ + if (s1->cc_pending) + av_buffer_unref(&s1->a53_buf_ref); + s1->cc_pending = 0; +} + static int mpeg_decode_a53_cc(AVCodecContext *avctx, const uint8_t *p, int buf_size) { @@ -2274,6 +2289,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture, switch (start_code) { case SEQ_START_CODE: if (last_code == 0) { + mpeg_drop_skipped_cc(s); mpeg1_decode_sequence(avctx, buf_ptr, input_size); if (buf != avctx->extradata) s->sync = 1; @@ -2323,6 +2339,9 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture, return ret; } + mpeg_drop_skipped_cc(s); + s->cc_pending = 1; + /* We have a complete image: we try to decompress it. */ if (mpeg1_decode_picture(avctx, buf_ptr, input_size) < 0) s2->pict_type = 0; @@ -2382,6 +2401,7 @@ static int decode_chunks(AVCodecContext *avctx, AVFrame *picture, break; case GOP_START_CODE: if (last_code == 0) { + mpeg_drop_skipped_cc(s); s2->first_field = 0; ret = mpeg_decode_gop(avctx, buf_ptr, input_size); if (ret < 0) @@ -2627,6 +2647,7 @@ static av_cold void flush(AVCodecContext *avctx) s->closed_gop = 0; av_buffer_unref(&s->a53_buf_ref); + s->cc_pending = 0; ff_mpeg_flush(avctx); } diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak index 3ded96e044f..b2c4f4beaeb 100644 --- a/tests/fate/subtitles.mak +++ b/tests/fate/subtitles.mak @@ -139,7 +139,7 @@ fate-sub-rcwt: REF = d86f179094a5752d68aa97d82cf887b0 FATE_SUBTITLES-$(call ALLYES, AVDEVICE LAVFI_INDEV MOVIE_FILTER MPEGTS_DEMUXER MCC_MUXER EIA608_TO_SMPTE436M_BSF) += fate-sub-mcc fate-sub-mcc: CMD = md5 -f lavfi -i "movie=$(TARGET_SAMPLES)/sub/scte20.ts[out0+subcc]" -map 0:s -c copy -override_time_code_rate ntsc -creation_time "1970-01-02T00:00:00" -bitexact -f mcc fate-sub-mcc: CMP = oneline -fate-sub-mcc: REF = 73982c4db86f3a809c1e8262d3701fc2 +fate-sub-mcc: REF = 1253c9f01124447cf03e79f34b52b7fa FATE_SUBTITLES-$(call DEMMUX, MCC, MCC, SMPTE436M_TO_EIA608_BSF EIA608_TO_SMPTE436M_BSF) += fate-sub-mcc-remux-eia608-bsf fate-sub-mcc-remux-eia608-bsf: CMD = fmtstdout mcc -f mcc -i $(SRC_PATH)/tests/ref/fate/sub-mcc-remux -map 0:s -c copy -bsf "eia608_to_smpte436m=cdp_frame_rate=60000/1001:initial_cdp_sequence_cntr=65535:line_number=11" -override_time_code_rate ntsc -creation_time "1970-01-02T00:00:00" -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
