From 90d97053a867b78ecd5b52daa1935e04cf3beeb2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 6 Mar 2021 20:33:37 +0100 Subject: [PATCH 01/54] win32/ComWorker: make COMWorker a real class, no static members --- src/mixer/plugins/WasapiMixerPlugin.cxx | 12 +++++- src/output/plugins/wasapi/ForMixer.hxx | 7 ++++ .../plugins/wasapi/WasapiOutputPlugin.cxx | 31 +++++++++++----- src/win32/ComWorker.cxx | 4 -- src/win32/ComWorker.hxx | 37 +++++++------------ 5 files changed, 52 insertions(+), 39 deletions(-) diff --git a/src/mixer/plugins/WasapiMixerPlugin.cxx b/src/mixer/plugins/WasapiMixerPlugin.cxx index 18c862f29..d87f1ca26 100644 --- a/src/mixer/plugins/WasapiMixerPlugin.cxx +++ b/src/mixer/plugins/WasapiMixerPlugin.cxx @@ -44,7 +44,11 @@ public: void Close() noexcept override {} int GetVolume() override { - auto future = COMWorker::Async([&]() -> int { + auto com_worker = wasapi_output_get_com_worker(output); + if (!com_worker) + return -1; + + auto future = com_worker->Async([&]() -> int { HRESULT result; float volume_level; @@ -76,7 +80,11 @@ public: } void SetVolume(unsigned volume) override { - COMWorker::Async([&]() { + auto com_worker = wasapi_output_get_com_worker(output); + if (!com_worker) + throw std::runtime_error("Cannot set WASAPI volume"); + + com_worker->Async([&]() { HRESULT result; const float volume_level = volume / 100.0f; diff --git a/src/output/plugins/wasapi/ForMixer.hxx b/src/output/plugins/wasapi/ForMixer.hxx index 2d815ce61..0d090a3ae 100644 --- a/src/output/plugins/wasapi/ForMixer.hxx +++ b/src/output/plugins/wasapi/ForMixer.hxx @@ -20,10 +20,13 @@ #ifndef MPD_WASAPI_OUTPUT_FOR_MIXER_HXX #define MPD_WASAPI_OUTPUT_FOR_MIXER_HXX +#include + struct IMMDevice; struct IAudioClient; class AudioOutput; class WasapiOutput; +class COMWorker; [[gnu::pure]] WasapiOutput & @@ -33,6 +36,10 @@ wasapi_output_downcast(AudioOutput &output) noexcept; bool wasapi_is_exclusive(WasapiOutput &output) noexcept; +[[gnu::pure]] +std::shared_ptr +wasapi_output_get_com_worker(WasapiOutput &output) noexcept; + [[gnu::pure]] IMMDevice * wasapi_output_get_device(WasapiOutput &output) noexcept; diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 3882474f1..f9dfb5429 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -214,22 +214,28 @@ class WasapiOutput final : public AudioOutput { public: static AudioOutput *Create(EventLoop &, const ConfigBlock &block); WasapiOutput(const ConfigBlock &block); + + auto GetComWorker() noexcept { + // TODO: protect access to the shard_ptr + return com_worker; + } + void Enable() override { - COMWorker::Aquire(); + com_worker = std::make_shared(); try { - COMWorker::Async([&]() { OpenDevice(); }).get(); + com_worker->Async([&]() { OpenDevice(); }).get(); } catch (...) { - COMWorker::Release(); + com_worker.reset(); throw; } } void Disable() noexcept override { - COMWorker::Async([&]() { DoDisable(); }).get(); - COMWorker::Release(); + com_worker->Async([&]() { DoDisable(); }).get(); + com_worker.reset(); } void Open(AudioFormat &audio_format) override { - COMWorker::Async([&]() { DoOpen(audio_format); }).get(); + com_worker->Async([&]() { DoOpen(audio_format); }).get(); } void Close() noexcept override; std::chrono::steady_clock::duration Delay() const noexcept override; @@ -253,6 +259,7 @@ private: bool dop_setting; #endif std::string device_config; + std::shared_ptr com_worker; ComPtr enumerator; ComPtr device; ComPtr client; @@ -283,6 +290,12 @@ WasapiOutput &wasapi_output_downcast(AudioOutput &output) noexcept { bool wasapi_is_exclusive(WasapiOutput &output) noexcept { return output.is_exclusive; } +std::shared_ptr +wasapi_output_get_com_worker(WasapiOutput &output) noexcept +{ + return output.GetComWorker(); +} + IMMDevice *wasapi_output_get_device(WasapiOutput &output) noexcept { return output.device.get(); } @@ -524,7 +537,7 @@ void WasapiOutput::Close() noexcept { assert(thread); try { - COMWorker::Async([&]() { + com_worker->Async([&]() { Stop(*client); }).get(); thread->CheckException(); @@ -535,7 +548,7 @@ void WasapiOutput::Close() noexcept { is_started = false; thread->Finish(); thread->Join(); - COMWorker::Async([&]() { + com_worker->Async([&]() { thread.reset(); client.reset(); }).get(); @@ -586,7 +599,7 @@ size_t WasapiOutput::Play(const void *chunk, size_t size) { if (!is_started) { is_started = true; thread->Play(); - COMWorker::Async([&]() { + com_worker->Async([&]() { Start(*client); }).wait(); } diff --git a/src/win32/ComWorker.cxx b/src/win32/ComWorker.cxx index 702c2c427..01f67a5f9 100644 --- a/src/win32/ComWorker.cxx +++ b/src/win32/ComWorker.cxx @@ -21,10 +21,6 @@ #include "Com.hxx" #include "thread/Name.hxx" -Mutex COMWorker::mutex; -unsigned int COMWorker::reference_count = 0; -std::optional COMWorker::thread; - void COMWorker::COMWorkerThread::Work() noexcept { SetThreadName("COM Worker"); COM com{true}; diff --git a/src/win32/ComWorker.hxx b/src/win32/ComWorker.hxx index 10f2123a4..f7e29a395 100644 --- a/src/win32/ComWorker.hxx +++ b/src/win32/ComWorker.hxx @@ -22,12 +22,9 @@ #include "WinEvent.hxx" #include "thread/Future.hxx" -#include "thread/Mutex.hxx" #include "thread/Thread.hxx" #include -#include -#include #include @@ -56,31 +53,25 @@ private: }; public: - static void Aquire() { - std::unique_lock locker(mutex); - if (reference_count == 0) { - thread.emplace(); - thread->Start(); - } - ++reference_count; - } - static void Release() noexcept { - std::unique_lock locker(mutex); - --reference_count; - if (reference_count == 0) { - thread->Finish(); - thread->Join(); - thread.reset(); - } + COMWorker() { + thread.Start(); } + ~COMWorker() noexcept { + thread.Finish(); + thread.Join(); + } + + COMWorker(const COMWorker &) = delete; + COMWorker &operator=(const COMWorker &) = delete; + template - static auto Async(Function &&function, Args &&...args) { + auto Async(Function &&function, Args &&...args) { using R = std::invoke_result_t, std::decay_t...>; auto promise = std::make_shared>(); auto future = promise->get_future(); - thread->Push([function = std::forward(function), + thread.Push([function = std::forward(function), args = std::make_tuple(std::forward(args)...), promise = std::move(promise)]() mutable { try { @@ -101,9 +92,7 @@ public: } private: - static Mutex mutex; - static unsigned int reference_count; - static std::optional thread; + COMWorkerThread thread; }; #endif From cf108c389f88deb6fbc70390b9877a9df6e8701e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Mar 2021 18:17:16 +0100 Subject: [PATCH 02/54] win32/ComWorker: remove parameter passing from Async() Parameters should better be captured. This removes some complexity from Async(). --- src/win32/ComWorker.hxx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/win32/ComWorker.hxx b/src/win32/ComWorker.hxx index f7e29a395..3fa0db44a 100644 --- a/src/win32/ComWorker.hxx +++ b/src/win32/ComWorker.hxx @@ -65,24 +65,19 @@ public: COMWorker(const COMWorker &) = delete; COMWorker &operator=(const COMWorker &) = delete; - template - auto Async(Function &&function, Args &&...args) { - using R = std::invoke_result_t, - std::decay_t...>; + template + auto Async(Function &&function) { + using R = std::invoke_result_t>; auto promise = std::make_shared>(); auto future = promise->get_future(); thread.Push([function = std::forward(function), - args = std::make_tuple(std::forward(args)...), promise = std::move(promise)]() mutable { try { if constexpr (std::is_void_v) { - std::apply(std::forward(function), - std::move(args)); + std::invoke(std::forward(function)); promise->set_value(); } else { - promise->set_value(std::apply( - std::forward(function), - std::move(args))); + promise->set_value(std::invoke(std::forward(function))); } } catch (...) { promise->set_exception(std::current_exception()); From 48bdd09f6458b93605fd5700a6e0fe9b8ccd22aa Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Mar 2021 18:21:56 +0100 Subject: [PATCH 03/54] win32/ComWorker: fold class COMWorkerThread into class COMWorker --- src/win32/ComWorker.cxx | 4 +++- src/win32/ComWorker.hxx | 40 +++++++++++++++++----------------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/win32/ComWorker.cxx b/src/win32/ComWorker.cxx index 01f67a5f9..5de211071 100644 --- a/src/win32/ComWorker.cxx +++ b/src/win32/ComWorker.cxx @@ -21,7 +21,9 @@ #include "Com.hxx" #include "thread/Name.hxx" -void COMWorker::COMWorkerThread::Work() noexcept { +void +COMWorker::Work() noexcept +{ SetThreadName("COM Worker"); COM com{true}; while (true) { diff --git a/src/win32/ComWorker.hxx b/src/win32/ComWorker.hxx index 3fa0db44a..d4c410652 100644 --- a/src/win32/ComWorker.hxx +++ b/src/win32/ComWorker.hxx @@ -30,27 +30,11 @@ // Worker thread for all COM operation class COMWorker { -private: - class COMWorkerThread : public Thread { - public: - COMWorkerThread() : Thread{BIND_THIS_METHOD(Work)} {} + Thread thread{BIND_THIS_METHOD(Work)}; - private: - friend class COMWorker; - void Work() noexcept; - void Finish() noexcept { - running_flag.clear(); - event.Set(); - } - void Push(const std::function &function) { - spsc_buffer.push(function); - event.Set(); - } - - boost::lockfree::spsc_queue> spsc_buffer{32}; - std::atomic_flag running_flag = true; - WinEvent event{}; - }; + boost::lockfree::spsc_queue> spsc_buffer{32}; + std::atomic_flag running_flag = true; + WinEvent event{}; public: COMWorker() { @@ -58,7 +42,7 @@ public: } ~COMWorker() noexcept { - thread.Finish(); + Finish(); thread.Join(); } @@ -70,7 +54,7 @@ public: using R = std::invoke_result_t>; auto promise = std::make_shared>(); auto future = promise->get_future(); - thread.Push([function = std::forward(function), + Push([function = std::forward(function), promise = std::move(promise)]() mutable { try { if constexpr (std::is_void_v) { @@ -87,7 +71,17 @@ public: } private: - COMWorkerThread thread; + void Finish() noexcept { + running_flag.clear(); + event.Set(); + } + + void Push(const std::function &function) { + spsc_buffer.push(function); + event.Set(); + } + + void Work() noexcept; }; #endif From 6a75c48dba2baacb7ca02a607f5ff80ca2f375d8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 13:44:10 +0100 Subject: [PATCH 04/54] win32/HResult: add MakeHResultError() None of the current FormatHResultError() callers need the format string. --- src/mixer/plugins/WasapiMixerPlugin.cxx | 12 ++++++------ src/output/plugins/wasapi/AudioClient.hxx | 18 +++++++++--------- src/output/plugins/wasapi/Device.hxx | 18 +++++++++--------- .../plugins/wasapi/WasapiOutputPlugin.cxx | 15 +++++++-------- src/win32/Com.hxx | 4 ++-- src/win32/ComPtr.hxx | 2 +- src/win32/HResult.hxx | 7 +++++++ 7 files changed, 41 insertions(+), 35 deletions(-) diff --git a/src/mixer/plugins/WasapiMixerPlugin.cxx b/src/mixer/plugins/WasapiMixerPlugin.cxx index d87f1ca26..d0145405a 100644 --- a/src/mixer/plugins/WasapiMixerPlugin.cxx +++ b/src/mixer/plugins/WasapiMixerPlugin.cxx @@ -59,9 +59,9 @@ public: result = endpoint_volume->GetMasterVolumeLevelScalar( &volume_level); if (FAILED(result)) { - throw FormatHResultError(result, - "Unable to get master " - "volume level"); + throw MakeHResultError(result, + "Unable to get master " + "volume level"); } } else { auto session_volume = @@ -69,7 +69,7 @@ public: result = session_volume->GetMasterVolume(&volume_level); if (FAILED(result)) { - throw FormatHResultError( + throw MakeHResultError( result, "Unable to get master volume"); } } @@ -95,7 +95,7 @@ public: result = endpoint_volume->SetMasterVolumeLevelScalar( volume_level, nullptr); if (FAILED(result)) { - throw FormatHResultError( + throw MakeHResultError( result, "Unable to set master volume level"); } @@ -106,7 +106,7 @@ public: result = session_volume->SetMasterVolume(volume_level, nullptr); if (FAILED(result)) { - throw FormatHResultError( + throw MakeHResultError( result, "Unable to set master volume"); } } diff --git a/src/output/plugins/wasapi/AudioClient.hxx b/src/output/plugins/wasapi/AudioClient.hxx index 3ed3da319..70ec56a69 100644 --- a/src/output/plugins/wasapi/AudioClient.hxx +++ b/src/output/plugins/wasapi/AudioClient.hxx @@ -33,8 +33,8 @@ GetBufferSizeInFrames(IAudioClient &client) HRESULT result = client.GetBufferSize(&buffer_size_in_frames); if (FAILED(result)) - throw FormatHResultError(result, - "Unable to get audio client buffer size"); + throw MakeHResultError(result, + "Unable to get audio client buffer size"); return buffer_size_in_frames; } @@ -46,8 +46,8 @@ GetCurrentPaddingFrames(IAudioClient &client) HRESULT result = client.GetCurrentPadding(&padding_frames); if (FAILED(result)) - throw FormatHResultError(result, - "Failed to get current padding"); + throw MakeHResultError(result, + "Failed to get current padding"); return padding_frames; } @@ -59,7 +59,7 @@ GetMixFormat(IAudioClient &client) HRESULT result = client.GetMixFormat(&f); if (FAILED(result)) - throw FormatHResultError(result, "GetMixFormat failed"); + throw MakeHResultError(result, "GetMixFormat failed"); return ComHeapPtr{f}; } @@ -69,7 +69,7 @@ Start(IAudioClient &client) { HRESULT result = client.Start(); if (FAILED(result)) - throw FormatHResultError(result, "Failed to start client"); + throw MakeHResultError(result, "Failed to start client"); } inline void @@ -77,7 +77,7 @@ Stop(IAudioClient &client) { HRESULT result = client.Stop(); if (FAILED(result)) - throw FormatHResultError(result, "Failed to stop client"); + throw MakeHResultError(result, "Failed to stop client"); } inline void @@ -85,7 +85,7 @@ SetEventHandle(IAudioClient &client, HANDLE h) { HRESULT result = client.SetEventHandle(h); if (FAILED(result)) - throw FormatHResultError(result, "Unable to set event handle"); + throw MakeHResultError(result, "Unable to set event handle"); } template @@ -95,7 +95,7 @@ GetService(IAudioClient &client) T *p = nullptr; HRESULT result = client.GetService(IID_PPV_ARGS(&p)); if (FAILED(result)) - throw FormatHResultError(result, "Unable to get service"); + throw MakeHResultError(result, "Unable to get service"); return ComPtr{p}; } diff --git a/src/output/plugins/wasapi/Device.hxx b/src/output/plugins/wasapi/Device.hxx index 863f327a7..87663c0dc 100644 --- a/src/output/plugins/wasapi/Device.hxx +++ b/src/output/plugins/wasapi/Device.hxx @@ -33,8 +33,8 @@ GetDefaultAudioEndpoint(IMMDeviceEnumerator &e) HRESULT result = e.GetDefaultAudioEndpoint(eRender, eMultimedia, &device); if (FAILED(result)) - throw FormatHResultError(result, - "Unable to get default device for multimedia"); + throw MakeHResultError(result, + "Unable to get default device for multimedia"); return ComPtr{device}; } @@ -47,7 +47,7 @@ EnumAudioEndpoints(IMMDeviceEnumerator &e) HRESULT result = e.EnumAudioEndpoints(eRender, DEVICE_STATE_ACTIVE, &dc); if (FAILED(result)) - throw FormatHResultError(result, "Unable to enumerate devices"); + throw MakeHResultError(result, "Unable to enumerate devices"); return ComPtr{dc}; } @@ -59,7 +59,7 @@ GetCount(IMMDeviceCollection &dc) HRESULT result = dc.GetCount(&count); if (FAILED(result)) - throw FormatHResultError(result, "Collection->GetCount failed"); + throw MakeHResultError(result, "Collection->GetCount failed"); return count; } @@ -71,7 +71,7 @@ Item(IMMDeviceCollection &dc, UINT i) auto result = dc.Item(i, &device); if (FAILED(result)) - throw FormatHResultError(result, "Collection->Item failed"); + throw MakeHResultError(result, "Collection->Item failed"); return ComPtr{device}; } @@ -83,7 +83,7 @@ GetState(IMMDevice &device) HRESULT result = device.GetState(&state);; if (FAILED(result)) - throw FormatHResultError(result, "Unable to get device status"); + throw MakeHResultError(result, "Unable to get device status"); return state; } @@ -96,7 +96,7 @@ Activate(IMMDevice &device) HRESULT result = device.Activate(__uuidof(T), CLSCTX_ALL, nullptr, (void **)&p); if (FAILED(result)) - throw FormatHResultError(result, "Unable to activate device"); + throw MakeHResultError(result, "Unable to activate device"); return ComPtr{p}; } @@ -108,8 +108,8 @@ OpenPropertyStore(IMMDevice &device) HRESULT result = device.OpenPropertyStore(STGM_READ, &property_store); if (FAILED(result)) - throw FormatHResultError(result, - "Device->OpenPropertyStore failed"); + throw MakeHResultError(result, + "Device->OpenPropertyStore failed"); return ComPtr{property_store}; } diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index f9dfb5429..f49d9748b 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -336,7 +336,7 @@ void WasapiOutputThread::Work() noexcept { if (HRESULT result = render_client->GetBuffer(write_in_frames, &data); FAILED(result)) { - throw FormatHResultError(result, "Failed to get buffer"); + throw MakeHResultError(result, "Failed to get buffer"); } AtScopeExit(&) { @@ -457,7 +457,7 @@ void WasapiOutput::DoOpen(AudioFormat &audio_format) { if (HRESULT result = client->GetDevicePeriod(&default_device_period, &min_device_period); FAILED(result)) { - throw FormatHResultError(result, "Unable to get device period"); + throw MakeHResultError(result, "Unable to get device period"); } FormatDebug(wasapi_output_domain, "Default device period: %I64u ns, Minimum device period: " @@ -505,8 +505,7 @@ void WasapiOutput::DoOpen(AudioFormat &audio_format) { } if (FAILED(result)) { - throw FormatHResultError( - result, "Unable to initialize audio client"); + throw MakeHResultError(result, "Unable to initialize audio client"); } } } else { @@ -515,8 +514,8 @@ void WasapiOutput::DoOpen(AudioFormat &audio_format) { buffer_duration, 0, reinterpret_cast(&device_format), nullptr); FAILED(result)) { - throw FormatHResultError(result, - "Unable to initialize audio client"); + throw MakeHResultError(result, + "Unable to initialize audio client"); } } @@ -773,7 +772,7 @@ void WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) { } if (FAILED(result) && result != AUDCLNT_E_UNSUPPORTED_FORMAT) { - throw FormatHResultError(result, "IsFormatSupported failed"); + throw MakeHResultError(result, "IsFormatSupported failed"); } switch (result) { @@ -802,7 +801,7 @@ void WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) { result_string.c_str()); } if (FAILED(result)) { - throw FormatHResultError(result, "Format is not supported"); + throw MakeHResultError(result, "Format is not supported"); } break; case S_FALSE: diff --git a/src/win32/Com.hxx b/src/win32/Com.hxx index a2aa062a1..34350b62d 100644 --- a/src/win32/Com.hxx +++ b/src/win32/Com.hxx @@ -31,7 +31,7 @@ public: COM() { if (HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); FAILED(result)) { - throw FormatHResultError( + throw MakeHResultError( result, "Unable to initialize COM with COINIT_MULTITHREADED"); } @@ -39,7 +39,7 @@ public: COM(bool) { if (HRESULT result = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); FAILED(result)) { - throw FormatHResultError( + throw MakeHResultError( result, "Unable to initialize COM with COINIT_APARTMENTTHREADED"); } diff --git a/src/win32/ComPtr.hxx b/src/win32/ComPtr.hxx index 01a8ef525..c7efa88b8 100644 --- a/src/win32/ComPtr.hxx +++ b/src/win32/ComPtr.hxx @@ -85,7 +85,7 @@ public: ::CoCreateInstance(class_id, unknown_outer, class_context, __uuidof(T), reinterpret_cast(&ptr)); if (FAILED(result)) { - throw FormatHResultError(result, "Unable to create instance"); + throw MakeHResultError(result, "Unable to create instance"); } } diff --git a/src/win32/HResult.hxx b/src/win32/HResult.hxx index 2f48c08f2..e61447fb6 100644 --- a/src/win32/HResult.hxx +++ b/src/win32/HResult.hxx @@ -74,6 +74,13 @@ static inline const std::error_category &hresult_category() noexcept { return hresult_category_instance; } +inline std::system_error +MakeHResultError(HRESULT result, const char *msg) noexcept +{ + return std::system_error(std::error_code(result, hresult_category()), + msg); +} + gcc_printf(2, 3) std::system_error FormatHResultError(HRESULT result, const char *fmt, ...) noexcept; From f2c679cfec46a26297056abb845bfdd3f36930e8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 14:02:05 +0100 Subject: [PATCH 05/54] win32/Com: remove the unused COINIT_MULTITHREADED constructor --- src/win32/Com.hxx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/win32/Com.hxx b/src/win32/Com.hxx index 34350b62d..284fca5ea 100644 --- a/src/win32/Com.hxx +++ b/src/win32/Com.hxx @@ -28,14 +28,6 @@ // https://docs.microsoft.com/en-us/windows/win32/api/_com/ class COM { public: - COM() { - if (HRESULT result = CoInitializeEx(nullptr, COINIT_MULTITHREADED); - FAILED(result)) { - throw MakeHResultError( - result, - "Unable to initialize COM with COINIT_MULTITHREADED"); - } - } COM(bool) { if (HRESULT result = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); FAILED(result)) { From 927f1e03a3e53879e737a1696c4b226aa4017418 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 14:02:16 +0100 Subject: [PATCH 06/54] win32/Com: make COINIT_APARTMENTTHREADED the default constructor --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 2 +- src/win32/Com.hxx | 2 +- src/win32/ComWorker.cxx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index f49d9748b..a0c08008f 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -307,7 +307,7 @@ IAudioClient *wasapi_output_get_client(WasapiOutput &output) noexcept { void WasapiOutputThread::Work() noexcept { SetThreadName("Wasapi Output Worker"); FormatDebug(wasapi_output_domain, "Working thread started"); - COM com{true}; + COM com; while (true) { try { event.Wait(); diff --git a/src/win32/Com.hxx b/src/win32/Com.hxx index 284fca5ea..ba749a8c1 100644 --- a/src/win32/Com.hxx +++ b/src/win32/Com.hxx @@ -28,7 +28,7 @@ // https://docs.microsoft.com/en-us/windows/win32/api/_com/ class COM { public: - COM(bool) { + COM() { if (HRESULT result = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); FAILED(result)) { throw MakeHResultError( diff --git a/src/win32/ComWorker.cxx b/src/win32/ComWorker.cxx index 5de211071..c5ada8c91 100644 --- a/src/win32/ComWorker.cxx +++ b/src/win32/ComWorker.cxx @@ -25,7 +25,7 @@ void COMWorker::Work() noexcept { SetThreadName("COM Worker"); - COM com{true}; + COM com; while (true) { if (!running_flag.test_and_set()) { return; From ec76583c33bb9a53985ae0ff9db602e2852d9c70 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 14:03:05 +0100 Subject: [PATCH 07/54] win32/Com: add COINIT_DISABLE_OLE1DDE MSDN documentation suggests always passing this flag to reduce overhead for an "obsolete technology". --- src/win32/Com.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/win32/Com.hxx b/src/win32/Com.hxx index ba749a8c1..173b0fcc9 100644 --- a/src/win32/Com.hxx +++ b/src/win32/Com.hxx @@ -29,7 +29,7 @@ class COM { public: COM() { - if (HRESULT result = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); + if (HRESULT result = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED|COINIT_DISABLE_OLE1DDE); FAILED(result)) { throw MakeHResultError( result, From e227596c2052f28956d1767c5562d2c5a133049d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:36:52 +0100 Subject: [PATCH 08/54] test/run_output: pass FileDescriptor to run_output() --- test/run_output.cxx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/run_output.cxx b/test/run_output.cxx index bb7ba71b8..f7d86a825 100644 --- a/test/run_output.cxx +++ b/test/run_output.cxx @@ -113,7 +113,8 @@ LoadAudioOutput(const ConfigData &config, EventLoop &event_loop, } static void -run_output(AudioOutput &ao, AudioFormat audio_format) +RunOutput(AudioOutput &ao, AudioFormat audio_format, + FileDescriptor in_fd) { /* open the audio output */ @@ -134,8 +135,8 @@ run_output(AudioOutput &ao, AudioFormat audio_format) char buffer[4096]; while (true) { if (length < sizeof(buffer)) { - ssize_t nbytes = read(0, buffer + length, - sizeof(buffer) - length); + ssize_t nbytes = in_fd.Read(buffer + length, + sizeof(buffer) - length); if (nbytes <= 0) break; @@ -174,7 +175,7 @@ try { /* do it */ - run_output(*ao, c.audio_format); + RunOutput(*ao, c.audio_format, FileDescriptor(STDIN_FILENO)); /* cleanup and exit */ From 3fb25d40628d7c6f84abf4b6ebc916af26b12aee Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:41:46 +0100 Subject: [PATCH 09/54] test/run_convert: move code to RunConvert() --- test/run_convert.cxx | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/test/run_convert.cxx b/test/run_convert.cxx index 3cba693fd..25b0e3831 100644 --- a/test/run_convert.cxx +++ b/test/run_convert.cxx @@ -101,18 +101,9 @@ public: } }; -int -main(int argc, char **argv) -try { - const auto c = ParseCommandLine(argc, argv); - - SetLogThreshold(c.verbose ? LogLevel::DEBUG : LogLevel::INFO); - const GlobalInit init(c.config_path); - - const size_t in_frame_size = c.in_audio_format.GetFrameSize(); - - PcmConvert state(c.in_audio_format, c.out_audio_format); - +static void +RunConvert(PcmConvert &convert, size_t in_frame_size) +{ StaticFifoBuffer buffer; while (true) { @@ -136,20 +127,32 @@ try { buffer.Consume(src.size); - auto output = state.Convert({src.data, src.size}); + auto output = convert.Convert({src.data, src.size}); [[maybe_unused]] ssize_t ignored = write(1, output.data, output.size); } while (true) { - auto output = state.Flush(); + auto output = convert.Flush(); if (output.IsNull()) break; [[maybe_unused]] ssize_t ignored = write(1, output.data, output.size); } +} + +int +main(int argc, char **argv) +try { + const auto c = ParseCommandLine(argc, argv); + + SetLogThreshold(c.verbose ? LogLevel::DEBUG : LogLevel::INFO); + const GlobalInit init(c.config_path); + + PcmConvert state(c.in_audio_format, c.out_audio_format); + RunConvert(state, c.in_audio_format.GetFrameSize()); return EXIT_SUCCESS; } catch (...) { From e777fb4edb94090955a292b1aa8ac039aef66905 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:50:32 +0100 Subject: [PATCH 10/54] test/run_convert: pass FileDescriptor to RunConvert() --- test/run_convert.cxx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/run_convert.cxx b/test/run_convert.cxx index 25b0e3831..7d20ea2f9 100644 --- a/test/run_convert.cxx +++ b/test/run_convert.cxx @@ -29,6 +29,7 @@ #include "pcm/Convert.hxx" #include "fs/Path.hxx" #include "fs/NarrowPath.hxx" +#include "io/FileDescriptor.hxx" #include "util/ConstBuffer.hxx" #include "util/StaticFifoBuffer.hxx" #include "util/OptionDef.hxx" @@ -102,7 +103,8 @@ public: }; static void -RunConvert(PcmConvert &convert, size_t in_frame_size) +RunConvert(PcmConvert &convert, size_t in_frame_size, + FileDescriptor in_fd, FileDescriptor out_fd) { StaticFifoBuffer buffer; @@ -111,7 +113,7 @@ RunConvert(PcmConvert &convert, size_t in_frame_size) const auto dest = buffer.Write(); assert(!dest.empty()); - ssize_t nbytes = read(0, dest.data, dest.size); + ssize_t nbytes = in_fd.Read(dest.data, dest.size); if (nbytes <= 0) break; @@ -128,9 +130,7 @@ RunConvert(PcmConvert &convert, size_t in_frame_size) buffer.Consume(src.size); auto output = convert.Convert({src.data, src.size}); - - [[maybe_unused]] ssize_t ignored = write(1, output.data, - output.size); + out_fd.FullWrite(output.data, output.size); } while (true) { @@ -138,8 +138,7 @@ RunConvert(PcmConvert &convert, size_t in_frame_size) if (output.IsNull()) break; - [[maybe_unused]] ssize_t ignored = write(1, output.data, - output.size); + out_fd.FullWrite(output.data, output.size); } } @@ -152,7 +151,9 @@ try { const GlobalInit init(c.config_path); PcmConvert state(c.in_audio_format, c.out_audio_format); - RunConvert(state, c.in_audio_format.GetFrameSize()); + RunConvert(state, c.in_audio_format.GetFrameSize(), + FileDescriptor(STDIN_FILENO), + FileDescriptor(STDOUT_FILENO)); return EXIT_SUCCESS; } catch (...) { From 2bebc793639f54edd23183355cfeab64f36aab88 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:58:38 +0100 Subject: [PATCH 11/54] test/run_convert: use std::byte --- test/run_convert.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/run_convert.cxx b/test/run_convert.cxx index 7d20ea2f9..24d64c462 100644 --- a/test/run_convert.cxx +++ b/test/run_convert.cxx @@ -106,7 +106,7 @@ static void RunConvert(PcmConvert &convert, size_t in_frame_size, FileDescriptor in_fd, FileDescriptor out_fd) { - StaticFifoBuffer buffer; + StaticFifoBuffer buffer; while (true) { { From eff50b263a9b0a738e0f26d08b2a5ce782761b7c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:58:12 +0100 Subject: [PATCH 12/54] test/run_output: use class StaticFifoBuffer --- test/run_output.cxx | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/test/run_output.cxx b/test/run_output.cxx index f7d86a825..4fd3ff2ea 100644 --- a/test/run_output.cxx +++ b/test/run_output.cxx @@ -31,6 +31,7 @@ #include "util/StringBuffer.hxx" #include "util/RuntimeError.hxx" #include "util/ScopeExit.hxx" +#include "util/StaticFifoBuffer.hxx" #include "util/PrintException.hxx" #include "LogBackend.hxx" @@ -127,32 +128,37 @@ RunOutput(AudioOutput &ao, AudioFormat audio_format, fprintf(stderr, "audio_format=%s\n", ToString(audio_format).c_str()); - size_t frame_size = audio_format.GetFrameSize(); + const size_t in_frame_size = audio_format.GetFrameSize(); /* play */ - size_t length = 0; - char buffer[4096]; + StaticFifoBuffer buffer; + while (true) { - if (length < sizeof(buffer)) { - ssize_t nbytes = in_fd.Read(buffer + length, - sizeof(buffer) - length); + { + const auto dest = buffer.Write(); + assert(!dest.empty()); + + ssize_t nbytes = in_fd.Read(dest.data, dest.size); if (nbytes <= 0) break; - length += (size_t)nbytes; + buffer.Append(nbytes); } - size_t play_length = (length / frame_size) * frame_size; - if (play_length > 0) { - size_t consumed = ao.Play(buffer, play_length); + auto src = buffer.Read(); + assert(!src.empty()); - assert(consumed <= length); - assert(consumed % frame_size == 0); + src.size -= src.size % in_frame_size; + if (src.empty()) + continue; - length -= consumed; - memmove(buffer, buffer + consumed, length); - } + size_t consumed = ao.Play(src.data, src.size); + + assert(consumed <= src.size); + assert(consumed % in_frame_size == 0); + + buffer.Consume(consumed); } } From d61341c0e3769ec4b9bfb7c2f2b0e98cd3834741 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 16:36:08 +0100 Subject: [PATCH 13/54] io/FileDescriptor: add method SetBinaryMode() --- src/io/FileDescriptor.cxx | 10 +++++++++- src/io/FileDescriptor.hxx | 3 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/io/FileDescriptor.cxx b/src/io/FileDescriptor.cxx index 08b457831..8351446c6 100644 --- a/src/io/FileDescriptor.cxx +++ b/src/io/FileDescriptor.cxx @@ -172,7 +172,15 @@ FileDescriptor::CreatePipe(FileDescriptor &r, FileDescriptor &w) noexcept #endif } -#ifndef _WIN32 +#ifdef _WIN32 + +void +FileDescriptor::SetBinaryMode() noexcept +{ + _setmode(fd, _O_BINARY); +} + +#else // !_WIN32 bool FileDescriptor::CreatePipeNonBlock(FileDescriptor &r, diff --git a/src/io/FileDescriptor.hxx b/src/io/FileDescriptor.hxx index 186c5870c..a036d0d58 100644 --- a/src/io/FileDescriptor.hxx +++ b/src/io/FileDescriptor.hxx @@ -148,10 +148,13 @@ public: #ifdef _WIN32 void EnableCloseOnExec() noexcept {} void DisableCloseOnExec() noexcept {} + void SetBinaryMode() noexcept; #else static bool CreatePipeNonBlock(FileDescriptor &r, FileDescriptor &w) noexcept; + void SetBinaryMode() noexcept {} + /** * Enable non-blocking mode on this file descriptor. */ From 4d9af9a81bb317585a1dce0620c1bb3468e5a0a6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 17:25:57 +0100 Subject: [PATCH 14/54] test/run_{input,output,convert}: switch file descriptors to binary mode Fixes those programs on Windows. --- test/run_convert.cxx | 3 +++ test/run_input.cxx | 2 ++ test/run_output.cxx | 2 ++ 3 files changed, 7 insertions(+) diff --git a/test/run_convert.cxx b/test/run_convert.cxx index 24d64c462..b7a687c03 100644 --- a/test/run_convert.cxx +++ b/test/run_convert.cxx @@ -106,6 +106,9 @@ static void RunConvert(PcmConvert &convert, size_t in_frame_size, FileDescriptor in_fd, FileDescriptor out_fd) { + in_fd.SetBinaryMode(); + out_fd.SetBinaryMode(); + StaticFifoBuffer buffer; while (true) { diff --git a/test/run_input.cxx b/test/run_input.cxx index 0607dd3c6..6c2f360a3 100644 --- a/test/run_input.cxx +++ b/test/run_input.cxx @@ -164,6 +164,8 @@ static int dump_input_stream(InputStream &is, FileDescriptor out, offset_type seek, size_t chunk_size) { + out.SetBinaryMode(); + std::unique_lock lock(is.mutex); if (seek > 0) diff --git a/test/run_output.cxx b/test/run_output.cxx index 4fd3ff2ea..6d917600d 100644 --- a/test/run_output.cxx +++ b/test/run_output.cxx @@ -117,6 +117,8 @@ static void RunOutput(AudioOutput &ao, AudioFormat audio_format, FileDescriptor in_fd) { + in_fd.SetBinaryMode(); + /* open the audio output */ ao.Enable(); From cd53ca22c6e4596f7606445ed3b0875e9faa213a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 17:43:33 +0100 Subject: [PATCH 15/54] output/wasapi: remove unused function SafeTry() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index a0c08008f..e387a48e4 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -85,17 +85,6 @@ gcc_const constexpr uint32_t GetChannelMask(const uint8_t channels) noexcept { } } -template -inline bool SafeTry(Functor &&functor) { - try { - functor(); - return true; - } catch (...) { - FormatError(std::current_exception(), "%s"); - return false; - } -} - template inline bool SafeSilenceTry(Functor &&functor) { try { From 4833d0891de4d838571fbe5c2aad94c6288e2dca Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 17:46:58 +0100 Subject: [PATCH 16/54] output/wasapi: eliminate kErrorId --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index e387a48e4..71e55bfc7 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -147,8 +147,6 @@ void SetDSDFallback(AudioFormat &audio_format) noexcept { } #endif -inline constexpr const unsigned int kErrorId = -1; - } // namespace class WasapiOutputThread : public Thread { @@ -637,8 +635,8 @@ void WasapiOutput::OpenDevice() { } } - unsigned int id = kErrorId; if (!device_config.empty()) { + unsigned int id; if (!SafeSilenceTry([this, &id]() { id = std::stoul(device_config); })) { device = SearchDevice(device_config); if (!device) From 84a06a72dfd67508e0fbffdc78b543f56fc7a0a9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 17:42:53 +0100 Subject: [PATCH 17/54] output/wasapi: fix coding style --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 193 ++++++++++++------ 1 file changed, 125 insertions(+), 68 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 71e55bfc7..3dfdd61fc 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -61,7 +61,9 @@ namespace { static constexpr Domain wasapi_output_domain("wasapi_output"); -gcc_const constexpr uint32_t GetChannelMask(const uint8_t channels) noexcept { +constexpr uint32_t +GetChannelMask(const uint8_t channels) noexcept +{ switch (channels) { case 1: return KSAUDIO_SPEAKER_MONO; @@ -86,7 +88,9 @@ gcc_const constexpr uint32_t GetChannelMask(const uint8_t channels) noexcept { } template -inline bool SafeSilenceTry(Functor &&functor) { +inline bool +SafeSilenceTry(Functor &&functor) noexcept +{ try { functor(); return true; @@ -95,7 +99,9 @@ inline bool SafeSilenceTry(Functor &&functor) { } } -std::vector GetFormats(const AudioFormat &audio_format) noexcept { +std::vector +GetFormats(const AudioFormat &audio_format) noexcept +{ #ifdef ENABLE_DSD if (audio_format.format == SampleFormat::DSD) { AudioFormat dop_format = audio_format; @@ -141,7 +147,9 @@ std::vector GetFormats(const AudioFormat &audio_format) no } #ifdef ENABLE_DSD -void SetDSDFallback(AudioFormat &audio_format) noexcept { +void +SetDSDFallback(AudioFormat &audio_format) noexcept +{ audio_format.format = SampleFormat::FLOAT; audio_format.sample_rate = 384000; } @@ -150,16 +158,37 @@ void SetDSDFallback(AudioFormat &audio_format) noexcept { } // namespace class WasapiOutputThread : public Thread { -public: + friend class WasapiOutput; + WinEvent event; + WinEvent data_poped; + IAudioClient *client; + ComPtr render_client; + const UINT32 frame_size; + const UINT32 buffer_size_in_frames; + bool is_exclusive; + enum class Status : uint32_t { FINISH, PLAY, PAUSE }; + alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = + Status::PAUSE; + alignas(BOOST_LOCKFREE_CACHELINE_BYTES) struct { + std::atomic_bool occur = false; + std::exception_ptr ptr = nullptr; + WinEvent thrown; + } error; + boost::lockfree::spsc_queue spsc_buffer; + +public: WasapiOutputThread(IAudioClient *_client, ComPtr &&_render_client, const UINT32 _frame_size, const UINT32 _buffer_size_in_frames, bool _is_exclusive) - : Thread(BIND_THIS_METHOD(Work)), client(_client), - render_client(std::move(_render_client)), frame_size(_frame_size), - buffer_size_in_frames(_buffer_size_in_frames), is_exclusive(_is_exclusive), - spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) {} + :Thread(BIND_THIS_METHOD(Work)), client(_client), + render_client(std::move(_render_client)), frame_size(_frame_size), + buffer_size_in_frames(_buffer_size_in_frames), is_exclusive(_is_exclusive), + spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) + { + } + void Finish() noexcept { return SetStatus(Status::FINISH); } void Play() noexcept { return SetStatus(Status::PLAY); } void Pause() noexcept { return SetStatus(Status::PAUSE); } @@ -173,23 +202,6 @@ public: } private: - friend class WasapiOutput; - WinEvent event; - WinEvent data_poped; - IAudioClient *client; - ComPtr render_client; - const UINT32 frame_size; - const UINT32 buffer_size_in_frames; - bool is_exclusive; - alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = - Status::PAUSE; - alignas(BOOST_LOCKFREE_CACHELINE_BYTES) struct { - std::atomic_bool occur = false; - std::exception_ptr ptr = nullptr; - WinEvent thrown; - } error; - boost::lockfree::spsc_queue spsc_buffer; - void SetStatus(Status s) noexcept { status.store(s); event.Set(); @@ -198,6 +210,23 @@ private: }; class WasapiOutput final : public AudioOutput { + std::atomic_flag not_interrupted = true; + bool is_started = false; + bool is_exclusive; + bool enumerate_devices; +#ifdef ENABLE_DSD + bool dop_setting; +#endif + std::string device_config; + std::shared_ptr com_worker; + ComPtr enumerator; + ComPtr device; + ComPtr client; + WAVEFORMATEXTENSIBLE device_format; + std::optional thread; + std::size_t watermark; + std::optional pcm_export; + public: static AudioOutput *Create(EventLoop &, const ConfigBlock &block); WasapiOutput(const ConfigBlock &block); @@ -238,23 +267,6 @@ public: } private: - std::atomic_flag not_interrupted = true; - bool is_started = false; - bool is_exclusive; - bool enumerate_devices; -#ifdef ENABLE_DSD - bool dop_setting; -#endif - std::string device_config; - std::shared_ptr com_worker; - ComPtr enumerator; - ComPtr device; - ComPtr client; - WAVEFORMATEXTENSIBLE device_format; - std::optional thread; - std::size_t watermark; - std::optional pcm_export; - friend bool wasapi_is_exclusive(WasapiOutput &output) noexcept; friend IMMDevice *wasapi_output_get_device(WasapiOutput &output) noexcept; friend IAudioClient *wasapi_output_get_client(WasapiOutput &output) noexcept; @@ -271,11 +283,17 @@ private: ComPtr SearchDevice(std::string_view name); }; -WasapiOutput &wasapi_output_downcast(AudioOutput &output) noexcept { +WasapiOutput & +wasapi_output_downcast(AudioOutput &output) noexcept +{ return static_cast(output); } -bool wasapi_is_exclusive(WasapiOutput &output) noexcept { return output.is_exclusive; } +bool +wasapi_is_exclusive(WasapiOutput &output) noexcept +{ + return output.is_exclusive; +} std::shared_ptr wasapi_output_get_com_worker(WasapiOutput &output) noexcept @@ -283,15 +301,21 @@ wasapi_output_get_com_worker(WasapiOutput &output) noexcept return output.GetComWorker(); } -IMMDevice *wasapi_output_get_device(WasapiOutput &output) noexcept { +IMMDevice * +wasapi_output_get_device(WasapiOutput &output) noexcept +{ return output.device.get(); } -IAudioClient *wasapi_output_get_client(WasapiOutput &output) noexcept { +IAudioClient * +wasapi_output_get_client(WasapiOutput &output) noexcept +{ return output.client.get(); } -void WasapiOutputThread::Work() noexcept { +void +WasapiOutputThread::Work() noexcept +{ SetThreadName("Wasapi Output Worker"); FormatDebug(wasapi_output_domain, "Working thread started"); COM com; @@ -350,22 +374,27 @@ void WasapiOutputThread::Work() noexcept { } } -AudioOutput *WasapiOutput::Create(EventLoop &, const ConfigBlock &block) { +AudioOutput * +WasapiOutput::Create(EventLoop &, const ConfigBlock &block) +{ return new WasapiOutput(block); } WasapiOutput::WasapiOutput(const ConfigBlock &block) -: AudioOutput(FLAG_ENABLE_DISABLE | FLAG_PAUSE), - is_exclusive(block.GetBlockValue("exclusive", false)), - enumerate_devices(block.GetBlockValue("enumerate", false)), + :AudioOutput(FLAG_ENABLE_DISABLE | FLAG_PAUSE), + is_exclusive(block.GetBlockValue("exclusive", false)), + enumerate_devices(block.GetBlockValue("enumerate", false)), #ifdef ENABLE_DSD - dop_setting(block.GetBlockValue("dop", false)), + dop_setting(block.GetBlockValue("dop", false)), #endif - device_config(block.GetBlockValue("device", "")) { + device_config(block.GetBlockValue("device", "")) +{ } /// run inside COMWorkerThread -void WasapiOutput::DoDisable() noexcept { +void +WasapiOutput::DoDisable() noexcept +{ if (thread) { try { thread->Finish(); @@ -382,7 +411,9 @@ void WasapiOutput::DoDisable() noexcept { } /// run inside COMWorkerThread -void WasapiOutput::DoOpen(AudioFormat &audio_format) { +void +WasapiOutput::DoOpen(AudioFormat &audio_format) +{ client.reset(); if (GetState(*device) != DEVICE_STATE_ACTIVE) { @@ -519,7 +550,9 @@ void WasapiOutput::DoOpen(AudioFormat &audio_format) { thread->Start(); } -void WasapiOutput::Close() noexcept { +void +WasapiOutput::Close() noexcept +{ assert(thread); try { @@ -541,7 +574,9 @@ void WasapiOutput::Close() noexcept { pcm_export.reset(); } -std::chrono::steady_clock::duration WasapiOutput::Delay() const noexcept { +std::chrono::steady_clock::duration +WasapiOutput::Delay() const noexcept +{ if (!is_started) { // idle while paused return std::chrono::seconds(1); @@ -558,7 +593,9 @@ std::chrono::steady_clock::duration WasapiOutput::Delay() const noexcept { return result; } -size_t WasapiOutput::Play(const void *chunk, size_t size) { +size_t +WasapiOutput::Play(const void *chunk, size_t size) +{ assert(thread); not_interrupted.test_and_set(); @@ -599,7 +636,9 @@ size_t WasapiOutput::Play(const void *chunk, size_t size) { } while (true); } -bool WasapiOutput::Pause() { +bool +WasapiOutput::Pause() +{ if (is_started) { thread->Pause(); is_started = false; @@ -608,14 +647,18 @@ bool WasapiOutput::Pause() { return true; } -void WasapiOutput::Interrupt() noexcept { +void +WasapiOutput::Interrupt() noexcept +{ if (thread) { not_interrupted.clear(); thread->data_poped.Set(); } } -void WasapiOutput::Drain() { +void +WasapiOutput::Drain() +{ assert(thread); thread->spsc_buffer.consume_all([](auto &&) {}); @@ -623,7 +666,9 @@ void WasapiOutput::Drain() { } /// run inside COMWorkerThread -void WasapiOutput::OpenDevice() { +void +WasapiOutput::OpenDevice() +{ enumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER); @@ -650,7 +695,9 @@ void WasapiOutput::OpenDevice() { } /// run inside COMWorkerThread -bool WasapiOutput::TryFormatExclusive(const AudioFormat &audio_format) { +bool +WasapiOutput::TryFormatExclusive(const AudioFormat &audio_format) +{ for (auto test_format : GetFormats(audio_format)) { HRESULT result = client->IsFormatSupported( AUDCLNT_SHAREMODE_EXCLUSIVE, @@ -674,7 +721,9 @@ bool WasapiOutput::TryFormatExclusive(const AudioFormat &audio_format) { } /// run inside COMWorkerThread -void WasapiOutput::FindExclusiveFormatSupported(AudioFormat &audio_format) { +void +WasapiOutput::FindExclusiveFormatSupported(AudioFormat &audio_format) +{ for (uint8_t channels : {0, 2, 6, 8, 7, 1, 4, 5, 3}) { if (audio_format.channels == channels) { continue; @@ -734,7 +783,9 @@ void WasapiOutput::FindExclusiveFormatSupported(AudioFormat &audio_format) { } /// run inside COMWorkerThread -void WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) { +void +WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) +{ HRESULT result; // In shared mode, different sample rate is always unsupported. @@ -837,7 +888,9 @@ void WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) { } /// run inside COMWorkerThread -void WasapiOutput::EnumerateDevices() { +void +WasapiOutput::EnumerateDevices() +{ const auto device_collection = EnumAudioEndpoints(*enumerator); const UINT count = GetCount(*device_collection); @@ -884,7 +937,11 @@ WasapiOutput::SearchDevice(std::string_view name) return nullptr; } -static bool wasapi_output_test_default_device() { return true; } +static bool +wasapi_output_test_default_device() +{ + return true; +} const struct AudioOutputPlugin wasapi_output_plugin = { "wasapi", From 980ef822163c5b7b32f561c28724624dfce6105d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 17:42:42 +0100 Subject: [PATCH 18/54] output/wasapi: move SetEventHandle() call to thread constructor --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 3dfdd61fc..afe1c3e3c 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -187,6 +187,7 @@ public: buffer_size_in_frames(_buffer_size_in_frames), is_exclusive(_is_exclusive), spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) { + SetEventHandle(*client, event.handle()); } void Finish() noexcept { return SetStatus(Status::FINISH); } @@ -545,8 +546,6 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) thread.emplace(client.get(), std::move(render_client), FrameSize(), buffer_size_in_frames, is_exclusive); - SetEventHandle(*client, thread->event.handle()); - thread->Start(); } From 22c329cdb47d3a3e4a99678c869792b0f72ff061 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 19:55:49 +0100 Subject: [PATCH 19/54] output/wasapi: convert pointer to reference --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index afe1c3e3c..a7388773c 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -161,7 +161,7 @@ class WasapiOutputThread : public Thread { friend class WasapiOutput; WinEvent event; WinEvent data_poped; - IAudioClient *client; + IAudioClient &client; ComPtr render_client; const UINT32 frame_size; const UINT32 buffer_size_in_frames; @@ -178,7 +178,7 @@ class WasapiOutputThread : public Thread { boost::lockfree::spsc_queue spsc_buffer; public: - WasapiOutputThread(IAudioClient *_client, + WasapiOutputThread(IAudioClient &_client, ComPtr &&_render_client, const UINT32 _frame_size, const UINT32 _buffer_size_in_frames, bool _is_exclusive) @@ -187,7 +187,7 @@ public: buffer_size_in_frames(_buffer_size_in_frames), is_exclusive(_is_exclusive), spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) { - SetEventHandle(*client, event.handle()); + SetEventHandle(client, event.handle()); } void Finish() noexcept { return SetStatus(Status::FINISH); } @@ -334,7 +334,7 @@ WasapiOutputThread::Work() noexcept UINT32 write_in_frames = buffer_size_in_frames; if (!is_exclusive) { UINT32 data_in_frames = - GetCurrentPaddingFrames(*client); + GetCurrentPaddingFrames(client); if (data_in_frames >= buffer_size_in_frames) { continue; @@ -543,7 +543,7 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) const UINT32 buffer_size_in_frames = GetBufferSizeInFrames(*client); watermark = buffer_size_in_frames * 3 * FrameSize(); - thread.emplace(client.get(), std::move(render_client), FrameSize(), + thread.emplace(*client, std::move(render_client), FrameSize(), buffer_size_in_frames, is_exclusive); thread->Start(); From 34d4d9157a249780cb0f0ab6d80b1dbe7da77a63 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 19:57:36 +0100 Subject: [PATCH 20/54] output/wasapi: add `inline` --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index a7388773c..970f6e8cd 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -314,7 +314,7 @@ wasapi_output_get_client(WasapiOutput &output) noexcept return output.client.get(); } -void +inline void WasapiOutputThread::Work() noexcept { SetThreadName("Wasapi Output Worker"); From 5f656dffda1f62604cc97553b2cf4951be206328 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 19:55:14 +0100 Subject: [PATCH 21/54] output/wasapi: implement Cancel() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 970f6e8cd..508d3f71d 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -258,6 +258,7 @@ public: std::chrono::steady_clock::duration Delay() const noexcept override; size_t Play(const void *chunk, size_t size) override; void Drain() override; + void Cancel() noexcept override; bool Pause() override; void Interrupt() noexcept override; @@ -664,6 +665,14 @@ WasapiOutput::Drain() thread->CheckException(); } +void +WasapiOutput::Cancel() noexcept +{ + assert(thread); + + thread->spsc_buffer.consume_all([](auto &&) {}); +} + /// run inside COMWorkerThread void WasapiOutput::OpenDevice() From 5823e79fe78aaa4fd5d9a1f202ca678995a66179 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 21:41:11 +0100 Subject: [PATCH 22/54] output/wasapi: remove broken Drain() implementation The current Drain() implementation does what Cancel() should do; it does not wait for completion, but instead discards the buffer. --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 508d3f71d..ec3ec6a56 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -661,7 +661,8 @@ WasapiOutput::Drain() { assert(thread); - thread->spsc_buffer.consume_all([](auto &&) {}); + // TODO implement + thread->CheckException(); } From 3d6c9d1b88c5fd5d056b5ece27111b7178319284 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:06:28 +0100 Subject: [PATCH 23/54] output/wasapi: catch all exception --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index ec3ec6a56..4c7353c18 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -401,9 +401,9 @@ WasapiOutput::DoDisable() noexcept try { thread->Finish(); thread->Join(); - } catch (std::exception &err) { - FormatError(wasapi_output_domain, "exception while disabling: %s", - err.what()); + } catch (...) { + LogError(std::current_exception(), + "exception while disabling"); } thread.reset(); client.reset(); @@ -560,9 +560,9 @@ WasapiOutput::Close() noexcept Stop(*client); }).get(); thread->CheckException(); - } catch (std::exception &err) { - FormatError(wasapi_output_domain, "exception while stoping: %s", - err.what()); + } catch (...) { + FormatError(std::current_exception(), + "exception while stopping"); } is_started = false; thread->Finish(); From 3a0dbb0a6750e9a62de8716a8d312484b475be0d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:08:20 +0100 Subject: [PATCH 24/54] output/wasapi: make WasapiOutputThread::is_exclusive const --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 4c7353c18..19a8ddf2e 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -165,7 +165,7 @@ class WasapiOutputThread : public Thread { ComPtr render_client; const UINT32 frame_size; const UINT32 buffer_size_in_frames; - bool is_exclusive; + const bool is_exclusive; enum class Status : uint32_t { FINISH, PLAY, PAUSE }; alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = From 9256190a9b8fb36b69bd6c73315bf09a10363204 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:14:23 +0100 Subject: [PATCH 25/54] output/wasapi: move catch block to the Work() function level If an exception has been caught, the method cannot continue playback, therefore it doesn't make sense to have the "catch" block inside the "while" block (and not break the loop after catching an exception). --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 88 +++++++++---------- 1 file changed, 43 insertions(+), 45 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 19a8ddf2e..922604bef 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -317,63 +317,61 @@ wasapi_output_get_client(WasapiOutput &output) noexcept inline void WasapiOutputThread::Work() noexcept -{ +try { SetThreadName("Wasapi Output Worker"); FormatDebug(wasapi_output_domain, "Working thread started"); COM com; while (true) { - try { - event.Wait(); + event.Wait(); - Status current_state = status.load(); - if (current_state == Status::FINISH) { - FormatDebug(wasapi_output_domain, - "Working thread stopped"); - return; + Status current_state = status.load(); + if (current_state == Status::FINISH) { + FormatDebug(wasapi_output_domain, + "Working thread stopped"); + return; + } + + UINT32 write_in_frames = buffer_size_in_frames; + if (!is_exclusive) { + UINT32 data_in_frames = + GetCurrentPaddingFrames(client); + + if (data_in_frames >= buffer_size_in_frames) { + continue; } + write_in_frames -= data_in_frames; + } - UINT32 write_in_frames = buffer_size_in_frames; - if (!is_exclusive) { - UINT32 data_in_frames = - GetCurrentPaddingFrames(client); + BYTE *data; + DWORD mode = 0; - if (data_in_frames >= buffer_size_in_frames) { - continue; - } - write_in_frames -= data_in_frames; - } + if (HRESULT result = + render_client->GetBuffer(write_in_frames, &data); + FAILED(result)) { + throw MakeHResultError(result, "Failed to get buffer"); + } - BYTE *data; - DWORD mode = 0; + AtScopeExit(&) { + render_client->ReleaseBuffer(write_in_frames, mode); + }; - if (HRESULT result = - render_client->GetBuffer(write_in_frames, &data); - FAILED(result)) { - throw MakeHResultError(result, "Failed to get buffer"); - } - - AtScopeExit(&) { - render_client->ReleaseBuffer(write_in_frames, mode); - }; - - if (current_state == Status::PLAY) { - const UINT32 write_size = write_in_frames * frame_size; - UINT32 new_data_size = 0; - new_data_size = spsc_buffer.pop(data, write_size); - std::fill_n(data + new_data_size, - write_size - new_data_size, 0); - data_poped.Set(); - } else { - mode = AUDCLNT_BUFFERFLAGS_SILENT; - FormatDebug(wasapi_output_domain, - "Working thread paused"); - } - } catch (...) { - error.ptr = std::current_exception(); - error.occur.store(true); - error.thrown.Wait(); + if (current_state == Status::PLAY) { + const UINT32 write_size = write_in_frames * frame_size; + UINT32 new_data_size = 0; + new_data_size = spsc_buffer.pop(data, write_size); + std::fill_n(data + new_data_size, + write_size - new_data_size, 0); + data_poped.Set(); + } else { + mode = AUDCLNT_BUFFERFLAGS_SILENT; + FormatDebug(wasapi_output_domain, + "Working thread paused"); } } +} catch (...) { + error.ptr = std::current_exception(); + error.occur.store(true); + error.thrown.Wait(); } AudioOutput * From 79397db5b4edc20ea4bc6e7b035ff79a245ee6b7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:17:00 +0100 Subject: [PATCH 26/54] output/wasapi: remove the "thrown" field It is pointless to let WasapiOutputThread wait for the CheckException() call. --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 922604bef..37ec0cfd6 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -173,7 +173,6 @@ class WasapiOutputThread : public Thread { alignas(BOOST_LOCKFREE_CACHELINE_BYTES) struct { std::atomic_bool occur = false; std::exception_ptr ptr = nullptr; - WinEvent thrown; } error; boost::lockfree::spsc_queue spsc_buffer; @@ -197,7 +196,6 @@ public: void CheckException() { if (error.occur.load()) { auto err = std::exchange(error.ptr, nullptr); - error.thrown.Set(); std::rethrow_exception(err); } } @@ -371,7 +369,6 @@ try { } catch (...) { error.ptr = std::current_exception(); error.occur.store(true); - error.thrown.Wait(); } AudioOutput * From 798e68ef623192a97bde77c015b4056ad3a8765c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:18:01 +0100 Subject: [PATCH 27/54] output/wasapi: don't clear the exception in CheckException() This is pointless; the method cannot be called again anyway. --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 37ec0cfd6..3314204fc 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -195,8 +195,7 @@ public: void WaitDataPoped() noexcept { data_poped.Wait(); } void CheckException() { if (error.occur.load()) { - auto err = std::exchange(error.ptr, nullptr); - std::rethrow_exception(err); + std::rethrow_exception(error.ptr); } } From d19b3df3b09913743a80adf79517ad5d11e3b619 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:20:54 +0100 Subject: [PATCH 28/54] test/run_output: call AudioOutput::Drain() --- test/run_output.cxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/run_output.cxx b/test/run_output.cxx index 6d917600d..bc6782c32 100644 --- a/test/run_output.cxx +++ b/test/run_output.cxx @@ -162,6 +162,8 @@ RunOutput(AudioOutput &ao, AudioFormat audio_format, buffer.Consume(consumed); } + + ao.Drain(); } int main(int argc, char **argv) From 29346dc9c54fd0d469edcf7e4ff9558454d01554 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:27:16 +0100 Subject: [PATCH 29/54] output/wasapi: remove the thread management code from DoDisable() This is duplicate; this has already been done in Close(). --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 3314204fc..e023725bd 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -391,17 +391,8 @@ WasapiOutput::WasapiOutput(const ConfigBlock &block) void WasapiOutput::DoDisable() noexcept { - if (thread) { - try { - thread->Finish(); - thread->Join(); - } catch (...) { - LogError(std::current_exception(), - "exception while disabling"); - } - thread.reset(); - client.reset(); - } + assert(!thread); + device.reset(); enumerator.reset(); } From 01d3c2705e93ce4cf83cb2ed223ed6311ef19fd5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:28:17 +0100 Subject: [PATCH 30/54] output/wasapi: Finish() calls Join() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index e023725bd..4f6068a5e 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -189,7 +189,11 @@ public: SetEventHandle(client, event.handle()); } - void Finish() noexcept { return SetStatus(Status::FINISH); } + void Finish() noexcept { + SetStatus(Status::FINISH); + Join(); + } + void Play() noexcept { return SetStatus(Status::PLAY); } void Pause() noexcept { return SetStatus(Status::PAUSE); } void WaitDataPoped() noexcept { data_poped.Wait(); } @@ -551,7 +555,6 @@ WasapiOutput::Close() noexcept } is_started = false; thread->Finish(); - thread->Join(); com_worker->Async([&]() { thread.reset(); client.reset(); From d6fb07a3e48873f4706cd718a504582b04278f31 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:29:14 +0100 Subject: [PATCH 31/54] output/wasapi: start the WasapiOutputThread in its constructor --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 4f6068a5e..d049ad802 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -187,6 +187,7 @@ public: spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) { SetEventHandle(client, event.handle()); + Start(); } void Finish() noexcept { @@ -535,8 +536,6 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) watermark = buffer_size_in_frames * 3 * FrameSize(); thread.emplace(*client, std::move(render_client), FrameSize(), buffer_size_in_frames, is_exclusive); - - thread->Start(); } void From 6931ce9558854abd4bb70f61f3f465896f0a87b0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:26:11 +0100 Subject: [PATCH 32/54] output/wasapi: make the Thread a field, not a base class --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index d049ad802..dc9f27107 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -157,8 +157,9 @@ SetDSDFallback(AudioFormat &audio_format) noexcept } // namespace -class WasapiOutputThread : public Thread { +class WasapiOutputThread { friend class WasapiOutput; + Thread thread{BIND_THIS_METHOD(Work)}; WinEvent event; WinEvent data_poped; IAudioClient &client; @@ -181,18 +182,18 @@ public: ComPtr &&_render_client, const UINT32 _frame_size, const UINT32 _buffer_size_in_frames, bool _is_exclusive) - :Thread(BIND_THIS_METHOD(Work)), client(_client), + :client(_client), render_client(std::move(_render_client)), frame_size(_frame_size), buffer_size_in_frames(_buffer_size_in_frames), is_exclusive(_is_exclusive), spsc_buffer(_buffer_size_in_frames * 4 * _frame_size) { SetEventHandle(client, event.handle()); - Start(); + thread.Start(); } void Finish() noexcept { SetStatus(Status::FINISH); - Join(); + thread.Join(); } void Play() noexcept { return SetStatus(Status::PLAY); } From 9ade93983c61f90afa89aedf7119a17df3cbe1ba Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:43:38 +0100 Subject: [PATCH 33/54] output/wasapi: rename method WaitDataPoped() to Wait() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index dc9f27107..e08a4d985 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -198,7 +198,15 @@ public: void Play() noexcept { return SetStatus(Status::PLAY); } void Pause() noexcept { return SetStatus(Status::PAUSE); } - void WaitDataPoped() noexcept { data_poped.Wait(); } + + /** + * Wait for the thread to finish some work (e.g. until some + * buffer space becomes available). + */ + void Wait() noexcept { + data_poped.Wait(); + } + void CheckException() { if (error.occur.load()) { std::rethrow_exception(error.ptr); @@ -600,7 +608,7 @@ WasapiOutput::Play(const void *chunk, size_t size) static_cast(input.data), input.size); if (consumed_size == 0) { assert(is_started); - thread->WaitDataPoped(); + thread->Wait(); if (!not_interrupted.test_and_set()) { throw AudioOutputInterrupted{}; } From 3a948515ceccd0d2da44f5aad567d76a665e3d1a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:43:20 +0100 Subject: [PATCH 34/54] output/wasapi: check for exceptions after Wait() This finishes problems which occur early in the WasapiOutputThread; previously, the error was ignored and the output blocked forever without doing anything (and without reporting the error). --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index e08a4d985..ccf4f524c 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -382,6 +382,10 @@ try { } catch (...) { error.ptr = std::current_exception(); error.occur.store(true); + + /* wake up the client thread which may be inside + WaitDataPoped() */ + data_poped.Set(); } AudioOutput * @@ -609,6 +613,7 @@ WasapiOutput::Play(const void *chunk, size_t size) if (consumed_size == 0) { assert(is_started); thread->Wait(); + thread->CheckException(); if (!not_interrupted.test_and_set()) { throw AudioOutputInterrupted{}; } From 24a205a1aa80318c72ffd9d2a84b05b88845cb3a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:53:59 +0100 Subject: [PATCH 35/54] win32/HResult: try to use FormatMessage() --- src/win32/HResult.cxx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/win32/HResult.cxx b/src/win32/HResult.cxx index a40116a1b..0daa80163 100644 --- a/src/win32/HResult.cxx +++ b/src/win32/HResult.cxx @@ -18,6 +18,7 @@ */ #include "HResult.hxx" +#include "system/Error.hxx" #include #include @@ -27,11 +28,21 @@ std::string HResultCategory::message(int Errcode) const { + char buffer[256]; + + /* FormatMessage() supports some HRESULT values (depending on + the Windows version) */ + if (FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + nullptr, Errcode, 0, + buffer, sizeof(buffer), + nullptr)) + return buffer; + const auto msg = HRESULTToString(Errcode); if (!msg.empty()) return std::string(msg); - char buffer[11]; // "0x12345678\0" int size = snprintf(buffer, sizeof(buffer), "0x%1x", Errcode); assert(2 <= size && size <= 10); return std::string(buffer, size); From d72263d28d9b3715d31b069483bb874ede50771a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:55:16 +0100 Subject: [PATCH 36/54] win32/HResult: support AUDCLNT_E_NOT_{INITIALIZED,STOPPED} --- src/win32/HResult.hxx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/win32/HResult.hxx b/src/win32/HResult.hxx index e61447fb6..9bf612727 100644 --- a/src/win32/HResult.hxx +++ b/src/win32/HResult.hxx @@ -50,6 +50,8 @@ case x: C(AUDCLNT_E_SERVICE_NOT_RUNNING); C(AUDCLNT_E_UNSUPPORTED_FORMAT); C(AUDCLNT_E_WRONG_ENDPOINT_TYPE); + C(AUDCLNT_E_NOT_INITIALIZED); + C(AUDCLNT_E_NOT_STOPPED); C(CO_E_NOTINITIALIZED); C(E_INVALIDARG); C(E_OUTOFMEMORY); From 69783a44c8b926ea5858852fa7a35cc9ed22c539 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 22:08:03 +0100 Subject: [PATCH 37/54] output/wasapi: move Start()/Stop() calls to WasapiOutputThread::Work() --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index ccf4f524c..a9787cf4d 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -168,6 +168,8 @@ class WasapiOutputThread { const UINT32 buffer_size_in_frames; const bool is_exclusive; + bool started = false; + enum class Status : uint32_t { FINISH, PLAY, PAUSE }; alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = Status::PAUSE; @@ -332,6 +334,17 @@ try { SetThreadName("Wasapi Output Worker"); FormatDebug(wasapi_output_domain, "Working thread started"); COM com; + + AtScopeExit(this) { + if (started) { + try { + Stop(client); + } catch (...) { + LogError(std::current_exception()); + } + } + }; + while (true) { event.Wait(); @@ -342,11 +355,21 @@ try { return; } + if (!started) { + if (current_state != Status::PLAY) + /* don't bother starting the + IAudioClient if we're + paused */ + continue; + + Start(client); + started = true; + } + UINT32 write_in_frames = buffer_size_in_frames; if (!is_exclusive) { UINT32 data_in_frames = GetCurrentPaddingFrames(client); - if (data_in_frames >= buffer_size_in_frames) { continue; } @@ -557,9 +580,6 @@ WasapiOutput::Close() noexcept assert(thread); try { - com_worker->Async([&]() { - Stop(*client); - }).get(); thread->CheckException(); } catch (...) { FormatError(std::current_exception(), @@ -623,9 +643,6 @@ WasapiOutput::Play(const void *chunk, size_t size) if (!is_started) { is_started = true; thread->Play(); - com_worker->Async([&]() { - Start(*client); - }).wait(); } thread->CheckException(); From 0a97e68aa9dfaea4dab9961479134d23c6007276 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 23:02:43 +0100 Subject: [PATCH 38/54] output/wasapi: start after the buffer has been filled Postpone the Start() call until there is something to be played. --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index a9787cf4d..eed47591b 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -355,16 +355,10 @@ try { return; } - if (!started) { - if (current_state != Status::PLAY) - /* don't bother starting the - IAudioClient if we're - paused */ - continue; - - Start(client); - started = true; - } + if (!started && current_state != Status::PLAY) + /* don't bother starting the IAudioClient if + we're paused */ + continue; UINT32 write_in_frames = buffer_size_in_frames; if (!is_exclusive) { @@ -387,6 +381,11 @@ try { AtScopeExit(&) { render_client->ReleaseBuffer(write_in_frames, mode); + + if (!started) { + Start(client); + started = true; + } }; if (current_state == Status::PLAY) { From 3e93c392d77938b1b5e4a0345aad45ea01366551 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:21:26 +0100 Subject: [PATCH 39/54] output/wasapi: make `enumerator` a local variable --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index eed47591b..5d1e546b9 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -233,7 +233,6 @@ class WasapiOutput final : public AudioOutput { #endif std::string device_config; std::shared_ptr com_worker; - ComPtr enumerator; ComPtr device; ComPtr client; WAVEFORMATEXTENSIBLE device_format; @@ -293,9 +292,11 @@ private: bool TryFormatExclusive(const AudioFormat &audio_format); void FindExclusiveFormatSupported(AudioFormat &audio_format); void FindSharedFormatSupported(AudioFormat &audio_format); - void EnumerateDevices(); - ComPtr GetDevice(unsigned int index); - ComPtr SearchDevice(std::string_view name); + static void EnumerateDevices(IMMDeviceEnumerator &enumerator); + static ComPtr GetDevice(IMMDeviceEnumerator &enumerator, + unsigned index); + static ComPtr SearchDevice(IMMDeviceEnumerator &enumerator, + std::string_view name); }; WasapiOutput & @@ -434,7 +435,6 @@ WasapiOutput::DoDisable() noexcept assert(!thread); device.reset(); - enumerator.reset(); } /// run inside COMWorkerThread @@ -695,12 +695,13 @@ WasapiOutput::Cancel() noexcept void WasapiOutput::OpenDevice() { + ComPtr enumerator; enumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, CLSCTX_INPROC_SERVER); if (enumerate_devices) { try { - EnumerateDevices(); + EnumerateDevices(*enumerator); } catch (...) { LogError(std::current_exception()); } @@ -709,12 +710,12 @@ WasapiOutput::OpenDevice() if (!device_config.empty()) { unsigned int id; if (!SafeSilenceTry([this, &id]() { id = std::stoul(device_config); })) { - device = SearchDevice(device_config); + device = SearchDevice(*enumerator, device_config); if (!device) throw FormatRuntimeError("Device '%s' not found", device_config.c_str()); } else - device = GetDevice(id); + device = GetDevice(*enumerator, id); } else { device = GetDefaultAudioEndpoint(*enumerator); } @@ -915,9 +916,9 @@ WasapiOutput::FindSharedFormatSupported(AudioFormat &audio_format) /// run inside COMWorkerThread void -WasapiOutput::EnumerateDevices() +WasapiOutput::EnumerateDevices(IMMDeviceEnumerator &enumerator) { - const auto device_collection = EnumAudioEndpoints(*enumerator); + const auto device_collection = EnumAudioEndpoints(enumerator); const UINT count = GetCount(*device_collection); for (UINT i = 0; i < count; ++i) { @@ -938,17 +939,18 @@ WasapiOutput::EnumerateDevices() /// run inside COMWorkerThread ComPtr -WasapiOutput::GetDevice(unsigned int index) +WasapiOutput::GetDevice(IMMDeviceEnumerator &enumerator, unsigned index) { - const auto device_collection = EnumAudioEndpoints(*enumerator); + const auto device_collection = EnumAudioEndpoints(enumerator); return Item(*device_collection, index); } /// run inside COMWorkerThread ComPtr -WasapiOutput::SearchDevice(std::string_view name) +WasapiOutput::SearchDevice(IMMDeviceEnumerator &enumerator, + std::string_view name) { - const auto device_collection = EnumAudioEndpoints(*enumerator); + const auto device_collection = EnumAudioEndpoints(enumerator); const UINT count = GetCount(*device_collection); for (UINT i = 0; i < count; ++i) { From 3e484637f90588707f928fc0a83f5c0836feeaca Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:29:03 +0100 Subject: [PATCH 40/54] output/wasapi: rename OpenDevice() to ChooseDevice() OpenDevice was a confusing name because it does not actually open a device. --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 5d1e546b9..f08b2fad4 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -253,7 +253,7 @@ public: com_worker = std::make_shared(); try { - com_worker->Async([&]() { OpenDevice(); }).get(); + com_worker->Async([&]() { ChooseDevice(); }).get(); } catch (...) { com_worker.reset(); throw; @@ -288,7 +288,7 @@ private: void DoDisable() noexcept; void DoOpen(AudioFormat &audio_format); - void OpenDevice(); + void ChooseDevice(); bool TryFormatExclusive(const AudioFormat &audio_format); void FindExclusiveFormatSupported(AudioFormat &audio_format); void FindSharedFormatSupported(AudioFormat &audio_format); @@ -445,7 +445,7 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) if (GetState(*device) != DEVICE_STATE_ACTIVE) { device.reset(); - OpenDevice(); + ChooseDevice(); } client = Activate(*device); @@ -693,7 +693,7 @@ WasapiOutput::Cancel() noexcept /// run inside COMWorkerThread void -WasapiOutput::OpenDevice() +WasapiOutput::ChooseDevice() { ComPtr enumerator; enumerator.CoCreateInstance(__uuidof(MMDeviceEnumerator), nullptr, From 579428172e5411a4530244ce5c7c78eb1f8bb41c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:38:28 +0100 Subject: [PATCH 41/54] output/wasapi: remove the broken Delay() calculation code This code is complicated - and broken: the producer thread is not allowed to call consumer methods. Also the code is not necessary because this plugin implements Interrupt(). --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index f08b2fad4..a3501c94d 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -601,15 +601,7 @@ WasapiOutput::Delay() const noexcept return std::chrono::seconds(1); } - assert(thread); - - const size_t data_size = thread->spsc_buffer.read_available(); - const size_t delay_size = std::max(data_size, watermark) - watermark; - - using s = std::chrono::seconds; - using duration = std::chrono::steady_clock::duration; - auto result = duration(s(delay_size)) / device_format.Format.nAvgBytesPerSec; - return result; + return std::chrono::steady_clock::duration::zero(); } size_t From a2be91aea580cc08c0f551d3a598132f799e43bc Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:42:25 +0100 Subject: [PATCH 42/54] output/wasapi: add method WasapiOutputThread::InterruptWaiter() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index a3501c94d..7a58ffe2e 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -209,6 +209,10 @@ public: data_poped.Wait(); } + void InterruptWaiter() noexcept { + data_poped.Set(); + } + void CheckException() { if (error.occur.load()) { std::rethrow_exception(error.ptr); @@ -395,7 +399,7 @@ try { new_data_size = spsc_buffer.pop(data, write_size); std::fill_n(data + new_data_size, write_size - new_data_size, 0); - data_poped.Set(); + InterruptWaiter(); } else { mode = AUDCLNT_BUFFERFLAGS_SILENT; FormatDebug(wasapi_output_domain, @@ -406,9 +410,8 @@ try { error.ptr = std::current_exception(); error.occur.store(true); - /* wake up the client thread which may be inside - WaitDataPoped() */ - data_poped.Set(); + /* wake up the client thread which may be inside Wait() */ + InterruptWaiter(); } AudioOutput * @@ -661,7 +664,7 @@ WasapiOutput::Interrupt() noexcept { if (thread) { not_interrupted.clear(); - thread->data_poped.Set(); + thread->InterruptWaiter(); } } From 2ac2bd26f85d08c0b3156409e421aea5bf71e2f3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:44:55 +0100 Subject: [PATCH 43/54] output/wasapi: combine two `if` statements to one `switch` --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 7a58ffe2e..7092deb92 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -354,16 +354,23 @@ try { event.Wait(); Status current_state = status.load(); - if (current_state == Status::FINISH) { + switch (current_state) { + case Status::FINISH: FormatDebug(wasapi_output_domain, "Working thread stopped"); return; - } - if (!started && current_state != Status::PLAY) - /* don't bother starting the IAudioClient if - we're paused */ - continue; + case Status::PAUSE: + if (!started) + /* don't bother starting the + IAudioClient if we're paused */ + continue; + + break; + + case Status::PLAY: + break; + } UINT32 write_in_frames = buffer_size_in_frames; if (!is_exclusive) { From 5907656bbbeefbdba3c7a64e38ff98efe438e16e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 17:47:14 +0100 Subject: [PATCH 44/54] output/wasapi: stop the IAudioClient while paused Instead of generating silence, do nothing, don't waste CPU time. --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 7092deb92..f393f8871 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -366,7 +366,12 @@ try { IAudioClient if we're paused */ continue; - break; + /* stop the IAudioClient while paused; it will + be restarted as soon as we're asked to + resume playback */ + Stop(client); + started = false; + continue; case Status::PLAY: break; @@ -400,18 +405,12 @@ try { } }; - if (current_state == Status::PLAY) { - const UINT32 write_size = write_in_frames * frame_size; - UINT32 new_data_size = 0; - new_data_size = spsc_buffer.pop(data, write_size); - std::fill_n(data + new_data_size, - write_size - new_data_size, 0); - InterruptWaiter(); - } else { - mode = AUDCLNT_BUFFERFLAGS_SILENT; - FormatDebug(wasapi_output_domain, - "Working thread paused"); - } + const UINT32 write_size = write_in_frames * frame_size; + UINT32 new_data_size = 0; + new_data_size = spsc_buffer.pop(data, write_size); + std::fill_n(data + new_data_size, + write_size - new_data_size, 0); + InterruptWaiter(); } } catch (...) { error.ptr = std::current_exception(); From 08135f2cb752b59a91eb0536a59911d18138b22a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 19:58:32 +0100 Subject: [PATCH 45/54] output/wasapi: make configuration fields `const` --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index f393f8871..944ec9370 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -230,10 +230,10 @@ private: class WasapiOutput final : public AudioOutput { std::atomic_flag not_interrupted = true; bool is_started = false; - bool is_exclusive; - bool enumerate_devices; + const bool is_exclusive; + const bool enumerate_devices; #ifdef ENABLE_DSD - bool dop_setting; + const bool dop_setting; #endif std::string device_config; std::shared_ptr com_worker; From 1da27be84d9b4736e8980191061eae0260b6fa30 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 19:59:57 +0100 Subject: [PATCH 46/54] output/wasapi: move runtime fields below configuration fields --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 944ec9370..e324afc6f 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -228,13 +228,15 @@ private: }; class WasapiOutput final : public AudioOutput { - std::atomic_flag not_interrupted = true; - bool is_started = false; const bool is_exclusive; const bool enumerate_devices; #ifdef ENABLE_DSD const bool dop_setting; #endif + + bool is_started = false; + std::atomic_flag not_interrupted = true; + std::string device_config; std::shared_ptr com_worker; ComPtr device; From 14f0134097664a8bf6ba9ee36863378207127eba Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:05:03 +0100 Subject: [PATCH 47/54] output/wasapi: make device_config `const` --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index e324afc6f..a1c3d69ee 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -237,7 +237,8 @@ class WasapiOutput final : public AudioOutput { bool is_started = false; std::atomic_flag not_interrupted = true; - std::string device_config; + const std::string device_config; + std::shared_ptr com_worker; ComPtr device; ComPtr client; From 8024f7e84ddbeaae1cee3179174886b8083d629f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:06:59 +0100 Subject: [PATCH 48/54] output/wasapi: move the thread->Play() call right before the consumed_size check Fixes a bogus assertion failure (which can now be removed). --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index a1c3d69ee..db61fff3f 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -633,8 +633,13 @@ WasapiOutput::Play(const void *chunk, size_t size) do { const size_t consumed_size = thread->spsc_buffer.push( static_cast(input.data), input.size); + + if (!is_started) { + is_started = true; + thread->Play(); + } + if (consumed_size == 0) { - assert(is_started); thread->Wait(); thread->CheckException(); if (!not_interrupted.test_and_set()) { @@ -643,11 +648,6 @@ WasapiOutput::Play(const void *chunk, size_t size) continue; } - if (!is_started) { - is_started = true; - thread->Play(); - } - thread->CheckException(); if (pcm_export) { From fe7c5a420821811f51163b6a303e996130afea6c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:03:13 +0100 Subject: [PATCH 49/54] output/wasapi: initialize is_started in Open() --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index db61fff3f..f415c51e0 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -234,7 +234,11 @@ class WasapiOutput final : public AudioOutput { const bool dop_setting; #endif - bool is_started = false; + /** + * Only valid if the output is open. + */ + bool is_started; + std::atomic_flag not_interrupted = true; const std::string device_config; @@ -583,6 +587,8 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) watermark = buffer_size_in_frames * 3 * FrameSize(); thread.emplace(*client, std::move(render_client), FrameSize(), buffer_size_in_frames, is_exclusive); + + is_started = false; } void @@ -596,7 +602,6 @@ WasapiOutput::Close() noexcept FormatError(std::current_exception(), "exception while stopping"); } - is_started = false; thread->Finish(); com_worker->Async([&]() { thread.reset(); From 8a045207a72b1fdff161b53ef740be762e7c78f7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 19:59:06 +0100 Subject: [PATCH 50/54] output/wasapi: add field `paused` Fixes bogus Delay() results at the start of playback, because Delay() thinks the output is paused. --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index f415c51e0..ba6b61a80 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -239,6 +239,11 @@ class WasapiOutput final : public AudioOutput { */ bool is_started; + /** + * Only valid if the output is open. + */ + bool paused; + std::atomic_flag not_interrupted = true; const std::string device_config; @@ -276,6 +281,7 @@ public: } void Open(AudioFormat &audio_format) override { com_worker->Async([&]() { DoOpen(audio_format); }).get(); + paused = false; } void Close() noexcept override; std::chrono::steady_clock::duration Delay() const noexcept override; @@ -589,6 +595,7 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) buffer_size_in_frames, is_exclusive); is_started = false; + paused = false; } void @@ -613,7 +620,7 @@ WasapiOutput::Close() noexcept std::chrono::steady_clock::duration WasapiOutput::Delay() const noexcept { - if (!is_started) { + if (paused) { // idle while paused return std::chrono::seconds(1); } @@ -626,6 +633,8 @@ WasapiOutput::Play(const void *chunk, size_t size) { assert(thread); + paused = false; + not_interrupted.test_and_set(); ConstBuffer input(chunk, size); @@ -665,6 +674,7 @@ WasapiOutput::Play(const void *chunk, size_t size) bool WasapiOutput::Pause() { + paused = true; if (is_started) { thread->Pause(); is_started = false; From 1fe0c673bc3ead4e99e47e61e7aa3f0ed590bbba Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:25:20 +0100 Subject: [PATCH 51/54] output/wasapi: implement Cancel() properly Calling consume_all() is illegal in the producer thread. --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index ba6b61a80..e7075c0b5 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -170,7 +170,10 @@ class WasapiOutputThread { bool started = false; + std::atomic_bool cancel = false; + enum class Status : uint32_t { FINISH, PLAY, PAUSE }; + alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = Status::PAUSE; alignas(BOOST_LOCKFREE_CACHELINE_BYTES) struct { @@ -201,6 +204,24 @@ public: void Play() noexcept { return SetStatus(Status::PLAY); } void Pause() noexcept { return SetStatus(Status::PAUSE); } + /** + * Instruct the thread to discard the buffer (and wait for + * completion). This needs to be done inside this thread, + * because only the consumer thread is allowed to do that. + */ + void Cancel() noexcept { + cancel.store(true); + event.Set(); + + while (cancel.load() && !error.occur.load()) + Wait(); + + /* not rethrowing the exception here via + CheckException() because this method must be + "noexcept"; the next WasapiOutput::Play() call will + throw */ + } + /** * Wait for the thread to finish some work (e.g. until some * buffer space becomes available). @@ -366,6 +387,12 @@ try { while (true) { event.Wait(); + if (cancel.load()) { + spsc_buffer.consume_all([](auto &&) {}); + cancel.store(false); + InterruptWaiter(); + } + Status current_state = status.load(); switch (current_state) { case Status::FINISH: @@ -707,7 +734,7 @@ WasapiOutput::Cancel() noexcept { assert(thread); - thread->spsc_buffer.consume_all([](auto &&) {}); + thread->Cancel(); } /// run inside COMWorkerThread From 73f9824ddf7c7a917b89f0bb500985c15273530d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 8 Mar 2021 19:53:50 +0100 Subject: [PATCH 52/54] output/wasapi: eliminate `friend` declaration --- src/output/plugins/wasapi/WasapiOutputPlugin.cxx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index e7075c0b5..5e0226930 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -158,7 +158,6 @@ SetDSDFallback(AudioFormat &audio_format) noexcept } // namespace class WasapiOutputThread { - friend class WasapiOutput; Thread thread{BIND_THIS_METHOD(Work)}; WinEvent event; WinEvent data_poped; @@ -204,6 +203,11 @@ public: void Play() noexcept { return SetStatus(Status::PLAY); } void Pause() noexcept { return SetStatus(Status::PAUSE); } + std::size_t Push(ConstBuffer input) noexcept { + return spsc_buffer.push(static_cast(input.data), + input.size); + } + /** * Instruct the thread to discard the buffer (and wait for * completion). This needs to be done inside this thread, @@ -672,8 +676,7 @@ WasapiOutput::Play(const void *chunk, size_t size) return size; do { - const size_t consumed_size = thread->spsc_buffer.push( - static_cast(input.data), input.size); + const size_t consumed_size = thread->Push({chunk, size}); if (!is_started) { is_started = true; From 77fe727e696099ca65553f5c8d49f719b8d16678 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:39:54 +0100 Subject: [PATCH 53/54] output/wasapi: move the "is_started" flag to class WasapiOutputThread --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 5e0226930..203896c7a 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -167,6 +167,16 @@ class WasapiOutputThread { const UINT32 buffer_size_in_frames; const bool is_exclusive; + /** + * This flag is only used by the calling thread + * (i.e. #OutputThread), and specifies whether the + * WasapiOutputThread has been told to play via Play(). This + * variable is somewhat redundant because we already have + * "state", but using this variable saves some overhead for + * atomic operations. + */ + bool playing = false; + bool started = false; std::atomic_bool cancel = false; @@ -200,12 +210,30 @@ public: thread.Join(); } - void Play() noexcept { return SetStatus(Status::PLAY); } - void Pause() noexcept { return SetStatus(Status::PAUSE); } + void Play() noexcept { + playing = true; + SetStatus(Status::PLAY); + } + + void Pause() noexcept { + if (!playing) + return; + + playing = false; + SetStatus(Status::PAUSE); + } std::size_t Push(ConstBuffer input) noexcept { - return spsc_buffer.push(static_cast(input.data), - input.size); + std::size_t consumed = + spsc_buffer.push(static_cast(input.data), + input.size); + + if (!playing) { + playing = true; + Play(); + } + + return consumed; } /** @@ -259,11 +287,6 @@ class WasapiOutput final : public AudioOutput { const bool dop_setting; #endif - /** - * Only valid if the output is open. - */ - bool is_started; - /** * Only valid if the output is open. */ @@ -625,7 +648,6 @@ WasapiOutput::DoOpen(AudioFormat &audio_format) thread.emplace(*client, std::move(render_client), FrameSize(), buffer_size_in_frames, is_exclusive); - is_started = false; paused = false; } @@ -678,11 +700,6 @@ WasapiOutput::Play(const void *chunk, size_t size) do { const size_t consumed_size = thread->Push({chunk, size}); - if (!is_started) { - is_started = true; - thread->Play(); - } - if (consumed_size == 0) { thread->Wait(); thread->CheckException(); @@ -705,10 +722,7 @@ bool WasapiOutput::Pause() { paused = true; - if (is_started) { - thread->Pause(); - is_started = false; - } + thread->Pause(); thread->CheckException(); return true; } From 25b01940369d746debd0240bcab617b3451aa8c3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Mar 2021 20:47:15 +0100 Subject: [PATCH 54/54] output/wasapi: implement Drain() --- .../plugins/wasapi/WasapiOutputPlugin.cxx | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx index 203896c7a..fed16fbee 100644 --- a/src/output/plugins/wasapi/WasapiOutputPlugin.cxx +++ b/src/output/plugins/wasapi/WasapiOutputPlugin.cxx @@ -181,6 +181,8 @@ class WasapiOutputThread { std::atomic_bool cancel = false; + std::atomic_bool empty = true; + enum class Status : uint32_t { FINISH, PLAY, PAUSE }; alignas(BOOST_LOCKFREE_CACHELINE_BYTES) std::atomic status = @@ -224,6 +226,8 @@ public: } std::size_t Push(ConstBuffer input) noexcept { + empty.store(false); + std::size_t consumed = spsc_buffer.push(static_cast(input.data), input.size); @@ -236,6 +240,24 @@ public: return consumed; } + /** + * Check if the buffer is empty, and if not, wait a bit. + * + * Throws on error. + * + * @return true if the buffer is now empty + */ + bool Drain() { + if (empty) + return true; + + CheckException(); + Wait(); + CheckException(); + + return empty; + } + /** * Instruct the thread to discard the buffer (and wait for * completion). This needs to be done inside this thread, @@ -417,6 +439,7 @@ try { if (cancel.load()) { spsc_buffer.consume_all([](auto &&) {}); cancel.store(false); + empty.store(true); InterruptWaiter(); } @@ -475,6 +498,9 @@ try { const UINT32 write_size = write_in_frames * frame_size; UINT32 new_data_size = 0; new_data_size = spsc_buffer.pop(data, write_size); + if (new_data_size == 0) + empty.store(true); + std::fill_n(data + new_data_size, write_size - new_data_size, 0); InterruptWaiter(); @@ -741,9 +767,15 @@ WasapiOutput::Drain() { assert(thread); - // TODO implement + not_interrupted.test_and_set(); - thread->CheckException(); + while (!thread->Drain()) { + if (!not_interrupted.test_and_set()) + throw AudioOutputInterrupted{}; + } + + /* TODO: this needs to wait until the hardware has really + finished playing */ } void