[FFmpeg-devel] [PATCH 1/2] hwcontext_vulkan: let users enable device and instance extensions using options

[Lynne]

May 10, 2020, 14:14 by sw at jkqxz.net:

> On 10/05/2020 11:51, Lynne wrote:
>
>> Also documents all options supported by the hwdevice. 
>> This lets users enable all extensions they need without writing their own
>> instance initialization code.
>>
>> Patch attached.
>>
>>
>> From cf91acc2a907a5ff7af753bf3b2ab495dbc37db0 Mon Sep 17 00:00:00 2001
>> From: Lynne <dev at lynne.ee>
>> Date: Sun, 10 May 2020 11:26:40 +0100
>> Subject: [PATCH 1/2] hwcontext_vulkan: let users enable device and instance
>>  extensions using options
>>
>> Also documents all options supported by the hwdevice.
>> This lets users enable all extensions they need without writing their own
>> instance initialization code.
>> ---
>>  doc/ffmpeg.texi              | 15 +++++++++++++++
>>  libavutil/hwcontext_vulkan.c | 31 ++++++++++++++++++++++++++++---
>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
>> index 29753f06ca..d2a30fc868 100644
>> --- a/doc/ffmpeg.texi
>> +++ b/doc/ffmpeg.texi
>> @@ -1034,6 +1034,18 @@ If @var{device} is an integer, it selects the device by its index in a
>>  system-dependent list of devices.  If @var{device} is any other string, it
>>  selects the first device with a name containing that string as a substring.
>>  
>> +The following options are recognized:
>> + at table @option
>> + at item debug
>> +If set to 1, enables the validation layer, if installed.
>> + at item linear_images
>> +If set to 1, images allocated by the hwcontext will be linear and locally mappable.
>> + at item instance_extensions
>> +A vertical bar separated list of additional instance extensions to enable.
>> + at item device_extensions
>> +A vertical bar separated list of additional device extensions to enable.
>> + at end table
>> +
>>  Examples:
>>  @table @emph
>>  @item -init_hw_device vulkan:1
>> @@ -1041,6 +1053,9 @@ Choose the second device on the system.
>>  
>>  @item -init_hw_device vulkan:RADV
>>  Choose the first device with a name containing the string @emph{RADV}.
>> +
>> + at item -init_hw_device vulkan:0,instance_extensions=VK_KHR_wayland_surface|VK_KHR_xcb_surface
>> +Choose the first device and enable the Wayland and XCB instance extensions.
>>  @end table
>>  
>>  @end table
>> diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
>> index 43e7cddbc5..a35c1d3a4f 100644
>> --- a/libavutil/hwcontext_vulkan.c
>> +++ b/libavutil/hwcontext_vulkan.c
>> @@ -261,7 +261,7 @@ static VkBool32 vk_dbg_callback(VkDebugUtilsMessageSeverityFlagBitsEXT severity,
>>  return 0;
>>  }
>>  
>> -static int check_extensions(AVHWDeviceContext *ctx, int dev,
>> +static int check_extensions(AVHWDeviceContext *ctx, int dev, AVDictionary *opts,
>>  const char * const **dst, uint32_t *num, int debug)
>>  {
>>  const char *tstr;
>> @@ -274,12 +274,14 @@ static int check_extensions(AVHWDeviceContext *ctx, int dev,
>>  int optional_exts_num;
>>  uint32_t sup_ext_count;
>>  VkExtensionProperties *sup_ext;
>> +    AVDictionaryEntry *user_exts = NULL;
>>  const VulkanOptExtension *optional_exts;
>>  
>>  if (!dev) {
>>  mod = "instance";
>>  optional_exts = optional_instance_exts;
>>  optional_exts_num = FF_ARRAY_ELEMS(optional_instance_exts);
>> +        user_exts = av_dict_get(opts, "instance_extensions", NULL, 0);w
>>  vkEnumerateInstanceExtensionProperties(NULL, &sup_ext_count, NULL);
>>  sup_ext = av_malloc_array(sup_ext_count, sizeof(VkExtensionProperties));
>>  if (!sup_ext)
>> @@ -289,6 +291,7 @@ static int check_extensions(AVHWDeviceContext *ctx, int dev,
>>  mod = "device";
>>  optional_exts = optional_device_exts;
>>  optional_exts_num = FF_ARRAY_ELEMS(optional_device_exts);
>> +        user_exts = av_dict_get(opts, "device_extensions", NULL, 0);
>>  vkEnumerateDeviceExtensionProperties(hwctx->phys_dev, NULL,
>>  &sup_ext_count, NULL);
>>  sup_ext = av_malloc_array(sup_ext_count, sizeof(VkExtensionProperties));
>> @@ -345,6 +348,28 @@ static int check_extensions(AVHWDeviceContext *ctx, int dev,
>>  }
>>  }
>>  
>> +    if (user_exts) {
>> +        char *save, *token = av_strtok(user_exts->value, "|", &save);
>>
>
> Using strtok modifies the string, but that isn't allowed.  Maybe strdup() first?
>

Didn't know it did. Changed locally.
Had to strdup and then free all values, since previously the array was filled with
static strings only.



>> +        while (token) {
>> +            found = 0;
>> +            for (int j = 0; j < sup_ext_count; j++) {
>> +                if (!strcmp(token, sup_ext[j].extensionName)) {
>> +                    found = 1;
>> +                    break;
>> +                }
>> +            }
>> +            if (found) {
>> +                ADD_VAL_TO_LIST(extension_names, extensions_found, token);
>> +            } else {
>> +                av_log(ctx, AV_LOG_ERROR, "%s extension \"%s\" not found!\n",
>> +                       mod, token);
>> +                err = AVERROR(EINVAL);
>> +                goto end;
>> +            }
>> +            token = av_strtok(NULL, "|", &save);
>> +        }
>> +    }
>> +
>>  *dst = extension_names;
>>  *num = extensions_found;
>>  
>> @@ -376,7 +401,7 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
>>  };
>>  
>>  /* Check for present/missing extensions */
>> -    err = check_extensions(ctx, 0, &inst_props.ppEnabledExtensionNames,
>> +    err = check_extensions(ctx, 0, opts, &inst_props.ppEnabledExtensionNames,
>>  &inst_props.enabledExtensionCount, debug_mode);
>>  if (err < 0)
>>  return err;
>> @@ -777,7 +802,7 @@ static int vulkan_device_create_internal(AVHWDeviceContext *ctx,
>>  if ((err = search_queue_families(ctx, &dev_info)))
>>  goto end;
>>  
>> -    if ((err = check_extensions(ctx, 1, &dev_info.ppEnabledExtensionNames,
>> +    if ((err = check_extensions(ctx, 1, opts, &dev_info.ppEnabledExtensionNames,
>>  &dev_info.enabledExtensionCount, 0)))
>>  goto end;
>>  
>> -- 
>> 2.26.2
>>
>
> A nitpick: '+' separation might be nicer than '|'?  It's a combination of values which should be combined like "plus" (as in "flags=a+b+c"), rather than a set you pick one of like "or" (as in "format=a|b|c").
>

Makes sense, changed. Also fits in better with our other code (fflags).

New version attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-hwcontext_vulkan-let-users-enable-device-and-instanc.patch
Type: text/x-patch
Size: 6639 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200510/7e4bd2ef/attachment.bin>