Vulkan extensions loaded as layers do not work

[obscenelyvague]

I lifted the v1.2 limitation in hopes of getting Vulkan to work through layers. From the log it seems libplacebo correctly recognizes the layers and respective extensions they're providing support for but fails afterwards and complains about the extensions not being present anyway.

Log from mpv:
mpv_vk.txt

[haasn]

Hi, what code/changes are you testing with?

In particular, did you add any request to load the relevant extension? It seems it's probed as available, but unless something requests it, it won't automatically be loaded.

[obscenelyvague]

I didn't make any other code changes besides building https://github.com/KhronosGroup/Vulkan-ExtensionLayer to package libraries into the APK and lowering Vulkan requirement version in libplacebo.

[obscenelyvague]

I dropped the requirement through PL_VK_MIN_VERSION. I had an expectation that the feature check would automatically pass because extension layers are present. Do you have any suggestions regarding how to actually make use of them or what else I would have to change?

[haasn]

There's a few other things that need to be done. We've gone back and forth on the minimum vk version in the past. I'm not sure what currently requires 1.2, but see these possibly relevant commits:

• 44fdc77
• e5fb4e6
• efdfbbf
• 351447a

I can take a proper look at it tomorrow and throw up a patch. At the very least, the timeline extension needs to be added to the instance extension list.

[haasn]

Something like this seems to work. Quite verbose though.

Maybe we should just not care about the extra boilerplate extensions (which I probably didn't even list all of), under the theory that devices providing these features probably provide the relevant vulkan version as well.

diff --git a/src/include/libplacebo/vulkan.h b/src/include/libplacebo/vulkan.h
index 505ea293..71565140 100644
--- a/src/include/libplacebo/vulkan.h
+++ b/src/include/libplacebo/vulkan.h
@@ -24,7 +24,7 @@
 
 PL_API_BEGIN
 
-#define PL_VK_MIN_VERSION VK_API_VERSION_1_2
+#define PL_VK_MIN_VERSION VK_API_VERSION_1_1
 
 // Structure representing a VkInstance. Using this is not required.
 typedef const struct pl_vk_inst_t {
diff --git a/src/vulkan/command.c b/src/vulkan/command.c
index 5020affa..92e7570b 100644
--- a/src/vulkan/command.c
+++ b/src/vulkan/command.c
@@ -22,7 +22,7 @@
 static VkResult vk_cmd_poll(struct vk_cmd *cmd, uint64_t timeout)
 {
     struct vk_ctx *vk = cmd->pool->vk;
-    return vk->WaitSemaphores(vk->dev, &(VkSemaphoreWaitInfo) {
+    return vk->WaitSemaphoresKHR(vk->dev, &(VkSemaphoreWaitInfo) {
         .sType = VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO,
         .semaphoreCount = 1,
         .pSemaphores = &cmd->sync.sem,
diff --git a/src/vulkan/common.h b/src/vulkan/common.h
index 71a41c25..034e2152 100644
--- a/src/vulkan/common.h
+++ b/src/vulkan/common.h
@@ -215,12 +215,12 @@ struct vk_ctx {
     PL_VK_FUN(QueueSubmit2KHR);
     PL_VK_FUN(QueueWaitIdle);
     PL_VK_FUN(ResetFences);
-    PL_VK_FUN(ResetQueryPool);
+    PL_VK_FUN(ResetQueryPoolEXT);
     PL_VK_FUN(SetDebugUtilsObjectNameEXT);
     PL_VK_FUN(SetHdrMetadataEXT);
     PL_VK_FUN(UpdateDescriptorSets);
     PL_VK_FUN(WaitForFences);
-    PL_VK_FUN(WaitSemaphores);
+    PL_VK_FUN(WaitSemaphoresKHR);
 
 #ifdef PL_HAVE_WIN32
     PL_VK_FUN(GetMemoryWin32HandleKHR);
diff --git a/src/vulkan/context.c b/src/vulkan/context.c
index 37048672..a2dae93e 100644
--- a/src/vulkan/context.c
+++ b/src/vulkan/context.c
@@ -187,7 +187,27 @@ static const struct vk_ext vk_device_extensions[] = {
             PL_VK_DEV_FUN(QueueSubmit2KHR),
             {0}
         },
+    }, {
+        .name = VK_KHR_TIMELINE_SEMAPHORE_EXTENSION_NAME,
+        .funs = (const struct vk_fun[]) {
+            PL_VK_DEV_FUN(WaitSemaphoresKHR),
+        },
+    }, {
+        .name = VK_EXT_HOST_QUERY_RESET_EXTENSION_NAME,
+        .funs = (const struct vk_fun[]) {
+            PL_VK_DEV_FUN(ResetQueryPoolEXT),
+        },
     },
+    // Extensions which only provide features
+    { VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME },
+    { VK_KHR_8BIT_STORAGE_EXTENSION_NAME },
+    { VK_KHR_SHADER_ATOMIC_INT64_EXTENSION_NAME },
+    { VK_KHR_SHADER_FLOAT16_INT8_EXTENSION_NAME },
+    { VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME },
+    { VK_KHR_VULKAN_MEMORY_MODEL_EXTENSION_NAME },
+    { VK_EXT_SUBGROUP_SIZE_CONTROL_EXTENSION_NAME },
+    { VK_KHR_ZERO_INITIALIZE_WORKGROUP_MEMORY_EXTENSION_NAME },
+    { VK_KHR_MAINTENANCE_4_EXTENSION_NAME },
 };
 
 // Make sure to keep this in sync with the above!
@@ -203,6 +223,7 @@ const char * const pl_vulkan_recommended_extensions[] = {
 #endif
     VK_EXT_PCI_BUS_INFO_EXTENSION_NAME,
     VK_EXT_HDR_METADATA_EXTENSION_NAME,
+    VK_KHR_IMAGE_FORMAT_LIST_EXTENSION_NAME,
     VK_EXT_IMAGE_DRM_FORMAT_MODIFIER_EXTENSION_NAME,
 #ifdef VK_KHR_portability_subset
     VK_KHR_PORTABILITY_SUBSET_EXTENSION_NAME,
@@ -214,6 +235,16 @@ const char * const pl_vulkan_recommended_extensions[] = {
     VK_EXT_FULL_SCREEN_EXCLUSIVE_EXTENSION_NAME,
 #endif
     VK_KHR_SYNCHRONIZATION_2_EXTENSION_NAME,
+    VK_KHR_TIMELINE_SEMAPHORE_EXTENSION_NAME,
+    VK_EXT_HOST_QUERY_RESET_EXTENSION_NAME,
+    VK_KHR_8BIT_STORAGE_EXTENSION_NAME,
+    VK_KHR_SHADER_ATOMIC_INT64_EXTENSION_NAME,
+    VK_KHR_SHADER_FLOAT16_INT8_EXTENSION_NAME,
+    VK_KHR_BUFFER_DEVICE_ADDRESS_EXTENSION_NAME,
+    VK_KHR_VULKAN_MEMORY_MODEL_EXTENSION_NAME,
+    VK_EXT_SUBGROUP_SIZE_CONTROL_EXTENSION_NAME,
+    VK_KHR_ZERO_INITIALIZE_WORKGROUP_MEMORY_EXTENSION_NAME,
+    VK_KHR_MAINTENANCE_4_EXTENSION_NAME,
 };
 
 const int pl_vulkan_num_recommended_extensions =
@@ -395,11 +426,9 @@ static const struct vk_fun vk_dev_funs[] = {
     PL_VK_DEV_FUN(QueueSubmit),
     PL_VK_DEV_FUN(QueueWaitIdle),
     PL_VK_DEV_FUN(ResetFences),
-    PL_VK_DEV_FUN(ResetQueryPool),
     PL_VK_DEV_FUN(SetDebugUtilsObjectNameEXT),
     PL_VK_DEV_FUN(UpdateDescriptorSets),
     PL_VK_DEV_FUN(WaitForFences),
-    PL_VK_DEV_FUN(WaitSemaphores),
 };
 
 static void load_vk_fun(struct vk_ctx *vk, const struct vk_fun *fun)
diff --git a/src/vulkan/gpu.c b/src/vulkan/gpu.c
index 6b8ad809..01125223 100644
--- a/src/vulkan/gpu.c
+++ b/src/vulkan/gpu.c
@@ -132,7 +132,7 @@ static void timer_begin(pl_gpu gpu, struct vk_cmd *cmd, pl_timer timer)
         vk->CmdResetQueryPool(cmd->buf, timer->qpool, timer->index_write, 2);
     } else {
         // Use host query reset
-        vk->ResetQueryPool(vk->dev, timer->qpool, timer->index_write, 2);
+        vk->ResetQueryPoolEXT(vk->dev, timer->qpool, timer->index_write, 2);
     }
 
     vk->CmdWriteTimestamp(cmd->buf, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT,
diff --git a/src/vulkan/gpu_buf.c b/src/vulkan/gpu_buf.c
index 91a2195c..b373d040 100644
--- a/src/vulkan/gpu_buf.c
+++ b/src/vulkan/gpu_buf.c
@@ -405,7 +405,7 @@ bool vk_buf_read(pl_gpu gpu, pl_buf buf, size_t offset, void *dest, size_t size)
 
     if (vk_buf_poll(gpu, buf, 0) && buf_vk->sem.write.sync.sem) {
         // ensure no more queued writes
-        VK(vk->WaitSemaphores(vk->dev, &(VkSemaphoreWaitInfo) {
+        VK(vk->WaitSemaphoresKHR(vk->dev, &(VkSemaphoreWaitInfo) {
             .sType = VK_STRUCTURE_TYPE_SEMAPHORE_WAIT_INFO,
             .semaphoreCount = 1,
             .pSemaphores = &buf_vk->sem.write.sync.sem,

[obscenelyvague]

Still seemingly fails at the feature check, unfortunately.
mpv_vk_new.txt

[haasn]

Ah, the problem is that we still have no code actually enabling this layer. Doing it the 'proper' way would unfortunately require some sort of dependency loop:

• We need to create the instance before we can probe the physical devices
• We need to probe the physical devices to gain information about layer extension support
• We need to know layer extension support to figure out what layers to enable during instance creation

As this is unresolvable, there's no way to automatically enable layers based on what device-level extensions they will enable, and we should instead hard-code a list of layers to try enabling.

[haasn]

Try this diff (on top of the previous):

Note: Also available at https://code.videolan.org/haasn/libplacebo/-/commits/vk1.1

diff --git a/src/vulkan/context.c b/src/vulkan/context.c
index a2dae93e..ec447185 100644
--- a/src/vulkan/context.c
+++ b/src/vulkan/context.c
@@ -50,6 +50,12 @@ struct vk_ext {
       .device_level = true,                 \
     }
 
+// Table of optional layers
+static const char *vk_opt_layers[] = {
+    "VK_LAYER_KHRONOS_timeline_semaphore",
+    "VK_LAYER_KHRONOS_synchronization2",
+};
+
 // Table of optional vulkan instance extensions
 static const char *vk_instance_extensions[] = {
     VK_KHR_SURFACE_EXTENSION_NAME,
@@ -670,6 +676,15 @@ debug_layers_done: ;
     for (int i = 0; i < params->num_layers; i++)
         PL_ARRAY_APPEND(tmp, layers, params->layers[i]);
 
+    for (int i = 0; i < PL_ARRAY_SIZE(vk_opt_layers); i++) {
+        for (int n = 0; n < num_layers_avail; n++) {
+            if (strcmp(vk_opt_layers[i], layers_avail[n].layerName) == 0) {
+                PL_ARRAY_APPEND(tmp, layers, vk_opt_layers[i]);
+                break;
+            }
+        }
+    }
+
     for (int i = 0; i < params->num_opt_layers; i++) {
         const char *layer = params->opt_layers[i];
         for (int n = 0; n < num_layers_avail; n++) {

[obscenelyvague]

Hi, Instance creation works now! Although, for me the Android app crashes afterwards. Attaching all files that I think may be of relevance. CC @sfan5 as well.
logcat.log
tombstones.zip
systemlibs.zip

Debug build is available here (need to set gpu-api=androidvk) in case this turns out to be a system issue. I don't have other devices to test at hand.

[sfan5]

Also crashes on my phone. What I'd do is turn the backtrace into symbols and check the code closer.

[obscenelyvague]

Any idea as to why it's not showing line numbers?
https://files.catbox.moe/1br9sd.txt
https://files.catbox.moe/b7ctuq.txt