PR #25770 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25770 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25770.patch
When H.264 film-grain synthesis fails, clearing `needs_fg` only updates the decoding context's picture. Frame contexts that copied the picture earlier retain `needs_fg = 1` and can output an unwritten grain buffer. The stream in #25691 reproduces this with unsupported grain model 1. Copy the decoded pixels into the existing shared grain buffer on failure and keep `needs_fg` unchanged. This preserves the decoded-picture fallback for every picture reference before completion is reported, and propagates any copy error. Add deterministic FATE coverage that calls both picture-copy functions before completing synthesis. It exercises unsupported models, unsupported pixel formats, missing side data, and successful synthesis with no component models. Expected checksums are in the FATE reference file; no external sample or scheduler timing is required. Validation on Linux AArch64: - Unpatched master: incorrect output in 30/40 three-frame-thread runs and 25/40 default-threading runs. - Patched decoder: correct output in 1,000 runs each with one thread, three frame threads, default threading, and grain export. - Expanded H.264 FATE run: 218 checks passed, including the new test. - New test fails without the fix and passes with it. - New test also passes with ASan/UBSan, assertion level 2, and threading disabled, without sanitizer diagnostics. The regression was verified at 66845cffc3bb against its immediate parent 6bc29a6b571c. The supplied stream also reproduces on release branches 5.1, 6.1, 7.1, 8.0, 8.1, 9.0, and 9.1. Fixes #25691. >From 2564bec90c894b6ba25c78c9740f6032516e593d Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 5 Nov 2026 01:23:45 +0000 Subject: [PATCH 1/2] avcodec/h264_picture: fill the grain buffer when synthesis fails Frame threads may copy a picture before film grain synthesis completes. Clearing needs_fg only changes the decoding thread local picture, leaving other contexts to output the uninitialized grain buffer. Copy the decoded pixels into the shared grain buffer on failure and keep needs_fg unchanged. This preserves the existing fallback for every copy of the picture, before reporting decoding progress. Fixes #25691. Assisted-by: Fairy --- libavcodec/h264_picture.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c index 8cb91e371c6..33825df501c 100644 --- a/libavcodec/h264_picture.c +++ b/libavcodec/h264_picture.c @@ -220,8 +220,10 @@ int ff_h264_field_end(H264Context *h, H264SliceContext *sl, int in_setup) if (err < 0) { av_log(h->avctx, AV_LOG_WARNING, "Failed synthesizing film " "grain, ignoring: %s\n", av_err2str(err)); - cur->needs_fg = 0; - err = 0; + /* Other frame threads may already reference f_grain. Keep + * needs_fg unchanged and fill the shared buffer with the decoded + * picture so that those references also see the fallback. */ + err = av_frame_copy(cur->f_grain, cur->f); } } -- 2.54.0 >From 609b0d03428bb35041809477d8cf15a25b5be969 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 5 Nov 2026 01:23:45 +0000 Subject: [PATCH 2/2] tests: cover H.264 film grain fallback in copied pictures Create picture references before completing synthesis to exercise the frame-thread failure ordering deterministically. Check unsupported grain models, unsupported pixel formats, missing side data and a successful no-component synthesis case. Expected pixel checksums live in FATE refs. Assisted-by: Fairy --- libavcodec/Makefile | 1 + libavcodec/tests/h264_film_grain.c | 132 +++++++++++++++++++++++++++++ tests/fate/libavcodec.mak | 4 + tests/ref/fate/h264-film-grain | 20 +++++ 4 files changed, 157 insertions(+) create mode 100644 libavcodec/tests/h264_film_grain.c create mode 100644 tests/ref/fate/h264-film-grain diff --git a/libavcodec/Makefile b/libavcodec/Makefile index bb2c32c250f..e7ac372ff40 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1404,6 +1404,7 @@ TESTPROGS-$(CONFIG_DXV_ENCODER) += hashtable TESTPROGS-$(CONFIG_MJPEG_ENCODER) += mjpegenc_huffman TESTPROGS-$(CONFIG_MPEGAUDIOHEADER) += mpegaudiodecheader TESTPROGS-$(CONFIG_MPEGVIDEO) += mpeg12framerate +TESTPROGS-$(CONFIG_H264_DECODER) += h264_film_grain TESTPROGS-$(CONFIG_H264_METADATA_BSF) += h264_levels TESTPROGS-$(CONFIG_HEVC_METADATA_BSF) += h265_levels TESTPROGS-$(CONFIG_RANGECODER) += rangecoder diff --git a/libavcodec/tests/h264_film_grain.c b/libavcodec/tests/h264_film_grain.c new file mode 100644 index 00000000000..401d7a61773 --- /dev/null +++ b/libavcodec/tests/h264_film_grain.c @@ -0,0 +1,132 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdio.h> +#include <string.h> + +#include "libavutil/adler32.h" +#include "libavutil/film_grain_params.h" +#include "libavutil/imgutils.h" +#include "libavutil/mem.h" +#include "libavcodec/h264dec.h" + +static int print_frame(const char *name, const AVFrame *frame) +{ + uint8_t pixels[16 * 16 * 3]; + int size = av_image_copy_to_buffer(pixels, sizeof(pixels), + (const uint8_t * const *)frame->data, + frame->linesize, frame->format, + frame->width, frame->height, 1); + if (size < 0) + return size; + printf("%s: %d bytes, %08lx\n", name, size, + (unsigned long)av_adler32_update(0, pixels, size)); + return 0; +} + +static int test_fallback(enum AVPixelFormat format, int model_id) +{ + AVCodecContext *avctx = avcodec_alloc_context3(NULL); + H264Context *h = av_mallocz(sizeof(*h)); + H264Picture *pic; + int ret = AVERROR(ENOMEM); + + if (!avctx || !h) + goto end; + + h->avctx = avctx; + avctx->active_thread_type = FF_THREAD_FRAME; + /* Skip reference marking and progress reporting: no worker is running. */ + h->droppable = 1; + h->picture_structure = PICT_FRAME; + h->cur_pic_ptr = pic = &h->DPB[0]; + + for (int i = 0; i < 3; i++) { + h->DPB[i].f = av_frame_alloc(); + h->DPB[i].f_grain = av_frame_alloc(); + if (!h->DPB[i].f || !h->DPB[i].f_grain) + goto end; + } + + for (int i = 0; i < 2; i++) { + AVFrame *f = i ? pic->f_grain : pic->f; + f->format = format; + f->width = f->height = 16; + ret = av_frame_get_buffer(f, 32); + if (ret < 0) + goto end; + for (int c = 0; c < 3; c++) + memset(f->data[c], i ? 0x2a : 0x60 + c, + f->linesize[c] * (c ? 8 : 16)); + } + pic->tf.f = pic->f; + pic->needs_fg = 1; + + if (model_id >= 0) { + AVFilmGrainParams *params = av_film_grain_params_create_side_data(pic->f); + if (!params) { + ret = AVERROR(ENOMEM); + goto end; + } + params->type = AV_FILM_GRAIN_PARAMS_H274; + params->codec.h274.model_id = model_id; + } + + /* Force the ordering which exposes the bug, without scheduler timing: + * both kinds of picture copy happen before grain synthesis fails. */ + ret = ff_h264_ref_picture(&h->DPB[1], pic); + if (ret < 0) + goto end; + ret = ff_h264_replace_picture(&h->DPB[2], pic); + if (ret < 0) + goto end; + ret = ff_h264_field_end(h, NULL, 0); + if (ret < 0) + goto end; + + printf("format %s, model %d\n", av_get_pix_fmt_name(format), model_id); + ret = print_frame("decoded", pic->f); + for (int i = 0; i < 3 && ret >= 0; i++) { + static const char *const names[] = { "local", "ref", "replace" }; + H264Picture *p = &h->DPB[i]; + ret = print_frame(names[i], p->needs_fg ? p->f_grain : p->f); + } + +end: + if (h) { + for (int i = 0; i < 3; i++) { + ff_h264_unref_picture(&h->DPB[i]); + av_frame_free(&h->DPB[i].f); + av_frame_free(&h->DPB[i].f_grain); + } + } + av_free(h); + avcodec_free_context(&avctx); + return ret; +} + +int main(void) +{ + av_log_set_level(AV_LOG_ERROR); + if (test_fallback(AV_PIX_FMT_YUV420P, 1) < 0 || + test_fallback(AV_PIX_FMT_YUV420P10LE, 0) < 0 || + test_fallback(AV_PIX_FMT_YUV420P, -1) < 0 || + test_fallback(AV_PIX_FMT_YUV420P, 0) < 0) + return 1; + return 0; +} diff --git a/tests/fate/libavcodec.mak b/tests/fate/libavcodec.mak index 82cdb041d10..d0b52d9d8b0 100644 --- a/tests/fate/libavcodec.mak +++ b/tests/fate/libavcodec.mak @@ -83,6 +83,10 @@ fate-dct8x8: libavcodec/tests/dct$(EXESUF) fate-dct8x8: CMD = run libavcodec/tests/dct$(EXESUF) fate-dct8x8: CMP = null +FATE_LIBAVCODEC-$(CONFIG_H264_DECODER) += fate-h264-film-grain +fate-h264-film-grain: libavcodec/tests/h264_film_grain$(EXESUF) +fate-h264-film-grain: CMD = run libavcodec/tests/h264_film_grain$(EXESUF) + FATE_LIBAVCODEC-$(CONFIG_H264_METADATA_BSF) += fate-h264-levels fate-h264-levels: libavcodec/tests/h264_levels$(EXESUF) fate-h264-levels: CMD = run libavcodec/tests/h264_levels$(EXESUF) diff --git a/tests/ref/fate/h264-film-grain b/tests/ref/fate/h264-film-grain new file mode 100644 index 00000000000..00713b89d6e --- /dev/null +++ b/tests/ref/fate/h264-film-grain @@ -0,0 +1,20 @@ +format yuv420p, model 1 +decoded: 384 bytes, 76b490c0 +local: 384 bytes, 76b490c0 +ref: 384 bytes, 76b490c0 +replace: 384 bytes, 76b490c0 +format yuv420p10le, model 0 +decoded: 768 bytes, 4a1f218f +local: 768 bytes, 4a1f218f +ref: 768 bytes, 4a1f218f +replace: 768 bytes, 4a1f218f +format yuv420p, model -1 +decoded: 384 bytes, 76b490c0 +local: 384 bytes, 76b490c0 +ref: 384 bytes, 76b490c0 +replace: 384 bytes, 76b490c0 +format yuv420p, model 0 +decoded: 384 bytes, 76b490c0 +local: 384 bytes, 76b490c0 +ref: 384 bytes, 76b490c0 +replace: 384 bytes, 76b490c0 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
