From bb156f81335972204200d51c32915870165c4caa Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Wed, 2 Aug 2023 19:05:47 -0500 Subject: [PATCH] LibAudio: Add a Serenity implementation of PlaybackStream This implementation is very naive compared to the PulseAudio one. Instead of using a callback implemented by the audio server connection to push audio to the buffer, we have to poll on a timer to check when we need to push the audio buffers. Implementing cross-process condition variables into the audio queue class could allow us to avoid polling, which may prove beneficial to CPU usage. Audio timestamps will be accurate to the number of samples available, but will count in increments of about 100ms and run ahead of the actual audio being pushed to the device by the server. Buffer underruns are completely ignored for now as well, since the `AudioServer` has no way to know how many samples are actually written in a single audio buffer. --- Tests/LibAudio/TestPlaybackStream.cpp | 18 ++- Userland/Libraries/LibAudio/CMakeLists.txt | 1 + .../Libraries/LibAudio/ConnectionToServer.cpp | 5 + .../Libraries/LibAudio/ConnectionToServer.h | 2 + .../Libraries/LibAudio/PlaybackStream.cpp | 9 +- .../LibAudio/PlaybackStreamSerenity.cpp | 112 ++++++++++++++++++ .../LibAudio/PlaybackStreamSerenity.h | 41 +++++++ 7 files changed, 185 insertions(+), 3 deletions(-) create mode 100644 Userland/Libraries/LibAudio/PlaybackStreamSerenity.cpp create mode 100644 Userland/Libraries/LibAudio/PlaybackStreamSerenity.h diff --git a/Tests/LibAudio/TestPlaybackStream.cpp b/Tests/LibAudio/TestPlaybackStream.cpp index 246c22a87f..27fcd1cf78 100644 --- a/Tests/LibAudio/TestPlaybackStream.cpp +++ b/Tests/LibAudio/TestPlaybackStream.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -15,10 +16,23 @@ # include #endif -TEST_CASE(create_and_destroy_playback_stream) +// FIXME: CI doesn't run an AudioServer currently. Creating one in /etc/SystemServer.ini does not +// allow this test to pass since CI runs in a Shell that will setsid() if it finds that the +// current session ID is 0, and AudioServer's socket address depends on the current sid. +// If we can fix that, this test can run on CI. +// https://github.com/SerenityOS/serenity/issues/20538 +#if defined(AK_OS_SERENITY) +# define STREAM_TEST BENCHMARK_CASE +#else +# define STREAM_TEST TEST_CASE +#endif + +STREAM_TEST(create_and_destroy_playback_stream) { + Core::EventLoop event_loop; + bool has_implementation = false; -#if defined(HAVE_PULSEAUDIO) +#if defined(AK_OS_SERENITY) || defined(HAVE_PULSEAUDIO) has_implementation = true; #endif diff --git a/Userland/Libraries/LibAudio/CMakeLists.txt b/Userland/Libraries/LibAudio/CMakeLists.txt index 40e8fdb4e5..600f4e0d83 100644 --- a/Userland/Libraries/LibAudio/CMakeLists.txt +++ b/Userland/Libraries/LibAudio/CMakeLists.txt @@ -19,6 +19,7 @@ set(SOURCES if (SERENITYOS) list(APPEND SOURCES ConnectionToServer.cpp) list(APPEND SOURCES ConnectionToManagerServer.cpp) + list(APPEND SOURCES PlaybackStreamSerenity.cpp) set(GENERATED_SOURCES ../../Services/AudioServer/AudioClientEndpoint.h ../../Services/AudioServer/AudioServerEndpoint.h diff --git a/Userland/Libraries/LibAudio/ConnectionToServer.cpp b/Userland/Libraries/LibAudio/ConnectionToServer.cpp index 520d2fb0b4..1634db6233 100644 --- a/Userland/Libraries/LibAudio/ConnectionToServer.cpp +++ b/Userland/Libraries/LibAudio/ConnectionToServer.cpp @@ -150,6 +150,11 @@ size_t ConnectionToServer::remaining_buffers() const return m_buffer->size() - m_buffer->weak_remaining_capacity(); } +bool ConnectionToServer::can_enqueue() const +{ + return m_buffer->can_enqueue(); +} + void ConnectionToServer::client_volume_changed(double volume) { if (on_client_volume_change) diff --git a/Userland/Libraries/LibAudio/ConnectionToServer.h b/Userland/Libraries/LibAudio/ConnectionToServer.h index d35a6078d7..133620ab94 100644 --- a/Userland/Libraries/LibAudio/ConnectionToServer.h +++ b/Userland/Libraries/LibAudio/ConnectionToServer.h @@ -54,6 +54,8 @@ public: // How many buffers (i.e. short sample arrays) the server hasn't played yet. // Non-realtime code needn't worry about this. size_t remaining_buffers() const; + // Whether there is room in the realtime audio queue for another sample buffer. + bool can_enqueue() const; void set_self_sample_rate(u32 sample_rate); diff --git a/Userland/Libraries/LibAudio/PlaybackStream.cpp b/Userland/Libraries/LibAudio/PlaybackStream.cpp index 8811fbe2eb..5f60d92492 100644 --- a/Userland/Libraries/LibAudio/PlaybackStream.cpp +++ b/Userland/Libraries/LibAudio/PlaybackStream.cpp @@ -6,8 +6,13 @@ #include "PlaybackStream.h" +#include #include +#if defined(AK_OS_SERENITY) +# include +#endif + #if defined(HAVE_PULSEAUDIO) # include #endif @@ -28,7 +33,9 @@ ErrorOr> PlaybackStream::create(OutputState initia { VERIFY(data_request_callback); // Create the platform-specific implementation for this stream. -#if defined(HAVE_PULSEAUDIO) +#if defined(AK_OS_SERENITY) + return PlaybackStreamSerenity::create(initial_output_state, sample_rate, channels, target_latency_ms, move(data_request_callback)); +#elif defined(HAVE_PULSEAUDIO) return PlaybackStreamPulseAudio::create(initial_output_state, sample_rate, channels, target_latency_ms, move(data_request_callback)); #else (void)initial_output_state, (void)sample_rate, (void)channels, (void)target_latency_ms; diff --git a/Userland/Libraries/LibAudio/PlaybackStreamSerenity.cpp b/Userland/Libraries/LibAudio/PlaybackStreamSerenity.cpp new file mode 100644 index 0000000000..7cc3bec215 --- /dev/null +++ b/Userland/Libraries/LibAudio/PlaybackStreamSerenity.cpp @@ -0,0 +1,112 @@ +/* + * Copyright (c) 2023, Gregory Bertilson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "PlaybackStreamSerenity.h" + +#include + +namespace Audio { + +ErrorOr> PlaybackStreamSerenity::create(OutputState initial_state, u32 sample_rate, [[maybe_unused]] u8 channels, [[maybe_unused]] u32 target_latency_ms, AudioDataRequestCallback&& data_request_callback) +{ + VERIFY(data_request_callback); + auto connection = TRY(ConnectionToServer::try_create()); + if (auto result = connection->try_set_self_sample_rate(sample_rate); result.is_error()) + return Error::from_string_literal("Failed to set sample rate"); + + auto polling_timer = TRY(Core::Timer::try_create()); + auto implementation = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) PlaybackStreamSerenity(connection, move(polling_timer), move(data_request_callback)))); + if (initial_state == OutputState::Playing) + connection->async_start_playback(); + return implementation; +} + +PlaybackStreamSerenity::PlaybackStreamSerenity(NonnullRefPtr stream, NonnullRefPtr polling_timer, AudioDataRequestCallback&& data_request_callback) + : m_connection(move(stream)) + , m_polling_timer(move(polling_timer)) + , m_data_request_callback(move(data_request_callback)) +{ + // Ensure that our audio buffers are filled when they are more than 3/4 empty. + // FIXME: Add an event to ConnectionToServer track the sample rate and update this interval, or + // implement the data request into ConnectionToServer so each client doesn't need to poll + // on a timer with an arbitrary interval. + m_polling_timer->set_interval(static_cast((AUDIO_BUFFERS_COUNT * 3 / 4) * AUDIO_BUFFER_SIZE * 1000 / m_connection->get_self_sample_rate())); + m_polling_timer->on_timeout = [this]() { + fill_buffers(); + }; + m_polling_timer->start(); +} + +void PlaybackStreamSerenity::fill_buffers() +{ + while (m_connection->can_enqueue()) { + Array buffer; + buffer.fill({ 0.0f, 0.0f }); + auto written_data = m_data_request_callback(Bytes { reinterpret_cast(buffer.data()), sizeof(buffer) }, PcmSampleFormat::Float32, AUDIO_BUFFER_SIZE); + // FIXME: The buffer we are enqueuing here is a fixed size, meaning that the server will not be + // aware of exactly how many samples we have written here. We should allow the server to + // consume sized buffers to allow us to obtain sample-accurate timing information even + // when we run out of samples on a sample count that is not a multiple of AUDIO_BUFFER_SIZE. + m_number_of_samples_enqueued += written_data.size() / sizeof(Sample); + MUST(m_connection->realtime_enqueue(buffer)); + } +} + +void PlaybackStreamSerenity::set_underrun_callback(Function callback) +{ + // FIXME: Implement underrun callback in AudioServer + (void)callback; +} + +NonnullRefPtr> PlaybackStreamSerenity::resume() +{ + auto promise = Core::ThreadedPromise::create(); + // FIXME: We need to get the time played at the correct time from the server. If a message to + // start playback is sent while there is any other message being processed, this may end + // up being inaccurate. + auto time = MUST(total_time_played()); + fill_buffers(); + m_connection->async_start_playback(); + m_polling_timer->start(); + promise->resolve(move(time)); + return promise; +} + +NonnullRefPtr> PlaybackStreamSerenity::drain_buffer_and_suspend() +{ + // FIXME: Play back all samples on the server before pausing. This can be achieved by stopping + // enqueuing samples and receiving a message that a buffer underrun has occurred. + auto promise = Core::ThreadedPromise::create(); + m_connection->async_pause_playback(); + m_polling_timer->stop(); + promise->resolve(); + return promise; +} + +NonnullRefPtr> PlaybackStreamSerenity::discard_buffer_and_suspend() +{ + auto promise = Core::ThreadedPromise::create(); + m_connection->async_clear_buffer(); + m_connection->async_pause_playback(); + m_polling_timer->stop(); + promise->resolve(); + return promise; +} + +ErrorOr PlaybackStreamSerenity::total_time_played() +{ + return Duration::from_milliseconds(m_number_of_samples_enqueued * 1000 / m_connection->get_self_sample_rate()); +} + +NonnullRefPtr> PlaybackStreamSerenity::set_volume(double volume) +{ + auto promise = Core::ThreadedPromise::create(); + m_connection->async_set_self_volume(volume); + promise->resolve(); + return promise; +} + +} diff --git a/Userland/Libraries/LibAudio/PlaybackStreamSerenity.h b/Userland/Libraries/LibAudio/PlaybackStreamSerenity.h new file mode 100644 index 0000000000..d8f65fb1c3 --- /dev/null +++ b/Userland/Libraries/LibAudio/PlaybackStreamSerenity.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2023, Gregory Bertilson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include + +namespace Audio { + +class PlaybackStreamSerenity final + : public PlaybackStream { +public: + static ErrorOr> create(OutputState initial_state, u32 sample_rate, u8 channels, u32 target_latency_ms, AudioDataRequestCallback&& data_request_callback); + + virtual void set_underrun_callback(Function) override; + + virtual NonnullRefPtr> resume() override; + virtual NonnullRefPtr> drain_buffer_and_suspend() override; + virtual NonnullRefPtr> discard_buffer_and_suspend() override; + + virtual ErrorOr total_time_played() override; + + virtual NonnullRefPtr> set_volume(double) override; + +private: + PlaybackStreamSerenity(NonnullRefPtr, NonnullRefPtr polling_timer, AudioDataRequestCallback&& data_request_callback); + + void fill_buffers(); + + NonnullRefPtr m_connection; + size_t m_number_of_samples_enqueued { 0 }; + NonnullRefPtr m_polling_timer; + AudioDataRequestCallback m_data_request_callback; + bool m_paused { false }; +}; + +}