PR #24953 opened by Marcos Ashton (MarcosAsh) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24953 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24953.patch
Three Vulkan decode fixes my nightly GPU job found. vp90-2-05-resize.ivf crashed on close. An exec context held the last reference to a frame whose frames context was already freed. The exec layer now references the frames context too, except in the pools hwcontext_vulkan keeps inside it. vulkan_hevc used the DPB index as the slot index, which can go past maxDpbSlots (16 here). Each picture now gets its own slot, like vulkan_h264. The long-term reference flag was also always 0, since HEVC_FRAME_FLAG_LONG_REF doesn't fit in a 1-bit field. With both, RPS_B_qualcomm_5, RPS_C_ericsson_4, RPS_C_ericsson_5, RPS_D_ericsson_5, RPS_D_ericsson_6, RPS_E_qualcomm_5 and LTRPSPS_A_Qualcomm_1 are now bit-exact. The other 181 HEVC conformance streams are unchanged. Tested on an RTX 5070 Ti (NVIDIA 595.91.07) with HEVC, H.264, VP9 and AV1 conformance streams, Vulkan filters and fate-hwdevice. >From 0a2f08d14e76688217d48b23413be305cc665b12 Mon Sep 17 00:00:00 2001 From: Marcos Ashton Iglesias <[email protected]> Date: Thu, 8 Oct 2026 17:44:08 +0100 Subject: [PATCH 1/3] avutil/vulkan: keep the frames context of a dependency frame alive ff_vk_exec_add_dep_frame() only referenced the frame's buffer. Exec contexts keep their dependencies until their pool is next used or freed, so after a resolution change one could hold the last reference to a frame whose frames context was already freed, and the buffer pool then freed the frame through it. Decoding vp90-2-05-resize.ivf with -hwaccel vulkan -hwaccel_output_format vulkan crashed in vulkan_frame_free() when the decoder was closed. Reference the frames context too, except from the exec pools that hwcontext_vulkan keeps in the frames context itself, which would then never be freed. --- libavutil/hwcontext_vulkan.c | 4 ++++ libavutil/vulkan.c | 11 +++++++++++ libavutil/vulkan.h | 3 +++ 3 files changed, 18 insertions(+) diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c index 63d2ab5c43..3f692af287 100644 --- a/libavutil/hwcontext_vulkan.c +++ b/libavutil/hwcontext_vulkan.c @@ -3390,6 +3390,10 @@ static int vulkan_frames_init(AVHWFramesContext *hwfc) if (err) return err; + fp->compute_exec.owner = hwfc; + fp->upload_exec.owner = hwfc; + fp->download_exec.owner = hwfc; + /* Test to see if allocation will fail */ err = create_frame(hwfc, &f, hwctx->tiling, hwctx->usage, hwctx->img_flags, hwctx->nb_layers, hwctx->create_pnext); diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c index 3987c33080..68337a6807 100644 --- a/libavutil/vulkan.c +++ b/libavutil/vulkan.c @@ -909,6 +909,17 @@ int ff_vk_exec_add_dep_frame(FFVulkanContext *s, FFVkExecContext *e, AVFrame *f, if (!e->buf_deps[e->nb_buf_deps]) return AVERROR(ENOMEM); e->nb_buf_deps++; + + /* The buffer pool frees the frame through its frames context, which + * must outlive it. Deps are released in order, so the frame goes + * first. A pool of that frames context must not keep it alive. */ + if (e->parent->owner != hwfc) { + av_assert1(e->nb_buf_deps < FF_VK_EXEC_MAX_BUF_DEPS); + e->buf_deps[e->nb_buf_deps] = av_buffer_ref(f->hw_frames_ctx); + if (!e->buf_deps[e->nb_buf_deps]) + return AVERROR(ENOMEM); + e->nb_buf_deps++; + } } e->frame_deps[e->nb_frame_deps] = f; diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h index 30850c07d9..f38c30a93b 100644 --- a/libavutil/vulkan.h +++ b/libavutil/vulkan.h @@ -287,6 +287,9 @@ typedef struct FFVkExecPool { /* Registered shaders' data */ FFVulkanShaderData reg_shd[FF_VK_MAX_SHADERS]; int nb_reg_shd; + + /* Frames context the pool belongs to, if any */ + const AVHWFramesContext *owner; } FFVkExecPool; typedef struct FFVulkanContext { -- 2.52.0 >From 99de87850e1dca2d50424180cfd84f2062700394 Mon Sep 17 00:00:00 2001 From: Marcos Ashton Iglesias <[email protected]> Date: Thu, 8 Oct 2026 17:44:08 +0100 Subject: [PATCH 2/3] avcodec/vulkan_hevc: give each picture a DPB slot below maxDpbSlots The index into the DPB was used as the slot index and, with a layered DPB, as the image layer. The DPB has 32 entries and also holds pictures that only wait for output, but the session has maxDpbSlots slots (16 on NVIDIA), so slots and layers past the end were used. Give each picture the lowest slot no reference holds once it is decoded, as vulkan_h264 does since 82a2875192b, and use the slots in the RefPicSet lists too. A picture dropped from the RPS also gives up its slot, so a damaged stream that refers to it again can't bind a slot twice. Missing references generated by the decoder hold no slot and are no longer bound. Fixes RPS_B_qualcomm_5, RPS_C_ericsson_4 and RPS_C_ericsson_5 on NVIDIA. --- libavcodec/vulkan_hevc.c | 92 ++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 36 deletions(-) diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c index 6c0cd56363..d7e4eafab7 100644 --- a/libavcodec/vulkan_hevc.c +++ b/libavcodec/vulkan_hevc.c @@ -115,6 +115,10 @@ static int alloc_hevc_header_structs(FFVulkanDecodeContext *s, typedef struct HEVCVulkanDecodePicture { FFVulkanDecodePicture vp; + /* DPB slot the picture was decoded into, valid while has_slot is set */ + int slot; + int has_slot; + /* Current picture */ StdVideoDecodeH265ReferenceInfo h265_ref; VkVideoDecodeH265DpbSlotInfoKHR vkh265_ref; @@ -134,7 +138,7 @@ static int vk_hevc_fill_pict(AVCodecContext *avctx, HEVCFrame **ref_src, VkVideoPictureResourceInfoKHR *ref, /* Goes in ^ */ VkVideoDecodeH265DpbSlotInfoKHR *vkh265_ref, /* Goes in ^ */ StdVideoDecodeH265ReferenceInfo *h265_ref, /* Goes in ^ */ - HEVCFrame *pic, int is_current, int pic_id) + HEVCFrame *pic, int is_current) { FFVulkanDecodeContext *dec = avctx->internal->hwaccel_priv_data; FFVulkanDecodeShared *ctx = dec->shared_ctx; @@ -163,14 +167,14 @@ static int vk_hevc_fill_pict(AVCodecContext *avctx, HEVCFrame **ref_src, .sType = VK_STRUCTURE_TYPE_VIDEO_PICTURE_RESOURCE_INFO_KHR, .codedOffset = (VkOffset2D){ 0, 0 }, .codedExtent = (VkExtent2D){ pic->f->width, pic->f->height }, - .baseArrayLayer = ctx->common.layered_dpb ? pic_id : 0, + .baseArrayLayer = ctx->common.layered_dpb ? hp->slot : 0, .imageViewBinding = vkpic->view.ref, }; *ref_slot = (VkVideoReferenceSlotInfoKHR) { .sType = VK_STRUCTURE_TYPE_VIDEO_REFERENCE_SLOT_INFO_KHR, .pNext = vkh265_ref, - .slotIndex = pic_id, + .slotIndex = hp->slot, .pPictureResource = ref, }; @@ -708,6 +712,12 @@ static int vk_hevc_create_params(AVCodecContext *avctx, VkVideoSessionParameters return 0; } +static uint8_t vk_hevc_ref_slot(const HEVCFrame *f) +{ + const HEVCVulkanDecodePicture *hp = f->hwaccel_picture_private; + return hp && hp->has_slot ? hp->slot : STD_VIDEO_H265_NO_REFERENCE_PICTURE; +} + static int vk_hevc_start_frame(AVCodecContext *avctx, av_unused const AVBufferRef *buffer_ref, av_unused const uint8_t *buffer, @@ -716,6 +726,8 @@ static int vk_hevc_start_frame(AVCodecContext *avctx, int err; HEVCContext *h = avctx->priv_data; HEVCLayerContext *l = &h->layers[h->cur_layer]; + FFVulkanDecodeContext *dec = avctx->internal->hwaccel_priv_data; + FFVulkanDecodeShared *ctx = dec->shared_ctx; HEVCFrame *pic = h->cur_frame; HEVCVulkanDecodePicture *hp = pic->hwaccel_picture_private; @@ -723,6 +735,28 @@ static int vk_hevc_start_frame(AVCodecContext *avctx, const HEVCPPS *pps = h->pps; const HEVCSPS *sps = pps->sps; int nb_refs = 0; + uint32_t used = 0; + + /* The DPB has more entries than the session has slots, so slots cannot + * follow DPB indices: take the lowest one no reference holds. A picture + * dropped from the RPS loses its slot, so that a damaged stream referring + * to it again cannot bind the slot twice. */ + for (int i = 0; i < FF_ARRAY_ELEMS(l->DPB); i++) { + HEVCVulkanDecodePicture *rp = l->DPB[i].hwaccel_picture_private; + if (!rp) + continue; + if (!(l->DPB[i].flags & (HEVC_FRAME_FLAG_SHORT_REF | HEVC_FRAME_FLAG_LONG_REF))) + rp->has_slot = 0; + else if (rp->has_slot) + used |= 1U << rp->slot; + } + /* ff_ctz(0) is undefined; a fully-set mask means no slot is free */ + hp->slot = ~used ? ff_ctz(~used) : ctx->caps.maxDpbSlots; + if (hp->slot >= ctx->caps.maxDpbSlots) { + av_log(avctx, AV_LOG_ERROR, "Not enough DPB slots for the " + "stream's references\n"); + return AVERROR(ENOTSUP); + } hp->h265pic = (StdVideoDecodeH265PictureInfo) { .flags = (StdVideoDecodeH265PictureInfoFlags) { @@ -750,13 +784,17 @@ static int vk_hevc_start_frame(AVCodecContext *avctx, if (ref == pic) { err = vk_hevc_fill_pict(avctx, NULL, &vp->ref_slot, &vp->ref, - &hp->vkh265_ref, &hp->h265_ref, pic, 1, i); + &hp->vkh265_ref, &hp->h265_ref, pic, 1); if (err < 0) return err; continue; } + /* Missing references generated by the decoder hold no slot */ + if (vk_hevc_ref_slot(ref) == STD_VIDEO_H265_NO_REFERENCE_PICTURE) + continue; + if (nb_refs >= HEVC_MAX_REFS) { av_log(avctx, AV_LOG_ERROR, "Too many reference frames in DPB: %d >= %d\n", @@ -766,7 +804,7 @@ static int vk_hevc_start_frame(AVCodecContext *avctx, err = vk_hevc_fill_pict(avctx, &hp->ref_src[idx], &vp->ref_slots[idx], &vp->refs[idx], &hp->vkh265_refs[idx], - &hp->h265_refs[idx], (HEVCFrame *)ref, 0, i); + &hp->h265_refs[idx], (HEVCFrame *)ref, 0); if (err < 0) return err; @@ -781,38 +819,14 @@ static int vk_hevc_start_frame(AVCodecContext *avctx, } memset(hp->h265pic.RefPicSetStCurrBefore, 0xff, 8); - for (int i = 0; i < h->rps[ST_CURR_BEF].nb_refs; i++) { - HEVCFrame *frame = h->rps[ST_CURR_BEF].ref[i]; - for (int j = 0; j < FF_ARRAY_ELEMS(l->DPB); j++) { - const HEVCFrame *ref = &l->DPB[j]; - if (ref == frame) { - hp->h265pic.RefPicSetStCurrBefore[i] = j; - break; - } - } - } + for (int i = 0; i < h->rps[ST_CURR_BEF].nb_refs; i++) + hp->h265pic.RefPicSetStCurrBefore[i] = vk_hevc_ref_slot(h->rps[ST_CURR_BEF].ref[i]); memset(hp->h265pic.RefPicSetStCurrAfter, 0xff, 8); - for (int i = 0; i < h->rps[ST_CURR_AFT].nb_refs; i++) { - HEVCFrame *frame = h->rps[ST_CURR_AFT].ref[i]; - for (int j = 0; j < FF_ARRAY_ELEMS(l->DPB); j++) { - const HEVCFrame *ref = &l->DPB[j]; - if (ref == frame) { - hp->h265pic.RefPicSetStCurrAfter[i] = j; - break; - } - } - } + for (int i = 0; i < h->rps[ST_CURR_AFT].nb_refs; i++) + hp->h265pic.RefPicSetStCurrAfter[i] = vk_hevc_ref_slot(h->rps[ST_CURR_AFT].ref[i]); memset(hp->h265pic.RefPicSetLtCurr, 0xff, 8); - for (int i = 0; i < h->rps[LT_CURR].nb_refs; i++) { - HEVCFrame *frame = h->rps[LT_CURR].ref[i]; - for (int j = 0; j < FF_ARRAY_ELEMS(l->DPB); j++) { - const HEVCFrame *ref = &l->DPB[j]; - if (ref == frame) { - hp->h265pic.RefPicSetLtCurr[i] = j; - break; - } - } - } + for (int i = 0; i < h->rps[LT_CURR].nb_refs; i++) + hp->h265pic.RefPicSetLtCurr[i] = vk_hevc_ref_slot(h->rps[LT_CURR].ref[i]); hp->h265_pic_info = (VkVideoDecodeH265PictureInfoKHR) { .sType = VK_STRUCTURE_TYPE_VIDEO_DECODE_H265_PICTURE_INFO_KHR, @@ -941,7 +955,13 @@ static int vk_hevc_end_frame(AVCodecContext *avctx) av_log(avctx, AV_LOG_DEBUG, "Decoding frame, %zu bytes, %i slices\n", vp->slices_size, hp->h265_pic_info.sliceSegmentCount); - return ff_vk_decode_frame(avctx, pic->f, vp, rav, rvp); + err = ff_vk_decode_frame(avctx, pic->f, vp, rav, rvp); + if (err < 0) + return err; + + hp->has_slot = 1; + + return 0; } static void vk_hevc_free_frame_priv(AVRefStructOpaque _hwctx, void *data) -- 2.52.0 >From bfaf364f80081b8d64cdbe8bbcfc09c742e9bec1 Mon Sep 17 00:00:00 2001 From: Marcos Ashton Iglesias <[email protected]> Date: Thu, 8 Oct 2026 17:44:08 +0100 Subject: [PATCH 3/3] avcodec/vulkan_hevc: set used_for_long_term_reference HEVC_FRAME_FLAG_LONG_REF is 4, which doesn't fit in the 1 bit field, so long-term references were always marked short-term. Together with the previous commit this fixes RPS_D_ericsson_5, RPS_D_ericsson_6, RPS_E_qualcomm_5 and LTRPSPS_A_Qualcomm_1 on NVIDIA. --- libavcodec/vulkan_hevc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/vulkan_hevc.c b/libavcodec/vulkan_hevc.c index d7e4eafab7..5ac13c4552 100644 --- a/libavcodec/vulkan_hevc.c +++ b/libavcodec/vulkan_hevc.c @@ -152,7 +152,7 @@ static int vk_hevc_fill_pict(AVCodecContext *avctx, HEVCFrame **ref_src, *h265_ref = (StdVideoDecodeH265ReferenceInfo) { .flags = (StdVideoDecodeH265ReferenceInfoFlags) { - .used_for_long_term_reference = pic->flags & HEVC_FRAME_FLAG_LONG_REF, + .used_for_long_term_reference = !!(pic->flags & HEVC_FRAME_FLAG_LONG_REF), .unused_for_reference = 0, }, .PicOrderCntVal = pic->poc, -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
