[FFmpeg-devel] [PATCH 2/3] hwcontext_vulkan: add queue and frame locking functions

[Anton Khirnov]

Quoting Lynne (2022-04-03 16:52:24)
> This allows for multiple threads to access the same frame. This is
> unfortunately necessary, as in Vulkan, queues are considered to be
> up to the user to synchronize, while frames often have their layout
> changed upon reading.
> 
> Patch attached.
> 
> 
> From d8bd429859f9dc90325dbd0a7355b21ad5a80b6f Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Sun, 3 Apr 2022 16:44:58 +0200
> Subject: [PATCH 2/3] hwcontext_vulkan: add queue and frame locking functions
> 
> This allows for multiple threads to access the same frame. This is
> unfortunately necessary, as in Vulkan, queues are considered to be
> up to the user to synchronize, while frames often have their layout
> changed upon reading.
> ---
>  libavutil/hwcontext_vulkan.c | 180 ++++++++++++++++++++++++++---------
>  libavutil/hwcontext_vulkan.h |  28 ++++++
>  2 files changed, 164 insertions(+), 44 deletions(-)
> 
> diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
> index 1176858545..5bd0cab7ef 100644
> --- a/libavutil/hwcontext_vulkan.c
> +++ b/libavutil/hwcontext_vulkan.c
> @@ -27,6 +27,7 @@
>  #include <dlfcn.h>
>  #endif
>  
> +#include <pthread.h>

Either make vulkan depend on threads or make this all conditional somehow.

> @@ -4129,7 +4209,19 @@ static int vulkan_frames_derive_to(AVHWFramesContext *dst_fc,
>  
>  AVVkFrame *av_vk_frame_alloc(void)
>  {
> -    return av_mallocz(sizeof(AVVkFrame));
> +    AVVkFrame *f = av_mallocz(sizeof(AVVkFrame));
> +    if (!f)
> +        return NULL;
> +
> +    f->internal = av_mallocz(sizeof(*f->internal));

Doxy says AVVkFrame can be freed with av_free.

> +    if (!f->internal) {
> +        av_free(f);
> +        return NULL;
> +    }
> +
> +    pthread_mutex_init(&f->internal->lock, NULL);
> +
> +    return f;
>  }
>  
>  const HWContextType ff_hwcontext_type_vulkan = {
> diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h
> index ce8a835c6f..5864ae1264 100644
> --- a/libavutil/hwcontext_vulkan.h
> +++ b/libavutil/hwcontext_vulkan.h
> @@ -135,6 +135,19 @@ typedef struct AVVulkanDeviceContext {
>       */
>      int queue_family_decode_index;
>      int nb_decode_queues;
> +
> +    /**
> +     * Locks a queue, preventing other threads from submitting any command
> +     * buffers to this queue.
> +     * If set to NULL, will be set to lavu-internal functions that utilize a
> +     * mutex.

I'd expect some prescription for who calls this and when. Like
"Any users accessing any queues associated with this device MUST call
 this callback before manipulating the queue and unlock_queue() after
 they are done."

Same for lock_frame()

> +     */
> +    void (*lock_queue)(AVHWDeviceContext *ctx, int queue_family, int index);

any reason those are signed?

> +
> +    /**
> +     * Similar to lock_queue(), unlocks a queue. Must only be called after it.
s/similar/complementary/

> +     */
> +    void (*unlock_queue)(AVHWDeviceContext *ctx, int queue_family, int index);
>  } AVVulkanDeviceContext;
>  
>  /**
> @@ -195,6 +208,21 @@ typedef struct AVVulkanFramesContext {
>       * av_hwframe_ctx_init().
>       */
>      AVVkFrameFlags flags;
> +
> +    /**
> +     * Locks a frame, preventing other threads from changing frame properties.
> +     * If set to NULL, will be set to lavu-internal functions that utilize a
> +     * mutex.
> +     * Users SHOULD only ever lock just before command submission in order
> +     * to get accurate frame properties, and unlock immediately after command
> +     * submission without waiting for it to finish.
> +     */
> +    void (*lock_frame)(AVHWFramesContext *fc, AVFrame *f);
> +
> +    /**
> +     * Similar to lock_frame(), unlocks a frame. Must only be called after it.
> +     */
> +    void (*unlock_frame)(AVHWFramesContext *fc, AVFrame *f);
>  } AVVulkanFramesContext;
>  
>  /*
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

-- 
Anton Khirnov