LibAudio: Fixed stuttery playback of audio

When playing an ABuffer, the count of samples were determined by the
size of the SharedBuffer. This caused small pauses of up to 512
samples during the playback, when the size of the shared buffer was
rounded up to a multiple of 4096. This problem was amplified by the
fact that the AResampleHelper was created every time a new chunk of
audio was to be processed, causing inconsistencies in the playback of
wav files.

Libraries/LibAudio/ABuffer.h

@@ -1,6 +1,5 @@
 #pragma once
 
-#include <AK/RefCounted.h>
 #include <AK/ByteBuffer.h>
 #include <AK/Types.h>
 #include <AK/Vector.h>
@@ -12,19 +11,22 @@ struct ASample {
     ASample()
         : left(0)
         , right(0)
-    {}
+    {
+    }
 
     // For mono
     ASample(float left)
         : left(left)
         , right(left)
-    {}
+    {
+    }
 
     // For stereo
     ASample(float left, float right)
         : left(left)
         , right(right)
-    {}
+    {
+    }
 
     void clip()
     {
@@ -57,38 +59,57 @@ struct ASample {
     float right;
 };
 
+// Small helper to resample from one playback rate to another
+// This isn't really "smart", in that we just insert (or drop) samples.
+// Should do better...
+class AResampleHelper {
+public:
+    AResampleHelper(float source, float target);
+
+    void process_sample(float sample_l, float sample_r);
+    bool read_sample(float& next_l, float& next_r);
+
+private:
+    const float m_ratio;
+    float m_current_ratio { 0 };
+    float m_last_sample_l { 0 };
+    float m_last_sample_r { 0 };
+};
+
 // A buffer of audio samples, normalized to 44100hz.
 class ABuffer : public RefCounted<ABuffer> {
 public:
-    static RefPtr<ABuffer> from_pcm_data(ByteBuffer& data, int num_channels, int bits_per_sample, int source_rate);
+    static RefPtr<ABuffer> from_pcm_data(ByteBuffer& data, AResampleHelper& resampler, int num_channels, int bits_per_sample);
     static NonnullRefPtr<ABuffer> create_with_samples(Vector<ASample>&& samples)
     {
         return adopt(*new ABuffer(move(samples)));
     }
-    static NonnullRefPtr<ABuffer> create_with_shared_buffer(NonnullRefPtr<SharedBuffer>&& buffer)
+    static NonnullRefPtr<ABuffer> create_with_shared_buffer(NonnullRefPtr<SharedBuffer>&& buffer, int sample_count)
     {
-        return adopt(*new ABuffer(move(buffer)));
+        return adopt(*new ABuffer(move(buffer), sample_count));
     }
 
     const ASample* samples() const { return (const ASample*)data(); }
-    int sample_count() const { return m_buffer->size() / (int)sizeof(ASample); }
+    int sample_count() const { return m_sample_count; }
     const void* data() const { return m_buffer->data(); }
-    int size_in_bytes() const { return m_buffer->size(); }
+    int size_in_bytes() const { return m_sample_count * (int)sizeof(ASample); }
     int shared_buffer_id() const { return m_buffer->shared_buffer_id(); }
     SharedBuffer& shared_buffer() { return *m_buffer; }
 
 private:
     explicit ABuffer(Vector<ASample>&& samples)
-        : m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(ASample)))
+        : m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(ASample))),
+        m_sample_count(samples.size())
     {
         memcpy(m_buffer->data(), samples.data(), samples.size() * sizeof(ASample));
     }
 
-    explicit ABuffer(NonnullRefPtr<SharedBuffer>&& buffer)
-        : m_buffer(move(buffer))
+    explicit ABuffer(NonnullRefPtr<SharedBuffer>&& buffer, int sample_count)
+        : m_buffer(move(buffer)),
+        m_sample_count(sample_count)
     {
     }
 
     NonnullRefPtr<SharedBuffer> m_buffer;
-    int m_sample_count { 0 };
+    const int m_sample_count;
 };

Libraries/LibAudio/AClientConnection.cpp

@@ -18,7 +18,7 @@ void AClientConnection::enqueue(const ABuffer& buffer)
 {
     for (;;) {
         const_cast<ABuffer&>(buffer).shared_buffer().share_with(server_pid());
-        auto response = send_sync<AudioServer::EnqueueBuffer>(buffer.shared_buffer_id());
+        auto response = send_sync<AudioServer::EnqueueBuffer>(buffer.shared_buffer_id(), buffer.sample_count());
         if (response->success())
             break;
         sleep(1);
@@ -28,7 +28,7 @@ void AClientConnection::enqueue(const ABuffer& buffer)
 bool AClientConnection::try_enqueue(const ABuffer& buffer)
 {
     const_cast<ABuffer&>(buffer).shared_buffer().share_with(server_pid());
-    auto response = send_sync<AudioServer::EnqueueBuffer>(buffer.shared_buffer_id());
+    auto response = send_sync<AudioServer::EnqueueBuffer>(buffer.shared_buffer_id(), buffer.sample_count());
     return response->success();
 }
 

Libraries/LibAudio/AWavLoader.cpp

@@ -1,5 +1,4 @@
 #include <AK/BufferStream.h>
-#include <LibAudio/ABuffer.h>
 #include <LibAudio/AWavLoader.h>
 #include <LibCore/CFile.h>
 #include <LibCore/CIODeviceStreamReader.h>
@@ -14,6 +13,7 @@ AWavLoader::AWavLoader(const StringView& path)
     }
 
     parse_header();
+    m_resampler = make<AResampleHelper>(m_sample_rate, 44100);
 }
 
 RefPtr<ABuffer> AWavLoader::get_more_samples(size_t max_bytes_to_read_from_input)
@@ -26,7 +26,7 @@ RefPtr<ABuffer> AWavLoader::get_more_samples(size_t max_bytes_to_read_from_input
     if (raw_samples.is_empty())
         return nullptr;
 
-    auto buffer = ABuffer::from_pcm_data(raw_samples, m_num_channels, m_bits_per_sample, m_sample_rate);
+    auto buffer = ABuffer::from_pcm_data(raw_samples, *m_resampler, m_num_channels, m_bits_per_sample);
     m_loaded_samples += buffer->sample_count();
     return buffer;
 }
@@ -80,7 +80,7 @@ bool AWavLoader::parse_header()
 
     u16 audio_format;
     stream >> audio_format;
-    CHECK_OK("Audio format");     // incomplete read check
+    CHECK_OK("Audio format"); // incomplete read check
     ok = ok && audio_format == 1; // WAVE_FORMAT_PCM
     ASSERT(audio_format == 1);
     CHECK_OK("Audio format"); // value check
@@ -137,64 +137,62 @@ bool AWavLoader::parse_header()
     return true;
 }
 
-// Small helper to resample from one playback rate to another
-// This isn't really "smart", in that we just insert (or drop) samples.
-// Should do better...
-class AResampleHelper {
-public:
-    AResampleHelper(float source, float target);
-    bool read_sample();
-    void prepare();
-
-private:
-    const float m_ratio;
-    float m_current_ratio { 0 };
-};
-
 AResampleHelper::AResampleHelper(float source, float target)
     : m_ratio(source / target)
 {
 }
 
-void AResampleHelper::prepare()
+void AResampleHelper::process_sample(float sample_l, float sample_r)
 {
-    m_current_ratio += m_ratio;
+    m_last_sample_l = sample_l;
+    m_last_sample_r = sample_r;
+    m_current_ratio += 1;
 }
 
-bool AResampleHelper::read_sample()
+bool AResampleHelper::read_sample(float& next_l, float& next_r)
 {
-    if (m_current_ratio > 1) {
-        m_current_ratio--;
+    if (m_current_ratio > 0) {
+        m_current_ratio -= m_ratio;
+        next_l = m_last_sample_l;
+        next_r = m_last_sample_r;
         return true;
     }
 
     return false;
 }
 
-template<typename SampleReader>
-static void read_samples_from_stream(BufferStream& stream, SampleReader read_sample, Vector<ASample>& samples, int num_channels, int source_rate)
+template <typename SampleReader>
+static void read_samples_from_stream(BufferStream& stream, SampleReader read_sample, Vector<ASample>& samples, AResampleHelper& resampler, int num_channels)
 {
-    AResampleHelper resampler(source_rate, 44100);
     float norm_l = 0;
     float norm_r = 0;
+
     switch (num_channels) {
     case 1:
-        while (!stream.handle_read_failure()) {
-            resampler.prepare();
-            while (resampler.read_sample()) {
-                norm_l = read_sample(stream);
+        for(;;) {
+            while (resampler.read_sample(norm_l, norm_r)) {
+                samples.append(ASample(norm_l));
             }
-            samples.append(ASample(norm_l));
+            norm_l = read_sample(stream);
+
+            if (stream.handle_read_failure()) {
+                break;
+            }
+            resampler.process_sample(norm_l, norm_r);
         }
         break;
     case 2:
-        while (!stream.handle_read_failure()) {
-            resampler.prepare();
-            while (resampler.read_sample()) {
-                norm_l = read_sample(stream);
-                norm_r = read_sample(stream);
+        for(;;) {
+            while (resampler.read_sample(norm_l, norm_r)) {
+                samples.append(ASample(norm_l, norm_r));
             }
-            samples.append(ASample(norm_l, norm_r));
+            norm_l = read_sample(stream);
+            norm_r = read_sample(stream);
+
+            if (stream.handle_read_failure()) {
+                break;
+            }
+            resampler.process_sample(norm_l, norm_r);
         }
         break;
     default:
@@ -236,7 +234,7 @@ static float read_norm_sample_8(BufferStream& stream)
 // ### can't const this because BufferStream is non-const
 // perhaps we need a reading class separate from the writing one, that can be
 // entirely consted.
-RefPtr<ABuffer> ABuffer::from_pcm_data(ByteBuffer& data, int num_channels, int bits_per_sample, int source_rate)
+RefPtr<ABuffer> ABuffer::from_pcm_data(ByteBuffer& data, AResampleHelper& resampler, int num_channels, int bits_per_sample)
 {
     BufferStream stream(data);
     Vector<ASample> fdata;
@@ -248,13 +246,13 @@ RefPtr<ABuffer> ABuffer::from_pcm_data(ByteBuffer& data, int num_channels, int b
 
     switch (bits_per_sample) {
     case 8:
-        read_samples_from_stream(stream, read_norm_sample_8, fdata, num_channels, source_rate);
+        read_samples_from_stream(stream, read_norm_sample_8, fdata, resampler, num_channels);
         break;
     case 16:
-        read_samples_from_stream(stream, read_norm_sample_16, fdata, num_channels, source_rate);
+        read_samples_from_stream(stream, read_norm_sample_16, fdata, resampler, num_channels);
         break;
     case 24:
-        read_samples_from_stream(stream, read_norm_sample_24, fdata, num_channels, source_rate);
+        read_samples_from_stream(stream, read_norm_sample_24, fdata, resampler, num_channels);
         break;
     default:
         ASSERT_NOT_REACHED();
@@ -265,10 +263,5 @@ RefPtr<ABuffer> ABuffer::from_pcm_data(ByteBuffer& data, int num_channels, int b
     // don't belong.
     ASSERT(!stream.handle_read_failure());
 
-    // HACK: This is a total hack to remove an unnecessary sample at the end of the buffer.
-    // FIXME: Don't generate the extra sample... :^)
-    for (int i = 0; i < 1; ++i)
-        fdata.take_last();
-
     return ABuffer::create_with_samples(move(fdata));
 }

Libraries/LibAudio/AWavLoader.h

@@ -4,6 +4,7 @@
 #include <AK/RefPtr.h>
 #include <AK/StringView.h>
 #include <LibCore/CFile.h>
+#include <LibAudio/ABuffer.h>
 
 class ABuffer;
 
@@ -31,6 +32,7 @@ private:
     bool parse_header();
     RefPtr<CFile> m_file;
     String m_error_string;
+    OwnPtr<AResampleHelper> m_resampler;
 
     u32 m_sample_rate { 0 };
     u16 m_num_channels { 0 };

Servers/AudioServer/ASClientConnection.cpp

@@ -68,6 +68,6 @@ OwnPtr<AudioServer::EnqueueBufferResponse> ASClientConnection::handle(const Audi
     if (m_queue->is_full())
         return make<AudioServer::EnqueueBufferResponse>(false);
 
-    m_queue->enqueue(ABuffer::create_with_shared_buffer(*shared_buffer));
+    m_queue->enqueue(ABuffer::create_with_shared_buffer(*shared_buffer, message.sample_count()));
     return make<AudioServer::EnqueueBufferResponse>(true);
 }

Servers/AudioServer/AudioServer.ipc

@@ -8,5 +8,5 @@ endpoint AudioServer
     SetMainMixVolume(i32 volume) => ()
 
     // Buffer playback
-    EnqueueBuffer(i32 buffer_id) => (bool success)
+    EnqueueBuffer(i32 buffer_id, int sample_count) => (bool success)
 }