search

bozbez / win-capture-audio

An OBS plugin that allows capture of independant application audio streams on Windows, in a similar fashion to OBS's game capture and Discord's application streaming.
GNU General Public License v2.0
3.78k stars 205 forks source link

Exception when detaching from process #158

Closed mauvealerts closed 2 years ago

mauvealerts commented 2 years ago

I've observed that the plugin seems to stop working sporadically, no longer capturing audio until I restart OBS.

Looking through OBS logs, the exception below seems a likely culprit. This occurred when the game I was capturing exited. Both game-capture and win-capture-audio should be capturing from the same process. The exception is thrown in SessionMonitor::RemoveSession on the line THROW_IF_FAILED(session_control2->GetSessionIdentifier(session_id_raw.put()));

15:19:44.667: ==== Recording Stop ================================================
15:19:46.820: [game-capture: 'Spel2 Video'] capture window no longer exists, terminating capture
15:19:46.820: [game-capture: 'Spel2 Video'] capture stopped
15:34:33.740: [game-capture: 'Spel2 Video'] attempting to hook process: Spel2.exe
15:34:33.771: [game-capture: 'Spel2 Video'] using helper (compatibility hook)
15:34:33.772: [game-capture: 'Spel2 Video'] hook not loaded yet, retrying..
15:34:36.755: [game-capture: 'Spel2 Video'] attempting to hook process: Spel2.exe
15:34:36.763: [game-capture: 'Spel2 Video'] d3d11 shared texture capture successful
15:34:36.770: [game-capture: 'Spel2 Video'] shared texture capture successful
15:36:38.319: [game-capture: 'Spel2 Video'] capture window no longer exists, terminating capture
15:36:38.320: [game-capture: 'Spel2 Video'] capture stopped
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(1) tid(5cac) 88890004 
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(2) tid(5c68) 88890004 
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(6) tid(5d3c) 88890004 
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(7) tid(5b40) 88890004 
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(5) tid(523c) 88890004 
16:55:53.654: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(4) tid(586c) 88890004 
16:55:53.655: [win-capture-audio] (SafeRun) ..\src\session-monitor.cpp(204)\win-capture-audio.dll!00007FFE10EAFF33: (caller: 00007FFE10EB026C) Exception(3) tid(5ccc) 88890004
mauvealerts commented 2 years ago

API documentation suggests the cause is probably AUDCLNT_E_DEVICE_INVALIDATED ("The audio session is disconnected on the default audio device.") That makes me think it's an inherent race condition... I don't understand the purpose of this code well enough to speculate about better handling for this case.

mauvealerts commented 2 years ago

IIUC, a SessionEventNotificationClient instance is 1-1 with a session and SessionWatcher instance. The SessionWatcher has the info to construct the SessionKey, butSessionEventNotificationClient instance doesn't; the latter is what sends the event.

If that's correct, then we "just" need to plumb pid+session_id through so there's no need to query it in SessionMonitor::RemoveSession?

bozbez commented 2 years ago

Yeah so the reason this code is such a mess is because IAudioSessionNotification (which we need to create per-device after session enumeration) only gives us session creation notifications (who would care about sessions expiring right?). To get the session expiry notifications we have to register a IAudioSessionEvents per session which as you have seen is done by each session's SessionWatcher.

The the SessionWatchers themselves need to keep the IAudioSessionControl around to unregister the IAudioSessionEvents notification client, but I agree there isn't any need to actually pass the session controls back up through SessionEvents::SessionExpired. There should be a race issue every time we try to query the PID/SID pair from the session control since there is a chance they arrive already expired in IAudioSessionNotification::OnSessionCreated but for sure the more common case would be on normal session removal.

If you are wondering why both the PID and SID are used it is because the SID on it's own is not actually unique (lol): if two processes of the same application create two sessions they will share an SID so we must also package it with the PID.

5ecaa981a69e19779d0a5540975e1444f15269c0 should fix all the races, could you give it go and see if the issue persists?

mauvealerts commented 2 years ago

That change looks promising. I'm giving it a go, using a build against OBS main (via #155). I'm not sure I can gain confidence in it tonight... I tried repo-ing the exception with the release version, and just starting+exiting the game ~20 times didn't make it happen.

I'll try to report back in a few days. Feel free to close tho; I can reopen if the issue persists.

Re: Windows API stuff: I note there's IAudioSessionControl2::GetSessionInstanceIdentifier. I haven't tried it, but sounds like it'd remove the need to cart around the pid. (Please excuse the pun)

mauvealerts commented 2 years ago

It's been a few days and a bunch of restarting games. It's hard to be sure with race conditions, but I think this is likely resolved. Plugin definitely seems to be working fine