[PATCH 0/1] MR11717: faudio: Initialize COM before creating the WMA decoder.
Found while testing STEINS;GATE RE:BOOT (https://store.steampowered.com/app/4012810/STEINSGATE_REBOOT/). The visual novel voiceovers were not playing and every time one was supposed to play, the following showed up in logs: `err:ole:com_get_class_object apartment not initialised` Tracing this led me to `FAudio_WMADEC_init`, which runs CoCreateInstance on the thread the application called IXAudio2::CreateSourceVoice() from, with FAudio however never initializing COM on that path. That makes combase then reject the call with the decoder never being created. I believe most games don't run into this because Media Foundation is usually initialized before this code path is hit, which leaves a process-wide MTA in place. Since the voiceovers play correctly on Windows, handling it in FAudio seemed like the right call. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717
From: Nello De Gregoris <bluechxindv@gmail.com> --- libs/faudio/src/FAudio_platform_win32_wmadec.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libs/faudio/src/FAudio_platform_win32_wmadec.c b/libs/faudio/src/FAudio_platform_win32_wmadec.c index 8d7b9185a94..c5485fcd5c6 100644 --- a/libs/faudio/src/FAudio_platform_win32_wmadec.c +++ b/libs/faudio/src/FAudio_platform_win32_wmadec.c @@ -22,6 +22,7 @@ struct FAudioWMADEC { IMFTransform *decoder; IMFSample *output_sample; + BOOL co_init; char *output_buf; size_t output_pos; @@ -266,6 +267,10 @@ uint32_t FAudio_WMADEC_init(FAudioSourceVoice *voice, uint32_t type) if (!(impl = voice->audio->pMalloc(sizeof(*impl)))) return -1; FAudio_memset(impl, 0, sizeof(*impl)); + /* the app's thread might not have initialised COM, and CoCreateInstance + * below fails if it has not, leaving the voice to decode to silence */ + impl->co_init = SUCCEEDED(CoInitializeEx(NULL, COINIT_MULTITHREADED)); + hr = CoCreateInstance( &CLSID_CWMADecMediaObject, 0, @@ -275,6 +280,7 @@ uint32_t FAudio_WMADEC_init(FAudioSourceVoice *voice, uint32_t type) ); if (FAILED(hr)) { + if (impl->co_init) CoUninitialize(); voice->audio->pFree(impl->output_buf); return -2; } @@ -578,6 +584,7 @@ void FAudio_WMADEC_free(FAudioSourceVoice *voice) if (impl->output_sample) IMFSample_Release(impl->output_sample); IMFTransform_Release(impl->decoder); voice->audio->pFree(impl->output_buf); + if (impl->co_init) CoUninitialize(); voice->audio->pFree(voice->src.wmadec); voice->src.wmadec = NULL; voice->src.decode = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11717
I don't think libraries should be using CoInitialize() at all. Unless of course xaudio2 happens to do that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149422
That would ultimately need a test to check if xaudio really initializes COM on that path (likely not indeed). Note that it is not only a matter of abstract correctness, initializing COM sets specific aparentment, and initialized state affects the app and can easily produce a bug. Where COM is initialized (and how, if it is, is that really multithreaded apt?) needs to be precise. Likely COM is initiliased elsewhere implicitly and the game depends of that, while finding what is supposed to do that exactly might be not exactly easy. Like it might be some random directshow component doing it, I found one not so long ago (2bcb70b3293c82b0a5bbbe5e56913e7ea99e2716, ba79f532310a5f6819b4807806af8d8ba0bb5a44) in the context of some other game but there may be more. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149425
Note in specific case I linked there is not even anything initializing COM on the thread where the game cares about it, that is implicit MTA initialized by wmvcode in its own thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149428
Native xaudio probably doesn't initialize COM, but it probably doesn't use CoCreateInstance() either. I don't think it uses an external WMA decoder at all, but rather one embedded in the DLL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149430
Oh, maybe, in that case it should probably either try to avoid using CoCreateInstance (not sure offhand if that is possible) or go hard way using COM initiliaze spy, somewhat similar to how it is done in dlls/imm32/imm.c. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149432
FWIW whether xaudio works with the same calls without COM initialized can also be tested, even if ad-hoc without finalizing upstream patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149433
Thanks everyone for taking a look and for the pointers! I ran a few tests on Windows and indeed xaudio does not initialize COM, but it seems to require it, since CreateMasteringVoice() fails outright when the process has no apartment with CO_E_NOTINITIALIZED. However what it seems to initialize COM implicitly is CreateMasteringVoice() itself. I put the main thread in an STA, then checked the apartment from a new thread that never touched COM (so if I'm correct it should only report IMPLICIT_MTA if an MTA exists somewhere in the process) and after CreateMasteringVoice there does seem to be one. Windows reports CO_E_NOTINITIALIZED on that new thread before and right after XAudio2Create(), and then APTTYPE_MTA with APTTYPEQUALIFIER_IMPLICIT_MTA once the mastering voice has been created. The main thread stays in its STA the whole time, so native creates that MTA without touching the caller's apartment, which looks a lot more like CoIncrementMTAUsage() to me. Should I try drafting a solution using that? It's my first time working with COM so I might be missing something else... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11717#note_149530
participants (5)
-
Elizabeth Figura (＠zfigura) -
Nello De Gregoris
-
Nello De Gregoris (＠bluechxin)
-
Nikolay Sivov (＠nsivov)
-
Paul Gofman (＠gofman)