PR #24955 opened by ffmpeg-devel URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24955 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24955.patch
**Backport:** https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24941 # Summary of changes Fixes an intermittent crash in `av_buffer_replace()` when decoding H.264 with D3D11VA or DXVA2 and frame threading on Windows on Arm. **Problem** `ff_dxva2_common_end_frame()` adds a reference to the decoder to `frame->buf[]`. The H.264 decoder calls the hwaccel `end_frame()` after `ff_thread_finish_setup()`, so the next frame thread may already be copying the same picture in `ff_h264_update_thread_context()` (through `ff_h264_replace_picture()` and `av_frame_replace()`), which reads `frame->buf[]`. `thread.h` does not allow a decoder to change anything that `update_thread_context()` reads after `ff_thread_finish_setup()`; fa77cb258b fixed the same kind of race for `decode_error_flags`. On x86 the race does not show up in practice. On AArch64 the copying thread can see the new `buf[1]` pointer before the contents of the `AVBufferRef` it points to, and then dereferences a NULL `buffer`. Crash dumps show the access violation in `av_buffer_replace()` at address 0x10, the offset of the reference count in `AVBuffer`. The code dates from 70143a3954 (2017). HEVC and VP9 share frames through RefStruct and are not affected. **Fix** Store the decoder reference in `FrameDecodeData.hwaccel_priv` instead. The decode data is attached when the frame buffer is allocated, before `ff_thread_finish_setup()`, and all references to the frame share it, so `update_thread_context()` only takes a new reference to it. `hwaccel_priv_post_process()` adds the decoder reference to `frame->buf[]` in the caller's thread when the frame is returned. nvdec uses the same mechanism for its per-frame state, and videotoolbox moved to it for the same reason in 63d875772d. Visible changes: - Every returned frame now carries the decoder reference. Before, a frame could be returned from a copy made before `end_frame()` had added it. - The second field of a field pair no longer adds a second decoder reference. **Testing** Windows on Arm (aarch64), MSYS2 CLANGARM64, clang 22.1.8, static build of master at a25ba44c0c: ``` ffmpeg -y -thread_type frame+slice -hwaccel d3d11va -i bbb_sunflower_2160p_60fps_normal.mp4 \ -vf scale=1920:1080 -c:v hevc_nvenc -f hevc tmp.hevc ``` The input is Big Buck Bunny 2160p60, H.264 High (`bbb_sunflower_2160p_60fps_normal.mp4` from download.blender.org). - Without this patch: crashes with an access violation (MSYS2 prints `Segmentation fault`). - With this patch: three consecutive runs completed, each with 38074 frames and the same output size (172962 KiB). FATE has no DXVA2/D3D11VA coverage. >From 2af5b6f6c05140aac39ff118a35a3cf9f053c108 Mon Sep 17 00:00:00 2001 From: Diego de Souza <[email protected]> Date: Thu, 8 Oct 2026 15:23:51 +0200 Subject: [PATCH] avcodec/dxva2: fix frame->buf[] data race with frame threading ff_dxva2_common_end_frame() adds a reference to the decoder to frame->buf[], so that the decoder outlives the frames decoded with it. It is called from the hwaccel end_frame() callback, which with frame threading may run after ff_thread_finish_setup(). From that point on, other threads may copy the frame: the H.264 decoder keeps a separate AVFrame per picture in each thread and synchronizes them in update_thread_context() with av_frame_replace(), which reads frame->buf[] while the decoding thread may be writing to it. As noted in fa77cb258b ("avcodec/h264dec: Fix data race when updating decode_error_flags"), a decoding thread must not modify any field that av_frame_ref() copies after ff_thread_finish_setup(). This has been observed as an intermittent access violation in av_buffer_replace() when decoding H.264 with D3D11VA and frame threading on Windows on Arm: the copying thread read a non-NULL frame->buf[1] entry, but saw the fields of the AVBufferRef it points to as zero, and dereferenced the NULL buffer pointer when incrementing the reference count. Store the decoder reference in FrameDecodeData.hwaccel_priv instead, as nvdec does for its per-frame state. The decode data is attached to frame->private_ref when the buffer is allocated, before ff_thread_finish_setup(), and all references to the frame share it, so update_thread_context() only takes a new reference to it. The decoder reference is added to frame->buf[] by hwaccel_priv_post_process(), which runs in the caller's thread when the frame is returned, so returned frames keep the decoder alive. Since the decode data is shared, the second field of a field pair no longer adds a second decoder reference to the frame. Signed-off-by: Diego de Souza <[email protected]> (cherry picked from commit 538d10d1878fb546c3a2645efa56119848f9ccfc) --- libavcodec/dxva2.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c index 22ecd5acaf..3833dcccaa 100644 --- a/libavcodec/dxva2.c +++ b/libavcodec/dxva2.c @@ -889,6 +889,20 @@ static int frame_add_buf(AVFrame *frame, AVBufferRef *ref) return AVERROR(EINVAL); } +static void dxva2_frame_priv_free(void *priv) +{ + AVBufferRef *decoder_ref = priv; + + av_buffer_unref(&decoder_ref); +} + +static int dxva2_frame_post_process(void *logctx, AVFrame *frame) +{ + const FrameDecodeData *fdd = frame->private_ref; + + return frame_add_buf(frame, fdd->hwaccel_priv); +} + int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame, const void *pp, unsigned pp_size, const void *qm, unsigned qm_size, @@ -911,9 +925,24 @@ int ff_dxva2_common_end_frame(AVCodecContext *avctx, AVFrame *frame, FFDXVASharedContext *sctx = DXVA_SHARED_CONTEXT(avctx); if (sctx->decoder_ref) { - result = frame_add_buf(frame, sctx->decoder_ref); - if (result < 0) - return result; + FrameDecodeData *fdd = frame->private_ref; + + /* With frame threading, this may run after ff_thread_finish_setup(), + * when other threads may already be copying this AVFrame, so its + * buf[] array must not be modified here. Store the decoder + * reference in the per-frame decode data, which all references to + * the frame share, and add it to frame->buf[] once the frame is + * output. The second field of a field pair reuses the reference + * stored for the first. */ + if (!fdd->hwaccel_priv) { + AVBufferRef *decoder_ref = av_buffer_ref(sctx->decoder_ref); + if (!decoder_ref) + return AVERROR(ENOMEM); + + fdd->hwaccel_priv = decoder_ref; + fdd->hwaccel_priv_free = dxva2_frame_priv_free; + fdd->hwaccel_priv_post_process = dxva2_frame_post_process; + } } do { -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
