PR #24435 opened by Kacper Michajłow (kasper93) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24435 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24435.patch
From a070cf747b3985043f206ce63861b80ec027bf7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Michaj=C5=82ow?= <[email protected]> Date: Thu, 10 Sep 2026 03:47:46 +0200 Subject: [PATCH 1/2] fate/vcodec: cover the encoder taking the input picture by reference An mpegvideo encoder takes the input picture by reference when its linesize matches the one the encoder uses itself, which is the coded width plus a border on either side, and copies the picture otherwise. Both have to encode the same. fate-vsynth1-mpeg4-adap feeds the encoder an unpadded picture, which gets copied. Pad the input to 352 + 2 * 16 and the same encode has the picture taken by reference instead, so the two cover a path each. The two references currently do not agree, which is the bug the next commit fixes. --- tests/fate/vcodec.mak | 14 ++++++++++++-- tests/ref/vsynth/vsynth1-mpeg4-layout | 4 ++++ 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 tests/ref/vsynth/vsynth1-mpeg4-layout diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index 895d5d55bc..b23cca677b 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -315,11 +315,18 @@ FATE_VCODEC-$(call ENCDEC, MPEG4, AVI) += $(FATE_MPEG4_AVI) fate-vsynth%-mpeg4: ENCOPTS = -qscale 10 -flags +mv4 -mbd bits fate-vsynth%-mpeg4: FMT = mp4 -fate-vsynth%-mpeg4-adap: ENCOPTS = -b 550k -bf 2 -flags +mv4 \ +MPEG4_ADAP_OPTS = -b 550k -bf 2 -flags +mv4 \ -trellis 1 -cmp 1 -subcmp 2 \ -mbd rd -scplx_mask 0.3 \ -mpv_flags +mv0 \ -b_strategy 1 -b_sensitivity 5 +fate-vsynth%-mpeg4-adap: ENCOPTS = $(MPEG4_ADAP_OPTS) + +# This is the same test as fate-vsynth%-mpeg4-adap but with an input padded +# so the encoder can take it by reference, without copy. +FATE_VCODEC_LAYOUT-$(call ENCDEC, MPEG4, AVI) += fate-vsynth1-mpeg4-layout +fate-vsynth1-mpeg4-layout: ENCOPTS = -vf pad=384:288:0:0,crop=352:288:0:0 \ + $(MPEG4_ADAP_OPTS) fate-vsynth%-mpeg4-adv: ENCOPTS = -qscale 9 -flags +mv4+aic \ -data_partitioning 1 -trellis 1 \ @@ -525,7 +532,10 @@ $(FATE_VSYNTH2): tests/data/vsynth2.yuv $(FATE_VSYNTH_LENA): tests/data/vsynth_lena.yuv $(FATE_VSYNTH3): tests/data/vsynth3.yuv -FATE_AVCONV += $(FATE_VSYNTH1) $(FATE_VSYNTH2) $(FATE_VSYNTH3) +FATE_VCODEC_LAYOUT = $(FATE_VCODEC_LAYOUT-yes) +$(FATE_VCODEC_LAYOUT): tests/data/vsynth1.yuv + +FATE_AVCONV += $(FATE_VSYNTH1) $(FATE_VSYNTH2) $(FATE_VSYNTH3) $(FATE_VCODEC_LAYOUT) FATE_SAMPLES_AVCONV += $(FATE_VSYNTH_LENA) fate-vsynth1: $(FATE_VSYNTH1) diff --git a/tests/ref/vsynth/vsynth1-mpeg4-layout b/tests/ref/vsynth/vsynth1-mpeg4-layout new file mode 100644 index 0000000000..984e6b125c --- /dev/null +++ b/tests/ref/vsynth/vsynth1-mpeg4-layout @@ -0,0 +1,4 @@ +f268fdbe7f6810b87fd3f6069ef44a96 *tests/data/fate/vsynth1-mpeg4-layout.avi +260786 tests/data/fate/vsynth1-mpeg4-layout.avi +bab18a36ec88a100e6ba497bfc4928db *tests/data/fate/vsynth1-mpeg4-layout.out.rawvideo +stddev: 14.01 PSNR: 25.20 MAXDIFF: 164 bytes: 7603200/ 7603200 -- 2.52.0 From d92bd13fcaeba2d7356946be296faf6baecf4427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kacper=20Michaj=C5=82ow?= <[email protected]> Date: Thu, 10 Sep 2026 03:48:53 +0200 Subject: [PATCH 2/2] avcodec/mpegvideo_enc: score B frame chains on the input pixels When the reconstruction reuses the input buffer, the input picture is stored at INPLACE_OFFSET inside that buffer. get_intra_count() was handed f->data[0], so for such a picture it scored the padding margin rather than the input, and b_frame_score picked the B frame chain length from the wrong pixels. A picture that the encoder can reference without copying is not offset, so the very same encode produced different output depending on whether the input buffer could be taken directly. Both mpeg4-adap tests were scored on the wrong pixels, hence their references change here. Additionally, I've added sanity check that mpeg4-adap and mpeg4-layout produce exactly the same output, by comparing thier test references directly. Just to avoid missing this fact in the future. --- libavcodec/mpegvideo_enc.c | 11 +++++++++-- tests/fate/vcodec.mak | 5 ++++- tests/ref/vsynth/vsynth1-mpeg4-adap | 8 ++++---- tests/ref/vsynth/vsynth3-mpeg4-adap | 8 ++++---- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index 7751de8510..01a934bd1c 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -1224,6 +1224,13 @@ static int get_sae(const uint8_t *src, int ref, int stride) return acc; } +static const uint8_t *input_pic_luma(const MPVEncContext *const s, + const MPVPicture *const pic) +{ + return pic->f->data[0] + + (pic->shared || s->c.avctx->rc_buffer_size ? 0 : INPLACE_OFFSET); +} + static int get_intra_count(MPVEncContext *const s, const uint8_t *src, const uint8_t *ref, int stride) { @@ -1696,8 +1703,8 @@ static int set_bframe_chain_length(MPVMainEncContext *const m) m->input_picture[i]->b_frame_score == 0) { m->input_picture[i]->b_frame_score = get_intra_count(s, - m->input_picture[i ]->f->data[0], - m->input_picture[i - 1]->f->data[0], + input_pic_luma(s, m->input_picture[i ]), + input_pic_luma(s, m->input_picture[i - 1]), s->c.linesize) + 1; } } diff --git a/tests/fate/vcodec.mak b/tests/fate/vcodec.mak index b23cca677b..ff45ad2a49 100644 --- a/tests/fate/vcodec.mak +++ b/tests/fate/vcodec.mak @@ -324,9 +324,12 @@ fate-vsynth%-mpeg4-adap: ENCOPTS = $(MPEG4_ADAP_OPTS) # This is the same test as fate-vsynth%-mpeg4-adap but with an input padded # so the encoder can take it by reference, without copy. -FATE_VCODEC_LAYOUT-$(call ENCDEC, MPEG4, AVI) += fate-vsynth1-mpeg4-layout +FATE_VCODEC_LAYOUT-$(call ENCDEC, MPEG4, AVI) += fate-vsynth1-mpeg4-layout \ + fate-vsynth1-mpeg4-layout-ref fate-vsynth1-mpeg4-layout: ENCOPTS = -vf pad=384:288:0:0,crop=352:288:0:0 \ $(MPEG4_ADAP_OPTS) +fate-vsynth1-mpeg4-layout-ref: CMD = sed s/mpeg4-layout/mpeg4-adap/g $(SRC_PATH)/tests/ref/vsynth/vsynth1-mpeg4-layout +fate-vsynth1-mpeg4-layout-ref: REF = $(SRC_PATH)/tests/ref/vsynth/vsynth1-mpeg4-adap fate-vsynth%-mpeg4-adv: ENCOPTS = -qscale 9 -flags +mv4+aic \ -data_partitioning 1 -trellis 1 \ diff --git a/tests/ref/vsynth/vsynth1-mpeg4-adap b/tests/ref/vsynth/vsynth1-mpeg4-adap index 8fd0384f31..8bde82345e 100644 --- a/tests/ref/vsynth/vsynth1-mpeg4-adap +++ b/tests/ref/vsynth/vsynth1-mpeg4-adap @@ -1,4 +1,4 @@ -3a889f498a36c3bce59924887e73990f *tests/data/fate/vsynth1-mpeg4-adap.avi -264468 tests/data/fate/vsynth1-mpeg4-adap.avi -8333845e5dfdb913d08570b7edf8682f *tests/data/fate/vsynth1-mpeg4-adap.out.rawvideo -stddev: 13.94 PSNR: 25.24 MAXDIFF: 165 bytes: 7603200/ 7603200 +f268fdbe7f6810b87fd3f6069ef44a96 *tests/data/fate/vsynth1-mpeg4-adap.avi +260786 tests/data/fate/vsynth1-mpeg4-adap.avi +bab18a36ec88a100e6ba497bfc4928db *tests/data/fate/vsynth1-mpeg4-adap.out.rawvideo +stddev: 14.01 PSNR: 25.20 MAXDIFF: 164 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth3-mpeg4-adap b/tests/ref/vsynth/vsynth3-mpeg4-adap index d85d648729..f059c599ad 100644 --- a/tests/ref/vsynth/vsynth3-mpeg4-adap +++ b/tests/ref/vsynth/vsynth3-mpeg4-adap @@ -1,4 +1,4 @@ -974af2dfb1f81c8030d813617f989b9d *tests/data/fate/vsynth3-mpeg4-adap.avi -42096 tests/data/fate/vsynth3-mpeg4-adap.avi -96839ddca31edf7c9d392c358262a6d4 *tests/data/fate/vsynth3-mpeg4-adap.out.rawvideo -stddev: 4.94 PSNR: 34.25 MAXDIFF: 49 bytes: 86700/ 86700 +40b2c13177bb7fb95165ed5bc2d14a67 *tests/data/fate/vsynth3-mpeg4-adap.avi +41754 tests/data/fate/vsynth3-mpeg4-adap.avi +2df698f349ef31cb11b96c941a0dbd22 *tests/data/fate/vsynth3-mpeg4-adap.out.rawvideo +stddev: 5.05 PSNR: 34.06 MAXDIFF: 49 bytes: 86700/ 86700 -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
