From 55b50171f83ca247c19d5142d57c0583a4b07c1d Mon Sep 17 00:00:00 2001 From: squidbus <175574877+squidbus@users.noreply.github.com> Date: Thu, 2 Jan 2025 07:33:53 -0800 Subject: [PATCH] audio: Improve port state guards. (#1998) --- src/core/libraries/audio/audioout.cpp | 151 +++++++++++++------------ src/core/libraries/audio/audioout.h | 8 +- src/core/libraries/audio/sdl_audio.cpp | 27 +++-- 3 files changed, 103 insertions(+), 83 deletions(-) diff --git a/src/core/libraries/audio/audioout.cpp b/src/core/libraries/audio/audioout.cpp index d69454c3..f0ad59c3 100644 --- a/src/core/libraries/audio/audioout.cpp +++ b/src/core/libraries/audio/audioout.cpp @@ -3,13 +3,13 @@ #include #include -#include +#include +#include #include #include "common/assert.h" #include "common/config.h" #include "common/logging/log.h" -#include "common/polyfill_thread.h" #include "common/thread.h" #include "core/libraries/audio/audioout.h" #include "core/libraries/audio/audioout_backend.h" @@ -18,7 +18,7 @@ namespace Libraries::AudioOut { -std::shared_mutex ports_mutex; +std::mutex port_open_mutex{}; std::array ports_out{}; static std::unique_ptr audio; @@ -93,17 +93,20 @@ int PS4_SYSV_ABI sceAudioOutClose(s32 handle) { return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; } - std::scoped_lock lock(ports_mutex); + std::unique_lock open_lock{port_open_mutex}; auto& port = ports_out.at(handle - 1); - if (!port.impl) { - return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; + { + std::unique_lock lock{port.mutex}; + if (!port.IsOpen()) { + return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; + } + std::free(port.output_buffer); + port.output_buffer = nullptr; + port.output_ready = false; + port.impl = nullptr; } - + // Stop outside of port lock scope to prevent deadlocks. port.output_thread.Stop(); - std::free(port.output_buffer); - port.output_buffer = nullptr; - port.output_ready = false; - port.impl = nullptr; return ORBIS_OK; } @@ -172,35 +175,34 @@ int PS4_SYSV_ABI sceAudioOutGetPortState(s32 handle, OrbisAudioOutPortState* sta return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; } - std::scoped_lock lock(ports_mutex); - const auto& port = ports_out.at(handle - 1); - if (!port.impl) { - return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; + auto& port = ports_out.at(handle - 1); + { + std::unique_lock lock{port.mutex}; + if (!port.IsOpen()) { + return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; + } + switch (port.type) { + case OrbisAudioOutPort::Main: + case OrbisAudioOutPort::Bgm: + case OrbisAudioOutPort::Voice: + state->output = 1; + state->channel = port.format_info.num_channels > 2 ? 2 : port.format_info.num_channels; + break; + case OrbisAudioOutPort::Personal: + case OrbisAudioOutPort::Padspk: + state->output = 4; + state->channel = 1; + break; + case OrbisAudioOutPort::Aux: + state->output = 0; + state->channel = 0; + break; + default: + UNREACHABLE(); + } + state->rerouteCounter = 0; + state->volume = 127; } - - state->rerouteCounter = 0; - state->volume = 127; - - switch (port.type) { - case OrbisAudioOutPort::Main: - case OrbisAudioOutPort::Bgm: - case OrbisAudioOutPort::Voice: - state->output = 1; - state->channel = port.format_info.num_channels > 2 ? 2 : port.format_info.num_channels; - break; - case OrbisAudioOutPort::Personal: - case OrbisAudioOutPort::Padspk: - state->output = 4; - state->channel = 1; - break; - case OrbisAudioOutPort::Aux: - state->output = 0; - state->channel = 0; - break; - default: - UNREACHABLE(); - } - return ORBIS_OK; } @@ -279,15 +281,16 @@ static void AudioOutputThread(PortOut* port, const std::stop_token& stop) { while (true) { timer.Start(); { - std::unique_lock lock{port->output_mutex}; - Common::CondvarWait(port->output_cv, lock, stop, [&] { return port->output_ready; }); - if (stop.stop_requested()) { - break; + std::unique_lock lock{port->mutex}; + if (port->output_cv.wait(lock, stop, [&] { return port->output_ready; })) { + port->impl->Output(port->output_buffer); + port->output_ready = false; } - port->impl->Output(port->output_buffer); - port->output_ready = false; } port->output_cv.notify_one(); + if (stop.stop_requested()) { + break; + } timer.End(); } } @@ -332,27 +335,30 @@ s32 PS4_SYSV_ABI sceAudioOutOpen(UserService::OrbisUserServiceUserId user_id, return ORBIS_AUDIO_OUT_ERROR_INVALID_FORMAT; } - std::scoped_lock lock{ports_mutex}; + std::unique_lock open_lock{port_open_mutex}; const auto port = - std::ranges::find_if(ports_out, [&](const PortOut& p) { return p.impl == nullptr; }); + std::ranges::find_if(ports_out, [&](const PortOut& p) { return !p.IsOpen(); }); if (port == ports_out.end()) { LOG_ERROR(Lib_AudioOut, "Audio ports are full"); return ORBIS_AUDIO_OUT_ERROR_PORT_FULL; } - port->type = port_type; - port->format_info = GetFormatInfo(format); - port->sample_rate = sample_rate; - port->buffer_frames = length; - port->volume.fill(SCE_AUDIO_OUT_VOLUME_0DB); + { + std::unique_lock port_lock(port->mutex); - port->impl = audio->Open(*port); + port->type = port_type; + port->format_info = GetFormatInfo(format); + port->sample_rate = sample_rate; + port->buffer_frames = length; + port->volume.fill(SCE_AUDIO_OUT_VOLUME_0DB); - port->output_buffer = std::malloc(port->BufferSize()); - port->output_ready = false; - port->output_thread.Run( - [port](const std::stop_token& stop) { AudioOutputThread(&*port, stop); }); + port->impl = audio->Open(*port); + port->output_buffer = std::malloc(port->BufferSize()); + port->output_ready = false; + port->output_thread.Run( + [port](const std::stop_token& stop) { AudioOutputThread(&*port, stop); }); + } return std::distance(ports_out.begin(), port) + 1; } @@ -367,14 +373,13 @@ s32 PS4_SYSV_ABI sceAudioOutOutput(s32 handle, void* ptr) { } auto& port = ports_out.at(handle - 1); - if (!port.impl) { - return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; - } - { - std::unique_lock lock{port.output_mutex}; + std::unique_lock lock{port.mutex}; + if (!port.IsOpen()) { + return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; + } port.output_cv.wait(lock, [&] { return !port.output_ready; }); - if (ptr != nullptr) { + if (ptr != nullptr && port.IsOpen()) { std::memcpy(port.output_buffer, ptr, port.BufferSize()); port.output_ready = true; } @@ -488,19 +493,19 @@ s32 PS4_SYSV_ABI sceAudioOutSetVolume(s32 handle, s32 flag, s32* vol) { return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; } - std::scoped_lock lock(ports_mutex); auto& port = ports_out.at(handle - 1); - if (!port.impl) { - return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; - } - - for (int i = 0; i < port.format_info.num_channels; i++, flag >>= 1u) { - if (flag & 0x1u) { - port.volume[i] = vol[i]; + { + std::unique_lock lock{port.mutex}; + if (!port.IsOpen()) { + return ORBIS_AUDIO_OUT_ERROR_INVALID_PORT; } + for (int i = 0; i < port.format_info.num_channels; i++, flag >>= 1u) { + if (flag & 0x1u) { + port.volume[i] = vol[i]; + } + } + port.impl->SetVolume(port.volume); } - - port.impl->SetVolume(port.volume); return ORBIS_OK; } diff --git a/src/core/libraries/audio/audioout.h b/src/core/libraries/audio/audioout.h index 4f7378dc..5eafb43a 100644 --- a/src/core/libraries/audio/audioout.h +++ b/src/core/libraries/audio/audioout.h @@ -3,7 +3,9 @@ #pragma once +#include #include +#include #include "common/bit_field.h" #include "core/libraries/kernel/threads.h" @@ -74,10 +76,10 @@ struct AudioFormatInfo { }; struct PortOut { + std::mutex mutex; std::unique_ptr impl{}; void* output_buffer; - std::mutex output_mutex; std::condition_variable_any output_cv; bool output_ready; Kernel::Thread output_thread{}; @@ -88,6 +90,10 @@ struct PortOut { u32 buffer_frames; std::array volume; + [[nodiscard]] bool IsOpen() const { + return impl != nullptr; + } + [[nodiscard]] u32 BufferSize() const { return buffer_frames * format_info.FrameSize(); } diff --git a/src/core/libraries/audio/sdl_audio.cpp b/src/core/libraries/audio/sdl_audio.cpp index 59d2d5cf..762a9f68 100644 --- a/src/core/libraries/audio/sdl_audio.cpp +++ b/src/core/libraries/audio/sdl_audio.cpp @@ -14,7 +14,7 @@ namespace Libraries::AudioOut { class SDLPortBackend : public PortBackend { public: explicit SDLPortBackend(const PortOut& port) - : frame_size(port.format_info.FrameSize()), buffer_size(port.BufferSize()) { + : frame_size(port.format_info.FrameSize()), guest_buffer_size(port.BufferSize()) { // We want the latency for delivering frames out to be as small as possible, // so set the sample frames hint to the number of frames per buffer. const auto samples_num_str = std::to_string(port.buffer_frames); @@ -33,7 +33,7 @@ public: LOG_ERROR(Lib_AudioOut, "Failed to create SDL audio stream: {}", SDL_GetError()); return; } - queue_threshold = CalculateQueueThreshold(); + CalculateQueueThreshold(); if (!SDL_SetAudioStreamInputChannelMap(stream, port.format_info.channel_layout.data(), port.format_info.num_channels)) { LOG_ERROR(Lib_AudioOut, "Failed to configure SDL audio stream channel map: {}", @@ -71,9 +71,9 @@ public: queue_threshold); SDL_ClearAudioStream(stream); // Recalculate the threshold in case this happened because of a device change. - queue_threshold = CalculateQueueThreshold(); + CalculateQueueThreshold(); } - if (!SDL_PutAudioStreamData(stream, ptr, static_cast(buffer_size))) { + if (!SDL_PutAudioStreamData(stream, ptr, static_cast(guest_buffer_size))) { LOG_ERROR(Lib_AudioOut, "Failed to output to SDL audio stream: {}", SDL_GetError()); } } @@ -91,7 +91,7 @@ public: } private: - [[nodiscard]] u32 CalculateQueueThreshold() const { + void CalculateQueueThreshold() { SDL_AudioSpec discard; int sdl_buffer_frames; if (!SDL_GetAudioDeviceFormat(SDL_GetAudioStreamDevice(stream), &discard, @@ -100,13 +100,22 @@ private: SDL_GetError()); sdl_buffer_frames = 0; } - return std::max(buffer_size, sdl_buffer_frames * frame_size) * 4; + const auto sdl_buffer_size = sdl_buffer_frames * frame_size; + const auto new_threshold = std::max(guest_buffer_size, sdl_buffer_size) * 4; + if (host_buffer_size != sdl_buffer_size || queue_threshold != new_threshold) { + host_buffer_size = sdl_buffer_size; + queue_threshold = new_threshold; + LOG_INFO(Lib_AudioOut, + "SDL audio buffers: guest = {} bytes, host = {} bytes, threshold = {} bytes", + guest_buffer_size, host_buffer_size, queue_threshold); + } } u32 frame_size; - u32 buffer_size; - u32 queue_threshold; - SDL_AudioStream* stream; + u32 guest_buffer_size; + u32 host_buffer_size{}; + u32 queue_threshold{}; + SDL_AudioStream* stream{}; }; std::unique_ptr SDLAudioOut::Open(PortOut& port) {