search

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.26k stars 7.98k forks source link

Firefox crashes with [@ obs-virtualcam-module64.dll | BaseThreadInitThunk ] #4243

Closed Archaeopteryx closed 2 years ago

Archaeopteryx commented 3 years ago

Platform

Operating system and version: Windows (7, 8.1, 10) OBS Studio version: 26.*

Expected Behavior

Firefox doesn't crash on websites accessing webcams.

Current Behavior

I (person looking at crash reports for Firefox) and people I asked cannot reproduce the issue but some users encounters on websites which access web cams (e.g. Google Meet, https://webcamtests.com/ , BigBlueButton installs; interestingly no mention of Zoom). Recent crash reports (the "Reports" tab provides access to individual crash reports).

Steps to Reproduce

unknown

Additional information

Bug in Mozilla's bug tracker: bug 1691902

davidp3 commented 2 years ago

Just to +1 DllCanUnloadNow, the docs for it suggest that, apart from the CoGetClassObject special case, this is what it is for. Raymond Chen's blog has this post that takes it a step further to warn of a potential race with unloading, even if your DllCanUnloadNow is right. Looks like it will need that, too.

jp9000 commented 2 years ago

You know, while making my argument, I completely forgot that I even wrote a DllCanUnloadNow() already. I think I even forgot about it last time I tried to fix this issue. I forgot that it even existed as an export. What an embarrassing nightmare I put myself in. That really has to be the issue. (I should probably still modify that constructor/destructor logic though, because it's definitely still atypical)

Still, I have to admit that I feel pretty embarrassed if that's the case. And it really has to be the case. I'm going to try to figure out a different way to approach this for the time being. I would like to say that I really appreciate everyone's patience with this, their patience with me, and also the feedback. I apologize for any wild goose chases here. Thank you @gcp, @davidp3, @randomascii, and @notr1ch.

My guess for why this happens with this program particularly is just because it's likely more aggressive about cleaning up. Most COM applications either don't uninitialize or uninitialize on shutdown. I assumed it was erroneous thread usage but now I see that it was probably just more thorough. Kind of ironic in a way.

That being said, I really am not a fan of COM or DirectShow.

jp9000 commented 2 years ago

I'll have a fix for the destruction issue next patch. The buffer size issue should have been resolved already. And the issue the with the callbacks I will also attempt to fix.

I apologize if I gave you and your users any grief. I'm just going to go sulk in a corner and lament this whole thing for a long while.

crossan007 commented 2 years ago

I account for exactly 1 weekly occurrence of 1707060. It happens every single time without fail when I conclude my Tuesday morning meeting, so I'll be glad to re-test on the Tuesday following the release of the patch :-)

Fingers crossed for DllCanUnloadNow.

Thanks to everyone who contributed to this issue!

jp9000 commented 2 years ago

For the crash on filter unload, I've added this code: https://github.com/obsproject/obs-studio/commit/5efb3ea42df3ae01df065a93ae88ec04095a71ed

It's a bit of a weak/cheap fix, but the reason why it was likely to unload during destruction was in the th.join();, which could take up to a number of milliseconds. I would have liked to have changed the thread to only exist between run/stop calls, but the important thing is that the DLL can't unload while in that wait state, which was fundamentally the problem.

For the issue where the filter would still send data even when stopped, I've added this code: https://github.com/obsproject/obs-studio/commit/0f08432f539a3dd72fe88cf3191f5fc308d97107

Again another pretty weak/cheap fix; I'm a little ashamed of this one and because the loop is still technically going, but it'll prevent frames from submitting for now.

I want to do a bit of an overhaul on the code, but the reason I'm making these very simple fixes is mostly because I'm just a bit paranoid of doing an overhaul to the filter code until I can set up a better testing environment for it. I know how to test these issues, but I just need to build some tests that can emulate these scenarios. I just have a lot on my plate at the moment. But these should work fine for now.

notr1ch commented 2 years ago

There are several virtual camera bugs that are being conflated. I'm closing this issue as we've shipped two fixes in v28+ that should fix the unloaded virtual camera issue and the output-after-stopped issue. The original issue in this thread I believe was fixed in c5f06a2837b2e54d1a59f13ae31ba4e702cd9fd8.

There is one known bug remaining - switching resolutions (especially above 1080p) may cause corruption or crashing. A fix is being worked on. If you experience other virtual camera bugs, please file a new issue. I'll be aggressively deleting comments from future issues that mix up different bugs so we can better track things.