From 4692a45dea1e69436e669e3d7d4b4c1711dfc88d Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Tue, 25 Feb 2025 20:55:09 +1100 Subject: [PATCH] vkformat: fix format_from_video_info_2 to actually runtime check versions and extensions If the vulkan plugin was compiled against a newer version than the supported vulkan runtime instance or device, then it was possible for format retrieval to fail. Failure was due to unconditionally using newer extensions and features without runtime checking them. Part-of: --- girs/GstVulkan-1.0.gir | 6 +- .../gst-libs/gst/vulkan/gstvkformat.c | 73 +++++++++++++------ .../gst-libs/gst/vulkan/gstvkformat.h | 2 +- .../gst/vulkan/gstvkimagebufferpool.c | 2 +- .../tests/check/libs/vkformat.c | 9 +-- 5 files changed, 59 insertions(+), 33 deletions(-) diff --git a/girs/GstVulkan-1.0.gir b/girs/GstVulkan-1.0.gir index c92e3211b5..b166497be0 100644 --- a/girs/GstVulkan-1.0.gir +++ b/girs/GstVulkan-1.0.gir @@ -7410,9 +7410,9 @@ properties. - - a #GstVulkanPhysicalDevice - + + a #GstVulkanDevice + the #GstVideoInfo diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.c b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.c index 9c06d0efb3..ac39472f85 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.c @@ -560,11 +560,41 @@ _get_usage (guint64 feature) return usage; } +static gboolean +supports_KHR_get_physical_device_properties2 (GstVulkanDevice * device) +{ +#if defined (VK_KHR_get_physical_device_properties2) + return gst_vulkan_physical_device_check_api_version (device->physical_device, + 1, 1, 0) + || gst_vulkan_instance_is_extension_enabled (device->instance, + "VK_KHR_get_physical_device_properties2"); +#else + return FALSE; +#endif +} + +static gboolean +supports_KHR_format_feature_flags2 (GstVulkanDevice * device) +{ +#if defined (VK_KHR_format_feature_flags2) + if (gst_vulkan_physical_device_check_api_version (device->physical_device, 1, + 3, 0)) + return TRUE; + + if (supports_KHR_get_physical_device_properties2 (device) + && gst_vulkan_device_is_extension_enabled (device, + "VK_KHR_format_feature_flags2")) + return TRUE; +#endif + return FALSE; +} + static guint64 -_get_feature_flags (VkPhysicalDevice gpu, gpointer func, VkFormat format, - VkImageTiling tiling) +_get_feature_flags (GstVulkanDevice * device, gpointer func, + VkFormat format, VkImageTiling tiling) { VkFormatProperties prop = { 0 }; + VkPhysicalDevice gpu = gst_vulkan_device_get_physical_device (device); #if defined (VK_KHR_get_physical_device_properties2) #if defined (VK_KHR_format_feature_flags2) VkFormatProperties3KHR prop3 = { @@ -573,24 +603,24 @@ _get_feature_flags (VkPhysicalDevice gpu, gpointer func, VkFormat format, #endif VkFormatProperties2KHR prop2 = { .sType = VK_STRUCTURE_TYPE_FORMAT_PROPERTIES_2_KHR, -#if defined (VK_KHR_format_feature_flags2) - .pNext = &prop3, -#endif + .pNext = NULL, }; - if (func) { + if (func && supports_KHR_get_physical_device_properties2 (device)) { PFN_vkGetPhysicalDeviceFormatProperties2KHR gst_vkGetPhysicalDeviceFormatProperties2 = func; +#if defined (VK_KHR_format_feature_flags2) + prop2.pNext = &prop3; +#endif gst_vkGetPhysicalDeviceFormatProperties2 (gpu, format, &prop2); -#if defined (VK_KHR_format_feature_flags2) - return tiling == VK_IMAGE_TILING_LINEAR ? - prop3.linearTilingFeatures : prop3.optimalTilingFeatures; -#else - return tiling == VK_IMAGE_TILING_LINEAR ? - prop2.formatProperties.linearTilingFeatures : - prop2.formatProperties.optimalTilingFeatures; -#endif + if (supports_KHR_format_feature_flags2 (device)) + return tiling == VK_IMAGE_TILING_LINEAR ? + prop3.linearTilingFeatures : prop3.optimalTilingFeatures; + else + return tiling == VK_IMAGE_TILING_LINEAR ? + prop2.formatProperties.linearTilingFeatures : + prop2.formatProperties.optimalTilingFeatures; } #endif /* defined (VK_KHR_get_physical_device_properties2) */ @@ -602,7 +632,7 @@ _get_feature_flags (VkPhysicalDevice gpu, gpointer func, VkFormat format, /** * gst_vulkan_format_from_video_info_2: (skip) - * @physical_device: a #GstVulkanPhysicalDevice + * @device: a #GstVulkanDevice * @info: the #GstVideoInfo * @tiling: the tiling to use * @no_multiplane: query for vulkan formats without multiple images @@ -616,30 +646,27 @@ _get_feature_flags (VkPhysicalDevice gpu, gpointer func, VkFormat format, * Since: 1.24 */ gboolean -gst_vulkan_format_from_video_info_2 (GstVulkanPhysicalDevice * physical_device, +gst_vulkan_format_from_video_info_2 (GstVulkanDevice * device, GstVideoInfo * info, VkImageTiling tiling, gboolean no_multiplane, VkImageUsageFlags requested_usage, VkFormat fmts[GST_VIDEO_MAX_PLANES], int *n_imgs, VkImageUsageFlags * usage_ret) { int i; - VkPhysicalDevice gpu; #if defined (VK_KHR_get_physical_device_properties2) PFN_vkGetPhysicalDeviceFormatProperties2KHR gst_vkGetPhysicalDeviceFormatProperties2 = NULL; gst_vkGetPhysicalDeviceFormatProperties2 = - gst_vulkan_instance_get_proc_address (physical_device->instance, + gst_vulkan_instance_get_proc_address (device->instance, "vkGetPhysicalDeviceFormatProperties2"); if (!gst_vkGetPhysicalDeviceFormatProperties2) gst_vkGetPhysicalDeviceFormatProperties2 = - gst_vulkan_instance_get_proc_address (physical_device->instance, + gst_vulkan_instance_get_proc_address (device->instance, "vkGetPhysicalDeviceFormatProperties2KHR"); #else gpointer gst_vkGetPhysicalDeviceFormatProperties2 = NULL; #endif - gpu = gst_vulkan_physical_device_get_handle (physical_device); - for (i = 0; i < G_N_ELEMENTS (vk_formats_map); i++) { guint64 feats_primary, feats_secondary = 0; VkImageUsageFlags usage = 0; @@ -647,12 +674,12 @@ gst_vulkan_format_from_video_info_2 (GstVulkanPhysicalDevice * physical_device, if (vk_formats_map[i].format != GST_VIDEO_INFO_FORMAT (info)) continue; - feats_primary = _get_feature_flags (gpu, + feats_primary = _get_feature_flags (device, gst_vkGetPhysicalDeviceFormatProperties2, vk_formats_map[i].vkfrmt, tiling); if (vk_formats_map[i].vkfrmt != vk_formats_map[i].vkfrmts[0]) { - feats_secondary = _get_feature_flags (gpu, + feats_secondary = _get_feature_flags (device, gst_vkGetPhysicalDeviceFormatProperties2, vk_formats_map[i].vkfrmts[0], tiling); } diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.h b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.h index 2bf4a21f74..6d9aa1eb03 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.h +++ b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkformat.h @@ -159,7 +159,7 @@ VkFormat gst_vulkan_format_from_video_info guint plane); GST_VULKAN_API -gboolean gst_vulkan_format_from_video_info_2 (GstVulkanPhysicalDevice * physical_device, +gboolean gst_vulkan_format_from_video_info_2 (GstVulkanDevice * device, GstVideoInfo * info, VkImageTiling tiling, gboolean no_multiplane, diff --git a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkimagebufferpool.c b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkimagebufferpool.c index b295a33c3b..0f16402d6e 100644 --- a/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkimagebufferpool.c +++ b/subprojects/gst-plugins-bad/gst-libs/gst/vulkan/gstvkimagebufferpool.c @@ -275,7 +275,7 @@ gst_vulkan_image_buffer_pool_set_config (GstBufferPool * pool, } tiling = priv->raw_caps ? VK_IMAGE_TILING_LINEAR : VK_IMAGE_TILING_OPTIMAL; - found = gst_vulkan_format_from_video_info_2 (vk_pool->device->physical_device, + found = gst_vulkan_format_from_video_info_2 (vk_pool->device, &priv->v_info, tiling, no_multiplane, requested_usage, priv->vk_fmts, &priv->n_imgs, NULL); if (!found) diff --git a/subprojects/gst-plugins-bad/tests/check/libs/vkformat.c b/subprojects/gst-plugins-bad/tests/check/libs/vkformat.c index f57ffa8bfc..6fbc2c6bfb 100644 --- a/subprojects/gst-plugins-bad/tests/check/libs/vkformat.c +++ b/subprojects/gst-plugins-bad/tests/check/libs/vkformat.c @@ -47,7 +47,6 @@ teardown (void) GST_START_TEST (test_format_from_video_info_2) { - GstVulkanPhysicalDevice *phy_dev = device->physical_device; GstVideoInfo vinfo; VkFormat vk_fmts[GST_VIDEO_MAX_PLANES]; int n_imgs; @@ -56,14 +55,14 @@ GST_START_TEST (test_format_from_video_info_2) fail_unless (gst_video_info_set_format (&vinfo, GST_VIDEO_FORMAT_NV12, 620, 480)); - fail_unless (gst_vulkan_format_from_video_info_2 (phy_dev, &vinfo, + fail_unless (gst_vulkan_format_from_video_info_2 (device, &vinfo, VK_IMAGE_TILING_OPTIMAL, TRUE, 0, vk_fmts, &n_imgs, &supported_usage)); fail_unless (n_imgs == 2 && vk_fmts[0] == VK_FORMAT_R8_UNORM && vk_fmts[1] == VK_FORMAT_R8G8_UNORM); - fail_unless (gst_vulkan_format_from_video_info_2 (phy_dev, &vinfo, + fail_unless (gst_vulkan_format_from_video_info_2 (device, &vinfo, VK_IMAGE_TILING_LINEAR, FALSE, 0, vk_fmts, &n_imgs, &supported_usage)); @@ -74,7 +73,7 @@ GST_START_TEST (test_format_from_video_info_2) fail_unless (GST_VIDEO_INFO_COLORIMETRY (&vinfo).transfer == GST_VIDEO_TRANSFER_SRGB); - fail_unless (gst_vulkan_format_from_video_info_2 (phy_dev, &vinfo, + fail_unless (gst_vulkan_format_from_video_info_2 (device, &vinfo, VK_IMAGE_TILING_LINEAR, TRUE, 0, vk_fmts, &n_imgs, &supported_usage)); fail_unless (n_imgs == 1 && vk_fmts[0] == VK_FORMAT_R8G8B8A8_UNORM); @@ -84,7 +83,7 @@ GST_START_TEST (test_format_from_video_info_2) fail_unless (gst_video_colorimetry_from_string (&GST_VIDEO_INFO_COLORIMETRY (&vinfo), "smpte240m")); - fail_unless (gst_vulkan_format_from_video_info_2 (phy_dev, &vinfo, + fail_unless (gst_vulkan_format_from_video_info_2 (device, &vinfo, VK_IMAGE_TILING_LINEAR, TRUE, 0, vk_fmts, &n_imgs, &supported_usage)); fail_unless (n_imgs == 1 && vk_fmts[0] == VK_FORMAT_R8G8B8A8_UNORM);