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

Mark Thompson sw at jkqxz.net
Mon May 11 00:23:30 EEST 2020


On 10/05/2020 15:02, Lynne wrote:
> May 10, 2020, 14:14 by sw at jkqxz.net:
>> On 10/05/2020 11:51, Lynne wrote:
>>> ...
> 
> New version attached.
> 
> > From a4741ed92aecc43e64ca424206c3989008a222fd 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 | 39 ++++++++++++++++++++++++++++++++----
>  2 files changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index 29753f06ca..ed437bb16f 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 plus separated list of additional instance extensions to enable.
> + at item device_extensions
> +A plus 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..c853e2f502 100644
> --- a/libavutil/hwcontext_vulkan.c
> +++ b/libavutil/hwcontext_vulkan.c
> @@ -100,7 +100,7 @@ typedef struct AVVkFrameInternal {
>              err = AVERROR(ENOMEM);                                             \
>              goto end;                                                          \
>          }                                                                      \
> -        list[count - 1] = val;                                                 \
> +        list[count - 1] = av_strdup(val);                                      \

Memory allocation needs a check for failure.

>      } while(0)
>  
>  static const struct {
> @@ -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);
>          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,30 @@ static int check_extensions(AVHWDeviceContext *ctx, int dev,
>          }
>      }
>  
> +    if (user_exts) {
> +        char *user_exts_str = av_strdup(user_exts->value);

Also this one.

> +        char *save, *token = av_strtok(user_exts_str, "+", &save);
> +        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);
> +        }
> +        av_free(user_exts_str);

Leaks in the error case.

> +    }
> +
>      *dst = extension_names;
>      *num = extensions_found;
>  
> @@ -376,7 +403,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;
> @@ -391,6 +418,8 @@ static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
>      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 */
> @@ -777,13 +806,15 @@ 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;
>  
>      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) {
> -- 
> 2.26.2

LGTM with that fixed.

Thanks,

- Mark


More information about the ffmpeg-devel mailing list