PR #25560 opened by JuliusBairaktaris URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25560 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25560.patch
`amf_receive_frame()` stores the int result of `amf_amfsurface_to_avframe()` in an `AMF_RESULT`. `AMF_RESULT` has no negative values, so GCC and Clang make it unsigned, and the `ret >= 0` check always passes. A frame that failed to convert is returned as if it had decoded: without its HDR side data, or without its data or buffer after an allocation failure. The same mistake was fixed for the `amf_init_frames_context()` call for #21123. Fixing the check makes the fail path reachable, and two more problems show up there: - Once the surface is wrapped in `frame->buf[0]`, the frame owns it, but the fail path releases it as well. Forgejo_Fairy flagged this on #25525. It is now released only while no buffer owns it, and the frame is unreferenced. - `amf_decode_frame()` answers an unexpected receive result with `AVERROR(EAGAIN)`. That is not allowed while draining, and the ffmpeg CLI aborts with `Assertion pkt failed`. It now returns `AVERROR_EXTERNAL`. Tested on Windows 11, RX 9070 XT, driver 32.0.32015.7029: - With a temporary change that makes every third HDR metadata attachment fail while decoding a 30-frame HEVC stream: before, all 30 frames came out and no error was reported. Now the 10 failing frames are reported as decoding errors, and the other 20 come out with unchanged content and pts. - Without that change, `h264_amf`, `hevc_amf` (8 and 10 bit), `av1_amf` and `vp9_amf` give unchanged framemd5 with software and AMF-surface output. Independent of #25525, #25146 and #25147, and merges cleanly with each of them. Written with Claude Opus 5.5 in Claude Code; `Assisted-by` trailer on the commit. >From 96e870dd0d948875844e00999196f9f5e6c85b9c Mon Sep 17 00:00:00 2001 From: Julius Bairaktaris <[email protected]> Date: Wed, 28 Oct 2026 14:48:14 +0100 Subject: [PATCH] avcodec/amfdec: handle frame conversion failures amf_receive_frame() stores the int result of amf_amfsurface_to_avframe() in an AMF_RESULT. AMF_RESULT has no negative values, so GCC and Clang make it unsigned, the "ret >= 0" check always passes and a frame that failed to convert is returned as decoded, e.g. without its data or its buffer after an allocation failure. Keep the result in an int, as was done for the amf_init_frames_context() call for #21123. That makes the fail path reachable, which has two problems of its own: - Once the surface is wrapped in frame->buf[0], the frame owns it, but the fail path releases it as well. Release it only while no buffer owns it, and unref the frame. - amf_decode_frame() answers an unexpected receive result with AVERROR(EAGAIN). While draining, that is not allowed, and the ffmpeg CLI aborts with "Assertion pkt failed". Return AVERROR_EXTERNAL. Tested on Windows 11, RX 9070 XT, driver 32.0.32015.7029, with a temporary change that makes every third HDR metadata attachment fail while decoding a 30-frame HEVC stream: before, all 30 frames came out and no error was reported; now the 10 failing frames are reported as decoding errors, and the other 20 come out unchanged. Without that change, h264_amf, hevc_amf (8 and 10 bit), av1_amf and vp9_amf give unchanged framemd5 with software and AMF-surface output. Assisted-by: Claude Opus 5.5 Signed-off-by: Julius Bairaktaris <[email protected]> --- libavcodec/amfdec.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libavcodec/amfdec.c b/libavcodec/amfdec.c index 00a17360f55..6ddeab23345 100644 --- a/libavcodec/amfdec.c +++ b/libavcodec/amfdec.c @@ -583,6 +583,7 @@ static AMF_RESULT amf_receive_frame(AVCodecContext *avctx, AVFrame *frame) AMF_RESULT ret = AMF_OK; AMFSurface *surface = NULL; AMFData *data_out = NULL; + int err; ret = ctx->decoder->pVtbl->QueryOutput(ctx->decoder, &data_out); if (ret != AMF_OK && ret != AMF_REPEAT) { @@ -599,15 +600,14 @@ static AMF_RESULT amf_receive_frame(AVCodecContext *avctx, AVFrame *frame) data_out = NULL; } - ret = amf_amfsurface_to_avframe(avctx, surface, frame); - AMF_GOTO_FAIL_IF_FALSE(avctx, ret >= 0, AMF_FAIL, "Failed to convert AMFSurface to AVFrame = %d\n", ret); + err = amf_amfsurface_to_avframe(avctx, surface, frame); + AMF_GOTO_FAIL_IF_FALSE(avctx, err >= 0, AMF_FAIL, "Failed to convert AMFSurface to AVFrame = %d\n", err); return AMF_OK; fail: - if (surface) { + if (surface && !frame->buf[0]) surface->pVtbl->Release(surface); - surface = NULL; - } + av_frame_unref(frame); return ret; } @@ -773,6 +773,7 @@ static int amf_decode_frame(AVCodecContext *avctx, struct AVFrame *frame) return AVERROR_EOF; } else { av_log(avctx, AV_LOG_ERROR, "Unknown result from QueryOutput %d\n", res); + return AVERROR_EXTERNAL; } return got_frame ? 0 : AVERROR(EAGAIN); } -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
