[FFmpeg-devel] [PATCH 2/2] hwcontext_vulkan: expose the enabled device features

[Mark Thompson]

On 13/05/2020 16:53, Lynne wrote:
> With this, the puzze of making libplacebo, ffmpeg and any other Vulkan 
> API users interoperable is complete.
> Users of both libraries can initialize one another's contexts without having
> to create a new one.
> > From 28264793295b0d7861527f40fa7c7041a3b34907 Mon Sep 17 00:00:00 2001
> From: Lynne <dev at lynne.ee>
> Date: Wed, 13 May 2020 16:39:00 +0100
> Subject: [PATCH 2/2] hwcontext_vulkan: expose the enabled device features
> 
> With this, the puzze of making libplacebo, ffmpeg and any other Vulkan

The "puzze".

> API users interoperable is complete.
> Users of both libraries can initialize one another's contexts without having
> to create a new one.
> ---
>  libavutil/hwcontext_vulkan.c | 11 +++++++++++
>  libavutil/hwcontext_vulkan.h |  7 +++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
> index 82ceb7013a..d05dd2cf5d 100644
> --- a/libavutil/hwcontext_vulkan.c
> +++ b/libavutil/hwcontext_vulkan.c
> @@ -807,6 +807,7 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
>      AVDictionaryEntry *opt_d;
>      VulkanDevicePriv *p = ctx->internal->priv;
>      AVVulkanDeviceContext *hwctx = ctx->hwctx;
> +    VkPhysicalDeviceFeatures dev_features = { 0 };
>      VkDeviceQueueCreateInfo queue_create_info[3] = {
>          { .sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO, },
>          { .sType = VK_STRUCTURE_TYPE_DEVICE_QUEUE_CREATE_INFO, },
> @@ -815,6 +816,7 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
>  
>      VkDeviceCreateInfo dev_info = {
>          .sType                = VK_STRUCTURE_TYPE_DEVICE_CREATE_INFO,
> +        .pEnabledFeatures     = &hwctx->device_features,
>          .pQueueCreateInfos    = queue_create_info,
>          .queueCreateInfoCount = 0,
>      };
> @@ -839,6 +841,15 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
>      av_log(ctx, AV_LOG_VERBOSE, "    minMemoryMapAlignment:              %li\n",
>             p->props.limits.minMemoryMapAlignment);
>  
> +    vkGetPhysicalDeviceFeatures(hwctx->phys_dev, &dev_features);
> +#define COPY_FEATURE(DST, NAME) (DST).NAME = dev_features.NAME;
> +    COPY_FEATURE(hwctx->device_features, shaderImageGatherExtended)
> +    COPY_FEATURE(hwctx->device_features, shaderStorageImageExtendedFormats)
> +    COPY_FEATURE(hwctx->device_features, fragmentStoresAndAtomics)
> +    COPY_FEATURE(hwctx->device_features, vertexPipelineStoresAndAtomics)
> +    COPY_FEATURE(hwctx->device_features, shaderInt64)
> +#undef COPY_FEATURE

Forgive me if I'm being stupid here, but why can't the user run exactly the same code to find the features themselves?  They do have hwctx->phys_dev.

> +
>      /* Search queue family */
>      if ((err = search_queue_families(ctx, &dev_info)))
>          goto end;
> diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h
> index 9fbe8b9dcb..b73097d514 100644
> --- a/libavutil/hwcontext_vulkan.h
> +++ b/libavutil/hwcontext_vulkan.h
> @@ -94,6 +94,13 @@ typedef struct AVVulkanDeviceContext {
>       */
>      const char * const *enabled_dev_extensions;
>      int nb_enabled_dev_extensions;
> +    /**
> +     * This structure lists all device features that are present and enabled
> +     * during device creation. By default, if present, shaderImageGatherExtended,
> +     * shaderStorageImageExtendedFormats, fragmentStoresAndAtomics, shaderInt64,
> +     * and vertexPipelineStoresAndAtomics are enabled.

And what should an API user set it to?  Do they need to enable that same set of features?

> +     */
> +    VkPhysicalDeviceFeatures device_features;
>  } AVVulkanDeviceContext;
>  
>  /**
> -- 
> 2.26.2

- Mark