PR #25630 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25630 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25630.patch
When two H.264 fields share a packet and the PPS is replaced between them, `ff_h264_queue_decode_slice()` calculates `first_slice` before completing the preceding field. The second field consequently retains the first field's active PPS, including its scaling matrices, and produces incorrect pixels. Move that calculation after field completion and queued-slice processing. The fix preserves Fredrik Mellbin's authorship from the patch attached to #25495. A separate commit adds `fate-h264-second-field-pps`, with expected output under `tests/ref/fate/`. Fixes #25495. The regression was verified at 6fafc62b0bd0 and its immediate parent 31cc9c04ca38: all 13 sample frames match unpatched master at the former and corrected output at the latter. Validation on ARM64: - All 13 corrected frames match the same compressed stream repacketized through the raw H.264 parser, with one decoder thread, eight frame threads, and eight slice threads. - All 219 enabled existing H.264 FATE tests pass with `THREADS=8`. - The new regression test passes with `THREADS=1`, `THREADS=8`, and `THREADS=8 THREAD_TYPE=slice`; unpatched master differs on all 13 frame checksums. - `git diff --check` passes. The new FATE test requires the [existing sample attachment](https://code.ffmpeg.org/attachments/fcd5543e-cbe7-4f51-b680-4cceebc5757e), unchanged, installed as `h264/second-field-pps.mkv`. Size: 895,204 bytes. SHA-256: `02ee0adbefc7b900ba6889a6029ba30213b3f1f355b69f6660ee0ddef5711aa5`. Sample publication is pending: it was installed locally for testing but has not been submitted by this investigation. It needs submission to samples-request at ffmpeg dot org and installation in the public FATE suite before the test is merged. >From 20038f8a0dd81750bbd7fb4e03cad075409f099a Mon Sep 17 00:00:00 2001 From: Fredrik Mellbin <[email protected]> Date: Mon, 26 Oct 2026 14:00:00 +0100 Subject: [PATCH 1/2] avcodec/h264_slice: activate the PPS of a second field in the same packet ff_h264_queue_decode_slice() determines first_slice before it ends the previous picture. When both fields share a packet, the first slice of the second field therefore keeps the PPS of the first field active. A replacement PPS with different scaling matrices is parsed but is not activated, resulting in incorrect dequantization. Determine first_slice after ending the previous picture and completing any queued slices, so each new field activates its own PPS. For the sample attached to issue 25495, all 13 decoded frames now match the same bitstream repacketized by the raw H.264 parser. The result also matches the parent of the regression commit with single-thread decoding. Fixes: 6fafc62b0bd0e206deb77a7aabbf3a370ad80789 Fixes: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/25495 Co-Authored-By: Claude Opus 5.5 <[email protected]> Assisted-by: Fairy --- libavcodec/h264_slice.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 2bcb64378dc..d614c0b0a3d 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -2169,7 +2169,7 @@ int ff_h264_queue_decode_slice(H264Context *h, const H2645NAL *nal, H264SliceContext **queued) { H264SliceContext *sl = h->slice_ctx + h->nb_slice_ctx_queued; - int first_slice = sl == h->slice_ctx && !h->current_slice; + int first_slice; int ret; *queued = NULL; @@ -2247,6 +2247,11 @@ int ff_h264_queue_decode_slice(H264Context *h, const H2645NAL *nal, if (!h->current_slice) av_assert0(sl == h->slice_ctx); + /* Not determined any earlier: a picture that starts within the packet of + * the previous one ends that picture above, and its first slice has to + * activate the PPS like the first slice of any other picture. */ + first_slice = sl == h->slice_ctx && !h->current_slice; + if (h->current_slice == 0 && !h->first_field) { if ( (h->avctx->skip_frame >= AVDISCARD_NONREF && !h->nal_ref_idc) || -- 2.54.0 >From a9e6c27140b6fb751ea9638162eac67d39566ea9 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Wed, 28 Oct 2026 19:01:43 +0000 Subject: [PATCH 2/2] fate/h264: cover PPS replacement between fields in one packet Exercise a field pair whose second field replaces the PPS used by the first field, including its scaling matrices. Decode the following frames too, to cover propagation through the reference pictures. The sample is the 895204-byte Matroska attachment to issue 25495, installed as h264/second-field-pps.mkv in the FATE sample suite. Its SHA-256 is 02ee0adbefc7b900ba6889a6029ba30213b3f1f355b69f6660ee0ddef5711aa5. The reference agrees with decoding the same stream after repacketization through the raw H.264 parser; the unpatched decoder differs on every frame. Assisted-by: Fairy --- tests/fate/h264.mak | 3 ++- tests/ref/fate/h264-second-field-pps | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 tests/ref/fate/h264-second-field-pps diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak index 1ee70f216df..0dc1edcd07d 100644 --- a/tests/fate/h264.mak +++ b/tests/fate/h264.mak @@ -244,7 +244,7 @@ FATE_H264-$(call REMUX, H264, H264_PARSER H264_REDUNDANT_PPS_BSF H264_DECODER RA FATE_H264-$(call REMUX, NUT, MOV_DEMUXER H264_REDUNDANT_PPS_BSF H264_DECODER) += fate-h264-bsf-redundant-pps-side-data FATE_H264-$(call REMUX, NUT, MOV_DEMUXER H264_REDUNDANT_PPS_BSF H264_DECODER SCALE_FILTER RAWVIDEO_ENCODER) += fate-h264-bsf-redundant-pps-side-data2 -FATE_H264-$(call FRAMECRC, MATROSKA, H264) += fate-h264-direct-bff +FATE_H264-$(call FRAMECRC, MATROSKA, H264) += fate-h264-direct-bff fate-h264-second-field-pps FATE_H264-$(call FRAMECRC, FLV, H264, SCALE_FILTER) += fate-h264-brokensps-2580 FATE_H264-$(call FRAMECRC, MXF, H264, PCM_S24LE_DECODER SCALE_FILTER ARESAMPLE_FILTER) += fate-h264-xavc-4389 FATE_H264-$(call FRAMECRC, MOV, H264) += fate-h264-attachment-631 @@ -488,6 +488,7 @@ fate-h264-lossless: CMD = framecrc -i $(TARGET_SAM fate-h264-mixed-nal-coding: CMD = framecrc -i $(TARGET_SAMPLES)/h264/mixed-nal-coding.mp4 fate-h264-ref-pic-mod-overflow: CMD = framecrc -i $(TARGET_SAMPLES)/h264/ref-pic-mod-overflow.h264 fate-h264-twofields-packet: CMD = framecrc -i $(TARGET_SAMPLES)/h264/twofields_packet.mp4 -an -frames 30 +fate-h264-second-field-pps: CMD = framecrc -i $(TARGET_SAMPLES)/h264/second-field-pps.mkv -an fate-h264-unescaped-extradata: CMD = framecrc -i $(TARGET_SAMPLES)/h264/unescaped_extradata.mp4 -an -frames 10 fate-h264-3386: CMD = framecrc -i $(TARGET_SAMPLES)/h264/bbc2.sample.h264 fate-h264-missing-frame: CMD = framecrc -i $(TARGET_SAMPLES)/h264/nondeterministic_cut.h264 diff --git a/tests/ref/fate/h264-second-field-pps b/tests/ref/fate/h264-second-field-pps new file mode 100644 index 00000000000..630d7460135 --- /dev/null +++ b/tests/ref/fate/h264-second-field-pps @@ -0,0 +1,18 @@ +#tb 0: 1001/30000 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 1920x1080 +#sar 0: 1/1 +0, 0, 0, 1, 3110400, 0x85c14091 +0, 1, 1, 1, 3110400, 0x92f3c883 +0, 2, 2, 1, 3110400, 0x0c27b2f7 +0, 3, 3, 1, 3110400, 0x36d1ba1e +0, 4, 4, 1, 3110400, 0x82f590a8 +0, 5, 5, 1, 3110400, 0x86fddf93 +0, 6, 6, 1, 3110400, 0x121ceb9f +0, 7, 7, 1, 3110400, 0x2f91633e +0, 8, 8, 1, 3110400, 0x9a71e2c9 +0, 9, 9, 1, 3110400, 0xc2259b04 +0, 10, 10, 1, 3110400, 0xd6d7390a +0, 11, 11, 1, 3110400, 0x031f8190 +0, 12, 12, 1, 3110400, 0x579e5b18 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
