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
60.39k stars 7.99k forks source link

graphics-hook: Fix null pointer dereference #11430

Closed Charlese2 closed 2 weeks ago

Charlese2 commented 3 weeks ago

Multiple APIs may be setup to capture without being initialized in graphics-hook when multiple threads are sending present calls. This just prevents those invalid captures from completing.

Description

Adds a check for data.handle being null, as that currently is one indicator that the data struct has not been initialized.

Motivation and Context

In the NVIDIA 555.85 driver, an extra API queue was added when using VK_PRESENT_MODE_FIFO_KHR or VK_PRESENT_MODE_FIFO_RELAXED_KHR present modes. Two separate threads present to each of the queues. The application presents to the Vulkan queue, and the driver presents to the DirectX 12 queue (DXGI). The user-mode driver copies the contents of the vulkan framebuffer into in the the DirectX 12 queue. After that the DX12 queue is the one that is rendered to the window.

Fixes https://github.com/obsproject/obs-studio/issues/11403

How Has This Been Tested?

Tested under DX12, DX11, OpenGL, and Vulkan to make sure they still capture. (using Dolphin Emulator video backends) Tested disabling and re-enabling an active capture while using Vulkan and VSync to make sure it doesn't crash.

On Windows 11 23H2 using a NVIDIA GeForce 3080.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

Lain-B commented 2 weeks ago

Forgot to ask to update it to include the hook version change with this, but I just updated it manually.

Also just noticed the commit message, in the future before making commits for future reference,

OBS Studio uses the 50/72 standard for commits. 50 characters max for the title (excluding module prefix), an empty line, and then a full description of the commit, wrapped to 72 columns max.

But I fixed that too, so I'll just merge after it finishes building! Hope you don't mind me updating it, we just need to get it out for next Beta/RC.

And fix the whole "setup" vs "set up" thing Ryan just mentioned as I was typing this.

Charlese2 commented 2 weeks ago

Thank you for fixing it.