PR #25030 opened by Diego de Souza (ddesouza) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25030 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25030.patch
### Problem With `-export_side_data +venc_params`, h264dec attaches `AV_FRAME_DATA_VIDEO_ENC_PARAMS` to every output frame. It builds this data from `qscale_table`, and only the software macroblock decoding (h264_cabac.c / h264_cavlc.c) fills that table. When a hardware accelerator decodes the picture, `ff_h264_execute_decode_slices()` returns early. The table is allocated from a pool that is not zeroed and is never written. Every hardware-decoded frame therefore carries meaningless per-macroblock QP values, often stale ones left over from earlier pictures. ### Fix Export the parameters only for pictures that were decoded in software. The check looks at the picture's own pixel format (`AV_PIX_FMT_FLAG_HWACCEL`), not at `avctx->hwaccel`. The check has to be per picture because `ff_h264_flush_change()` keeps the delayed pictures when a new SPS makes `get_format()` run again. If that switches between software and hardware decoding, for example a 4:2:0 → 4:4:4 change that the hwaccel cannot decode, pictures decoded in one mode are output in the other. A decoder-wide check gets exactly those pictures wrong. Other per-picture markers were considered and rejected: - `hwaccel_picture_private` is NULL for nvdec and videotoolbox. - `hw_frames_ctx` is not reliable, because videotoolbox attaches it in post-processing. The film grain path is not affected. Film grain is applied only when there is no hwaccel, and the grain frame copies the source frame's format. The missing-field duplication in `finalize_frame()` uses the same decoder-wide check and is left unchanged. ### Testing - `make fate-h264-encparams` passes, so the software output is unchanged. - NVDEC (RTX 5090), through the decode API, with 1 thread and with automatic threading: - **Software-only and NVDEC-only decoding:** software frames carry the parameters. NVDEC frames (CUDA and downloaded) carry none. With the export flag off, no frames carry them. - **Switches between software and NVDEC:** the input is concatenated x264 4:2:0 / 4:4:4 Annex B streams with B-pyramid, so delayed pictures cross each switch. I tested NVDEC → software, software → NVDEC, and NVDEC → software → NVDEC. Every frame carries the parameters exactly when it was decoded in software. Signed-off-by: Diego de Souza <[email protected]> From 446a1b21ab2cb223759b9244814b910056dd50ad Mon Sep 17 00:00:00 2001 From: Diego de Souza <[email protected]> Date: Fri, 9 Oct 2026 17:28:09 +0200 Subject: [PATCH] avcodec/h264dec: do not export encoding parameters for hwaccel pictures h264_export_enc_params() builds AV_FRAME_DATA_VIDEO_ENC_PARAMS from the picture's qscale_table, which is only written by the software macroblock decoding (h264_cabac.c / h264_cavlc.c). With a hardware accelerator the table is allocated but never filled, so every frame carried meaningless per-macroblock QP values. Only export them for pictures decoded in software. Test the picture's own pixel format rather than avctx->hwaccel: ff_h264_flush_change() keeps the delayed pictures when a new SPS forces get_format() to run again, so pictures decoded before a switch between software and hardware decoding can be output after it. The missing-field duplication in finalize_frame() uses the same avctx->hwaccel check and is left unchanged. Signed-off-by: Diego de Souza <[email protected]> --- libavcodec/h264dec.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c index 597bdae548..1e0b6171ae 100644 --- a/libavcodec/h264dec.c +++ b/libavcodec/h264dec.c @@ -35,6 +35,7 @@ #include "libavutil/imgutils.h" #include "libavutil/mem.h" #include "libavutil/opt.h" +#include "libavutil/pixdesc.h" #include "libavutil/thread.h" #include "libavutil/video_enc_params.h" @@ -944,6 +945,7 @@ static int h264_export_enc_params(AVFrame *f, const H264Picture *p) static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp) { + const AVPixFmtDescriptor *desc; int ret; ret = av_frame_ref(dst, srcp->needs_fg ? srcp->f_grain : srcp->f); @@ -966,7 +968,13 @@ static int output_frame(H264Context *h, AVFrame *dst, H264Picture *srcp) if (srcp->sei_recovery_frame_cnt == 0) dst->flags |= AV_FRAME_FLAG_KEY; - if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) { + /* The per-macroblock QP values come from qscale_table, which only the + * software macroblock decoding fills. Check the picture rather than + * avctx->hwaccel: delayed pictures can be output after get_format() + * switched between software and hardware decoding. */ + desc = av_pix_fmt_desc_get(srcp->f->format); + if ((h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS) && + desc && !(desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) { ret = h264_export_enc_params(dst, srcp); if (ret < 0) goto fail; -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
