PR #25381 opened by voxeljorge URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25381 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25381.patch
qsvdec queues input packets in its own FIFO (`packet_fifo`, `buffer_pkt`) and can return a frame while it still holds data from an older packet. The surface that data is decoded into gets its properties through `ff_get_buffer()`, which copies them from `avctx->internal->last_pkt_props`: the packet libavcodec handed the decoder most recently, not the one being decoded. The frame then carries the newer packet's properties, including `opaque`/`opaque_ref` when `AV_CODEC_FLAG_COPY_OPAQUE` is set. pts is not affected, because qsvdec sets it from the surface's timestamp on output. This patch passes the packet being decoded down `qsv_decode()` → `get_surface()` → `alloc_frame()`, and calls `ff_decode_frame_props_from_pkt()` with it once the buffer is allocated. A flush packet carries no data, so frames allocated while draining keep the current behavior. **How it was found.** We stamp each packet with its read time through `opaque` and read it back from the decoded frame. With h264_qsv on H.264 with B-frames, every IDR frame came out with the next packet's time. **Testing.** A test serves 225 frames of H.264 in Matroska over loopback TCP, one packet per write, at irregular 70–190 ms intervals, so every packet's read time is known and distinct. It checks each frame's `opaque` against its own packet's write time. Run on an Intel iGPU (8086:a7ad, iHD driver, oneVPL) with the same change applied to n8.0: | Decoder | Without the patch | With the patch | | --- | --- | --- | | h264_qsv, no B-frames | 168 of 168 frames | 168 of 168 | | h264_qsv, B-frames | 166 of 172: every IDR had the next packet's `opaque` | 172 of 172 | | h264 (software), B-frames | 172 of 172 | 172 of 172 | On master: compiles with `--enable-libvpl` (gcc, `-Wall`, no warnings), and `tools/patcheck` reports only the changelog note. `make fate` was not run; FATE does not exercise qsvdec. **Known limitation.** `ff_decode_frame_props_from_pkt()` adds side data only for types the frame does not already have. Side data that `ff_get_buffer()` copied from the wrong packet can therefore survive. A more complete fix would have qsvdec set `FF_CODEC_CAP_SETS_FRAME_PROPS` and own its frame properties entirely, at the cost of handling the drain case itself. I can rework it that way if reviewers prefer. AI was used to help generate and test this PR, but I have reviewed the code myself. From b20654cea6091fc82eb0371317a88d86e15c3d07 Mon Sep 17 00:00:00 2001 From: Jorge Villatoro <[email protected]> Date: Fri, 23 Oct 2026 05:34:26 +0000 Subject: [PATCH] avcodec/qsvdec: set frame properties from the packet being decoded qsvdec queues input packets in its own FIFO (packet_fifo, buffer_pkt) and can return a frame while it still holds data from an older packet. The surface that data is decoded into gets its properties through ff_get_buffer(), which copies them from avctx->internal->last_pkt_props: the packet libavcodec handed the decoder most recently, not the one being decoded. The frame then carries the newer packet's duration, discard flag and, with AV_CODEC_FLAG_COPY_OPAQUE, its opaque and opaque_ref. pts is not affected: it is set from the surface's timestamp on output. With h264_qsv on H.264 with B-frames, every IDR frame came out with the opaque of the packet after it. Stamping each packet's read time into opaque and streaming 225 frames over TCP, one packet per write: before, 166 of 172 frames checked carried their own packet's time; after, all 172 did. Pass the packet being decoded down to alloc_frame() and set the frame properties from it once the buffer is allocated. A flush packet carries none, so frames allocated while draining keep the generic ones. Signed-off-by: Jorge Villatoro <[email protected]> --- libavcodec/qsvdec.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c index 990e46ea1d7..38cd12238a3 100644 --- a/libavcodec/qsvdec.c +++ b/libavcodec/qsvdec.c @@ -491,7 +491,8 @@ static int qsv_decode_header(AVCodecContext *avctx, QSVContext *q, return 0; } -static int alloc_frame(AVCodecContext *avctx, QSVContext *q, QSVFrame *frame) +static int alloc_frame(AVCodecContext *avctx, QSVContext *q, QSVFrame *frame, + const AVPacket *pkt) { int ret; @@ -503,6 +504,17 @@ static int alloc_frame(AVCodecContext *avctx, QSVContext *q, QSVFrame *frame) if (ret < 0) return ret; + /* The buffer took its properties from the packet the generic decode layer + * pulled last, but this decoder queues packets in packet_fifo and + * buffer_pkt, so that need not be the packet being decoded into this + * surface. Take them from the packet that is. A flush packet carries + * none, so frames allocated while draining keep the generic ones. */ + if (pkt->size) { + ret = ff_decode_frame_props_from_pkt(avctx, frame->frame, pkt); + if (ret < 0) + return ret; + } + if (frame->frame->format == AV_PIX_FMT_QSV) { frame->surface = *(mfxFrameSurface1*)frame->frame->data[3]; } else { @@ -572,7 +584,8 @@ static void qsv_clear_unused_frames(QSVContext *q) } } -static int get_surface(AVCodecContext *avctx, QSVContext *q, mfxFrameSurface1 **surf) +static int get_surface(AVCodecContext *avctx, QSVContext *q, mfxFrameSurface1 **surf, + const AVPacket *pkt) { QSVFrame *frame, **last; int ret; @@ -583,7 +596,7 @@ static int get_surface(AVCodecContext *avctx, QSVContext *q, mfxFrameSurface1 ** last = &q->work_frames; while (frame) { if (!frame->used) { - ret = alloc_frame(avctx, q, frame); + ret = alloc_frame(avctx, q, frame, pkt); if (ret < 0) return ret; *surf = &frame->surface; @@ -604,7 +617,7 @@ static int get_surface(AVCodecContext *avctx, QSVContext *q, mfxFrameSurface1 ** } *last = frame; - ret = alloc_frame(avctx, q, frame); + ret = alloc_frame(avctx, q, frame, pkt); if (ret < 0) return ret; @@ -789,7 +802,7 @@ static int qsv_decode(AVCodecContext *avctx, QSVContext *q, } do { - ret = get_surface(avctx, q, &insurf); + ret = get_surface(avctx, q, &insurf, avpkt); if (ret < 0) { av_freep(&sync); return ret; -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
