Vulkan extensions loaded as layers do not work #260
Comments
|
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. |
|
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. |
|
I dropped the requirement through |
|
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: 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. |
|
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, |
|
Still seemingly fails at the feature check, unfortunately. |
|
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:
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. |
|
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++) { |
|
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. Debug build is available here (need to set |
|
Also crashes on my phone. What I'd do is turn the backtrace into symbols and check the code closer. |
|
Any idea as to why it's not showing line numbers? |
|
@obscenelyvague @sfan5 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 caa12b41..93f8a99f 100644
--- a/src/vulkan/common.h
+++ b/src/vulkan/common.h
@@ -214,12 +214,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 1c70fe59..993134a4 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,
@@ -187,7 +193,29 @@ 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),
+ {0}
+ },
+ }, {
+ .name = VK_EXT_HOST_QUERY_RESET_EXTENSION_NAME,
+ .funs = (const struct vk_fun[]) {
+ PL_VK_DEV_FUN(ResetQueryPoolEXT),
+ {0}
+ },
},
+ // 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 +231,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 +243,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 =
@@ -250,7 +289,6 @@ static const VkPhysicalDeviceVulkan12Features recommended_vk12 = {
static const VkPhysicalDeviceVulkan11Features recommended_vk11 = {
.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VULKAN_1_1_FEATURES,
- .pNext = (void *) &recommended_vk12,
.samplerYcbcrConversion = true,
.storagePushConstant16 = true,
};
@@ -279,7 +317,6 @@ static const VkPhysicalDeviceVulkan12Features required_vk12 = {
static const VkPhysicalDeviceVulkan11Features required_vk11 = {
.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VULKAN_1_1_FEATURES,
- .pNext = (void *) &required_vk12,
};
const VkPhysicalDeviceFeatures2 pl_vulkan_required_features = {
@@ -299,8 +336,8 @@ static bool check_required_features(struct vk_ctx *vk)
} \
} while (0)
- CHECK_FEATURE(1, 2, hostQueryReset);
- CHECK_FEATURE(1, 2, timelineSemaphore);
+ // CHECK_FEATURE(1, 2, hostQueryReset);
+ // CHECK_FEATURE(1, 2, timelineSemaphore);
#undef CHECK_FEATURE
return true;
@@ -394,11 +431,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)
@@ -640,6 +675,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++) {
diff --git a/src/vulkan/gpu.c b/src/vulkan/gpu.c
index 6b8ad809..9eac4ab3 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,
@@ -399,10 +399,17 @@ static inline struct pl_spirv_version get_spirv_version(const struct vk_ctx *vk)
}
}
- pl_assert(vk->api_ver >= VK_API_VERSION_1_2);
+ if (vk->api_ver >= VK_API_VERSION_1_2) {
+ return (struct pl_spirv_version) {
+ .env_version = VK_API_VERSION_1_2,
+ .spv_version = PL_SPV_VERSION(1, 5),
+ };
+ }
+
+ pl_assert(vk->api_ver >= VK_API_VERSION_1_1);
return (struct pl_spirv_version) {
- .env_version = VK_API_VERSION_1_2,
- .spv_version = PL_SPV_VERSION(1, 5),
+ .env_version = VK_API_VERSION_1_1,
+ .spv_version = PL_SPV_VERSION(1, 3),
};
}
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,
|
|
@ijingo strange, I still get the same crash. Could you share your changes for mpv-android? |
|
@obscenelyvague you can try this apk |
|
@ijingo Also crashes. No layers are available though so I see that it avoids the two extensions altogether when creating instance. Which device are you using? I'll presume it's not one with a Qualcomm SoC. |
I tried Meta Quest 3, it uses Qualcomm XR2. |
Maybe you can try to add some more verbose logs in libplacebo during the device initialization, such as which vk function is being loaded, etc, to see during which step it crashed. That's how I made it run on my Quest 3 which only supports vulkan 1.1 |
|
If you want to use those two layers (KHRONOS_synchronization2, KHRONOS_timeline_semaphore) I think you have to package them with the app. |
|
Maybe they're different crashes. It's hard to tell because the backtrace looks mostly useless in both cases. @ijingo can you see if https://files.catbox.moe/x85m6z.apk ( |
@obscenelyvague On my device (meta quest3) it run normally. Here is the log, if you are intereseted. odh_logs_2024-10-16 06.32.17.420.zip |
|
|
sorry my bad, I forget to set |
|
It's segfaulting with a null pointer deref in your log. |
|
Current state of things look like this: Original patch now aborts early with with layers, the app crashes on every device tested, regardless of whether layers are needed or not without layers, the app crashes on Vulkan 1.1 devices without timeline extension (mpv_vk_nolayers_sm8250.txt | crashdumpnolayers.txt) BUT, works on Vulkan 1.1 devices with timeline extension This isn't what this issue was initially about but downgrading the requirement in this manner can potentially mean these GPU/driver combinations will be able to use newer versions of mpv with Vulkan. There are more devices in this database that identify by device name rather than GPU name which also qualify. This only covers some of them: |
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
The text was updated successfully, but these errors were encountered: