From d905498fb6080ae8607c233d5c829513b5091bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= Date: Fri, 11 Aug 2023 15:57:31 +0200 Subject: [PATCH] AudioServer: Clean up ClientAudioStream APIs - Use Optional references instead of pointers - Clean up some const and nullability weirdness - Use proper error return value for get_next_sample --- .../AudioServer/ClientAudioStream.cpp | 30 ++++++++----------- .../Services/AudioServer/ClientAudioStream.h | 16 ++++++---- Userland/Services/AudioServer/Mixer.cpp | 8 +++-- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/Userland/Services/AudioServer/ClientAudioStream.cpp b/Userland/Services/AudioServer/ClientAudioStream.cpp index 040f2f9dd4..bd5e3a2939 100644 --- a/Userland/Services/AudioServer/ClientAudioStream.cpp +++ b/Userland/Services/AudioServer/ClientAudioStream.cpp @@ -15,9 +15,9 @@ ClientAudioStream::ClientAudioStream(ConnectionFromClient& client) { } -ConnectionFromClient* ClientAudioStream::client() +Optional ClientAudioStream::client() { - return m_client.ptr(); + return m_client.has_value() ? *m_client : Optional {}; } bool ClientAudioStream::is_connected() const @@ -25,14 +25,14 @@ bool ClientAudioStream::is_connected() const return m_client && m_client->is_open(); } -bool ClientAudioStream::get_next_sample(Audio::Sample& sample, u32 audiodevice_sample_rate) +ErrorOr ClientAudioStream::get_next_sample(u32 audiodevice_sample_rate) { // Note: Even though we only check client state here, we will probably close the client much earlier. if (!is_connected()) - return false; + return ErrorState::ClientDisconnected; if (m_paused) - return false; + return ErrorState::ClientUnderrun; if (m_in_chunk_location >= m_current_audio_chunk.size()) { auto result = m_buffer->dequeue(); @@ -41,30 +41,26 @@ bool ClientAudioStream::get_next_sample(Audio::Sample& sample, u32 audiodevice_s dbgln_if(AUDIO_DEBUG, "Audio client {} can't keep up!", m_client->client_id()); } - return false; + return ErrorState::ClientUnderrun; } // FIXME: Our resampler and the way we resample here are bad. // Ideally, we should both do perfect band-corrected resampling, // as well as carry resampling state over between buffers. - auto attempted_resample = Audio::ResampleHelper { - m_sample_rate == 0 ? audiodevice_sample_rate : m_sample_rate, audiodevice_sample_rate - } - .try_resample(result.release_value()); - if (attempted_resample.is_error()) - return false; + auto maybe_resampled = Audio::ResampleHelper { m_sample_rate == 0 ? audiodevice_sample_rate : m_sample_rate, audiodevice_sample_rate } + .try_resample(result.release_value()); + if (maybe_resampled.is_error()) + return ErrorState::ResamplingError; // If the sample rate changes underneath us, we will still play the existing buffer unchanged until we're done. // This is not a significant problem since the buffers are very small (~100 samples or less). - m_current_audio_chunk = attempted_resample.release_value(); + m_current_audio_chunk = maybe_resampled.release_value(); m_in_chunk_location = 0; } - sample = m_current_audio_chunk[m_in_chunk_location++]; - - return true; + return m_current_audio_chunk[m_in_chunk_location++]; } -void ClientAudioStream::set_buffer(OwnPtr buffer) +void ClientAudioStream::set_buffer(NonnullOwnPtr buffer) { m_buffer = move(buffer); } diff --git a/Userland/Services/AudioServer/ClientAudioStream.h b/Userland/Services/AudioServer/ClientAudioStream.h index 06e61f2d3e..0de7a625e1 100644 --- a/Userland/Services/AudioServer/ClientAudioStream.h +++ b/Userland/Services/AudioServer/ClientAudioStream.h @@ -10,7 +10,6 @@ #include "ConnectionFromClient.h" #include "FadingProperty.h" #include -#include #include #include #include @@ -20,22 +19,29 @@ namespace AudioServer { class ClientAudioStream : public RefCounted { public: + enum class ErrorState { + ClientDisconnected, + ClientPaused, + ClientUnderrun, + ResamplingError, + }; + explicit ClientAudioStream(ConnectionFromClient&); ~ClientAudioStream() = default; - bool get_next_sample(Audio::Sample& sample, u32 audiodevice_sample_rate); + ErrorOr get_next_sample(u32 audiodevice_sample_rate); void clear(); bool is_connected() const; - ConnectionFromClient* client(); + Optional client(); - void set_buffer(OwnPtr buffer); + void set_buffer(NonnullOwnPtr buffer); void set_paused(bool paused); FadingProperty& volume(); double volume() const; - void set_volume(double const volume); + void set_volume(double volume); bool is_muted() const; void set_muted(bool muted); u32 sample_rate() const; diff --git a/Userland/Services/AudioServer/Mixer.cpp b/Userland/Services/AudioServer/Mixer.cpp index 79a3d10ad6..f9bddc7680 100644 --- a/Userland/Services/AudioServer/Mixer.cpp +++ b/Userland/Services/AudioServer/Mixer.cpp @@ -73,18 +73,20 @@ void Mixer::mix() // Mix the buffers together into the output for (auto& queue : active_mix_queues) { - if (!queue->client()) { + if (!queue->client().has_value()) { queue->clear(); continue; } queue->volume().advance_time(); + // FIXME: Perform sample extraction and mixing in two separate loops so they can be more easily vectorized. for (auto& mixed_sample : mixed_buffer) { - Audio::Sample sample; - if (!queue->get_next_sample(sample, audiodevice_get_sample_rate())) + auto sample_or_error = queue->get_next_sample(audiodevice_get_sample_rate()); + if (sample_or_error.is_error()) break; if (queue->is_muted()) continue; + auto sample = sample_or_error.release_value(); sample.log_multiply(SAMPLE_HEADROOM); sample.log_multiply(static_cast(queue->volume())); mixed_sample += sample;