Re: [FFmpeg-devel] [PATCH v2 4/4] avutil/hwcontext_vulkan: fully support customizable validation layers
Lynne:
Sent: 2021年11月24日 18:36
To: FFmpeg development discussions and patches<mailto:ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2 4/4] avutil/hwcontext_vulkan: fully
support customizable validation layers
24 Nov 2021, 05:11 by jianhua...@intel.com:
>> /* Creates a VkInstance */
>> static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
>> {
>> @@ -558,13 +651,16 @@ static int create_instance(AVHWDeviceContext *ctx,
>> AVDictionary *opts)
>> /* Check for present/missing extensions */
>> err = check_extensions(ctx, 0, opts, &inst_props.ppEnabledExtensionNames,
>> &inst_props.enabledExtensionCount, debug_mode);
>> +hwctx->enabled_inst_extensions = inst_props.ppEnabledExtensionNames;
>> +hwctx->nb_enabled_inst_extensions = inst_props.enabledExtensionCount;
>>
>
> Why did you move that assignment?
>
If the creation fails or something exception, assign them here to ensure
that they could be released in the vulkan_device_free() just like releasing
by a de-constructor, and it's no need to write more codes to free them in
this function. If the context creation failed, the vulkan_device_free() will
be called immediately, so they would not keep for a long time.
>
> I've pushed patches 2 and 3, just squash patch 1 and 4 (this one) and
> resubmit with the changes I mentioned.
>
Okay. No problem. I’ll resubmit it.
Thanks,
Jianhua
___
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".
Re: [FFmpeg-devel] [PATCH v2 4/4] avutil/hwcontext_vulkan: fully support customizable validation layers
24 Nov 2021, 05:11 by jianhua...@intel.com:
> +static int check_validation_layers(AVHWDeviceContext *ctx, AVDictionary
> *opts,
> + const char * const **dst, uint32_t *num)
> +{
> +static const char default_layer[] = { "VK_LAYER_KHRONOS_validation" };
> +
> +int found = 0, err = 0;
> +VulkanDevicePriv *priv = ctx->internal->priv;
> +FFVulkanFunctions *vk = &priv->vkfn;
> +
> +uint32_t sup_layer_count;
> +VkLayerProperties *sup_layers;
> +
> +AVDictionaryEntry *user_layers;
> +char *user_layers_str, *save, *token;
> +
> +const char **enabled_layers = NULL;
> +uint32_t enabled_layers_count = 0;
> +
> +user_layers = av_dict_get(opts, "validation_layers", NULL, 0);
> +if (!user_layers)
> +return 0;
>
With this change, unless users specify another validation layer,
then not even the default layer will get activated. That's not desirable.
This is how this should work:
- If `debug=1` is specified, enable the standard validation layer extension.
- If `validation_layers` is specified, without any `debug`, enable just the
layers specified in the list, and if the standard validation is amongst them,
enable debug mode to load its callback properly.
- If `debug=1` and `validation_layers` is specified, enable the standard
validation layer regardless of whether it's included in validation_layers, and
enable the rest of the layers.
- If `debug=0` and `validation_layers` is specified, enable no layers at all.
If any layer enabled, including the standard validation layer, is not
supported, error out and stop initialization.
> +user_layers_str = av_strdup(user_layers->value);
> +if (!user_layers_str) {
> +err = AVERROR(EINVAL);
> +goto fail;
> +}
> +
> +vk->EnumerateInstanceLayerProperties(&sup_layer_count, NULL);
> +sup_layers = av_malloc_array(sup_layer_count, sizeof(VkLayerProperties));
> +if (!sup_layers)
> +return AVERROR(ENOMEM);
>
You leak `user_layers_str` on error.
> +vk->EnumerateInstanceLayerProperties(&sup_layer_count, sup_layers);
> +
> +av_log(ctx, AV_LOG_VERBOSE, "Supported validation layers:\n");
> +for (int i = 0; i < sup_layer_count; i++) {
> +av_log(ctx, AV_LOG_VERBOSE, "\t%s\n", sup_layers[i].layerName);
> +if (!strcmp(default_layer, sup_layers[i].layerName))
> +found = 1;
> +}
> +
> +if (!found) {
> +av_log(ctx, AV_LOG_ERROR, "Default layer\"%s\" isn't supported.
> Please "
> + "check if vulkan-validation-layers installed\n",
> default_layer);
>
Return an error here to stop initialization.
> +} else {
> +av_log(ctx, AV_LOG_VERBOSE,
> + "Default validation layer %s is enabled\n", default_layer);
> +ADD_VAL_TO_LIST(enabled_layers, enabled_layers_count, default_layer);
> +}
> +
> +token = av_strtok(user_layers_str, "+", &save);
> +while (token) {
> +found = 0;
> +if (!strcmp(default_layer, token)) {
> +token = av_strtok(NULL, "+", &save);
> +continue;
> +}
> +for (int j = 0; j < sup_layer_count; j++) {
> +if (!strcmp(token, sup_layers[j].layerName)) {
> +found = 1;
> +break;
> +}
> +}
> +if (found) {
> +av_log(ctx, AV_LOG_VERBOSE, "Requested Validation Layer: %s\n",
> token);
> +ADD_VAL_TO_LIST(enabled_layers, enabled_layers_count, token);
> +} else {
> +av_log(ctx, AV_LOG_ERROR,
> + "Validation Layer \"%s\" not support.\n", token);
> +err = AVERROR(EINVAL);
> +goto fail;
> +}
> +token = av_strtok(NULL, "+", &save);
> +}
> +
> +*dst = enabled_layers;
> +*num = enabled_layers_count;
> +
> +av_free(sup_layers);
> +av_free(user_layers_str);
> +return 0;
> +
> +fail:
> +RELEASE_PROPS(enabled_layers, enabled_layers_count);
> +av_free(sup_layers);
> +av_free(user_layers_str);
> +return err;
> +}
> +
> /* Creates a VkInstance */
> static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
> {
> @@ -558,13 +651,16 @@ static int create_instance(AVHWDeviceContext *ctx,
> AVDictionary *opts)
> /* Check for present/missing extensions */
> err = check_extensions(ctx, 0, opts, &inst_props.ppEnabledExtensionNames,
> &inst_props.enabledExtensionCount, debug_mode);
> +hwctx->enabled_inst_extensions = inst_props.ppEnabledExtensionNames;
> +hwctx->nb_enabled_inst_extensions = inst_props.enabledExtensionCount;
>
Why did you move that assignment?
I've pushed patches 2 and 3, just squash patch 1 and 4 (this one) and
resubmit with the changes I mentioned.
___
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-devel] [PATCH v2 4/4] avutil/hwcontext_vulkan: fully support customizable validation layers
Validation layer is an indispensable part of developing on Vulkan.
The following commands is on how to enable validation layers:
ffmpeg -init_hw_device
vulkan=0,debug=1,validation_layers=VK_LAYER_LUNARG_monitor+VK_LAYER_LUNARG_api_dump
Signed-off-by: Wu Jianhua
---
libavutil/hwcontext_vulkan.c | 136 +--
1 file changed, 113 insertions(+), 23 deletions(-)
diff --git a/libavutil/hwcontext_vulkan.c b/libavutil/hwcontext_vulkan.c
index 644ed947f8..75f9f90d70 100644
--- a/libavutil/hwcontext_vulkan.c
+++ b/libavutil/hwcontext_vulkan.c
@@ -146,6 +146,13 @@ typedef struct AVVkFrameInternal {
}
\
} while(0)
+#define RELEASE_PROPS(props, count)
\
+if (props) {
\
+for (int i = 0; i < count; i++)
\
+av_free((void *)((props)[i]));
\
+av_free((void *)props);
\
+}
+
static const struct {
enum AVPixelFormat pixfmt;
const VkFormat vkfmts[4];
@@ -511,15 +518,101 @@ static int check_extensions(AVHWDeviceContext *ctx, int
dev, AVDictionary *opts,
return 0;
fail:
-if (extension_names)
-for (int i = 0; i < extensions_found; i++)
-av_free((void *)extension_names[i]);
-av_free(extension_names);
+RELEASE_PROPS(extension_names, extensions_found);
av_free(user_exts_str);
av_free(sup_ext);
return err;
}
+static int check_validation_layers(AVHWDeviceContext *ctx, AVDictionary *opts,
+ const char * const **dst, uint32_t *num)
+{
+static const char default_layer[] = { "VK_LAYER_KHRONOS_validation" };
+
+int found = 0, err = 0;
+VulkanDevicePriv *priv = ctx->internal->priv;
+FFVulkanFunctions *vk = &priv->vkfn;
+
+uint32_t sup_layer_count;
+VkLayerProperties *sup_layers;
+
+AVDictionaryEntry *user_layers;
+char *user_layers_str, *save, *token;
+
+const char **enabled_layers = NULL;
+uint32_t enabled_layers_count = 0;
+
+user_layers = av_dict_get(opts, "validation_layers", NULL, 0);
+if (!user_layers)
+return 0;
+
+user_layers_str = av_strdup(user_layers->value);
+if (!user_layers_str) {
+err = AVERROR(EINVAL);
+goto fail;
+}
+
+vk->EnumerateInstanceLayerProperties(&sup_layer_count, NULL);
+sup_layers = av_malloc_array(sup_layer_count, sizeof(VkLayerProperties));
+if (!sup_layers)
+return AVERROR(ENOMEM);
+vk->EnumerateInstanceLayerProperties(&sup_layer_count, sup_layers);
+
+av_log(ctx, AV_LOG_VERBOSE, "Supported validation layers:\n");
+for (int i = 0; i < sup_layer_count; i++) {
+av_log(ctx, AV_LOG_VERBOSE, "\t%s\n", sup_layers[i].layerName);
+if (!strcmp(default_layer, sup_layers[i].layerName))
+found = 1;
+}
+
+if (!found) {
+av_log(ctx, AV_LOG_ERROR, "Default layer\"%s\" isn't supported. Please
"
+ "check if vulkan-validation-layers installed\n", default_layer);
+} else {
+av_log(ctx, AV_LOG_VERBOSE,
+ "Default validation layer %s is enabled\n", default_layer);
+ADD_VAL_TO_LIST(enabled_layers, enabled_layers_count, default_layer);
+}
+
+token = av_strtok(user_layers_str, "+", &save);
+while (token) {
+found = 0;
+if (!strcmp(default_layer, token)) {
+token = av_strtok(NULL, "+", &save);
+continue;
+}
+for (int j = 0; j < sup_layer_count; j++) {
+if (!strcmp(token, sup_layers[j].layerName)) {
+found = 1;
+break;
+}
+}
+if (found) {
+av_log(ctx, AV_LOG_VERBOSE, "Requested Validation Layer: %s\n",
token);
+ADD_VAL_TO_LIST(enabled_layers, enabled_layers_count, token);
+} else {
+av_log(ctx, AV_LOG_ERROR,
+ "Validation Layer \"%s\" not support.\n", token);
+err = AVERROR(EINVAL);
+goto fail;
+}
+token = av_strtok(NULL, "+", &save);
+}
+
+*dst = enabled_layers;
+*num = enabled_layers_count;
+
+av_free(sup_layers);
+av_free(user_layers_str);
+return 0;
+
+fail:
+RELEASE_PROPS(enabled_layers, enabled_layers_count);
+av_free(sup_layers);
+av_free(user_layers_str);
+return err;
+}
+
/* Creates a VkInstance */
static int create_instance(AVHWDeviceContext *ctx, AVDictionary *opts)
{
@@ -558,13 +651,16 @@ static int create_instance(AVHWDeviceContext *ctx,
AVDictionary *opts)
/* Check for present/missing extensions */
err = check_extensions(ctx, 0, opts, &inst_props.ppEnabledExtensionNames,
&inst_props.enabledExtensionCount, debug_mod
