From 885500bbda97bbb21ed627226dbf27ac8c648649 Mon Sep 17 00:00:00 2001 From: Vestral <16190165+Vestrel@users.noreply.github.com> Date: Thu, 13 Oct 2022 21:39:16 +0900 Subject: [PATCH] Cubeb: rewrite locking --- rpcs3/Emu/Audio/AudioBackend.h | 6 +- rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp | 117 ++++++++++----------- rpcs3/Emu/Audio/Cubeb/CubebBackend.h | 5 +- rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp | 2 +- rpcs3/Emu/Audio/XAudio2/XAudio2Backend.cpp | 26 ++--- rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h | 2 +- rpcs3/Emu/Cell/Modules/cellAudio.cpp | 11 +- rpcs3/Emu/Cell/Modules/cellAudio.h | 6 +- rpcs3/Emu/Cell/lv2/sys_rsxaudio.cpp | 24 +++-- 9 files changed, 101 insertions(+), 98 deletions(-) diff --git a/rpcs3/Emu/Audio/AudioBackend.h b/rpcs3/Emu/Audio/AudioBackend.h index b74c4bb26003..d6978ea66dd4 100644 --- a/rpcs3/Emu/Audio/AudioBackend.h +++ b/rpcs3/Emu/Audio/AudioBackend.h @@ -40,7 +40,7 @@ enum class AudioChannelCnt : u32 enum class AudioStateEvent : u32 { UNSPECIFIED_ERROR, - DEFAULT_DEVICE_CHANGED, + DEFAULT_DEVICE_MAYBE_CHANGED, }; class AudioBackend @@ -226,10 +226,10 @@ class AudioBackend AudioFreq m_sampling_rate = AudioFreq::FREQ_48K; AudioChannelCnt m_channels = AudioChannelCnt::STEREO; - shared_mutex m_cb_mutex{}; + std::timed_mutex m_cb_mutex{}; std::function m_write_callback{}; - std::recursive_mutex m_state_cb_mutex{}; + shared_mutex m_state_cb_mutex{}; std::function m_state_callback{}; bool m_playing = false; diff --git a/rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp b/rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp index 5e60dee5e757..4c89ff8fbcaf 100644 --- a/rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp +++ b/rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp @@ -22,16 +22,14 @@ CubebBackend::CubebBackend() } #endif - std::lock_guard lock(m_dev_sw_mutex); - - if (int err = cubeb_init(&m_ctx, "RPCS3", nullptr)) + cubeb *ctx{}; + if (int err = cubeb_init(&ctx, "RPCS3", nullptr)) { Cubeb.error("cubeb_init() failed: %i", err); - m_ctx = nullptr; return; } - if (int err = cubeb_register_device_collection_changed(m_ctx, CUBEB_DEVICE_TYPE_OUTPUT, device_collection_changed_cb, this)) + if (int err = cubeb_register_device_collection_changed(ctx, CUBEB_DEVICE_TYPE_OUTPUT, device_collection_changed_cb, this)) { Cubeb.error("cubeb_register_device_collection_changed() failed: %i", err); } @@ -40,7 +38,10 @@ CubebBackend::CubebBackend() m_dev_collection_cb_enabled = true; } - Cubeb.notice("Using backend %s", cubeb_get_backend_id(m_ctx)); + Cubeb.notice("Using backend %s", cubeb_get_backend_id(ctx)); + + std::lock_guard cb_lock{m_state_cb_mutex}; + m_ctx = ctx; } CubebBackend::~CubebBackend() @@ -80,8 +81,19 @@ bool CubebBackend::Operational() bool CubebBackend::DefaultDeviceChanged() { - std::lock_guard lock{m_dev_sw_mutex}; - return !m_reset_req.observe() && m_default_dev_changed; + if (m_default_device.empty() || m_reset_req.observe()) + { + return false; + } + + device_handle device = GetDevice(); + if (!device.handle) + { + Cubeb.error("Selected device not found. Trying alternative approach..."); + device = GetDefaultDeviceAlt(m_sampling_rate, m_sample_size, m_channels); + } + + return !device.handle || device.id != m_default_device; } bool CubebBackend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSize sample_size, AudioChannelCnt ch_cnt) @@ -92,10 +104,8 @@ bool CubebBackend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSize return false; } - std::lock_guard lock(m_cb_mutex); - std::lock_guard dev_sw_lock{m_dev_sw_mutex}; - std::lock_guard state_cb_lock{m_state_cb_mutex}; - CloseUnlocked(); + Close(); + std::lock_guard lock{m_cb_mutex}; const bool use_default_device = dev_id.empty() || dev_id == audio_device_enumerator::DEFAULT_DEV_ID; @@ -128,6 +138,12 @@ bool CubebBackend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSize device.ch_cnt = 2; } + if (use_default_device) + { + std::lock_guard lock{m_state_cb_mutex}; + m_default_device = device.id; + } + m_sampling_rate = freq; m_sample_size = sample_size; m_channels = static_cast(std::min(static_cast(convert_channel_count(device.ch_cnt)), static_cast(ch_cnt))); @@ -169,7 +185,7 @@ bool CubebBackend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSize if (int err = cubeb_stream_start(m_stream)) { Cubeb.error("cubeb_stream_start() failed: %i", err); - CloseUnlocked(); + Close(); return false; } @@ -178,15 +194,10 @@ bool CubebBackend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSize Cubeb.error("cubeb_stream_set_volume() failed: %i", err); } - if (use_default_device) - { - m_default_device = device.id; - } - return true; } -void CubebBackend::CloseUnlocked() +void CubebBackend::Close() { if (m_stream != nullptr) { @@ -196,24 +207,19 @@ void CubebBackend::CloseUnlocked() } cubeb_stream_destroy(m_stream); - m_stream = nullptr; } - m_playing = false; - m_last_sample.fill(0); + { + std::lock_guard lock{m_cb_mutex}; + m_stream = nullptr; + m_playing = false; + m_last_sample.fill(0); + } - m_default_dev_changed = false; + std::lock_guard lock{m_state_cb_mutex}; m_default_device.clear(); } -void CubebBackend::Close() -{ - std::lock_guard lock(m_cb_mutex); - std::lock_guard dev_sw_lock{m_dev_sw_mutex}; - std::lock_guard state_cb_lock(m_state_cb_mutex); - CloseUnlocked(); -} - void CubebBackend::Play() { if (m_stream == nullptr) @@ -427,14 +433,14 @@ long CubebBackend::data_cb(cubeb_stream* stream, void* user_ptr, void const* /* std::unique_lock lock(cubeb->m_cb_mutex, std::defer_lock); - if (stream != cubeb->m_stream) + if (!cubeb->m_reset_req.observe() && lock.try_lock_for(std::chrono::microseconds{50}) && cubeb->m_write_callback && cubeb->m_playing) { - Cubeb.error("data_cb called with unknown stream"); - return CUBEB_ERROR; - } + if (stream != cubeb->m_stream) + { + // Cubeb.error("data_cb called with unknown stream"); + return CUBEB_ERROR; + } - if (!cubeb->m_reset_req.observe() && lock.try_lock() && cubeb->m_write_callback && cubeb->m_playing) - { const u32 sample_size = cubeb->full_sample_size.observe(); const u32 bytes_req = nframes * sample_size; u32 written = std::min(cubeb->m_write_callback(bytes_req, output_buffer), bytes_req); @@ -454,7 +460,7 @@ long CubebBackend::data_cb(cubeb_stream* stream, void* user_ptr, void const* /* { // Stream parameters are modified only after stream_destroy. stream_destroy will return // only after this callback returns, so it's safe to access full_sample_size here. - memset(output_buffer, 0, nframes * cubeb->full_sample_size.observe()); + memset(output_buffer, 0, nframes * cubeb->full_sample_size); } return nframes; @@ -528,7 +534,12 @@ void CubebBackend::device_collection_changed_cb(cubeb* context, void* user_ptr) CubebBackend* const cubeb = static_cast(user_ptr); ensure(cubeb); - std::lock_guard lock{cubeb->m_dev_sw_mutex}; + if (cubeb->m_reset_req.observe()) + { + return; + } + + std::lock_guard cb_lock{cubeb->m_state_cb_mutex}; if (context != cubeb->m_ctx) { @@ -539,34 +550,12 @@ void CubebBackend::device_collection_changed_cb(cubeb* context, void* user_ptr) // Non default device is used (or default device cannot be detected) if (cubeb->m_default_device.empty()) { - Cubeb.notice("Skipping default device enumeration."); + Cubeb.notice("Skipping default device notification"); return; } - device_handle device = cubeb->GetDevice(); - if (!device.handle) - { - Cubeb.notice("Selected device not found. Trying alternative approach..."); - device = cubeb->GetDefaultDeviceAlt(cubeb->m_sampling_rate, cubeb->m_sample_size, cubeb->m_channels); - } - - std::lock_guard cb_lock{cubeb->m_state_cb_mutex}; - - if (!device.handle) + if (cubeb->m_state_callback) { - // No devices available - if (!cubeb->m_reset_req.test_and_set() && cubeb->m_state_callback) - { - cubeb->m_state_callback(AudioStateEvent::UNSPECIFIED_ERROR); - } - } - else if (!cubeb->m_reset_req.observe() && device.id != cubeb->m_default_device) - { - cubeb->m_default_dev_changed = true; - - if (cubeb->m_state_callback) - { - cubeb->m_state_callback(AudioStateEvent::DEFAULT_DEVICE_CHANGED); - } + cubeb->m_state_callback(AudioStateEvent::DEFAULT_DEVICE_MAYBE_CHANGED); } } diff --git a/rpcs3/Emu/Audio/Cubeb/CubebBackend.h b/rpcs3/Emu/Audio/Cubeb/CubebBackend.h index c9ca9de03fe9..949d05b16d71 100644 --- a/rpcs3/Emu/Audio/Cubeb/CubebBackend.h +++ b/rpcs3/Emu/Audio/Cubeb/CubebBackend.h @@ -44,9 +44,8 @@ class CubebBackend final : public AudioBackend atomic_t m_reset_req = false; - shared_mutex m_dev_sw_mutex{}; + // Protected by callback mutex std::string m_default_device{}; - bool m_default_dev_changed = false; bool m_dev_collection_cb_enabled = false; @@ -55,8 +54,6 @@ class CubebBackend final : public AudioBackend static void state_cb(cubeb_stream* stream, void* user_ptr, cubeb_state state); static void device_collection_changed_cb(cubeb* context, void* user_ptr); - void CloseUnlocked(); - struct device_handle { cubeb_devid handle{}; diff --git a/rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp b/rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp index 7e9efb1d9a50..f28410cfec82 100644 --- a/rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp +++ b/rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp @@ -234,7 +234,7 @@ void FAudioBackend::OnVoiceProcessingPassStart_func(FAudioVoiceCallback *cb_obj, FAudioBackend *faudio = static_cast(cb_obj); std::unique_lock lock(faudio->m_cb_mutex, std::defer_lock); - if (BytesRequired && !faudio->m_reset_req.observe() && lock.try_lock() && faudio->m_write_callback && faudio->m_playing) + if (BytesRequired && !faudio->m_reset_req.observe() && lock.try_lock_for(std::chrono::microseconds{50}) && faudio->m_write_callback && faudio->m_playing) { ensure(BytesRequired <= faudio->m_data_buf.size(), "FAudio internal buffer is too small. Report to developers!"); diff --git a/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.cpp b/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.cpp index 9b1c9211e05b..05fdf4448d0d 100644 --- a/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.cpp +++ b/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.cpp @@ -80,12 +80,6 @@ XAudio2Backend::XAudio2Backend() return; } - if (HRESULT hr = enumerator->RegisterEndpointNotificationCallback(this); FAILED(hr)) - { - XAudio.error("RegisterEndpointNotificationCallback() failed: %s (0x%08x)", std::system_category().message(hr), static_cast(hr)); - return; - } - // All succeeded, "commit" m_xaudio2_instance = std::move(instance); m_device_enumerator = std::move(enumerator); @@ -125,7 +119,7 @@ bool XAudio2Backend::Operational() bool XAudio2Backend::DefaultDeviceChanged() { - std::lock_guard lock{m_dev_sw_mutex}; + std::lock_guard lock{m_state_cb_mutex}; return !m_reset_req.observe() && m_default_dev_changed; } @@ -162,9 +156,12 @@ void XAudio2Backend::CloseUnlocked() m_master_voice = nullptr; } + m_device_enumerator->UnregisterEndpointNotificationCallback(this); + m_playing = false; m_last_sample.fill(0); + std::lock_guard lock(m_state_cb_mutex); m_default_dev_changed = false; m_current_device.clear(); } @@ -172,7 +169,6 @@ void XAudio2Backend::CloseUnlocked() void XAudio2Backend::Close() { std::lock_guard lock(m_cb_mutex); - std::lock_guard dev_sw_lock{m_dev_sw_mutex}; CloseUnlocked(); } @@ -207,7 +203,6 @@ bool XAudio2Backend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSi } std::lock_guard lock(m_cb_mutex); - std::lock_guard dev_sw_lock{m_dev_sw_mutex}; CloseUnlocked(); const bool use_default_device = dev_id.empty() || dev_id == audio_device_enumerator::DEFAULT_DEV_ID; @@ -282,6 +277,13 @@ bool XAudio2Backend::Open(std::string_view dev_id, AudioFreq freq, AudioSampleSi return false; } + if (HRESULT hr = m_device_enumerator->RegisterEndpointNotificationCallback(this); FAILED(hr)) + { + XAudio.error("RegisterEndpointNotificationCallback() failed: %s (0x%08x)", std::system_category().message(hr), static_cast(hr)); + CloseUnlocked(); + return; + } + if (HRESULT hr = m_source_voice->SetVolume(1.0f); FAILED(hr)) { XAudio.error("SetVolume() failed: %s (0x%08x)", std::system_category().message(hr), static_cast(hr)); @@ -331,7 +333,7 @@ f64 XAudio2Backend::GetCallbackFrameLen() void XAudio2Backend::OnVoiceProcessingPassStart(UINT32 BytesRequired) { std::unique_lock lock(m_cb_mutex, std::defer_lock); - if (BytesRequired && !m_reset_req.observe() && lock.try_lock() && m_write_callback && m_playing) + if (BytesRequired && !m_reset_req.observe() && lock.try_lock_for(std::chrono::microseconds{50}) && m_write_callback && m_playing) { ensure(BytesRequired <= m_data_buf.size(), "XAudio internal buffer is too small. Report to developers!"); @@ -386,7 +388,7 @@ HRESULT XAudio2Backend::OnDefaultDeviceChanged(EDataFlow flow, ERole role, LPCWS return S_OK; } - std::lock_guard lock{m_dev_sw_mutex}; + std::lock_guard lock(m_state_cb_mutex); // Non default device is used if (m_current_device.empty()) @@ -404,7 +406,7 @@ HRESULT XAudio2Backend::OnDefaultDeviceChanged(EDataFlow flow, ERole role, LPCWS if (m_state_callback) { - m_state_callback(AudioStateEvent::DEFAULT_DEVICE_CHANGED); + m_state_callback(AudioStateEvent::DEFAULT_DEVICE_MAYBE_CHANGED); } } } diff --git a/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h b/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h index 81a7e8502d88..6a0e68836a1f 100644 --- a/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h +++ b/rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h @@ -45,7 +45,7 @@ class XAudio2Backend final : public AudioBackend, public IXAudio2VoiceCallback, Microsoft::WRL::ComPtr m_device_enumerator{}; - shared_mutex m_dev_sw_mutex{}; + // Protected by state callback mutex std::string m_current_device{}; bool m_default_dev_changed = false; diff --git a/rpcs3/Emu/Cell/Modules/cellAudio.cpp b/rpcs3/Emu/Cell/Modules/cellAudio.cpp index bf298f9c1c2e..c643d8df6bbd 100644 --- a/rpcs3/Emu/Cell/Modules/cellAudio.cpp +++ b/rpcs3/Emu/Cell/Modules/cellAudio.cpp @@ -158,6 +158,7 @@ audio_ringbuffer::audio_ringbuffer(cell_audio_config& _cfg) cb_ringbuf.set_buf_size(static_cast(static_cast(cfg.backend_ch_cnt) * cfg.audio_sampling_rate * cfg.audio_sample_size * buffer_dur_mult)); backend->SetWriteCallback(std::bind(&audio_ringbuffer::backend_write_callback, this, std::placeholders::_1, std::placeholders::_2)); + backend->SetStateCallback(std::bind(&audio_ringbuffer::backend_state_callback, this, std::placeholders::_1)); } audio_ringbuffer::~audio_ringbuffer() @@ -191,6 +192,14 @@ u32 audio_ringbuffer::backend_write_callback(u32 size, void *buf) return static_cast(cb_ringbuf.pop(buf, size, true)); } +void audio_ringbuffer::backend_state_callback(AudioStateEvent event) +{ + if (event == AudioStateEvent::DEFAULT_DEVICE_MAYBE_CHANGED) + { + backend_device_changed = true; + } +} + u64 audio_ringbuffer::get_timestamp() { return get_system_time(); @@ -806,7 +815,7 @@ void cell_audio_thread::operator()() cellAudio.success("Backend recovered"); m_backend_failed = false; } - + if (!cfg.buffering_enabled) { const u64 period_end = (m_counter * cfg.audio_block_period) + m_start_time; diff --git a/rpcs3/Emu/Cell/Modules/cellAudio.h b/rpcs3/Emu/Cell/Modules/cellAudio.h index ffc67106c46c..f047324b69e0 100644 --- a/rpcs3/Emu/Cell/Modules/cellAudio.h +++ b/rpcs3/Emu/Cell/Modules/cellAudio.h @@ -292,6 +292,7 @@ class audio_ringbuffer audio_resampler resampler{}; atomic_t backend_active = false; + atomic_t backend_device_changed = false; bool playing = false; u64 update_timestamp = 0; @@ -310,6 +311,7 @@ class audio_ringbuffer void commit_data(f32* buf, u32 sample_cnt); u32 backend_write_callback(u32 size, void *buf); + void backend_state_callback(AudioStateEvent event); public: audio_ringbuffer(cell_audio_config &cfg); @@ -345,9 +347,9 @@ class audio_ringbuffer return backend->Operational(); } - bool device_changed() const + bool device_changed() { - return backend->DefaultDeviceChanged(); + return backend_device_changed.test_and_reset() && backend->DefaultDeviceChanged(); } std::string_view get_backend_name() const diff --git a/rpcs3/Emu/Cell/lv2/sys_rsxaudio.cpp b/rpcs3/Emu/Cell/lv2/sys_rsxaudio.cpp index a2eb76dc8e7b..eb4b0bca8d99 100644 --- a/rpcs3/Emu/Cell/lv2/sys_rsxaudio.cpp +++ b/rpcs3/Emu/Cell/lv2/sys_rsxaudio.cpp @@ -1361,6 +1361,7 @@ void rsxaudio_backend_thread::operator()() { bool should_update_backend = false; bool reset_backend = false; + bool checkDefaultDevice = false; bool should_service_stream = false; { @@ -1376,16 +1377,24 @@ void rsxaudio_backend_thread::operator()() return; } + if (backend_device_changed) + { + should_update_backend = true; + checkDefaultDevice = true; + backend_device_changed = false; + } + // Emulated state changed if (ra_state_changed) { const callback_config cb_cfg = callback_cfg.observe(); - should_update_backend |= cb_cfg.cfg_changed; ra_state_changed = false; ra_state = new_ra_state; if (cb_cfg.cfg_changed) { + should_update_backend = true; + checkDefaultDevice = false; callback_cfg.atomic_op([&](callback_config& val) { val.cfg_changed = false; // Acknowledge cfg update @@ -1400,6 +1409,7 @@ void rsxaudio_backend_thread::operator()() emu_cfg_changed = false; emu_cfg = new_emu_cfg; should_update_backend = true; + checkDefaultDevice = false; } // Handle backend error notification @@ -1407,14 +1417,8 @@ void rsxaudio_backend_thread::operator()() { reset_backend = true; should_update_backend = true; + checkDefaultDevice = false; backend_error_occured = false; - backend_device_changed = false; - } - - if (backend_device_changed) - { - should_update_backend = true; - backend_device_changed = false; } if (should_update_backend) @@ -1449,7 +1453,7 @@ void rsxaudio_backend_thread::operator()() } } - if (should_update_backend) + if (should_update_backend && (!checkDefaultDevice || backend->DefaultDeviceChanged())) { backend_init(ra_state, emu_cfg, reset_backend); @@ -1893,7 +1897,7 @@ void rsxaudio_backend_thread::state_changed_callback(AudioStateEvent event) backend_error_occured = true; break; } - case AudioStateEvent::DEFAULT_DEVICE_CHANGED: + case AudioStateEvent::DEFAULT_DEVICE_MAYBE_CHANGED: { backend_device_changed = true; break;