From b4fbd30b70959b33bf93ac1e0c8c6dde1d61627f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= Date: Sat, 24 Jun 2023 13:42:06 +0200 Subject: [PATCH] AudioServer+Userland: Decouple client sample rates from device rate This change was a long time in the making ever since we obtained sample rate awareness in the system. Now, each client has its own sample rate, accessible via new IPC APIs, and the device sample rate is only accessible via the management interface. AudioServer takes care of resampling client streams into the device sample rate. Therefore, the main improvement introduced with this commit is full responsiveness to sample rate changes; all open audio programs will continue to play at correct speed with the audio resampled to the new device rate. The immediate benefits are manifold: - Gets rid of the legacy hardware sample rate IPC message in the non-managing client - Removes duplicate resampling and sample index rescaling code everywhere - Avoids potential sample index scaling bugs in SoundPlayer (which have happened many times before) and fixes a sample index scaling bug in aplay - Removes several FIXMEs - Reduces amount of sample copying in all applications (especially Piano, where this is critical), improving performance - Reduces number of resampling users, making future API changes (which will need to happen for correct resampling to be implemented) easier I also threw in a simple race condition fix for Piano's audio player loop. --- Base/usr/share/man/man1/asctl.md | 2 +- Ladybird/AudioCodecPluginLadybird.cpp | 6 ++--- .../Applications/Piano/AudioPlayerLoop.cpp | 18 ++++--------- Userland/Applications/Piano/AudioPlayerLoop.h | 2 -- Userland/Applications/Piano/Music.h | 4 +-- .../SoundPlayer/PlaybackManager.cpp | 12 ++------- .../SoundPlayer/PlaybackManager.h | 4 --- Userland/Applications/SoundPlayer/Player.cpp | 12 ++------- .../Libraries/LibAudio/ConnectionToServer.cpp | 12 +++++++-- .../Libraries/LibAudio/ConnectionToServer.h | 3 ++- .../LibWeb/Platform/AudioCodecPlugin.cpp | 16 +++++------ .../LibWeb/Platform/AudioCodecPlugin.h | 6 ++--- Userland/Services/AudioServer/AudioServer.ipc | 5 ++-- .../AudioServer/ConnectionFromClient.cpp | 19 ++++++++++--- .../AudioServer/ConnectionFromClient.h | 5 ++-- Userland/Services/AudioServer/Mixer.cpp | 10 ++++++- Userland/Services/AudioServer/Mixer.h | 27 ++++++++++++++++--- .../WebContent/AudioCodecPluginSerenity.cpp | 12 ++++----- .../WebContent/AudioCodecPluginSerenity.h | 2 -- Userland/Utilities/aplay.cpp | 16 +++-------- 20 files changed, 100 insertions(+), 93 deletions(-) diff --git a/Base/usr/share/man/man1/asctl.md b/Base/usr/share/man/man1/asctl.md index 9f0cecd3c2..43c1fe40fd 100644 --- a/Base/usr/share/man/man1/asctl.md +++ b/Base/usr/share/man/man1/asctl.md @@ -30,7 +30,7 @@ There are two commands available: `get` reports the state of audio variables, an The available variables are: * `(v)olume`: Audio server volume, in percent. Integer value. * `(m)ute`: Mute state. Boolean value, may be set with `0`, `false` or `1`, `true`. -* `sample(r)ate`: Sample rate of the sound card. **Attention:** Most audio applications need to be restarted after changing the sample rate. Integer value. +* `sample(r)ate`: Sample rate of the sound card. Integer value. Both commands and arguments can be abbreviated: Commands by their first letter, arguments by the letter in parenthesis. diff --git a/Ladybird/AudioCodecPluginLadybird.cpp b/Ladybird/AudioCodecPluginLadybird.cpp index 5fb048edfe..cba16fdadc 100644 --- a/Ladybird/AudioCodecPluginLadybird.cpp +++ b/Ladybird/AudioCodecPluginLadybird.cpp @@ -163,7 +163,7 @@ private: case AudioTask::Type::Seek: VERIFY(task.data.has_value()); - m_position = Web::Platform::AudioCodecPlugin::set_loader_position(m_loader, *task.data, m_duration, audio_output->format().sampleRate()); + m_position = Web::Platform::AudioCodecPlugin::set_loader_position(m_loader, *task.data, m_duration); if (paused == Paused::Yes) Q_EMIT playback_position_updated(m_position); @@ -211,10 +211,10 @@ private: auto channel_count = audio_output.format().channelCount(); auto samples_to_load = bytes_available / bytes_per_sample / channel_count; - auto samples = TRY(Web::Platform::AudioCodecPlugin::read_samples_from_loader(*m_loader, samples_to_load, audio_output.format().sampleRate())); + auto samples = TRY(Web::Platform::AudioCodecPlugin::read_samples_from_loader(*m_loader, samples_to_load)); enqueue_samples(audio_output, io_device, move(samples)); - m_position = Web::Platform::AudioCodecPlugin::current_loader_position(m_loader, audio_output.format().sampleRate()); + m_position = Web::Platform::AudioCodecPlugin::current_loader_position(m_loader); return Paused::No; } diff --git a/Userland/Applications/Piano/AudioPlayerLoop.cpp b/Userland/Applications/Piano/AudioPlayerLoop.cpp index 9117605ff9..1b7b2339ef 100644 --- a/Userland/Applications/Piano/AudioPlayerLoop.cpp +++ b/Userland/Applications/Piano/AudioPlayerLoop.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include #include @@ -59,11 +58,7 @@ AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, Atomic& need , m_wav_writer(wav_writer) { m_audio_client = Audio::ConnectionToServer::try_create().release_value_but_fixme_should_propagate_errors(); - - auto target_sample_rate = m_audio_client->get_sample_rate(); - if (target_sample_rate == 0) - target_sample_rate = Music::sample_rate; - m_resampler = Audio::ResampleHelper(Music::sample_rate, target_sample_rate); + m_audio_client->set_self_sample_rate(sample_rate); MUST(m_pipeline_thread->set_priority(sched_get_priority_max(0))); m_pipeline_thread->start(); @@ -108,15 +103,12 @@ intptr_t AudioPlayerLoop::pipeline_thread_main() ErrorOr AudioPlayerLoop::send_audio_to_server() { - TRY(m_resampler->try_resample_into_end(m_remaining_samples, m_buffer)); - - auto sample_rate = static_cast(m_resampler->target()); auto buffer_play_time_ns = 1'000'000'000.0 / (sample_rate / static_cast(Audio::AUDIO_BUFFER_SIZE)); auto good_sleep_time = Duration::from_nanoseconds(static_cast(buffer_play_time_ns)).to_timespec(); size_t start_of_chunk_to_write = 0; - while (start_of_chunk_to_write + Audio::AUDIO_BUFFER_SIZE <= m_remaining_samples.size()) { - auto const exact_chunk = m_remaining_samples.span().slice(start_of_chunk_to_write, Audio::AUDIO_BUFFER_SIZE); + while (start_of_chunk_to_write + Audio::AUDIO_BUFFER_SIZE <= m_buffer.size()) { + auto const exact_chunk = m_buffer.span().slice(start_of_chunk_to_write, Audio::AUDIO_BUFFER_SIZE); auto exact_chunk_array = Array::from_span(exact_chunk); TRY(m_audio_client->blocking_realtime_enqueue(exact_chunk_array, [&]() { @@ -125,8 +117,8 @@ ErrorOr AudioPlayerLoop::send_audio_to_server() start_of_chunk_to_write += Audio::AUDIO_BUFFER_SIZE; } - m_remaining_samples.remove(0, start_of_chunk_to_write); - VERIFY(m_remaining_samples.size() < Audio::AUDIO_BUFFER_SIZE); + // The buffer has to have been constructed with a size of an integer multiple of the audio buffer size. + VERIFY(start_of_chunk_to_write == m_buffer.size()); return {}; } diff --git a/Userland/Applications/Piano/AudioPlayerLoop.h b/Userland/Applications/Piano/AudioPlayerLoop.h index 60e86d8896..82d74ed0f0 100644 --- a/Userland/Applications/Piano/AudioPlayerLoop.h +++ b/Userland/Applications/Piano/AudioPlayerLoop.h @@ -39,10 +39,8 @@ private: TrackManager& m_track_manager; FixedArray m_buffer; - Optional> m_resampler; RefPtr m_audio_client; NonnullRefPtr m_pipeline_thread; - Vector m_remaining_samples {}; Atomic m_should_play_audio { true }; Atomic m_exit_requested { false }; diff --git a/Userland/Applications/Piano/Music.h b/Userland/Applications/Piano/Music.h index 8f2b2a3a2e..5d1d8235ce 100644 --- a/Userland/Applications/Piano/Music.h +++ b/Userland/Applications/Piano/Music.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include namespace Music { @@ -23,8 +24,7 @@ struct Sample { i16 right; }; -// HACK: needs to increase with device sample rate, but all of the sample_count stuff is static for now -constexpr int sample_count = 1 << 10; +constexpr int sample_count = Audio::AUDIO_BUFFER_SIZE * 10; constexpr double sample_rate = 44100; diff --git a/Userland/Applications/SoundPlayer/PlaybackManager.cpp b/Userland/Applications/SoundPlayer/PlaybackManager.cpp index d192fdb19c..c5b3f430ac 100644 --- a/Userland/Applications/SoundPlayer/PlaybackManager.cpp +++ b/Userland/Applications/SoundPlayer/PlaybackManager.cpp @@ -16,7 +16,6 @@ PlaybackManager::PlaybackManager(NonnullRefPtr connec return; next_buffer(); }).release_value_but_fixme_should_propagate_errors(); - m_device_sample_rate = connection->get_sample_rate(); } void PlaybackManager::set_loader(NonnullRefPtr&& loader) @@ -24,10 +23,9 @@ void PlaybackManager::set_loader(NonnullRefPtr&& loader) stop(); m_loader = loader; if (m_loader) { + m_connection->set_self_sample_rate(loader->sample_rate()); m_total_length = m_loader->total_samples() / static_cast(m_loader->sample_rate()); - m_device_samples_per_buffer = PlaybackManager::buffer_size_ms / 1000.0f * m_device_sample_rate; m_samples_to_load_per_buffer = PlaybackManager::buffer_size_ms / 1000.0f * m_loader->sample_rate(); - m_resampler = Audio::ResampleHelper(m_loader->sample_rate(), m_device_sample_rate); m_timer->start(); } else { m_timer->stop(); @@ -102,7 +100,7 @@ void PlaybackManager::next_buffer() if (m_paused) return; - while (m_connection->remaining_samples() < m_device_samples_per_buffer * always_enqueued_buffer_count) { + while (m_connection->remaining_samples() < m_samples_to_load_per_buffer * always_enqueued_buffer_count) { bool all_samples_loaded = (m_loader->loaded_samples() >= m_loader->total_samples()); bool audio_server_done = (m_connection->remaining_samples() == 0); @@ -121,12 +119,6 @@ void PlaybackManager::next_buffer() } auto buffer = buffer_or_error.release_value(); m_current_buffer.swap(buffer); - VERIFY(m_resampler.has_value()); - - m_resampler->reset(); - // FIXME: Handle OOM better. - auto resampled = MUST(FixedArray::create(m_resampler->resample(move(m_current_buffer)).span())); - m_current_buffer.swap(resampled); MUST(m_connection->async_enqueue(m_current_buffer)); } } diff --git a/Userland/Applications/SoundPlayer/PlaybackManager.h b/Userland/Applications/SoundPlayer/PlaybackManager.h index 3c711eb8e2..39da487c23 100644 --- a/Userland/Applications/SoundPlayer/PlaybackManager.h +++ b/Userland/Applications/SoundPlayer/PlaybackManager.h @@ -29,7 +29,6 @@ public: bool toggle_pause(); void set_loader(NonnullRefPtr&&); RefPtr loader() const { return m_loader; } - size_t device_sample_rate() const { return m_device_sample_rate; } bool is_paused() const { return m_paused; } float total_length() const { return m_total_length; } @@ -50,13 +49,10 @@ private: bool m_paused { true }; bool m_loop = { false }; float m_total_length { 0 }; - size_t m_device_sample_rate { 44100 }; - size_t m_device_samples_per_buffer { 0 }; size_t m_samples_to_load_per_buffer { 0 }; RefPtr m_loader { nullptr }; NonnullRefPtr m_connection; FixedArray m_current_buffer; - Optional> m_resampler; RefPtr m_timer; // Controls the GUI update rate. A smaller value makes the visualizations nicer. diff --git a/Userland/Applications/SoundPlayer/Player.cpp b/Userland/Applications/SoundPlayer/Player.cpp index ee479d017c..87946d5297 100644 --- a/Userland/Applications/SoundPlayer/Player.cpp +++ b/Userland/Applications/SoundPlayer/Player.cpp @@ -16,13 +16,11 @@ Player::Player(Audio::ConnectionToServer& audio_client_connection) m_playback_manager.on_update = [&]() { auto samples_played = m_playback_manager.loader()->loaded_samples(); auto sample_rate = m_playback_manager.loader()->sample_rate(); - float source_to_dest_ratio = static_cast(sample_rate) / m_playback_manager.device_sample_rate(); - samples_played *= source_to_dest_ratio; auto played_seconds = samples_played / sample_rate; time_elapsed(played_seconds); if (play_state() == PlayState::Playing) - sound_buffer_played(m_playback_manager.current_buffer(), m_playback_manager.device_sample_rate(), samples_played); + sound_buffer_played(m_playback_manager.current_buffer(), sample_rate, samples_played); }; m_playback_manager.on_finished_playing = [&]() { set_play_state(PlayState::Stopped); @@ -64,8 +62,7 @@ void Player::play_file_path(DeprecatedString const& path) m_loaded_filename = path; - // TODO: The combination of sample count, sample rate, and sample data should be properly abstracted for the source and the playback device. - total_samples_changed(loader->total_samples() * (static_cast(loader->sample_rate()) / m_playback_manager.device_sample_rate())); + total_samples_changed(loader->total_samples()); m_playback_manager.set_loader(move(loader)); file_name_changed(path); @@ -156,11 +153,6 @@ void Player::toggle_mute() void Player::seek(int sample) { - auto loader = m_playback_manager.loader(); - if (loader.is_null()) { - return; - } - sample *= (m_playback_manager.device_sample_rate() / static_cast(loader->sample_rate())); m_playback_manager.seek(sample); } diff --git a/Userland/Libraries/LibAudio/ConnectionToServer.cpp b/Userland/Libraries/LibAudio/ConnectionToServer.cpp index c055b5c0ab..520d2fb0b4 100644 --- a/Userland/Libraries/LibAudio/ConnectionToServer.cpp +++ b/Userland/Libraries/LibAudio/ConnectionToServer.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,7 @@ ConnectionToServer::ConnectionToServer(NonnullOwnPtr socket) return (intptr_t) nullptr; })) { + update_good_sleep_time(); async_pause_playback(); set_buffer(*m_buffer); } @@ -64,12 +66,12 @@ ErrorOr ConnectionToServer::async_enqueue(FixedArray&& samples) { if (!m_background_audio_enqueuer->is_started()) { m_background_audio_enqueuer->start(); + // Wait until the enqueuer has constructed its loop. A pseudo-spinlock is fine since this happens as soon as the other thread gets scheduled. while (!m_enqueuer_loop) usleep(1); TRY(m_background_audio_enqueuer->set_priority(THREAD_PRIORITY_MAX)); } - update_good_sleep_time(); m_user_queue->append(move(samples)); // Wake the background thread to make sure it starts enqueuing audio. m_enqueuer_loop->post_event(*this, make(0)); @@ -86,12 +88,18 @@ void ConnectionToServer::clear_client_buffer() void ConnectionToServer::update_good_sleep_time() { - auto sample_rate = static_cast(get_sample_rate()); + auto sample_rate = static_cast(get_self_sample_rate()); auto buffer_play_time_ns = 1'000'000'000.0 / (sample_rate / static_cast(AUDIO_BUFFER_SIZE)); // A factor of 1 should be good for now. m_good_sleep_time = Duration::from_nanoseconds(static_cast(buffer_play_time_ns)).to_timespec(); } +void ConnectionToServer::set_self_sample_rate(u32 sample_rate) +{ + IPC::ConnectionToServer::set_self_sample_rate(sample_rate); + update_good_sleep_time(); +} + // Non-realtime audio writing loop void ConnectionToServer::custom_event(Core::CustomEvent&) { diff --git a/Userland/Libraries/LibAudio/ConnectionToServer.h b/Userland/Libraries/LibAudio/ConnectionToServer.h index d7757765d8..eef31acad5 100644 --- a/Userland/Libraries/LibAudio/ConnectionToServer.h +++ b/Userland/Libraries/LibAudio/ConnectionToServer.h @@ -55,6 +55,8 @@ public: // Non-realtime code needn't worry about this. size_t remaining_buffers() const; + void set_self_sample_rate(u32 sample_rate); + virtual void die() override; Function on_client_volume_change; @@ -67,7 +69,6 @@ private: // We use this to perform the audio enqueuing on the background thread's event loop virtual void custom_event(Core::CustomEvent&) override; - // FIXME: This should be called every time the sample rate changes, but we just cautiously call it on every non-realtime enqueue. void update_good_sleep_time(); // Shared audio buffer: both server and client constantly read and write to/from this. diff --git a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp index c44e985f0d..3e3c1ba54c 100644 --- a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp +++ b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.cpp @@ -28,7 +28,7 @@ ErrorOr> AudioCodecPlugin::create(NonnullRefPtr< return s_creation_hook(move(loader)); } -ErrorOr> AudioCodecPlugin::read_samples_from_loader(Audio::Loader& loader, size_t samples_to_load, size_t device_sample_rate) +ErrorOr> AudioCodecPlugin::read_samples_from_loader(Audio::Loader& loader, size_t samples_to_load) { auto buffer_or_error = loader.get_more_samples(samples_to_load); if (buffer_or_error.is_error()) { @@ -36,30 +36,26 @@ ErrorOr> AudioCodecPlugin::read_samples_from_loader(Au return Error::from_string_literal("Error while loading samples"); } - Audio::ResampleHelper resampler(loader.sample_rate(), device_sample_rate); - return FixedArray::create(resampler.resample(buffer_or_error.release_value()).span()); + return buffer_or_error.release_value(); } -Duration AudioCodecPlugin::set_loader_position(Audio::Loader& loader, double position, Duration duration, size_t device_sample_rate) +Duration AudioCodecPlugin::set_loader_position(Audio::Loader& loader, double position, Duration duration) { if (loader.total_samples() == 0) - return current_loader_position(loader, device_sample_rate); + return current_loader_position(loader); auto duration_value = static_cast(duration.to_milliseconds()) / 1000.0; position = position / duration_value * static_cast(loader.total_samples() - 1); loader.seek(static_cast(position)).release_value_but_fixme_should_propagate_errors(); - return current_loader_position(loader, device_sample_rate); + return current_loader_position(loader); } -Duration AudioCodecPlugin::current_loader_position(Audio::Loader const& loader, size_t device_sample_rate) +Duration AudioCodecPlugin::current_loader_position(Audio::Loader const& loader) { auto samples_played = static_cast(loader.loaded_samples()); auto sample_rate = static_cast(loader.sample_rate()); - auto source_to_device_ratio = sample_rate / static_cast(device_sample_rate); - samples_played *= source_to_device_ratio; - return Duration::from_milliseconds(static_cast(samples_played / sample_rate * 1000.0)); } diff --git a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h index 6c15ca88d9..4e6b299649 100644 --- a/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h +++ b/Userland/Libraries/LibWeb/Platform/AudioCodecPlugin.h @@ -23,9 +23,9 @@ public: virtual ~AudioCodecPlugin(); - static ErrorOr> read_samples_from_loader(Audio::Loader&, size_t samples_to_load, size_t device_sample_rate); - static Duration set_loader_position(Audio::Loader&, double position, Duration duration, size_t device_sample_rate); - static Duration current_loader_position(Audio::Loader const&, size_t device_sample_rate); + static ErrorOr> read_samples_from_loader(Audio::Loader&, size_t samples_to_load); + static Duration set_loader_position(Audio::Loader&, double position, Duration duration); + static Duration current_loader_position(Audio::Loader const&); virtual void resume_playback() = 0; virtual void pause_playback() = 0; diff --git a/Userland/Services/AudioServer/AudioServer.ipc b/Userland/Services/AudioServer/AudioServer.ipc index e52694de55..6204cc4587 100644 --- a/Userland/Services/AudioServer/AudioServer.ipc +++ b/Userland/Services/AudioServer/AudioServer.ipc @@ -8,9 +8,8 @@ endpoint AudioServer get_self_volume() => (double volume) set_self_volume(double volume) => () - // FIXME: Decouple client sample rate from device sample rate, then remove this API - get_sample_rate() => (u32 sample_rate) - + set_self_sample_rate(u32 sample_rate) => () + get_self_sample_rate() => (u32 sample_rate) // Buffer playback set_buffer(Audio::AudioQueue buffer) => () clear_buffer() =| diff --git a/Userland/Services/AudioServer/ConnectionFromClient.cpp b/Userland/Services/AudioServer/ConnectionFromClient.cpp index 15d7014d34..52ab884730 100644 --- a/Userland/Services/AudioServer/ConnectionFromClient.cpp +++ b/Userland/Services/AudioServer/ConnectionFromClient.cpp @@ -40,8 +40,11 @@ void ConnectionFromClient::set_buffer(Audio::AudioQueue const& buffer) did_misbehave("Received an invalid buffer"); return; } - if (!m_queue) + if (!m_queue) { m_queue = m_mixer.create_queue(*this); + if (m_saved_sample_rate.has_value()) + m_queue->set_sample_rate(m_saved_sample_rate.release_value()); + } // This is ugly but we know nobody uses the buffer afterwards anyways. m_queue->set_buffer(make(move(const_cast(buffer)))); @@ -52,9 +55,19 @@ void ConnectionFromClient::did_change_client_volume(Badge, do async_client_volume_changed(volume); } -Messages::AudioServer::GetSampleRateResponse ConnectionFromClient::get_sample_rate() +Messages::AudioServer::GetSelfSampleRateResponse ConnectionFromClient::get_self_sample_rate() { - return { m_mixer.audiodevice_get_sample_rate() }; + if (m_queue) + return { m_queue->sample_rate() }; + // Fall back to device sample rate since that would mean no resampling. + return m_mixer.audiodevice_get_sample_rate(); +} +void ConnectionFromClient::set_self_sample_rate(u32 sample_rate) +{ + if (m_queue) + m_queue->set_sample_rate(sample_rate); + else + m_saved_sample_rate = sample_rate; } Messages::AudioServer::GetSelfVolumeResponse ConnectionFromClient::get_self_volume() diff --git a/Userland/Services/AudioServer/ConnectionFromClient.h b/Userland/Services/AudioServer/ConnectionFromClient.h index 148b0cd1a8..b1d04f574e 100644 --- a/Userland/Services/AudioServer/ConnectionFromClient.h +++ b/Userland/Services/AudioServer/ConnectionFromClient.h @@ -40,11 +40,12 @@ private: virtual void pause_playback() override; virtual Messages::AudioServer::IsSelfMutedResponse is_self_muted() override; virtual void set_self_muted(bool) override; - // FIXME: Decouple client sample rate from device sample rate, then remove this endpoint - virtual Messages::AudioServer::GetSampleRateResponse get_sample_rate() override; + virtual Messages::AudioServer::GetSelfSampleRateResponse get_self_sample_rate() override; + virtual void set_self_sample_rate(u32 sample_rate) override; Mixer& m_mixer; RefPtr m_queue; + Optional m_saved_sample_rate {}; }; } diff --git a/Userland/Services/AudioServer/Mixer.cpp b/Userland/Services/AudioServer/Mixer.cpp index 4826cf1598..d4747cca45 100644 --- a/Userland/Services/AudioServer/Mixer.cpp +++ b/Userland/Services/AudioServer/Mixer.cpp @@ -39,6 +39,7 @@ Mixer::Mixer(NonnullRefPtr config, NonnullOwnPtr d NonnullRefPtr Mixer::create_queue(ConnectionFromClient& client) { auto queue = adopt_ref(*new ClientAudioStream(client)); + queue->set_sample_rate(audiodevice_get_sample_rate()); { Threading::MutexLocker const locker(m_pending_mutex); m_pending_mixing.append(*queue); @@ -83,7 +84,7 @@ void Mixer::mix() for (auto& mixed_sample : mixed_buffer) { Audio::Sample sample; - if (!queue->get_next_sample(sample)) + if (!queue->get_next_sample(sample, audiodevice_get_sample_rate())) break; if (queue->is_muted()) continue; @@ -155,15 +156,22 @@ int Mixer::audiodevice_set_sample_rate(u32 sample_rate) int code = ioctl(m_device->fd(), SOUNDCARD_IOCTL_SET_SAMPLE_RATE, sample_rate); if (code != 0) dbgln("Error while setting sample rate to {}: ioctl error: {}", sample_rate, strerror(errno)); + // Note that the effective sample rate may be different depending on device restrictions. + // Therefore, we delete our cache, but for efficency don't immediately read the sample rate back. + m_cached_sample_rate = {}; return code; } u32 Mixer::audiodevice_get_sample_rate() const { + if (m_cached_sample_rate.has_value()) + return m_cached_sample_rate.value(); u32 sample_rate = 0; int code = ioctl(m_device->fd(), SOUNDCARD_IOCTL_GET_SAMPLE_RATE, &sample_rate); if (code != 0) dbgln("Error while getting sample rate: ioctl error: {}", strerror(errno)); + else + m_cached_sample_rate = sample_rate; return sample_rate; } diff --git a/Userland/Services/AudioServer/Mixer.h b/Userland/Services/AudioServer/Mixer.h index 8c00fd3678..921d0d18b3 100644 --- a/Userland/Services/AudioServer/Mixer.h +++ b/Userland/Services/AudioServer/Mixer.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -42,7 +43,7 @@ public: explicit ClientAudioStream(ConnectionFromClient&); ~ClientAudioStream() = default; - bool get_next_sample(Audio::Sample& sample) + bool get_next_sample(Audio::Sample& 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()) @@ -60,7 +61,19 @@ public: return false; } - m_current_audio_chunk = result.release_value(); + // 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; + + // 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_in_chunk_location = 0; } @@ -90,14 +103,21 @@ public: void set_volume(double const volume) { m_volume = volume; } bool is_muted() const { return m_muted; } void set_muted(bool muted) { m_muted = muted; } + u32 sample_rate() const { return m_sample_rate; } + void set_sample_rate(u32 sample_rate) + { + dbgln_if(AUDIO_DEBUG, "queue {} got sample rate {} Hz", m_client->client_id(), sample_rate); + m_sample_rate = sample_rate; + } private: OwnPtr m_buffer; - Array m_current_audio_chunk; + Vector m_current_audio_chunk; size_t m_in_chunk_location; bool m_paused { true }; bool m_muted { false }; + u32 m_sample_rate; WeakPtr m_client; FadingProperty m_volume { 1 }; @@ -137,6 +157,7 @@ private: Threading::ConditionVariable m_mixing_necessary { m_pending_mutex }; NonnullOwnPtr m_device; + mutable Optional m_cached_sample_rate {}; NonnullRefPtr m_sound_thread; diff --git a/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp b/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp index 0f4d2d8023..794e0fe463 100644 --- a/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp +++ b/Userland/Services/WebContent/AudioCodecPluginSerenity.cpp @@ -38,16 +38,16 @@ AudioCodecPluginSerenity::AudioCodecPluginSerenity(NonnullRefPtr(m_loader->total_samples()) / static_cast(m_loader->sample_rate()); m_duration = Duration::from_milliseconds(static_cast(duration * 1000.0)); - m_device_sample_rate = m_connection->get_sample_rate(); - m_device_samples_per_buffer = static_cast(BUFFER_SIZE_MS / 1000.0 * static_cast(m_device_sample_rate)); m_samples_to_load_per_buffer = static_cast(BUFFER_SIZE_MS / 1000.0 * static_cast(m_loader->sample_rate())); + + m_connection->set_self_sample_rate(m_loader->sample_rate()); } AudioCodecPluginSerenity::~AudioCodecPluginSerenity() = default; ErrorOr AudioCodecPluginSerenity::play_next_samples() { - while (m_connection->remaining_samples() < m_device_samples_per_buffer * ALWAYS_ENQUEUED_BUFFER_COUNT) { + while (m_connection->remaining_samples() < m_samples_to_load_per_buffer * ALWAYS_ENQUEUED_BUFFER_COUNT) { bool all_samples_loaded = m_loader->loaded_samples() >= m_loader->total_samples(); bool audio_server_done = m_connection->remaining_samples() == 0; @@ -62,10 +62,10 @@ ErrorOr AudioCodecPluginSerenity::play_next_samples() break; } - auto samples = TRY(read_samples_from_loader(m_loader, m_samples_to_load_per_buffer, m_device_sample_rate)); + auto samples = TRY(read_samples_from_loader(m_loader, m_samples_to_load_per_buffer)); TRY(m_connection->async_enqueue(move(samples))); - m_position = current_loader_position(m_loader, m_device_sample_rate); + m_position = current_loader_position(m_loader); } return {}; @@ -90,7 +90,7 @@ void AudioCodecPluginSerenity::set_volume(double volume) void AudioCodecPluginSerenity::seek(double position) { - m_position = set_loader_position(m_loader, position, m_duration, m_device_sample_rate); + m_position = set_loader_position(m_loader, position, m_duration); if (on_playback_position_updated) on_playback_position_updated(m_position); diff --git a/Userland/Services/WebContent/AudioCodecPluginSerenity.h b/Userland/Services/WebContent/AudioCodecPluginSerenity.h index 5e6fb1aafb..2423f40d6c 100644 --- a/Userland/Services/WebContent/AudioCodecPluginSerenity.h +++ b/Userland/Services/WebContent/AudioCodecPluginSerenity.h @@ -40,8 +40,6 @@ private: Duration m_duration; Duration m_position; - size_t m_device_sample_rate { 0 }; - size_t m_device_samples_per_buffer { 0 }; size_t m_samples_to_load_per_buffer { 0 }; }; diff --git a/Userland/Utilities/aplay.cpp b/Userland/Utilities/aplay.cpp index 07e0eb4fd3..cdef20d387 100644 --- a/Userland/Utilities/aplay.cpp +++ b/Userland/Utilities/aplay.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -17,8 +16,6 @@ #include #include -// The Kernel has issues with very large anonymous buffers. -// FIXME: This appears to be fine for now, but it's really a hack. constexpr size_t LOAD_CHUNK_SIZE = 128 * KiB; ErrorOr serenity_main(Main::Arguments arguments) @@ -62,10 +59,7 @@ ErrorOr serenity_main(Main::Arguments arguments) loader->num_channels() == 1 ? "Mono" : "Stereo"); out("\033[34;1mProgress\033[0m: \033[s"); - auto resampler = Audio::ResampleHelper(loader->sample_rate(), audio_client->get_sample_rate()); - - // If we're downsampling, we need to appropriately load more samples at once. - size_t const load_size = static_cast(LOAD_CHUNK_SIZE * static_cast(loader->sample_rate()) / static_cast(audio_client->get_sample_rate())); + audio_client->set_self_sample_rate(loader->sample_rate()); auto print_playback_update = [&]() { out("\033[u"); @@ -94,14 +88,12 @@ ErrorOr serenity_main(Main::Arguments arguments) }; for (;;) { - auto samples = loader->get_more_samples(load_size); + auto samples = loader->get_more_samples(LOAD_CHUNK_SIZE); if (!samples.is_error()) { if (samples.value().size() > 0) { print_playback_update(); // We can read and enqueue more samples - resampler.reset(); - auto resampled_samples = resampler.resample(move(samples.value())); - TRY(audio_client->async_enqueue(move(resampled_samples))); + TRY(audio_client->async_enqueue(samples.release_value())); } else if (should_loop) { // We're done: now loop auto result = loader->reset(); @@ -113,7 +105,7 @@ ErrorOr serenity_main(Main::Arguments arguments) // We're done and the server is done break; } - while (audio_client->remaining_samples() > load_size) { + while (audio_client->remaining_samples() > LOAD_CHUNK_SIZE) { // The server has enough data for now print_playback_update(); usleep(1'000'000 / 10);