From d8fc3f403b62e2b3d67ec08791fdc66847ddb4ac Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Sat, 18 Mar 2023 20:52:02 +0000 Subject: [PATCH 1/4] audio: Interpolate system manager sample count using host sink sample info This avoids the need to stall if the host sink sporadically misses the deadline, in such a case the previous implementation would report them samples as being played on-time, causing the guest to send more samples and leading to a gradual buildup. --- src/audio_core/device/device_session.cpp | 3 +-- src/audio_core/renderer/system_manager.cpp | 1 - src/audio_core/sink/sink_stream.cpp | 21 +++++++++++++++++++++ src/audio_core/sink/sink_stream.h | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/audio_core/device/device_session.cpp b/src/audio_core/device/device_session.cpp index 5a327a6068..ad0f40e28b 100644 --- a/src/audio_core/device/device_session.cpp +++ b/src/audio_core/device/device_session.cpp @@ -121,8 +121,7 @@ u64 DeviceSession::GetPlayedSampleCount() const { } std::optional DeviceSession::ThreadFunc() { - // Add 5ms of samples at a 48K sample rate. - played_sample_count += 48'000 * INCREMENT_TIME / 1s; + played_sample_count = stream->GetExpectedPlayedSampleCount(); if (type == Sink::StreamType::Out) { system.AudioCore().GetAudioManager().SetEvent(Event::Type::AudioOutManager, true); } else { diff --git a/src/audio_core/renderer/system_manager.cpp b/src/audio_core/renderer/system_manager.cpp index ce631f8108..9ddfa4a91d 100644 --- a/src/audio_core/renderer/system_manager.cpp +++ b/src/audio_core/renderer/system_manager.cpp @@ -15,7 +15,6 @@ MICROPROFILE_DEFINE(Audio_RenderSystemManager, "Audio", "Render System Manager", MP_RGB(60, 19, 97)); namespace AudioCore::AudioRenderer { -constexpr std::chrono::nanoseconds RENDER_TIME{5'000'000UL}; SystemManager::SystemManager(Core::System& core_) : core{core_}, adsp{core.AudioCore().GetADSP()}, mailbox{adsp.GetRenderMailbox()}, diff --git a/src/audio_core/sink/sink_stream.cpp b/src/audio_core/sink/sink_stream.cpp index 39a21b0f08..1af96f7938 100644 --- a/src/audio_core/sink/sink_stream.cpp +++ b/src/audio_core/sink/sink_stream.cpp @@ -14,6 +14,8 @@ #include "common/fixed_point.h" #include "common/settings.h" #include "core/core.h" +#include "core/core_timing.h" +#include "core/core_timing_util.h" namespace AudioCore::Sink { @@ -198,6 +200,7 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz const std::size_t frame_size = num_channels; const std::size_t frame_size_bytes = frame_size * sizeof(s16); size_t frames_written{0}; + size_t actual_frames_written{0}; // If we're paused or going to shut down, we don't want to consume buffers as coretiming is // paused and we'll desync, so just play silence. @@ -248,6 +251,7 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz frames_available * frame_size); frames_written += frames_available; + actual_frames_written += frames_available; playing_buffer.frames_played += frames_available; // If that's all the frames in the current buffer, add its samples and mark it as @@ -260,6 +264,13 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz std::memcpy(&last_frame[0], &output_buffer[(frames_written - 1) * frame_size], frame_size_bytes); + { + std::scoped_lock lk{sample_count_lock}; + last_sample_count_update_time = Core::Timing::CyclesToUs(system.CoreTiming().GetClockTicks()); + min_played_sample_count = max_played_sample_count; + max_played_sample_count += actual_frames_written; + } + if (system.IsMulticore() && queued_buffers <= max_queue_size) { Unstall(); } @@ -282,4 +293,14 @@ void SinkStream::Unstall() { stalled_lock.unlock(); } +u64 SinkStream::GetExpectedPlayedSampleCount() { + std::scoped_lock lk{sample_count_lock}; + auto cur_time{Core::Timing::CyclesToUs(system.CoreTiming().GetClockTicks())}; + auto time_delta{cur_time - last_sample_count_update_time}; + auto exp_played_sample_count{min_played_sample_count + + (TargetSampleRate * time_delta) / std::chrono::seconds{1}}; + + return std::min(exp_played_sample_count, max_played_sample_count); +} + } // namespace AudioCore::Sink diff --git a/src/audio_core/sink/sink_stream.h b/src/audio_core/sink/sink_stream.h index 5fea72ab72..2340c936c5 100644 --- a/src/audio_core/sink/sink_stream.h +++ b/src/audio_core/sink/sink_stream.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -14,6 +15,7 @@ #include "common/common_types.h" #include "common/reader_writer_queue.h" #include "common/ring_buffer.h" +#include "common/thread.h" namespace Core { class System; @@ -210,6 +212,13 @@ public: */ void Unstall(); + /** + * Get the total number of samples expected to have been played by this stream. + * + * @return The number of samples. + */ + u64 GetExpectedPlayedSampleCount(); + protected: /// Core system Core::System& system; @@ -237,6 +246,14 @@ private: std::atomic queued_buffers{}; /// The ring size for audio out buffers (usually 4, rarely 2 or 8) u32 max_queue_size{}; + /// Locks access to sample count tracking info + std::mutex sample_count_lock; + /// Minimum number of total samples that have been played since the last callback + u64 min_played_sample_count{}; + /// Maximum number of total samples that can be played since the last callback + u64 max_played_sample_count{}; + /// The time the two above tracking variables were last written to + std::chrono::microseconds last_sample_count_update_time{}; /// Set by the audio render/in/out system which uses this stream f32 system_volume{1.0f}; /// Set via IAudioDevice service calls From ea5dd02db9bdb9759a400907672ec6606bebb96b Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Sat, 18 Mar 2023 20:57:00 +0000 Subject: [PATCH 2/4] audio: Wait for samples on the emulated DSP side to avoid desyncs Waiting on the host side is inaccurate and leads to desyncs in the event of the sink missing a deadline that require stalls to fix. By waiting for the sink to have space before even starting rendering such desyncs can be avoided. --- .../renderer/adsp/audio_renderer.cpp | 2 ++ src/audio_core/renderer/adsp/audio_renderer.h | 1 + src/audio_core/renderer/system_manager.cpp | 18 +----------------- src/audio_core/renderer/system_manager.h | 7 ------- src/audio_core/sink/sink_stream.cpp | 15 +++++++++++++++ src/audio_core/sink/sink_stream.h | 9 +++++++++ 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/audio_core/renderer/adsp/audio_renderer.cpp b/src/audio_core/renderer/adsp/audio_renderer.cpp index 42b4b167a0..503f403496 100644 --- a/src/audio_core/renderer/adsp/audio_renderer.cpp +++ b/src/audio_core/renderer/adsp/audio_renderer.cpp @@ -189,6 +189,8 @@ void AudioRenderer::ThreadFunc() { max_time = std::min(command_buffer.time_limit, max_time); command_list_processor.SetProcessTimeMax(max_time); + streams[index]->WaitFreeSpace(); + // Process the command list { MICROPROFILE_SCOPE(Audio_Renderer); diff --git a/src/audio_core/renderer/adsp/audio_renderer.h b/src/audio_core/renderer/adsp/audio_renderer.h index 151f38c1bd..f97f9401e6 100644 --- a/src/audio_core/renderer/adsp/audio_renderer.h +++ b/src/audio_core/renderer/adsp/audio_renderer.h @@ -12,6 +12,7 @@ #include "common/common_types.h" #include "common/reader_writer_queue.h" #include "common/thread.h" +#include "common/polyfill_thread.h" namespace Core { namespace Timing { diff --git a/src/audio_core/renderer/system_manager.cpp b/src/audio_core/renderer/system_manager.cpp index 9ddfa4a91d..07d8ed093e 100644 --- a/src/audio_core/renderer/system_manager.cpp +++ b/src/audio_core/renderer/system_manager.cpp @@ -17,11 +17,7 @@ MICROPROFILE_DEFINE(Audio_RenderSystemManager, "Audio", "Render System Manager", namespace AudioCore::AudioRenderer { SystemManager::SystemManager(Core::System& core_) - : core{core_}, adsp{core.AudioCore().GetADSP()}, mailbox{adsp.GetRenderMailbox()}, - thread_event{Core::Timing::CreateEvent( - "AudioRendererSystemManager", [this](std::uintptr_t, s64 time, std::chrono::nanoseconds) { - return ThreadFunc2(time); - })} {} + : core{core_}, adsp{core.AudioCore().GetADSP()}, mailbox{adsp.GetRenderMailbox()} {} SystemManager::~SystemManager() { Stop(); @@ -32,8 +28,6 @@ bool SystemManager::InitializeUnsafe() { if (adsp.Start()) { active = true; thread = std::jthread([this](std::stop_token stop_token) { ThreadFunc(); }); - core.CoreTiming().ScheduleLoopingEvent(std::chrono::nanoseconds(0), RENDER_TIME, - thread_event); } } @@ -44,7 +38,6 @@ void SystemManager::Stop() { if (!active) { return; } - core.CoreTiming().UnscheduleEvent(thread_event, {}); active = false; update.store(true); update.notify_all(); @@ -110,16 +103,7 @@ void SystemManager::ThreadFunc() { adsp.Signal(); adsp.Wait(); - - update.wait(false); - update.store(false); } } -std::optional SystemManager::ThreadFunc2(s64 time) { - update.store(true); - update.notify_all(); - return std::nullopt; -} - } // namespace AudioCore::AudioRenderer diff --git a/src/audio_core/renderer/system_manager.h b/src/audio_core/renderer/system_manager.h index 415ddb74fd..1f0bbd8b4c 100644 --- a/src/audio_core/renderer/system_manager.h +++ b/src/audio_core/renderer/system_manager.h @@ -68,11 +68,6 @@ private: */ void ThreadFunc(); - /** - * Signalling core timing thread to run ThreadFunc. - */ - std::optional ThreadFunc2(s64 time); - enum class StreamState { Filling, Steady, @@ -95,8 +90,6 @@ private: ADSP::ADSP& adsp; /// AudioRenderer mailbox for communication ADSP::AudioRenderer_Mailbox* mailbox{}; - /// Core timing event to signal main thread - std::shared_ptr thread_event; /// Atomic for main thread to wait on std::atomic update{}; }; diff --git a/src/audio_core/sink/sink_stream.cpp b/src/audio_core/sink/sink_stream.cpp index 1af96f7938..a54c618454 100644 --- a/src/audio_core/sink/sink_stream.cpp +++ b/src/audio_core/sink/sink_stream.cpp @@ -205,6 +205,10 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz // If we're paused or going to shut down, we don't want to consume buffers as coretiming is // paused and we'll desync, so just play silence. if (system.IsPaused() || system.IsShuttingDown()) { + if (system.IsShuttingDown()) { + release_cv.notify_one(); + } + static constexpr std::array silence{}; for (size_t i = frames_written; i < num_frames; i++) { std::memcpy(&output_buffer[i * frame_size], &silence[0], frame_size_bytes); @@ -240,6 +244,12 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz } // Successfully dequeued a new buffer. queued_buffers--; + + { + std::unique_lock lk{release_mutex}; + } + + release_cv.notify_one(); } // Get the minimum frames available between the currently playing buffer, and the @@ -303,4 +313,9 @@ u64 SinkStream::GetExpectedPlayedSampleCount() { return std::min(exp_played_sample_count, max_played_sample_count); } +void SinkStream::WaitFreeSpace() { + std::unique_lock lk{release_mutex}; + release_cv.wait(lk, [this]() { return queued_buffers < max_queue_size || system.IsShuttingDown(); }); +} + } // namespace AudioCore::Sink diff --git a/src/audio_core/sink/sink_stream.h b/src/audio_core/sink/sink_stream.h index 2340c936c5..709f3b0ecb 100644 --- a/src/audio_core/sink/sink_stream.h +++ b/src/audio_core/sink/sink_stream.h @@ -16,6 +16,7 @@ #include "common/reader_writer_queue.h" #include "common/ring_buffer.h" #include "common/thread.h" +#include "common/polyfill_thread.h" namespace Core { class System; @@ -219,6 +220,11 @@ public: */ u64 GetExpectedPlayedSampleCount(); + /** + * Waits for free space in the sample ring buffer + */ + void WaitFreeSpace(); + protected: /// Core system Core::System& system; @@ -258,6 +264,9 @@ private: f32 system_volume{1.0f}; /// Set via IAudioDevice service calls f32 device_volume{1.0f}; + /// Signalled when ring buffer entries are consumed + std::condition_variable release_cv; + std::mutex release_mutex; std::mutex stall_guard; std::unique_lock stalled_lock; }; From 237934b73690a56ff756d6e682fa336dee8c95a4 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Sun, 26 Mar 2023 20:07:03 +0100 Subject: [PATCH 3/4] Run clang-format --- src/audio_core/renderer/adsp/audio_renderer.h | 2 +- src/audio_core/sink/sink_stream.cpp | 10 +++++----- src/audio_core/sink/sink_stream.h | 1 - 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/audio_core/renderer/adsp/audio_renderer.h b/src/audio_core/renderer/adsp/audio_renderer.h index f97f9401e6..85ce6a2690 100644 --- a/src/audio_core/renderer/adsp/audio_renderer.h +++ b/src/audio_core/renderer/adsp/audio_renderer.h @@ -10,9 +10,9 @@ #include "audio_core/renderer/adsp/command_buffer.h" #include "audio_core/renderer/adsp/command_list_processor.h" #include "common/common_types.h" +#include "common/polyfill_thread.h" #include "common/reader_writer_queue.h" #include "common/thread.h" -#include "common/polyfill_thread.h" namespace Core { namespace Timing { diff --git a/src/audio_core/sink/sink_stream.cpp b/src/audio_core/sink/sink_stream.cpp index a54c618454..084ac5edfe 100644 --- a/src/audio_core/sink/sink_stream.cpp +++ b/src/audio_core/sink/sink_stream.cpp @@ -245,9 +245,7 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz // Successfully dequeued a new buffer. queued_buffers--; - { - std::unique_lock lk{release_mutex}; - } + { std::unique_lock lk{release_mutex}; } release_cv.notify_one(); } @@ -276,7 +274,8 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz { std::scoped_lock lk{sample_count_lock}; - last_sample_count_update_time = Core::Timing::CyclesToUs(system.CoreTiming().GetClockTicks()); + last_sample_count_update_time = + Core::Timing::CyclesToUs(system.CoreTiming().GetClockTicks()); min_played_sample_count = max_played_sample_count; max_played_sample_count += actual_frames_written; } @@ -315,7 +314,8 @@ u64 SinkStream::GetExpectedPlayedSampleCount() { void SinkStream::WaitFreeSpace() { std::unique_lock lk{release_mutex}; - release_cv.wait(lk, [this]() { return queued_buffers < max_queue_size || system.IsShuttingDown(); }); + release_cv.wait( + lk, [this]() { return queued_buffers < max_queue_size || system.IsShuttingDown(); }); } } // namespace AudioCore::Sink diff --git a/src/audio_core/sink/sink_stream.h b/src/audio_core/sink/sink_stream.h index 709f3b0ecb..9806e6d986 100644 --- a/src/audio_core/sink/sink_stream.h +++ b/src/audio_core/sink/sink_stream.h @@ -16,7 +16,6 @@ #include "common/reader_writer_queue.h" #include "common/ring_buffer.h" #include "common/thread.h" -#include "common/polyfill_thread.h" namespace Core { class System; From 530fe24768357d4151ac6c6aca4a0e122ef8260a Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Sun, 26 Mar 2023 20:21:04 +0100 Subject: [PATCH 4/4] audio_core: No longer stall when sink queue is full Now the audout and audren update rates are tied to the sink status stalling is no longer necessary. --- src/audio_core/sink/cubeb_sink.cpp | 4 --- src/audio_core/sink/sdl2_sink.cpp | 2 -- src/audio_core/sink/sink_stream.cpp | 43 ----------------------------- src/audio_core/sink/sink_stream.h | 16 +---------- 4 files changed, 1 insertion(+), 64 deletions(-) diff --git a/src/audio_core/sink/cubeb_sink.cpp b/src/audio_core/sink/cubeb_sink.cpp index 9133f53884..9a08018888 100644 --- a/src/audio_core/sink/cubeb_sink.cpp +++ b/src/audio_core/sink/cubeb_sink.cpp @@ -101,8 +101,6 @@ public: ~CubebSinkStream() override { LOG_DEBUG(Service_Audio, "Destructing cubeb stream {}", name); - Unstall(); - if (!ctx) { return; } @@ -143,8 +141,6 @@ public: * Stop the sink stream. */ void Stop() override { - Unstall(); - if (!ctx || paused) { return; } diff --git a/src/audio_core/sink/sdl2_sink.cpp b/src/audio_core/sink/sdl2_sink.cpp index c138dc628e..ee1a0652f3 100644 --- a/src/audio_core/sink/sdl2_sink.cpp +++ b/src/audio_core/sink/sdl2_sink.cpp @@ -88,7 +88,6 @@ public: * Finalize the sink stream. */ void Finalize() override { - Unstall(); if (device == 0) { return; } @@ -116,7 +115,6 @@ public: * Stop the sink stream. */ void Stop() override { - Unstall(); if (device == 0 || paused) { return; } diff --git a/src/audio_core/sink/sink_stream.cpp b/src/audio_core/sink/sink_stream.cpp index 084ac5edfe..f99dbd8ec7 100644 --- a/src/audio_core/sink/sink_stream.cpp +++ b/src/audio_core/sink/sink_stream.cpp @@ -151,10 +151,6 @@ void SinkStream::ProcessAudioIn(std::span input_buffer, std::size_t n return; } - if (queued_buffers > max_queue_size) { - Stall(); - } - while (frames_written < num_frames) { // If the playing buffer has been consumed or has no frames, we need a new one if (playing_buffer.consumed || playing_buffer.frames == 0) { @@ -189,10 +185,6 @@ void SinkStream::ProcessAudioIn(std::span input_buffer, std::size_t n } std::memcpy(&last_frame[0], &input_buffer[(frames_written - 1) * frame_size], frame_size_bytes); - - if (queued_buffers <= max_queue_size) { - Unstall(); - } } void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::size_t num_frames) { @@ -216,20 +208,6 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz return; } - // Due to many frames being queued up with nvdec (5 frames or so?), a lot of buffers also get - // queued up (30+) but not all at once, which causes constant stalling here, so just let the - // video play out without attempting to stall. - // Can hopefully remove this later with a more complete NVDEC implementation. - const auto nvdec_active{system.AudioCore().IsNVDECActive()}; - - // Core timing cannot be paused in single-core mode, so Stall ends up being called over and over - // and never recovers to a normal state, so just skip attempting to sync things on single-core. - if (system.IsMulticore() && !nvdec_active && queued_buffers > max_queue_size) { - Stall(); - } else if (system.IsMulticore() && queued_buffers <= max_queue_size) { - Unstall(); - } - while (frames_written < num_frames) { // If the playing buffer has been consumed or has no frames, we need a new one if (playing_buffer.consumed || playing_buffer.frames == 0) { @@ -279,27 +257,6 @@ void SinkStream::ProcessAudioOutAndRender(std::span output_buffer, std::siz min_played_sample_count = max_played_sample_count; max_played_sample_count += actual_frames_written; } - - if (system.IsMulticore() && queued_buffers <= max_queue_size) { - Unstall(); - } -} - -void SinkStream::Stall() { - std::scoped_lock lk{stall_guard}; - if (stalled_lock) { - return; - } - stalled_lock = system.StallApplication(); -} - -void SinkStream::Unstall() { - std::scoped_lock lk{stall_guard}; - if (!stalled_lock) { - return; - } - system.UnstallApplication(); - stalled_lock.unlock(); } u64 SinkStream::GetExpectedPlayedSampleCount() { diff --git a/src/audio_core/sink/sink_stream.h b/src/audio_core/sink/sink_stream.h index 9806e6d986..23e289c7bd 100644 --- a/src/audio_core/sink/sink_stream.h +++ b/src/audio_core/sink/sink_stream.h @@ -55,9 +55,7 @@ struct SinkBuffer { class SinkStream { public: explicit SinkStream(Core::System& system_, StreamType type_) : system{system_}, type{type_} {} - virtual ~SinkStream() { - Unstall(); - } + virtual ~SinkStream() {} /** * Finalize the sink stream. @@ -202,16 +200,6 @@ public: */ void ProcessAudioOutAndRender(std::span output_buffer, std::size_t num_frames); - /** - * Stall core processes if the audio thread falls too far behind. - */ - void Stall(); - - /** - * Unstall core processes. - */ - void Unstall(); - /** * Get the total number of samples expected to have been played by this stream. * @@ -266,8 +254,6 @@ private: /// Signalled when ring buffer entries are consumed std::condition_variable release_cv; std::mutex release_mutex; - std::mutex stall_guard; - std::unique_lock stalled_lock; }; using SinkStreamPtr = std::unique_ptr;