[FFmpeg-devel] [PATCH] vulkan: take refs of frames using the regular buffer ref path

[Lynne]

On 02/03/2025 00:02, Lynne wrote:
> This simplifies the code, reduces allocations, and critically, does
> not store references of frames, along with references to hw_frames_ctx.
> The issue was that storing refs to frames while transferring stored
> refs to hw_frames_ctx of frames, and so created a circular dependency,
> which caused the Vulkan device to never be terminated.
> 
> This only stores what it strictly needs as a dependency, and enables
> the frames context to be freed, even while doing asynchronous transfers.
> ---
>   libavutil/vulkan.c | 20 ++++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
> index 1d18bf07c1..2ae619967a 100644
> --- a/libavutil/vulkan.c
> +++ b/libavutil/vulkan.c
> @@ -538,8 +538,6 @@ void ff_vk_exec_discard_deps(FFVulkanContext *s, FFVkExecContext *e)
>               e->frame_locked[j] = 0;
>           }
>           e->frame_update[j] = 0;
> -        if (f->buf[0])
> -            av_frame_free(&e->frame_deps[j]);
>       }
>       e->nb_frame_deps = 0;
>   
> @@ -700,6 +698,7 @@ int ff_vk_exec_add_dep_frame(FFVulkanContext *s, FFVkExecContext *e, AVFrame *f,
>       uint8_t *frame_locked;
>       uint8_t *frame_update;
>       AVFrame **frame_deps;
> +    AVBufferRef **buf_deps;
>       VkImageLayout *layout_dst;
>       uint32_t *queue_family_dst;
>       VkAccessFlagBits *access_dst;
> @@ -722,12 +721,21 @@ int ff_vk_exec_add_dep_frame(FFVulkanContext *s, FFVkExecContext *e, AVFrame *f,
>       ARR_REALLOC(e, frame_update, &e->frame_update_alloc_size, e->nb_frame_deps);
>       ARR_REALLOC(e, frame_deps,   &e->frame_deps_alloc_size,   e->nb_frame_deps);
>   
> -    e->frame_deps[e->nb_frame_deps] = f->buf[0] ? av_frame_clone(f) : f;
> -    if (!e->frame_deps[e->nb_frame_deps]) {
> -        ff_vk_exec_discard_deps(s, e);
> -        return AVERROR(ENOMEM);
> +    /* prepare_frame in hwcontext_vulkan.c uses the regular frame management
> +     * code but has no frame yet, and it doesn't need to actually store a ref
> +     * to the frame. */
> +    if (f->buf[0]) {
> +        ARR_REALLOC(e, buf_deps, &e->buf_deps_alloc_size, e->nb_buf_deps);
> +        e->buf_deps[e->nb_buf_deps] = av_buffer_ref(f->buf[0]);
> +        if (!e->buf_deps[e->nb_buf_deps]) {
> +            ff_vk_exec_discard_deps(s, e);
> +            return AVERROR(ENOMEM);
> +        }
> +        e->nb_buf_deps++;
>       }
>   
> +    e->frame_deps[e->nb_frame_deps] = f;
> +
>       vkfc->lock_frame(hwfc, vkf);
>       e->frame_locked[e->nb_frame_deps] = 1;
>       e->frame_update[e->nb_frame_deps] = 0;

Pushed.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xA2FEA5F03F034464.asc
Type: application/pgp-keys
Size: 624 bytes
Desc: OpenPGP public key
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250303/e75a0c36/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature.asc
Type: application/pgp-signature
Size: 236 bytes
Desc: OpenPGP digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20250303/e75a0c36/attachment.sig>