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

Userland/Services/AudioServer/ClientAudioStream.cpp

@@ -15,9 +15,9 @@ ClientAudioStream::ClientAudioStream(ConnectionFromClient& client)
 {
 }
 
-ConnectionFromClient* ClientAudioStream::client()
+Optional<ConnectionFromClient&> ClientAudioStream::client()
 {
-    return m_client.ptr();
+    return m_client.has_value() ? *m_client : Optional<ConnectionFromClient&> {};
 }
 
 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<Audio::Sample, ClientAudioStream::ErrorState> 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<Audio::Sample> {
-            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<Audio::Sample> { 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<Audio::AudioQueue> buffer)
+void ClientAudioStream::set_buffer(NonnullOwnPtr<Audio::AudioQueue> buffer)
 {
     m_buffer = move(buffer);
 }

Userland/Services/AudioServer/ClientAudioStream.h

@@ -10,7 +10,6 @@
 #include "ConnectionFromClient.h"
 #include "FadingProperty.h"
 #include <AK/Atomic.h>
-#include <AK/Badge.h>
 #include <AK/Debug.h>
 #include <AK/RefCounted.h>
 #include <AK/WeakPtr.h>
@@ -20,22 +19,29 @@ namespace AudioServer {
 
 class ClientAudioStream : public RefCounted<ClientAudioStream> {
 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<Audio::Sample, ErrorState> get_next_sample(u32 audiodevice_sample_rate);
     void clear();
 
     bool is_connected() const;
 
-    ConnectionFromClient* client();
+    Optional<ConnectionFromClient&> client();
 
-    void set_buffer(OwnPtr<Audio::AudioQueue> buffer);
+    void set_buffer(NonnullOwnPtr<Audio::AudioQueue> buffer);
 
     void set_paused(bool paused);
     FadingProperty<double>& 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;

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<float>(queue->volume()));
                 mixed_sample += sample;