raylib
raylib copied to clipboard
raysan5
[raudio] Fix crash when switching playback device
@jkaup those reviews should be pushed to miniaudio repo, not raylib, but probably @mackron is already aware of that potential issue.
I put in two comments with what I did when I fixed this myself.
-
I fetch the return value of
ma_CoInitializeExand store in a HRESULT and then check that one before uniniting. I do not know if this is needed. -
Because (1) uses windows specific stuff I also added the MA_WIN32 things checks. The crashes only happen for me on Windows anyways, so maybe best to only do these workaround there?
Like I said, I'm not sure if you need to do those extra things I did. Other then that it looks like the same thing I did to fix the crashes.
@karl-zylinski As per MSDN (https://learn.microsoft.com/en-us/windows/desktop/api/combaseapi/nf-combaseapi-couninitialize):
To close the COM library gracefully on a thread, each successful call to CoInitialize or CoInitializeEx, including any call that returns S_FALSE, must be balanced by a corresponding call to [CoUninitialize].
You don't really have to store HRESULT as it should always be successful, meaning you should always call CoUninitialize. (It can't return RPC_E_CHANGED_MODE since we always pass a constant COINIT value). And you also don't have to add MA_WIN32 checks since we are dealing with WASAPI which is Windows only.
@raysan5 On my initial investigation of this issue (link) the only place that required modification is ma_context_get_MMDevice__wasapi function, which solved the issue completely.
@veins1 Nice! Thanks for reporting!
@jkaup Please, could you update this PR as per the latest comment? It seems only ma_context_get_MMDevice__wasapi() should be modified.
Thanks team, and sorry for my silence.
When the device changes, WASAPI fires a callback from a separate thread. What I suspect is happening is that COM is not being initialized on that callback thread which is then resulting in this issue. If the COM initialization was moved to the ma_IMMNotificationClient_* callbacks I suspect that would also address the issue. However, since multiple people have confirmed the proposed fix works, and I'm planning on refactoring the WASAPI backend anyway, I'm happy enough to get that merged upstream. It would be good if @veins1 suggestion could be confirmed though just to reduce the amount of COM sitting around in the code base.
Thanks for the input on the issue @veins1 and @mackron. I will make the changes as proposed by @raysan5 as soon as possible.
Hi @raysan5 , sorry, got back to work today at last. I've updated the PR so that the proposed fix only affects ma_context_get_MMDevice__wasapi(), and have confirmed that this is enough to fix the crash as noted by @veins1 .
@jkaup Thanks a lot for this. @raysan5 This has been integrated upstream in the dev branch.