From 374629ef3069aa92e53fd033d0cb6e7f0fa386c5 Mon Sep 17 00:00:00 2001 From: Silent Date: Fri, 16 Aug 2019 20:36:50 +0200 Subject: [PATCH 1/9] AudioCommon: Make HandleWinAPI handle all success return values properly, not just S_OK --- Source/Core/AudioCommon/WASAPIStream.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 6e6cd1950a17..d1f3e788b1df 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -62,7 +62,7 @@ bool WASAPIStream::isValid() static bool HandleWinAPI(std::string_view message, HRESULT result) { - if (result != S_OK) + if (FAILED(result)) { _com_error err(result); std::string error = TStrToUTF8(err.ErrorMessage()); @@ -77,7 +77,7 @@ static bool HandleWinAPI(std::string_view message, HRESULT result) ERROR_LOG_FMT(AUDIO, "WASAPI: {}: {}", message, error); } - return result == S_OK; + return SUCCEEDED(result); } std::vector WASAPIStream::GetAvailableDevices() From c5a372ab2a74ff5b81c6611bbfefe3d8ddad11fd Mon Sep 17 00:00:00 2001 From: Silent Date: Fri, 16 Aug 2019 23:53:47 +0200 Subject: [PATCH 2/9] AudioCommon/WASAPI: Use WRL/WIL whenever possible This fixes numerous resource leaks, as not every return path cleaned every created resource Now they are all managed automatically and "commited" to WASAPIStream class fields only after it's certain they initialized properly --- Source/Core/AudioCommon/WASAPIStream.cpp | 241 +++++++++-------------- Source/Core/AudioCommon/WASAPIStream.h | 21 +- 2 files changed, 105 insertions(+), 157 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index d1f3e788b1df..2abebd0e97d3 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -12,18 +12,24 @@ #include #include #include -#include +#include // clang-format on +#include + +#include "Common/Assert.h" #include "Common/Logging/Log.h" #include "Common/StringUtil.h" #include "Common/Thread.h" #include "Core/ConfigManager.h" #include "VideoCommon/OnScreenDisplay.h" +using Microsoft::WRL::ComPtr; + WASAPIStream::WASAPIStream() { - CoInitialize(nullptr); + if (SUCCEEDED(CoInitializeEx(nullptr, COINIT_MULTITHREADED))) + m_coinitialize.activate(); m_format.Format.wFormatTag = WAVE_FORMAT_EXTENSIBLE; m_format.Format.nChannels = 2; @@ -39,20 +45,12 @@ WASAPIStream::WASAPIStream() WASAPIStream::~WASAPIStream() { - if (m_enumerator) - m_enumerator->Release(); - - if (m_need_data_event) - CloseHandle(m_need_data_event); - if (m_running) { m_running = false; if (m_thread.joinable()) m_thread.join(); } - - CoUninitialize(); } bool WASAPIStream::isValid() @@ -82,134 +80,119 @@ static bool HandleWinAPI(std::string_view message, HRESULT result) std::vector WASAPIStream::GetAvailableDevices() { - CoInitialize(nullptr); + HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + // RPC_E_CHANGED_MODE means that thread has COM already initialized with a different threading + // model. We don't necessarily need multithreaded model here, so don't treat this as an error + if (result != RPC_E_CHANGED_MODE && !HandleWinAPI("Failed to call CoInitialize", result)) + return {}; - IMMDeviceEnumerator* enumerator = nullptr; + wil::unique_couninitialize_call cleanup; + if (FAILED(result)) + cleanup.release(); // CoUninitialize must be matched with each successful CoInitialize call, so + // don't call it if initialize fails + + ComPtr enumerator; - HRESULT result = - CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, - __uuidof(IMMDeviceEnumerator), reinterpret_cast(&enumerator)); + result = CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, + IID_PPV_ARGS(enumerator.GetAddressOf())); if (!HandleWinAPI("Failed to create MMDeviceEnumerator", result)) return {}; - std::vector device_names; - IMMDeviceCollection* devices; - result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, &devices); + ComPtr devices; + result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, devices.GetAddressOf()); if (!HandleWinAPI("Failed to get available devices", result)) - { - CoUninitialize(); return {}; - } UINT count; devices->GetCount(&count); + std::vector device_names; + device_names.reserve(count); + for (u32 i = 0; i < count; i++) { - IMMDevice* device; - devices->Item(i, &device); + ComPtr device; + devices->Item(i, device.GetAddressOf()); if (!HandleWinAPI("Failed to get device " + std::to_string(i), result)) continue; - LPWSTR device_id; - device->GetId(&device_id); + ComPtr device_properties; - IPropertyStore* device_properties; - - result = device->OpenPropertyStore(STGM_READ, &device_properties); + result = device->OpenPropertyStore(STGM_READ, device_properties.GetAddressOf()); if (!HandleWinAPI("Failed to initialize IPropertyStore", result)) continue; - PROPVARIANT device_name; - PropVariantInit(&device_name); - - device_properties->GetValue(PKEY_Device_FriendlyName, &device_name); + wil::unique_prop_variant device_name; + device_properties->GetValue(PKEY_Device_FriendlyName, device_name.addressof()); device_names.push_back(TStrToUTF8(device_name.pwszVal)); - - PropVariantClear(&device_name); } - devices->Release(); - enumerator->Release(); - - CoUninitialize(); - return device_names; } -IMMDevice* WASAPIStream::GetDeviceByName(std::string name) +ComPtr WASAPIStream::GetDeviceByName(std::string name) { - CoInitialize(nullptr); + HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); + // RPC_E_CHANGED_MODE means that thread has COM already initialized with a different threading + // model. We don't necessarily need multithreaded model here, so don't treat this as an error + if (result != RPC_E_CHANGED_MODE && !HandleWinAPI("Failed to call CoInitialize", result)) + return nullptr; + + wil::unique_couninitialize_call cleanup; + if (FAILED(result)) + cleanup.release(); // CoUninitialize must be matched with each successful CoInitialize call, so + // don't call it if initialize fails - IMMDeviceEnumerator* enumerator = nullptr; + ComPtr enumerator; - HRESULT result = - CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, - __uuidof(IMMDeviceEnumerator), reinterpret_cast(&enumerator)); + result = CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, + IID_PPV_ARGS(enumerator.GetAddressOf())); if (!HandleWinAPI("Failed to create MMDeviceEnumerator", result)) return nullptr; - IMMDeviceCollection* devices; - result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, &devices); + ComPtr devices; + result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, devices.GetAddressOf()); if (!HandleWinAPI("Failed to get available devices", result)) - { - return {}; - } + return nullptr; UINT count; devices->GetCount(&count); for (u32 i = 0; i < count; i++) { - IMMDevice* device; - devices->Item(i, &device); + ComPtr device; + devices->Item(i, device.GetAddressOf()); if (!HandleWinAPI("Failed to get device " + std::to_string(i), result)) continue; - LPWSTR device_id; - device->GetId(&device_id); - - IPropertyStore* device_properties; + ComPtr device_properties; - result = device->OpenPropertyStore(STGM_READ, &device_properties); + result = device->OpenPropertyStore(STGM_READ, device_properties.GetAddressOf()); if (!HandleWinAPI("Failed to initialize IPropertyStore", result)) continue; - PROPVARIANT device_name; - PropVariantInit(&device_name); - - device_properties->GetValue(PKEY_Device_FriendlyName, &device_name); + wil::unique_prop_variant device_name; + device_properties->GetValue(PKEY_Device_FriendlyName, device_name.addressof()); if (TStrToUTF8(device_name.pwszVal) == name) - { - devices->Release(); - enumerator->Release(); - CoUninitialize(); return device; - } - - PropVariantClear(&device_name); } - devices->Release(); - enumerator->Release(); - CoUninitialize(); - return nullptr; } bool WASAPIStream::Init() { - HRESULT result = - CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, - __uuidof(IMMDeviceEnumerator), reinterpret_cast(&m_enumerator)); + ASSERT(m_enumerator == nullptr); + HRESULT result = CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, + IID_PPV_ARGS(m_enumerator.GetAddressOf())); if (!HandleWinAPI("Failed to create MMDeviceEnumerator", result)) return false; @@ -221,13 +204,13 @@ bool WASAPIStream::SetRunning(bool running) { if (running) { - IMMDevice* device = nullptr; + ComPtr device; HRESULT result; if (SConfig::GetInstance().sWASAPIDevice == "default") { - result = m_enumerator->GetDefaultAudioEndpoint(eRender, eConsole, &device); + result = m_enumerator->GetDefaultAudioEndpoint(eRender, eConsole, device.GetAddressOf()); } else { @@ -238,59 +221,46 @@ bool WASAPIStream::SetRunning(bool running) { ERROR_LOG_FMT(AUDIO, "Can't find device '{}', falling back to default", SConfig::GetInstance().sWASAPIDevice); - result = m_enumerator->GetDefaultAudioEndpoint(eRender, eConsole, &device); + result = m_enumerator->GetDefaultAudioEndpoint(eRender, eConsole, device.GetAddressOf()); } } if (!HandleWinAPI("Failed to obtain default endpoint", result)) return false; - LPWSTR device_id; - device->GetId(&device_id); - // Show a friendly name in the log - IPropertyStore* device_properties; + ComPtr device_properties; - result = device->OpenPropertyStore(STGM_READ, &device_properties); + result = device->OpenPropertyStore(STGM_READ, device_properties.GetAddressOf()); if (!HandleWinAPI("Failed to initialize IPropertyStore", result)) return false; - PROPVARIANT device_name; - PropVariantInit(&device_name); - - device_properties->GetValue(PKEY_Device_FriendlyName, &device_name); + wil::unique_prop_variant device_name; + device_properties->GetValue(PKEY_Device_FriendlyName, device_name.addressof()); INFO_LOG_FMT(AUDIO, "Using audio endpoint '{}'", TStrToUTF8(device_name.pwszVal)); - PropVariantClear(&device_name); + ComPtr audio_client; // Get IAudioDevice result = device->Activate(__uuidof(IAudioClient), CLSCTX_INPROC_SERVER, nullptr, - reinterpret_cast(&m_audio_client)); + reinterpret_cast(audio_client.GetAddressOf())); if (!HandleWinAPI("Failed to activate IAudioClient", result)) - { - device->Release(); return false; - } REFERENCE_TIME device_period = 0; - result = m_audio_client->GetDevicePeriod(nullptr, &device_period); + result = audio_client->GetDevicePeriod(nullptr, &device_period); device_period += SConfig::GetInstance().iLatency * (10000 / m_format.Format.nChannels); INFO_LOG_FMT(AUDIO, "Audio period set to {}", device_period); if (!HandleWinAPI("Failed to obtain device period", result)) - { - device->Release(); - m_audio_client->Release(); - m_audio_client = nullptr; return false; - } - result = m_audio_client->Initialize( + result = audio_client->Initialize( AUDCLNT_SHAREMODE_EXCLUSIVE, AUDCLNT_STREAMFLAGS_EVENTCALLBACK | AUDCLNT_STREAMFLAGS_NOPERSIST, device_period, device_period, reinterpret_cast(&m_format), nullptr); @@ -305,87 +275,61 @@ bool WASAPIStream::SetRunning(bool running) if (result == AUDCLNT_E_BUFFER_SIZE_NOT_ALIGNED) { - result = m_audio_client->GetBufferSize(&m_frames_in_buffer); - m_audio_client->Release(); + result = audio_client->GetBufferSize(&m_frames_in_buffer); if (!HandleWinAPI("Failed to get aligned buffer size", result)) - { - device->Release(); - m_audio_client->Release(); - m_audio_client = nullptr; return false; - } // Get IAudioDevice result = device->Activate(__uuidof(IAudioClient), CLSCTX_INPROC_SERVER, nullptr, - reinterpret_cast(&m_audio_client)); + reinterpret_cast(audio_client.ReleaseAndGetAddressOf())); if (!HandleWinAPI("Failed to reactivate IAudioClient", result)) - { - device->Release(); return false; - } device_period = static_cast( 10000.0 * 1000 * m_frames_in_buffer / m_format.Format.nSamplesPerSec + 0.5) + SConfig::GetInstance().iLatency * 10000; - result = m_audio_client->Initialize( + result = audio_client->Initialize( AUDCLNT_SHAREMODE_EXCLUSIVE, AUDCLNT_STREAMFLAGS_EVENTCALLBACK | AUDCLNT_STREAMFLAGS_NOPERSIST, device_period, device_period, reinterpret_cast(&m_format), nullptr); } if (!HandleWinAPI("Failed to initialize IAudioClient", result)) - { - device->Release(); - m_audio_client->Release(); - m_audio_client = nullptr; return false; - } - result = m_audio_client->GetBufferSize(&m_frames_in_buffer); + result = audio_client->GetBufferSize(&m_frames_in_buffer); if (!HandleWinAPI("Failed to get buffer size from IAudioClient", result)) - { - device->Release(); - m_audio_client->Release(); - m_audio_client = nullptr; return false; - } - result = m_audio_client->GetService(__uuidof(IAudioRenderClient), - reinterpret_cast(&m_audio_renderer)); + ComPtr audio_renderer; + + result = audio_client->GetService(IID_PPV_ARGS(audio_renderer.GetAddressOf())); if (!HandleWinAPI("Failed to get IAudioRenderClient from IAudioClient", result)) - { - device->Release(); - m_audio_client->Release(); - m_audio_client = nullptr; return false; - } - m_need_data_event = CreateEvent(nullptr, FALSE, FALSE, nullptr); - m_audio_client->SetEventHandle(m_need_data_event); + wil::unique_event_nothrow need_data_event; + need_data_event.create(); + + audio_client->SetEventHandle(need_data_event.get()); - result = m_audio_client->Start(); + result = audio_client->Start(); if (!HandleWinAPI("Failed to get IAudioRenderClient from IAudioClient", result)) - { - device->Release(); - m_audio_renderer->Release(); - m_audio_renderer = nullptr; - m_audio_client->Release(); - m_audio_client = nullptr; - CloseHandle(m_need_data_event); return false; - } - - device->Release(); INFO_LOG_FMT(AUDIO, "WASAPI: Successfully initialized!"); + // "Commit" audio client and audio renderer now + m_audio_client = std::move(audio_client); + m_audio_renderer = std::move(audio_renderer); + m_need_data_event = std::move(need_data_event); + m_running = true; m_thread = std::thread([this] { SoundLoop(); }); m_thread.detach(); @@ -401,14 +345,9 @@ bool WASAPIStream::SetRunning(bool running) { } - if (m_audio_client) - { - m_audio_renderer->Release(); - m_audio_renderer = nullptr; - - m_audio_client->Release(); - m_audio_client = nullptr; - } + m_need_data_event.reset(); + m_audio_renderer.Reset(); + m_audio_client.Reset(); } return true; @@ -432,7 +371,7 @@ void WASAPIStream::SoundLoop() if (!m_audio_renderer) continue; - WaitForSingleObject(m_need_data_event, 1000); + WaitForSingleObject(m_need_data_event.get(), 1000); m_audio_renderer->GetBuffer(m_frames_in_buffer, &data); GetMixer()->Mix(reinterpret_cast(data), m_frames_in_buffer); diff --git a/Source/Core/AudioCommon/WASAPIStream.h b/Source/Core/AudioCommon/WASAPIStream.h index 54becb68c042..39d2d5d9860e 100644 --- a/Source/Core/AudioCommon/WASAPIStream.h +++ b/Source/Core/AudioCommon/WASAPIStream.h @@ -5,16 +5,19 @@ #pragma once #ifdef _WIN32 + // clang-format off #include #include +#include +#include // clang-format on -#endif #include #include #include #include +#include #include "AudioCommon/SoundStream.h" @@ -23,6 +26,8 @@ struct IAudioRenderClient; struct IMMDevice; struct IMMDeviceEnumerator; +#endif + class WASAPIStream final : public SoundStream { #ifdef _WIN32 @@ -35,7 +40,7 @@ class WASAPIStream final : public SoundStream static bool isValid(); static std::vector GetAvailableDevices(); - static IMMDevice* GetDeviceByName(std::string name); + static Microsoft::WRL::ComPtr GetDeviceByName(std::string name); private: u32 m_frames_in_buffer = 0; @@ -43,10 +48,14 @@ class WASAPIStream final : public SoundStream std::atomic m_stopped = false; std::thread m_thread; - IAudioClient* m_audio_client = nullptr; - IAudioRenderClient* m_audio_renderer = nullptr; - IMMDeviceEnumerator* m_enumerator = nullptr; - HANDLE m_need_data_event = nullptr; + // CoUninitialize must be called after all WASAPI COM objects have been destroyed, + // therefore this member must be located before them, as first class fields are destructed last + wil::unique_couninitialize_call m_coinitialize{false}; + + Microsoft::WRL::ComPtr m_enumerator; + Microsoft::WRL::ComPtr m_audio_client; + Microsoft::WRL::ComPtr m_audio_renderer; + wil::unique_event_nothrow m_need_data_event; WAVEFORMATEXTENSIBLE m_format; #endif // _WIN32 }; From ee60be45010fed98a3d28f736bcf00efb036560c Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 10:08:16 +0200 Subject: [PATCH 3/9] AudioCommon/WASAPI: Simplify thread synchronization model by not detaching WASAPI handler thread --- Source/Core/AudioCommon/WASAPIStream.cpp | 18 +++--------------- Source/Core/AudioCommon/WASAPIStream.h | 1 - 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 2abebd0e97d3..2d502c3f7cde 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -45,12 +45,9 @@ WASAPIStream::WASAPIStream() WASAPIStream::~WASAPIStream() { - if (m_running) - { - m_running = false; - if (m_thread.joinable()) - m_thread.join(); - } + m_running = false; + if (m_thread.joinable()) + m_thread.join(); } bool WASAPIStream::isValid() @@ -332,7 +329,6 @@ bool WASAPIStream::SetRunning(bool running) m_running = true; m_thread = std::thread([this] { SoundLoop(); }); - m_thread.detach(); } else { @@ -341,10 +337,6 @@ bool WASAPIStream::SetRunning(bool running) if (m_thread.joinable()) m_thread.join(); - while (!m_stopped) - { - } - m_need_data_event.reset(); m_audio_renderer.Reset(); m_audio_client.Reset(); @@ -364,8 +356,6 @@ void WASAPIStream::SoundLoop() m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, AUDCLNT_BUFFERFLAGS_SILENT); } - m_stopped = false; - while (m_running) { if (!m_audio_renderer) @@ -383,8 +373,6 @@ void WASAPIStream::SoundLoop() m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, 0); } - - m_stopped = true; } #endif // _WIN32 diff --git a/Source/Core/AudioCommon/WASAPIStream.h b/Source/Core/AudioCommon/WASAPIStream.h index 39d2d5d9860e..b4337a378a20 100644 --- a/Source/Core/AudioCommon/WASAPIStream.h +++ b/Source/Core/AudioCommon/WASAPIStream.h @@ -45,7 +45,6 @@ class WASAPIStream final : public SoundStream private: u32 m_frames_in_buffer = 0; std::atomic m_running = false; - std::atomic m_stopped = false; std::thread m_thread; // CoUninitialize must be called after all WASAPI COM objects have been destroyed, From 7d59ad262f1e22f7f570c91c28f5d01cd93970a6 Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 10:13:01 +0200 Subject: [PATCH 4/9] AudioCommon/WASAPI: Use leaner memory model on m_running, no need for a full barrier --- Source/Core/AudioCommon/WASAPIStream.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 2d502c3f7cde..794fb6781a3c 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -45,7 +45,7 @@ WASAPIStream::WASAPIStream() WASAPIStream::~WASAPIStream() { - m_running = false; + m_running.store(false, std::memory_order_relaxed); if (m_thread.joinable()) m_thread.join(); } @@ -327,12 +327,12 @@ bool WASAPIStream::SetRunning(bool running) m_audio_renderer = std::move(audio_renderer); m_need_data_event = std::move(need_data_event); - m_running = true; + m_running.store(true, std::memory_order_relaxed); m_thread = std::thread([this] { SoundLoop(); }); } else { - m_running = false; + m_running.store(false, std::memory_order_relaxed); if (m_thread.joinable()) m_thread.join(); @@ -356,7 +356,7 @@ void WASAPIStream::SoundLoop() m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, AUDCLNT_BUFFERFLAGS_SILENT); } - while (m_running) + while (m_running.load(std::memory_order_relaxed)) { if (!m_audio_renderer) continue; From 5dbbf36563ce5a75e84f58f000d45e3b4c1ab82d Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 11:05:33 +0200 Subject: [PATCH 5/9] AudioCommon/WASAPI: Use std::string_view where applicable --- Source/Core/AudioCommon/WASAPIStream.cpp | 9 ++++++--- Source/Core/AudioCommon/WASAPIStream.h | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 794fb6781a3c..2ee39f668c66 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -59,14 +59,16 @@ static bool HandleWinAPI(std::string_view message, HRESULT result) { if (FAILED(result)) { - _com_error err(result); - std::string error = TStrToUTF8(err.ErrorMessage()); + std::string error; switch (result) { case AUDCLNT_E_DEVICE_IN_USE: error = "Audio endpoint already in use!"; break; + default: + error = TStrToUTF8(_com_error(result).ErrorMessage()).c_str(); + break; } ERROR_LOG_FMT(AUDIO, "WASAPI: {}: {}", message, error); @@ -75,6 +77,7 @@ static bool HandleWinAPI(std::string_view message, HRESULT result) return SUCCEEDED(result); } + std::vector WASAPIStream::GetAvailableDevices() { HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); @@ -131,7 +134,7 @@ std::vector WASAPIStream::GetAvailableDevices() return device_names; } -ComPtr WASAPIStream::GetDeviceByName(std::string name) +ComPtr WASAPIStream::GetDeviceByName(std::string_view name) { HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); // RPC_E_CHANGED_MODE means that thread has COM already initialized with a different threading diff --git a/Source/Core/AudioCommon/WASAPIStream.h b/Source/Core/AudioCommon/WASAPIStream.h index b4337a378a20..640d0155fcdf 100644 --- a/Source/Core/AudioCommon/WASAPIStream.h +++ b/Source/Core/AudioCommon/WASAPIStream.h @@ -40,7 +40,7 @@ class WASAPIStream final : public SoundStream static bool isValid(); static std::vector GetAvailableDevices(); - static Microsoft::WRL::ComPtr GetDeviceByName(std::string name); + static Microsoft::WRL::ComPtr GetDeviceByName(std::string_view name); private: u32 m_frames_in_buffer = 0; From 991b3ba8c2543ccfc08a89929e44e6e8a33ce559 Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 11:15:46 +0200 Subject: [PATCH 6/9] AudioCommon/WASAPI: Remove thread unsafe nullptr checks giving a false sense of safety --- Source/Core/AudioCommon/WASAPIStream.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 2ee39f668c66..9be46cb751d8 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -353,17 +353,11 @@ void WASAPIStream::SoundLoop() Common::SetCurrentThreadName("WASAPI Handler"); BYTE* data; - if (m_audio_renderer) - { - m_audio_renderer->GetBuffer(m_frames_in_buffer, &data); - m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, AUDCLNT_BUFFERFLAGS_SILENT); - } + m_audio_renderer->GetBuffer(m_frames_in_buffer, &data); + m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, AUDCLNT_BUFFERFLAGS_SILENT); while (m_running.load(std::memory_order_relaxed)) { - if (!m_audio_renderer) - continue; - WaitForSingleObject(m_need_data_event.get(), 1000); m_audio_renderer->GetBuffer(m_frames_in_buffer, &data); From c373890505ee3cce3687078d4fe99b8f38ca9d2b Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 11:44:51 +0200 Subject: [PATCH 7/9] AudioCommon/WASAPI: Do volume adjustment only when really needed This skips a potentially costly loop if volume is 100% or 0%, as for former there is no need for volume adjustment, while latter can be solved by specifying a AUDCLNT_BUFFERFLAGS_SILENT flag --- Source/Core/AudioCommon/WASAPIStream.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 9be46cb751d8..552e60cd5585 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -361,14 +361,23 @@ void WASAPIStream::SoundLoop() WaitForSingleObject(m_need_data_event.get(), 1000); m_audio_renderer->GetBuffer(m_frames_in_buffer, &data); - GetMixer()->Mix(reinterpret_cast(data), m_frames_in_buffer); - float volume = SConfig::GetInstance().m_IsMuted ? 0 : SConfig::GetInstance().m_Volume / 100.; + s16* audio_data = reinterpret_cast(data); + GetMixer()->Mix(audio_data, m_frames_in_buffer); - for (u32 i = 0; i < m_frames_in_buffer * 2; i++) - reinterpret_cast(data)[i] = static_cast(reinterpret_cast(data)[i] * volume); + const SConfig& config = SConfig::GetInstance(); + const bool is_muted = config.m_IsMuted || config.m_Volume == 0; + const bool need_volume_adjustment = config.m_Volume != 100 && !is_muted; - m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, 0); + if (need_volume_adjustment) + { + const float volume = config.m_Volume / 100.0f; + + for (u32 i = 0; i < m_frames_in_buffer * 2; i++) + *audio_data++ *= volume; + } + + m_audio_renderer->ReleaseBuffer(m_frames_in_buffer, is_muted ? AUDCLNT_BUFFERFLAGS_SILENT : 0); } } From 11c5150c168f13dca001c9217f7c27cd250fb1b4 Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 17 Aug 2019 13:08:58 +0200 Subject: [PATCH 8/9] AudioCommon/WASAPI: Factorize device enumeration logic into a function to greatly reduce code duplication --- Source/Core/AudioCommon/WASAPIStream.cpp | 83 ++++++++---------------- 1 file changed, 26 insertions(+), 57 deletions(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 552e60cd5585..766435613257 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -77,14 +77,13 @@ static bool HandleWinAPI(std::string_view message, HRESULT result) return SUCCEEDED(result); } - -std::vector WASAPIStream::GetAvailableDevices() +static void ForEachNamedDevice(const std::function, std::string)>& callback) { HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); // RPC_E_CHANGED_MODE means that thread has COM already initialized with a different threading // model. We don't necessarily need multithreaded model here, so don't treat this as an error if (result != RPC_E_CHANGED_MODE && !HandleWinAPI("Failed to call CoInitialize", result)) - return {}; + return; wil::unique_couninitialize_call cleanup; if (FAILED(result)) @@ -97,20 +96,17 @@ std::vector WASAPIStream::GetAvailableDevices() IID_PPV_ARGS(enumerator.GetAddressOf())); if (!HandleWinAPI("Failed to create MMDeviceEnumerator", result)) - return {}; + return; ComPtr devices; result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, devices.GetAddressOf()); if (!HandleWinAPI("Failed to get available devices", result)) - return {}; + return; UINT count; devices->GetCount(&count); - std::vector device_names; - device_names.reserve(count); - for (u32 i = 0; i < count; i++) { ComPtr device; @@ -128,64 +124,37 @@ std::vector WASAPIStream::GetAvailableDevices() wil::unique_prop_variant device_name; device_properties->GetValue(PKEY_Device_FriendlyName, device_name.addressof()); - device_names.push_back(TStrToUTF8(device_name.pwszVal)); + if (!callback(std::move(device), TStrToUTF8(device_name.pwszVal))) + break; } - - return device_names; } -ComPtr WASAPIStream::GetDeviceByName(std::string_view name) +std::vector WASAPIStream::GetAvailableDevices() { - HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); - // RPC_E_CHANGED_MODE means that thread has COM already initialized with a different threading - // model. We don't necessarily need multithreaded model here, so don't treat this as an error - if (result != RPC_E_CHANGED_MODE && !HandleWinAPI("Failed to call CoInitialize", result)) - return nullptr; - - wil::unique_couninitialize_call cleanup; - if (FAILED(result)) - cleanup.release(); // CoUninitialize must be matched with each successful CoInitialize call, so - // don't call it if initialize fails - - ComPtr enumerator; - - result = CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER, - IID_PPV_ARGS(enumerator.GetAddressOf())); - - if (!HandleWinAPI("Failed to create MMDeviceEnumerator", result)) - return nullptr; - - ComPtr devices; - result = enumerator->EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, devices.GetAddressOf()); - - if (!HandleWinAPI("Failed to get available devices", result)) - return nullptr; - - UINT count; - devices->GetCount(&count); - - for (u32 i = 0; i < count; i++) - { - ComPtr device; - devices->Item(i, device.GetAddressOf()); - if (!HandleWinAPI("Failed to get device " + std::to_string(i), result)) - continue; - - ComPtr device_properties; + std::vector device_names; - result = device->OpenPropertyStore(STGM_READ, device_properties.GetAddressOf()); + ForEachNamedDevice([&device_names](ComPtr, std::string n) { + device_names.push_back(std::move(n)); + return true; + }); - if (!HandleWinAPI("Failed to initialize IPropertyStore", result)) - continue; + return device_names; +} - wil::unique_prop_variant device_name; - device_properties->GetValue(PKEY_Device_FriendlyName, device_name.addressof()); +ComPtr WASAPIStream::GetDeviceByName(std::string_view name) +{ + ComPtr device; - if (TStrToUTF8(device_name.pwszVal) == name) - return device; - } + ForEachNamedDevice([&device, &name](ComPtr d, std::string n) { + if (n == name) + { + device = std::move(d); + return false; + } + return true; + }); - return nullptr; + return device; } bool WASAPIStream::Init() From cb854d78328cce91804bc9e439039ee3d8f11b51 Mon Sep 17 00:00:00 2001 From: Silent Date: Sun, 18 Aug 2019 12:48:19 +0200 Subject: [PATCH 9/9] AudioCommon/WASAPI: Construct std::thread with invoke semantics instead of a lambda --- Source/Core/AudioCommon/WASAPIStream.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/AudioCommon/WASAPIStream.cpp b/Source/Core/AudioCommon/WASAPIStream.cpp index 766435613257..a22d36912143 100644 --- a/Source/Core/AudioCommon/WASAPIStream.cpp +++ b/Source/Core/AudioCommon/WASAPIStream.cpp @@ -300,7 +300,7 @@ bool WASAPIStream::SetRunning(bool running) m_need_data_event = std::move(need_data_event); m_running.store(true, std::memory_order_relaxed); - m_thread = std::thread([this] { SoundLoop(); }); + m_thread = std::thread(&WASAPIStream::SoundLoop, this); } else {