Skip to content

Commit b4fbd30

Browse files
kleinesfilmroellchenlinusg
authored andcommitted
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.
1 parent d52a2ff commit b4fbd30

File tree

20 files changed

+100
-93
lines changed

20 files changed

+100
-93
lines changed

Base/usr/share/man/man1/asctl.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ There are two commands available: `get` reports the state of audio variables, an
3030
The available variables are:
3131
* `(v)olume`: Audio server volume, in percent. Integer value.
3232
* `(m)ute`: Mute state. Boolean value, may be set with `0`, `false` or `1`, `true`.
33-
* `sample(r)ate`: Sample rate of the sound card. **Attention:** Most audio applications need to be restarted after changing the sample rate. Integer value.
33+
* `sample(r)ate`: Sample rate of the sound card. Integer value.
3434

3535
Both commands and arguments can be abbreviated: Commands by their first letter, arguments by the letter in parenthesis.
3636

Ladybird/AudioCodecPluginLadybird.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ class AudioThread final : public QThread { // We have to use QThread, otherwise
163163

164164
case AudioTask::Type::Seek:
165165
VERIFY(task.data.has_value());
166-
m_position = Web::Platform::AudioCodecPlugin::set_loader_position(m_loader, *task.data, m_duration, audio_output->format().sampleRate());
166+
m_position = Web::Platform::AudioCodecPlugin::set_loader_position(m_loader, *task.data, m_duration);
167167

168168
if (paused == Paused::Yes)
169169
Q_EMIT playback_position_updated(m_position);
@@ -211,10 +211,10 @@ class AudioThread final : public QThread { // We have to use QThread, otherwise
211211
auto channel_count = audio_output.format().channelCount();
212212
auto samples_to_load = bytes_available / bytes_per_sample / channel_count;
213213

214-
auto samples = TRY(Web::Platform::AudioCodecPlugin::read_samples_from_loader(*m_loader, samples_to_load, audio_output.format().sampleRate()));
214+
auto samples = TRY(Web::Platform::AudioCodecPlugin::read_samples_from_loader(*m_loader, samples_to_load));
215215
enqueue_samples(audio_output, io_device, move(samples));
216216

217-
m_position = Web::Platform::AudioCodecPlugin::current_loader_position(m_loader, audio_output.format().sampleRate());
217+
m_position = Web::Platform::AudioCodecPlugin::current_loader_position(m_loader);
218218
return Paused::No;
219219
}
220220

Userland/Applications/Piano/AudioPlayerLoop.cpp

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#include <AK/Time.h>
1818
#include <LibAudio/ConnectionToServer.h>
1919
#include <LibAudio/Queue.h>
20-
#include <LibAudio/Resampler.h>
2120
#include <LibAudio/Sample.h>
2221
#include <LibIPC/Connection.h>
2322
#include <LibThreading/Thread.h>
@@ -59,11 +58,7 @@ AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, Atomic<bool>& need
5958
, m_wav_writer(wav_writer)
6059
{
6160
m_audio_client = Audio::ConnectionToServer::try_create().release_value_but_fixme_should_propagate_errors();
62-
63-
auto target_sample_rate = m_audio_client->get_sample_rate();
64-
if (target_sample_rate == 0)
65-
target_sample_rate = Music::sample_rate;
66-
m_resampler = Audio::ResampleHelper<DSP::Sample>(Music::sample_rate, target_sample_rate);
61+
m_audio_client->set_self_sample_rate(sample_rate);
6762

6863
MUST(m_pipeline_thread->set_priority(sched_get_priority_max(0)));
6964
m_pipeline_thread->start();
@@ -108,15 +103,12 @@ intptr_t AudioPlayerLoop::pipeline_thread_main()
108103

109104
ErrorOr<void> AudioPlayerLoop::send_audio_to_server()
110105
{
111-
TRY(m_resampler->try_resample_into_end(m_remaining_samples, m_buffer));
112-
113-
auto sample_rate = static_cast<double>(m_resampler->target());
114106
auto buffer_play_time_ns = 1'000'000'000.0 / (sample_rate / static_cast<double>(Audio::AUDIO_BUFFER_SIZE));
115107
auto good_sleep_time = Duration::from_nanoseconds(static_cast<unsigned>(buffer_play_time_ns)).to_timespec();
116108

117109
size_t start_of_chunk_to_write = 0;
118-
while (start_of_chunk_to_write + Audio::AUDIO_BUFFER_SIZE <= m_remaining_samples.size()) {
119-
auto const exact_chunk = m_remaining_samples.span().slice(start_of_chunk_to_write, Audio::AUDIO_BUFFER_SIZE);
110+
while (start_of_chunk_to_write + Audio::AUDIO_BUFFER_SIZE <= m_buffer.size()) {
111+
auto const exact_chunk = m_buffer.span().slice(start_of_chunk_to_write, Audio::AUDIO_BUFFER_SIZE);
120112
auto exact_chunk_array = Array<Audio::Sample, Audio::AUDIO_BUFFER_SIZE>::from_span(exact_chunk);
121113

122114
TRY(m_audio_client->blocking_realtime_enqueue(exact_chunk_array, [&]() {
@@ -125,8 +117,8 @@ ErrorOr<void> AudioPlayerLoop::send_audio_to_server()
125117

126118
start_of_chunk_to_write += Audio::AUDIO_BUFFER_SIZE;
127119
}
128-
m_remaining_samples.remove(0, start_of_chunk_to_write);
129-
VERIFY(m_remaining_samples.size() < Audio::AUDIO_BUFFER_SIZE);
120+
// The buffer has to have been constructed with a size of an integer multiple of the audio buffer size.
121+
VERIFY(start_of_chunk_to_write == m_buffer.size());
130122

131123
return {};
132124
}

Userland/Applications/Piano/AudioPlayerLoop.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,8 @@ class AudioPlayerLoop final : public Core::Object {
3939

4040
TrackManager& m_track_manager;
4141
FixedArray<DSP::Sample> m_buffer;
42-
Optional<Audio::ResampleHelper<DSP::Sample>> m_resampler;
4342
RefPtr<Audio::ConnectionToServer> m_audio_client;
4443
NonnullRefPtr<Threading::Thread> m_pipeline_thread;
45-
Vector<Audio::Sample, Audio::AUDIO_BUFFER_SIZE> m_remaining_samples {};
4644

4745
Atomic<bool> m_should_play_audio { true };
4846
Atomic<bool> m_exit_requested { false };

Userland/Applications/Piano/Music.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#pragma once
99

1010
#include <AK/Types.h>
11+
#include <LibAudio/Queue.h>
1112
#include <LibGfx/Color.h>
1213

1314
namespace Music {
@@ -23,8 +24,7 @@ struct Sample {
2324
i16 right;
2425
};
2526

26-
// HACK: needs to increase with device sample rate, but all of the sample_count stuff is static for now
27-
constexpr int sample_count = 1 << 10;
27+
constexpr int sample_count = Audio::AUDIO_BUFFER_SIZE * 10;
2828

2929
constexpr double sample_rate = 44100;
3030

Userland/Applications/SoundPlayer/PlaybackManager.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,16 @@ PlaybackManager::PlaybackManager(NonnullRefPtr<Audio::ConnectionToServer> connec
1616
return;
1717
next_buffer();
1818
}).release_value_but_fixme_should_propagate_errors();
19-
m_device_sample_rate = connection->get_sample_rate();
2019
}
2120

2221
void PlaybackManager::set_loader(NonnullRefPtr<Audio::Loader>&& loader)
2322
{
2423
stop();
2524
m_loader = loader;
2625
if (m_loader) {
26+
m_connection->set_self_sample_rate(loader->sample_rate());
2727
m_total_length = m_loader->total_samples() / static_cast<float>(m_loader->sample_rate());
28-
m_device_samples_per_buffer = PlaybackManager::buffer_size_ms / 1000.0f * m_device_sample_rate;
2928
m_samples_to_load_per_buffer = PlaybackManager::buffer_size_ms / 1000.0f * m_loader->sample_rate();
30-
m_resampler = Audio::ResampleHelper<Audio::Sample>(m_loader->sample_rate(), m_device_sample_rate);
3129
m_timer->start();
3230
} else {
3331
m_timer->stop();
@@ -102,7 +100,7 @@ void PlaybackManager::next_buffer()
102100
if (m_paused)
103101
return;
104102

105-
while (m_connection->remaining_samples() < m_device_samples_per_buffer * always_enqueued_buffer_count) {
103+
while (m_connection->remaining_samples() < m_samples_to_load_per_buffer * always_enqueued_buffer_count) {
106104
bool all_samples_loaded = (m_loader->loaded_samples() >= m_loader->total_samples());
107105
bool audio_server_done = (m_connection->remaining_samples() == 0);
108106

@@ -121,12 +119,6 @@ void PlaybackManager::next_buffer()
121119
}
122120
auto buffer = buffer_or_error.release_value();
123121
m_current_buffer.swap(buffer);
124-
VERIFY(m_resampler.has_value());
125-
126-
m_resampler->reset();
127-
// FIXME: Handle OOM better.
128-
auto resampled = MUST(FixedArray<Audio::Sample>::create(m_resampler->resample(move(m_current_buffer)).span()));
129-
m_current_buffer.swap(resampled);
130122
MUST(m_connection->async_enqueue(m_current_buffer));
131123
}
132124
}

Userland/Applications/SoundPlayer/PlaybackManager.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ class PlaybackManager final {
2929
bool toggle_pause();
3030
void set_loader(NonnullRefPtr<Audio::Loader>&&);
3131
RefPtr<Audio::Loader> loader() const { return m_loader; }
32-
size_t device_sample_rate() const { return m_device_sample_rate; }
3332

3433
bool is_paused() const { return m_paused; }
3534
float total_length() const { return m_total_length; }
@@ -50,13 +49,10 @@ class PlaybackManager final {
5049
bool m_paused { true };
5150
bool m_loop = { false };
5251
float m_total_length { 0 };
53-
size_t m_device_sample_rate { 44100 };
54-
size_t m_device_samples_per_buffer { 0 };
5552
size_t m_samples_to_load_per_buffer { 0 };
5653
RefPtr<Audio::Loader> m_loader { nullptr };
5754
NonnullRefPtr<Audio::ConnectionToServer> m_connection;
5855
FixedArray<Audio::Sample> m_current_buffer;
59-
Optional<Audio::ResampleHelper<Audio::Sample>> m_resampler;
6056
RefPtr<Core::Timer> m_timer;
6157

6258
// Controls the GUI update rate. A smaller value makes the visualizations nicer.

Userland/Applications/SoundPlayer/Player.cpp

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ Player::Player(Audio::ConnectionToServer& audio_client_connection)
1616
m_playback_manager.on_update = [&]() {
1717
auto samples_played = m_playback_manager.loader()->loaded_samples();
1818
auto sample_rate = m_playback_manager.loader()->sample_rate();
19-
float source_to_dest_ratio = static_cast<float>(sample_rate) / m_playback_manager.device_sample_rate();
20-
samples_played *= source_to_dest_ratio;
2119

2220
auto played_seconds = samples_played / sample_rate;
2321
time_elapsed(played_seconds);
2422
if (play_state() == PlayState::Playing)
25-
sound_buffer_played(m_playback_manager.current_buffer(), m_playback_manager.device_sample_rate(), samples_played);
23+
sound_buffer_played(m_playback_manager.current_buffer(), sample_rate, samples_played);
2624
};
2725
m_playback_manager.on_finished_playing = [&]() {
2826
set_play_state(PlayState::Stopped);
@@ -64,8 +62,7 @@ void Player::play_file_path(DeprecatedString const& path)
6462

6563
m_loaded_filename = path;
6664

67-
// TODO: The combination of sample count, sample rate, and sample data should be properly abstracted for the source and the playback device.
68-
total_samples_changed(loader->total_samples() * (static_cast<float>(loader->sample_rate()) / m_playback_manager.device_sample_rate()));
65+
total_samples_changed(loader->total_samples());
6966
m_playback_manager.set_loader(move(loader));
7067
file_name_changed(path);
7168

@@ -156,11 +153,6 @@ void Player::toggle_mute()
156153

157154
void Player::seek(int sample)
158155
{
159-
auto loader = m_playback_manager.loader();
160-
if (loader.is_null()) {
161-
return;
162-
}
163-
sample *= (m_playback_manager.device_sample_rate() / static_cast<float>(loader->sample_rate()));
164156
m_playback_manager.seek(sample);
165157
}
166158

Userland/Libraries/LibAudio/ConnectionToServer.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <LibAudio/UserSampleQueue.h>
1717
#include <LibCore/Event.h>
1818
#include <LibThreading/Mutex.h>
19+
#include <Userland/Services/AudioServer/AudioClientEndpoint.h>
1920
#include <sched.h>
2021
#include <time.h>
2122

@@ -37,6 +38,7 @@ ConnectionToServer::ConnectionToServer(NonnullOwnPtr<Core::LocalSocket> socket)
3738
return (intptr_t) nullptr;
3839
}))
3940
{
41+
update_good_sleep_time();
4042
async_pause_playback();
4143
set_buffer(*m_buffer);
4244
}
@@ -64,12 +66,12 @@ ErrorOr<void> ConnectionToServer::async_enqueue(FixedArray<Sample>&& samples)
6466
{
6567
if (!m_background_audio_enqueuer->is_started()) {
6668
m_background_audio_enqueuer->start();
69+
// Wait until the enqueuer has constructed its loop. A pseudo-spinlock is fine since this happens as soon as the other thread gets scheduled.
6770
while (!m_enqueuer_loop)
6871
usleep(1);
6972
TRY(m_background_audio_enqueuer->set_priority(THREAD_PRIORITY_MAX));
7073
}
7174

72-
update_good_sleep_time();
7375
m_user_queue->append(move(samples));
7476
// Wake the background thread to make sure it starts enqueuing audio.
7577
m_enqueuer_loop->post_event(*this, make<Core::CustomEvent>(0));
@@ -86,12 +88,18 @@ void ConnectionToServer::clear_client_buffer()
8688

8789
void ConnectionToServer::update_good_sleep_time()
8890
{
89-
auto sample_rate = static_cast<double>(get_sample_rate());
91+
auto sample_rate = static_cast<double>(get_self_sample_rate());
9092
auto buffer_play_time_ns = 1'000'000'000.0 / (sample_rate / static_cast<double>(AUDIO_BUFFER_SIZE));
9193
// A factor of 1 should be good for now.
9294
m_good_sleep_time = Duration::from_nanoseconds(static_cast<unsigned>(buffer_play_time_ns)).to_timespec();
9395
}
9496

97+
void ConnectionToServer::set_self_sample_rate(u32 sample_rate)
98+
{
99+
IPC::ConnectionToServer<AudioClientEndpoint, AudioServerEndpoint>::set_self_sample_rate(sample_rate);
100+
update_good_sleep_time();
101+
}
102+
95103
// Non-realtime audio writing loop
96104
void ConnectionToServer::custom_event(Core::CustomEvent&)
97105
{

Userland/Libraries/LibAudio/ConnectionToServer.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ class ConnectionToServer final
5555
// Non-realtime code needn't worry about this.
5656
size_t remaining_buffers() const;
5757

58+
void set_self_sample_rate(u32 sample_rate);
59+
5860
virtual void die() override;
5961

6062
Function<void(double volume)> on_client_volume_change;
@@ -67,7 +69,6 @@ class ConnectionToServer final
6769
// We use this to perform the audio enqueuing on the background thread's event loop
6870
virtual void custom_event(Core::CustomEvent&) override;
6971

70-
// FIXME: This should be called every time the sample rate changes, but we just cautiously call it on every non-realtime enqueue.
7172
void update_good_sleep_time();
7273

7374
// Shared audio buffer: both server and client constantly read and write to/from this.

0 commit comments

Comments
 (0)