obsproject / obs-studio

OBS Studio - Free and open source software for live streaming and screen recording
https://obsproject.com
GNU General Public License v2.0
58.68k stars 7.81k forks source link

Unvalidated parameters in Vulkan capture layer #6149

Open jorolf opened 2 years ago

jorolf commented 2 years ago

Operating System Info

Windows 10

OBS Studio Version

27.2.3

OBS Studio Log URL

N/A

Expected Behavior

When calling vkCreateSwapchainKHR in a Vulkan application the spec states: The implied image creation parameters of the swapchain must be supported as reported by vkGetPhysicalDeviceImageFormatProperties. OBS should respect the spec and check if its changes to the structure are valid.

Current Behavior

Current code: https://github.com/obsproject/obs-studio/blob/02e54103813f90eac34530de95c868251df01b9b/plugins/win-capture/graphics-hook/vulkan-capture.c#L1659-L1666

It just tries vkCreateSwapchainKHR with a modified imageUsage and if it fails it falls back to the original parameters.

Steps to Reproduce

I have been following this tutorial: https://vulkan-tutorial.com/Introduction At the swapchain creation step: https://vulkan-tutorial.com/Drawing_a_triangle/Presentation/Swap_chain

I had the Khronos validation layer enabled which then showed this error message:

VUID-VkSwapchainCreateInfoKHR-imageFormat-01778(ERROR / SPEC): msgNum: -1070202321 - Validation Error: [ VUID-VkSwapchai
nCreateInfoKHR-imageFormat-01778 ] Object 0: handle = 0x222d7f72230, type = VK_OBJECT_TYPE_DEVICE; | MessageID = 0xc0360
22f | vkCreateSwapchainKHR(): pCreateInfo->imageFormat VK_FORMAT_B8G8R8A8_SRGB with tiling VK_IMAGE_TILING_OPTIMAL has n
o supported format features on this physical device. The Vulkan spec states: The implied image creation parameters of th
e swapchain must be supported as reported by vkGetPhysicalDeviceImageFormatProperties (https://vulkan.lunarg.com/doc/vie
w/1.3.204.1/windows/1.3-extensions/vkspec.html#VUID-VkSwapchainCreateInfoKHR-imageFormat-01778)
    Objects: 1
        [0] 0x222d7f72230, type: 3, name: NULL

Anything else we should know?

GPU: AMD RX 580 Driver Version: 22.2.3

Using the Vulkan SDK provided here: https://vulkan.lunarg.com/sdk/home Version: 1.3.204.1

I tried disabling the layer using the Vulkan Configurator provided with the SDK which fixes the validation error. On that note: The Vulkan layer is implicitly on even if OBS is not running, it just has to be installed.

jpark37 commented 1 year ago

Sorry for the lateness of this reply. I didn't see this issue until now.

There isn't a way for us to call vkGetPhysicalDeviceImageFormatProperties since we don't have the tiling mode, and it looks like the validation layer is doing it by falsely assuming the swap chain needs to be VK_IMAGE_TILING_OPTIMAL, which shouldn't be the case. Linear tiling is perfectly fine for scan out, and possibly the only scan-out tiling on some GPUs IIRC, but my memory on this is fuzzy.

I think best practice for app developers is to not have implicit layers enabled when validating. LunarG has gone back and forth on how best to do this, and I don't remember what the current easiest method is. If you ask in the Vulkan Discord, you will probably get a speedy response, most likely from CharlesG.

I agree it sucks that we're always enabled (in normal circumstances). If we were in a position to launch the games people want to capture (e.g. Steam/RenderDoc are not always enabled because they expect you to use them to launch), we wouldn't have to, but we're not in position, so we have to be available preemptively to be able to hook games.

jpark37 commented 1 year ago

Thinking about the spec more, I guess VK_IMAGE_TILING_OPTIMAL could be linear under the hood if that's all the GPU supported.

As much as I like respecting specs, the drawback to adding this check using the implied parameters is that a driver that would have succeeded could fail because image format properties logic doesn't align completely with swap chain creation logic. It could be argued that's how it should be, but that's of small consolation to a user who can't use game capture because of spec OCD. There's not really an end-user benefit to adding this check, so I'm leaning towards leaving things as they are.

jorolf commented 1 year ago

Well, the reason I reported this in the first place was because of the notifications that appeared. Because the warning didn't have any indication that it belonged to OBS I had to manually turn on/off layers until I found which one caused it. (Not to mention I had to figure out it was caused by a layer in the first place)

However, since making that issue I reinstalled windows once and I can't figure out how to make it appear again. So maybe it was fixed? Idk.

jpark37 commented 1 year ago

It's possible that AMD updated their driver to support the combo. FWIW, I modified the vkcube sample to try BGRA8_SRGB/OPTIMAL/TRANSFER_SRC on my RX 5700 on 22.11.2, and it was VK_SUCCESS.