Skip to content

Commit

Permalink
Move COM initialization from object to GUI/audio threads
Browse files Browse the repository at this point in the history
CSoundInterface previously called CoInitializeEx and CoUninitialize in
its constructor and destructor. However this failed on Windows 7: On
startup or after changing configuration, CSoundInterface::OpenChannel
() failed to call IAudioClient::Initialize(), and it instead returned
0x800401f0 CO_E_NOTINITIALIZED. This is because CSoundInterface()
called CoInitializeEx() on the GUI thread, while
CSoundInterface::OpenChannel() is called on the audio thread. This is a
program bug, and Windows 10 and Wine happened to mask this error for
some reason.

To avoid this issue, call CoInitializeEx and CoUninitialize on the
GUI/audio threads' startup/teardown functions (CFamiTrackerApp and
CSoundGen::InitInstance/ExitInstance()), rather than the
CSoundInterface object's constructor and destructor.

Hopefully there aren't problems with accessing COM objects on the wrong
thread. It doesn't matter where IMMDeviceEnumerator/
IMMDeviceCollection/IMMDevice are constructed and accessed, since those
types are only accessed in startup/teardown(which don't need to be
real-time). All real-time logic talks to IAudioClient and
IAudioRenderClient, which are created on the audio thread in
CSoundInterface::OpenChannel() (and hopefully belong to the audio
thread, and if not they use a free-threaded marshaller anyway) and only
accessed on the audio thread.

COM threading is hard. RustAudio/cpal#597,
mozilla/cubeb#534
  • Loading branch information
nyanpasu64 committed Apr 17, 2022
1 parent 6ebff9f commit 295df07
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 17 deletions.
14 changes: 14 additions & 0 deletions Source/FamiTracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ END_MESSAGE_MAP()
// CFamiTrackerApp construction

CFamiTrackerApp::CFamiTrackerApp() :
m_CoInitialized(false),
m_bThemeActive(false),
m_pMIDI(NULL),
m_pAccel(NULL),
Expand Down Expand Up @@ -123,6 +124,15 @@ BOOL CFamiTrackerApp::InitInstance()
TRACE("OLE initialization failed\n");
}

HRESULT hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
if (FAILED(hr)) {
TRACE("CFamiTrackerApp: Failed to CoInitializeEx COM!\n");
}
if (!FAILED(hr)) {
// Call CoUninitialize() on shutdown.
m_CoInitialized = true;
}

// Standard initialization
// If you are not using these features and wish to reduce the size
// of your final executable, you should remove from the following
Expand Down Expand Up @@ -336,6 +346,10 @@ int CFamiTrackerApp::ExitInstance()

m_pVersionChecker.reset(); // // //

if (m_CoInitialized) {
CoUninitialize();
}

TRACE("App: End ExitInstance\n");

return CWinApp::ExitInstance();
Expand Down
1 change: 1 addition & 0 deletions Source/FamiTracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ class CFamiTrackerApp : public CWinApp
CMutex *m_pInstanceMutex;
HANDLE m_hWndMapFile;

bool m_CoInitialized;
bool m_bRunning = false; // // //
bool m_bThemeActive;

Expand Down
14 changes: 14 additions & 0 deletions Source/SoundGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ CSoundGen::CSoundGen() :
m_pInstRecorder(new CInstrumentRecorder(this)), // // //
m_bWaveChanged(0),
m_iMachineType(NTSC),
m_CoInitialized(false),
m_bRunning(false),
m_hInterruptEvent(NULL),
m_bBufferTimeout(false),
Expand Down Expand Up @@ -1989,6 +1990,15 @@ BOOL CSoundGen::InitInstance()
// Setup the sound player object, called when thread is started
//

HRESULT hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
if (FAILED(hr)) {
TRACE("SoundGen: Failed to CoInitializeEx COM!\n");
}
if (!FAILED(hr)) {
// Call CoUninitialize() on shutdown.
m_CoInitialized = true;
}

ASSERT(m_pDocument != NULL);
ASSERT(m_pTrackerView != NULL);

Expand Down Expand Up @@ -2061,6 +2071,10 @@ int CSoundGen::ExitInstance()

m_bRunning = false;

if (m_CoInitialized) {
CoUninitialize();
}

return CWinThread::ExitInstance();
}

Expand Down
1 change: 1 addition & 0 deletions Source/SoundGen.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ class CSoundGen : public CWinThread, IAudioCallback

const CDSample *m_pPreviewSample;

bool m_CoInitialized;
bool m_bRunning;

// Thread synchronization
Expand Down
17 changes: 1 addition & 16 deletions Source/SoundInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,12 @@
// Instance members

CSoundInterface::CSoundInterface(HANDLE hInterrupt) :
m_hInterrupt(hInterrupt),
m_CoInitialized(false)
m_hInterrupt(hInterrupt)
{
// Based off:
// https://github.com/wareya/AudioLibraryRosettaStone/blob/master/wasapi.cpp#L52
// https://github.com/thestk/rtaudio/blob/e9b1d0262a5e75e09c510eb9c5825daf86884d29/RtAudio.cpp#L4320

HRESULT hr = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED);
// Ignore error RPC_E_CHANGED_MODE and proceed.
if (FAILED(hr) && hr != RPC_E_CHANGED_MODE) {
// COM setup failed, don't try initializing audio.
return;
}
if (!FAILED(hr)) {
// Call CoUninitialize() on shutdown.
m_CoInitialized = true;
}

// Instantiate device enumerator
if (FAILED(CoCreateInstance(__uuidof(MMDeviceEnumerator), NULL,
CLSCTX_ALL, __uuidof(IMMDeviceEnumerator),
Expand All @@ -74,9 +62,6 @@ CSoundInterface::CSoundInterface(HANDLE hInterrupt) :

CSoundInterface::~CSoundInterface()
{
if (m_CoInitialized) {
CoUninitialize();
}
}

void CSoundInterface::EnumerateDevices()
Expand Down
1 change: 0 additions & 1 deletion Source/SoundInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ class CSoundInterface

private:
HANDLE m_hInterrupt;
bool m_CoInitialized;

// For enumeration
ComPtr<IMMDeviceEnumerator> m_maybeDeviceEnumerator;
Expand Down

0 comments on commit 295df07

Please sign in to comment.