May 10, 2020, 14:33 by s...@jkqxz.net:

> On 10/05/2020 11:54, Lynne wrote:
>
>>  */
>>  VkInstance inst;
>> +    /**
>> +     * Enabled instance extensions. By default, VK_KHR_surface is enabled 
>> if found.
>> +     */
>>
>
> Clarify how this should be set by the user if they are supplying the 
> instance?  (From code, looks like it's not used.)
>

Done.



>> +    const char * const *enabled_inst_extensions;
>> +    int num_enabled_inst_extensions;
>>  /**
>>  * Physical device
>>  */
>> @@ -50,6 +55,13 @@ typedef struct AVVulkanDeviceContext {
>>  * Active device
>>  */
>>  VkDevice act_dev;
>> +    /**
>> +     * Enabled device extensions. By default, VK_KHR_external_memory_fd,
>> +     * VK_EXT_external_memory_dma_buf, VK_EXT_image_drm_format_modifier and
>> +     * VK_KHR_external_semaphore_fd are enabled if found.
>> +     */
>>
>
> Clarify how this should be set by the user if they are supplying the device?  
> (From code, looks like it's required.)
>

Done (it isn't required, and can be NULL).



>> +    const char * const *enabled_dev_extensions;
>> +    int num_enabled_dev_extensions;
>>  /**
>>  * Queue family index for graphics
>>  * @note av_hwdevice_create() will set all 3 queue indices if unset
>> -- 
>> 2.26.2
>>
>
> I think you need to put the new fields at the end to avoid the worst ABI 
> break (upgrade libavutil and crash trying to use the device fields because 
> they've moved).
>
> The API change with the new field being required is ugly given that it's 
> months after the usual cutoff, but the effect hasn't changed (they wouldn't 
> have had any extensions) so it's probably ok.  Needs a notice in APIchanges, 
> though.
>

Done, with a minor lavu bump and an APIchanges notice. Looks tidier with the 
fields at the bottom anyway.

Patch attached.

>From 6ecc3547bcfcc450c8ffe8d93a3040fd863f6288 Mon Sep 17 00:00:00 2001
From: Lynne <d...@lynne.ee>
Date: Sun, 10 May 2020 11:47:50 +0100
Subject: [PATCH 2/3] hwcontext_vulkan: expose enabled device and instance
 extensions

This solves a huge oversight - it lets users reliably use their own
AVVulkanDeviceContext. Otherwise, the extensions supplied and enabled
are not discoverable by anything outside of hwcontext_vulkan.
---
 doc/APIchanges               |  4 ++++
 libavutil/hwcontext_vulkan.c | 40 ++++++++++++++++++++++++++++--------
 libavutil/hwcontext_vulkan.h | 21 ++++++++++++++++++-
 libavutil/version.h          |  2 +-
 4 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index b3e7e89412..75cfdb08b0 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,10 @@ libavutil:     2017-10-21
 
 API changes, most recent first:
 
+2020-05-10 - xxxxxxxxxx - lavu 56.44.100 - hwcontext_vulkan.h
+  Add enabled_inst_extensions, num_enabled_inst_extensions, enabled_dev_extensions
+  and num_enabled_dev_extensions fields to AVVulkanDeviceContext
+
 2020-04-22 - 0e1db79e37 - lavc 58.81.100 - packet.h
                         - lavu 56.43.100 - dovi_meta.h
   Add AV_PKT_DATA_DOVI_CONF and AVDOVIDecoderConfigurationRecord.
diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index c853e2f502..fad8c67818 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -417,15 +417,13 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
     /* Try to create the instance */
     ret = vkCreateInstance(&inst_props, hwctx->alloc, &hwctx->inst);
 
-    /* Free used memory */
-    for (int i = 0; i < inst_props.enabledExtensionCount; i++)
-        av_free((void *)inst_props.ppEnabledExtensionNames[i]);
-    av_free((void *)inst_props.ppEnabledExtensionNames);
-
     /* Check for errors */
     if (ret != VK_SUCCESS) {
         av_log(ctx, AV_LOG_ERROR, "Instance creation failure: %s\n",
                vk_ret2str(ret));
+        for (int i = 0; i < inst_props.enabledExtensionCount; i++)
+            av_free((void *)inst_props.ppEnabledExtensionNames[i]);
+        av_free((void *)inst_props.ppEnabledExtensionNames);
         return AVERROR_EXTERNAL;
     }
 
@@ -448,6 +446,9 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
                                            hwctx->alloc, &p->debug_ctx);
     }
 
+    hwctx->enabled_inst_extensions = inst_props.ppEnabledExtensionNames;
+    hwctx->num_enabled_inst_extensions = inst_props.enabledExtensionCount;
+
     return 0;
 }
 
@@ -753,6 +754,14 @@ static void vulkan_device_free(AVHWDeviceContext *ctx)
     }
 
     vkDestroyInstance(hwctx->inst, hwctx->alloc);
+
+    for (int i = 0; i < hwctx->num_enabled_inst_extensions; i++)
+        av_free((void *)hwctx->enabled_inst_extensions[i]);
+    av_free((void *)hwctx->enabled_inst_extensions);
+
+    for (int i = 0; i < hwctx->num_enabled_dev_extensions; i++)
+        av_free((void *)hwctx->enabled_dev_extensions[i]);
+    av_free((void *)hwctx->enabled_dev_extensions);
 }
 
 static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
@@ -813,13 +822,12 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
     ret = vkCreateDevice(hwctx->phys_dev, &dev_info, hwctx->alloc,
                          &hwctx->act_dev);
 
-    for (int i = 0; i < dev_info.enabledExtensionCount; i++)
-        av_free((void *)dev_info.ppEnabledExtensionNames[i]);
-    av_free((void *)dev_info.ppEnabledExtensionNames);
-
     if (ret != VK_SUCCESS) {
         av_log(ctx, AV_LOG_ERROR, "Device creation failure: %s\n",
                vk_ret2str(ret));
+        for (int i = 0; i < dev_info.enabledExtensionCount; i++)
+            av_free((void *)dev_info.ppEnabledExtensionNames[i]);
+        av_free((void *)dev_info.ppEnabledExtensionNames);
         err = AVERROR_EXTERNAL;
         goto end;
     }
@@ -829,6 +837,9 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
     if (opt_d)
         p->use_linear_images = strtol(opt_d->value, NULL, 10);
 
+    hwctx->enabled_dev_extensions = dev_info.ppEnabledExtensionNames;
+    hwctx->num_enabled_dev_extensions = dev_info.enabledExtensionCount;
+
 end:
     return err;
 }
@@ -840,6 +851,17 @@ static int vulkan_device_init(AVHWDeviceContext *ctx)
     AVVulkanDeviceContext *hwctx = ctx->hwctx;
     VulkanDevicePriv *p = ctx->internal->priv;
 
+    /* Set device extension flags */
+    for (int i = 0; i < hwctx->num_enabled_dev_extensions; i++) {
+        for (int j = 0; j < FF_ARRAY_ELEMS(optional_device_exts); j++) {
+            if (!strcmp(hwctx->enabled_dev_extensions[i],
+                        optional_device_exts[j].name)) {
+                p->extensions |= optional_device_exts[j].flag;
+                break;
+            }
+        }
+    }
+
     vkGetPhysicalDeviceQueueFamilyProperties(hwctx->phys_dev, &queue_num, NULL);
     if (!queue_num) {
         av_log(ctx, AV_LOG_ERROR, "Failed to get queues!\n");
diff --git a/libavutil/hwcontext_vulkan.h b/libavutil/hwcontext_vulkan.h
index ebc28916f3..909f88a7e4 100644
--- a/libavutil/hwcontext_vulkan.h
+++ b/libavutil/hwcontext_vulkan.h
@@ -39,7 +39,7 @@ typedef struct AVVulkanDeviceContext {
      */
     const VkAllocationCallbacks *alloc;
     /**
-     * Instance
+     * Vulkan instance. Must be at least version 1.1.
      */
     VkInstance inst;
     /**
@@ -65,6 +65,25 @@ typedef struct AVVulkanDeviceContext {
      * Queue family index for compute ops
      */
     int queue_family_comp_index;
+    /**
+     * Enabled instance extensions. By default, VK_KHR_surface is enabled if found.
+     * If supplying your own device context, set this to an array of strings, with
+     * each entry containing the specified Vulkan extension string to enable.
+     * Duplicates are possible and accepted.
+     * If no extensions are enabled, set these fields to NULL, and 0 respectively.
+     */
+    const char * const *enabled_inst_extensions;
+    int num_enabled_inst_extensions;
+    /**
+     * Enabled device extensions. By default, VK_KHR_external_memory_fd,
+     * VK_EXT_external_memory_dma_buf, VK_EXT_image_drm_format_modifier and
+     * VK_KHR_external_semaphore_fd are enabled if found.
+     * If supplying your own device context, these fields takes the same format as
+     * the above fields, with the same conditions that duplicates are possible
+     * and accepted, and that NULL and 0 respectively means no extensions are enabled.
+     */
+    const char * const *enabled_dev_extensions;
+    int num_enabled_dev_extensions;
 } AVVulkanDeviceContext;
 
 /**
diff --git a/libavutil/version.h b/libavutil/version.h
index ea9363e8e9..48d8a38c42 100644
--- a/libavutil/version.h
+++ b/libavutil/version.h
@@ -79,7 +79,7 @@
  */
 
 #define LIBAVUTIL_VERSION_MAJOR  56
-#define LIBAVUTIL_VERSION_MINOR  43
+#define LIBAVUTIL_VERSION_MINOR  44
 #define LIBAVUTIL_VERSION_MICRO 100
 
 #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
-- 
2.26.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to