PR #24941 opened by Diego de Souza (ddesouza)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24941
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24941.patch
# 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 48144b54522a209f45442cb76df9a16f128d557d 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]>
---
libavcodec/dxva2.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/libavcodec/dxva2.c b/libavcodec/dxva2.c
index 5817a0e7d7..6919d52680 100644
--- a/libavcodec/dxva2.c
+++ b/libavcodec/dxva2.c
@@ -927,6 +927,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,
@@ -949,9 +963,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]
