Cubeb: rewrite locking

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 @@ protected:
 	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<u32(u32, void *)> m_write_callback{};
 
-	std::recursive_mutex m_state_cb_mutex{};
+	shared_mutex m_state_cb_mutex{};
 	std::function<void(AudioStateEvent)> m_state_callback{};
 
 	bool m_playing = false;

rpcs3/Emu/Audio/Cubeb/CubebBackend.cpp

@@ -22,16 +22,14 @@ CubebBackend::CubebBackend()
 	}
 #endif
 
-	std::lock_guard lock(m_dev_sw_mutex);
+	cubeb *ctx{};
-
+	if (int err = cubeb_init(&ctx, "RPCS3", nullptr))
-	if (int err = cubeb_init(&m_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};
+	if (m_default_device.empty() || m_reset_req.observe())
-	return !m_reset_req.observe() && m_default_dev_changed;
+	{
+		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);
+	Close();
-	std::lock_guard dev_sw_lock{m_dev_sw_mutex};
+	std::lock_guard lock{m_cb_mutex};
-	std::lock_guard state_cb_lock{m_state_cb_mutex};
-	CloseUnlocked();
 
 	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<AudioChannelCnt>(std::min(static_cast<u32>(convert_channel_count(device.ch_cnt)), static_cast<u32>(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");
+		if (stream != cubeb->m_stream)
-		return CUBEB_ERROR;
+		{
-	}
+			// 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<CubebBackend*>(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 (cubeb->m_state_callback)
-	if (!device.handle)
 	{
-		Cubeb.notice("Selected device not found. Trying alternative approach...");
+		cubeb->m_state_callback(AudioStateEvent::DEFAULT_DEVICE_MAYBE_CHANGED);
-		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)
-	{
-		// 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);
-		}
 	}
 }

rpcs3/Emu/Audio/Cubeb/CubebBackend.h

@@ -44,9 +44,8 @@ private:
 
 	atomic_t<bool> 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 @@ private:
 	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{};

rpcs3/Emu/Audio/FAudio/FAudioBackend.cpp

@@ -234,7 +234,7 @@ void FAudioBackend::OnVoiceProcessingPassStart_func(FAudioVoiceCallback *cb_obj,
 	FAudioBackend *faudio = static_cast<FAudioBackend *>(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!");
 

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<u32>(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<u32>(hr));
+		CloseUnlocked();
+		return false;
+	}
+
 	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<u32>(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);
 			}
 		}
 	}

rpcs3/Emu/Audio/XAudio2/XAudio2Backend.h

@@ -45,7 +45,7 @@ private:
 
 	Microsoft::WRL::ComPtr<IMMDeviceEnumerator> 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;
 

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<u32>(static_cast<u32>(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<u32>(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;

rpcs3/Emu/Cell/Modules/cellAudio.h

@@ -292,6 +292,7 @@ private:
 	audio_resampler resampler{};
 
 	atomic_t<bool> backend_active = false;
+	atomic_t<bool> backend_device_changed = false;
 	bool playing = false;
 
 	u64 update_timestamp = 0;
@@ -310,6 +311,7 @@ private:
 
 	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 @@ public:
 		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

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;