From 5e01dde7b1a7ea8723e127c365331f10a9695546 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 27 Jul 2019 18:17:17 +0200 Subject: [PATCH] Audio: Make ABuffer sit on top of a SharedBuffer. This allows us to carry the same buffer all the way from the WAV loader to the AudioServer mixer. This alleviates some of the stutter, but there's still a noticeable skip when switching buffers. We're gonna need to do better. :^) --- Libraries/LibAudio/ABuffer.h | 27 ++++++++++++++++------ Libraries/LibAudio/AWavLoader.cpp | 2 +- Servers/AudioServer/ASClientConnection.cpp | 15 ++++-------- Servers/AudioServer/ASMixer.cpp | 18 +++++++-------- Servers/AudioServer/ASMixer.h | 5 ++-- Userland/aplay.cpp | 2 +- 6 files changed, 38 insertions(+), 31 deletions(-) diff --git a/Libraries/LibAudio/ABuffer.h b/Libraries/LibAudio/ABuffer.h index 364c84d40a..4aa23520cb 100644 --- a/Libraries/LibAudio/ABuffer.h +++ b/Libraries/LibAudio/ABuffer.h @@ -4,6 +4,7 @@ #include #include #include +#include // A single sample in an audio buffer. // Values are floating point, and should range from -1.0 to +1.0 @@ -57,17 +58,29 @@ public: { return adopt(*new ABuffer(move(samples))); } + static NonnullRefPtr create_with_shared_buffer(NonnullRefPtr&& buffer) + { + return adopt(*new ABuffer(move(buffer))); + } - const Vector& samples() const { return m_samples; } - Vector& samples() { return m_samples; } - const void* data() const { return m_samples.data(); } - int size_in_bytes() const { return m_samples.size() * sizeof(ASample); } + const ASample* samples() const { return (const ASample*)data(); } + int sample_count() const { return m_buffer->size() / (int)sizeof(ASample); } + const void* data() const { return m_buffer->data(); } + int size_in_bytes() const { return m_buffer->size(); } + int shared_buffer_id() const { return m_buffer->shared_buffer_id(); } private: - ABuffer(Vector&& samples) - : m_samples(move(samples)) + explicit ABuffer(Vector&& samples) + : m_buffer(*SharedBuffer::create_with_size(samples.size() * sizeof(ASample))) + { + memcpy(m_buffer->data(), samples.data(), samples.size() * sizeof(ASample)); + } + + explicit ABuffer(NonnullRefPtr&& buffer) + : m_buffer(move(buffer)) { } - Vector m_samples; + NonnullRefPtr m_buffer; + int m_sample_count { 0 }; }; diff --git a/Libraries/LibAudio/AWavLoader.cpp b/Libraries/LibAudio/AWavLoader.cpp index f5ff5d7cc8..afe2b9ffba 100644 --- a/Libraries/LibAudio/AWavLoader.cpp +++ b/Libraries/LibAudio/AWavLoader.cpp @@ -254,5 +254,5 @@ RefPtr ABuffer::from_pcm_data(ByteBuffer& data, int num_channels, int b // don't belong. ASSERT(!stream.handle_read_failure()); - return adopt(*new ABuffer(move(fdata))); + return ABuffer::create_with_samples(move(fdata)); } diff --git a/Servers/AudioServer/ASClientConnection.cpp b/Servers/AudioServer/ASClientConnection.cpp index 91351e6210..e70eb91fa1 100644 --- a/Servers/AudioServer/ASClientConnection.cpp +++ b/Servers/AudioServer/ASClientConnection.cpp @@ -40,16 +40,11 @@ bool ASClientConnection::handle_message(const ASAPI_ClientMessage& message, cons break; case ASAPI_ClientMessage::Type::PlayBuffer: { // ### ensure that the size is that of a Vector - Vector samples; - { - const auto& shared_buf = SharedBuffer::create_from_shared_buffer_id(message.play_buffer.buffer_id); - if (!shared_buf) { - did_misbehave(); - return false; - } - samples.resize(shared_buf->size() / sizeof(ASample)); - memcpy(samples.data(), shared_buf->data(), shared_buf->size()); + auto shared_buffer = SharedBuffer::create_from_shared_buffer_id(message.play_buffer.buffer_id); + if (!shared_buffer) { + did_misbehave(); + return false; } // we no longer need the buffer, so acknowledge that it's playing @@ -59,7 +54,7 @@ bool ASClientConnection::handle_message(const ASAPI_ClientMessage& message, cons reply.playing_buffer.buffer_id = message.play_buffer.buffer_id; post_message(reply); - m_mixer.queue(*this, ABuffer::create_with_samples(move(samples)), message.play_buffer.buffer_id); + m_mixer.queue(*this, ABuffer::create_with_shared_buffer(*shared_buffer)); break; } case ASAPI_ClientMessage::Type::Invalid: diff --git a/Servers/AudioServer/ASMixer.cpp b/Servers/AudioServer/ASMixer.cpp index 81d3180016..e4b618adc0 100644 --- a/Servers/AudioServer/ASMixer.cpp +++ b/Servers/AudioServer/ASMixer.cpp @@ -20,11 +20,11 @@ ASMixer::ASMixer() }, this); } -void ASMixer::queue(ASClientConnection& client, const ABuffer& buffer, int buffer_id) +void ASMixer::queue(ASClientConnection& client, const ABuffer& buffer) { ASSERT(buffer.size_in_bytes()); CLocker lock(m_lock); - m_pending_mixing.append(ASMixerBuffer(buffer, client, buffer_id)); + m_pending_mixing.append(ASMixerBuffer(buffer, client)); } void ASMixer::mix() @@ -68,19 +68,20 @@ void ASMixer::mix() for (auto& buffer : active_mix_buffers) { if (buffer.done) continue; - auto& samples = buffer.buffer->samples(); + auto* samples = buffer.buffer->samples(); + auto sample_count = buffer.buffer->sample_count(); - for (int i = 0; i < max_size && buffer.pos < samples.size(); ++buffer.pos, ++i) { + for (int i = 0; i < max_size && buffer.pos < sample_count; ++buffer.pos, ++i) { auto& mixed_sample = mixed_buffer[i]; mixed_sample += samples[buffer.pos]; } // clear it later - if (buffer.pos == samples.size()) { - if (buffer.m_buffer_id && buffer.m_client) { + if (buffer.pos == sample_count) { + if (buffer.m_client) { ASAPI_ServerMessage reply; reply.type = ASAPI_ServerMessage::Type::FinishedPlayingBuffer; - reply.playing_buffer.buffer_id = buffer.m_buffer_id; + reply.playing_buffer.buffer_id = buffer.buffer->shared_buffer_id(); buffer.m_client->post_message(reply); } buffer.done = true; @@ -118,9 +119,8 @@ void ASMixer::mix() } } -ASMixer::ASMixerBuffer::ASMixerBuffer(const NonnullRefPtr& buf, ASClientConnection& client, int buffer_id) +ASMixer::ASMixerBuffer::ASMixerBuffer(const NonnullRefPtr& buf, ASClientConnection& client) : buffer(buf) , m_client(client.make_weak_ptr()) - , m_buffer_id(buffer_id) { } diff --git a/Servers/AudioServer/ASMixer.h b/Servers/AudioServer/ASMixer.h index 8f39f42668..253f3dfc8e 100644 --- a/Servers/AudioServer/ASMixer.h +++ b/Servers/AudioServer/ASMixer.h @@ -14,16 +14,15 @@ class ASMixer : public RefCounted { public: ASMixer(); - void queue(ASClientConnection&, const ABuffer&, int buffer_id); + void queue(ASClientConnection&, const ABuffer&); private: struct ASMixerBuffer { - ASMixerBuffer(const NonnullRefPtr&, ASClientConnection&, int buffer_id = -1); + ASMixerBuffer(const NonnullRefPtr&, ASClientConnection&); NonnullRefPtr buffer; int pos { 0 }; bool done { false }; WeakPtr m_client; - int m_buffer_id { -1 }; }; Vector m_pending_mixing; diff --git a/Userland/aplay.cpp b/Userland/aplay.cpp index a86088c55c..158121534c 100644 --- a/Userland/aplay.cpp +++ b/Userland/aplay.cpp @@ -24,7 +24,7 @@ int main(int argc, char **argv) if (!samples) { break; } - printf("Playing %d sample(s)\n", samples->samples().size()); + printf("Playing %d sample(s)\n", samples->sample_count()); a_conn.play(*samples, true); }