Piano: Overhaul AudioPlayerLoop and throw out event loops

The audio player loop uses custom IPC plumbing to safely bypass any
event loop shenanigans. There is still work to be done, but this already
improves the realtime capabilities of Piano.

Userland/Applications/Piano/AudioPlayerLoop.cpp

@@ -6,18 +6,54 @@
  */
 
 #include "AudioPlayerLoop.h"
-
+#include "Music.h"
 #include "TrackManager.h"
+#include <AK/Assertions.h>
 #include <AK/FixedArray.h>
+#include <AK/Forward.h>
+#include <AK/NonnullOwnPtr.h>
 #include <AK/NumericLimits.h>
+#include <AK/StdLibExtras.h>
+#include <AK/Time.h>
 #include <LibAudio/ConnectionToServer.h>
+#include <LibAudio/Queue.h>
 #include <LibAudio/Resampler.h>
 #include <LibAudio/Sample.h>
-#include <LibCore/EventLoop.h>
+#include <LibIPC/Connection.h>
+#include <LibThreading/Thread.h>
+#include <sched.h>
+#include <time.h>
 
-AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, bool& need_to_write_wav, Audio::WavWriter& wav_writer)
+struct AudioLoopDeferredInvoker final : public IPC::DeferredInvoker {
+    static constexpr size_t INLINE_FUNCTIONS = 4;
+
+    virtual ~AudioLoopDeferredInvoker() = default;
+
+    virtual void schedule(Function<void()> function) override
+    {
+        deferred_functions.append(move(function));
+    }
+
+    void run_functions()
+    {
+        if (deferred_functions.size() > INLINE_FUNCTIONS)
+            dbgln("Warning: Audio loop has more than {} deferred functions, audio might glitch!", INLINE_FUNCTIONS);
+        while (!deferred_functions.is_empty()) {
+            auto function = deferred_functions.take_last();
+            function();
+        }
+    }
+
+    Vector<Function<void()>, INLINE_FUNCTIONS> deferred_functions;
+};
+
+AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, Atomic<bool>& need_to_write_wav, Threading::MutexProtected<Audio::WavWriter>& wav_writer)
     : m_track_manager(track_manager)
     , m_buffer(FixedArray<DSP::Sample>::must_create_but_fixme_should_propagate_errors(sample_count))
+    , m_pipeline_thread(Threading::Thread::construct([this]() {
+        return this->pipeline_thread_main();
+    },
+          "Audio pipeline"sv))
     , m_need_to_write_wav(need_to_write_wav)
     , m_wav_writer(wav_writer)
 {
@@ -28,37 +64,90 @@ AudioPlayerLoop::AudioPlayerLoop(TrackManager& track_manager, bool& need_to_writ
         target_sample_rate = Music::sample_rate;
     m_resampler = Audio::ResampleHelper<DSP::Sample>(Music::sample_rate, target_sample_rate);
 
-    // FIXME: I said I would never write such a hack again, but here we are.
-    // This code should die as soon as possible anyways, so it doesn't matter.
-    // Please don't use this as an example to write good audio code; it's just here as a temporary hack.
-    Core::EventLoop::register_timer(*this, 5, true, Core::TimerShouldFireWhenNotVisible::Yes);
+    MUST(m_pipeline_thread->set_priority(sched_get_priority_max(0)));
+    m_pipeline_thread->start();
 }
 
-void AudioPlayerLoop::timer_event(Core::TimerEvent&)
+AudioPlayerLoop::~AudioPlayerLoop()
 {
-    while (m_audio_client->remaining_samples() < sample_count)
-        enqueue_audio();
+    // Tell the pipeline to exit and wait for the last audio cycle to finish.
+    m_exit_requested.store(true);
+    auto result = m_pipeline_thread->join();
+    // FIXME: Get rid of the EINVAL/ESRCH check once we allow to join dead threads.
+    VERIFY(!result.is_error() || result.error() == EINVAL || result.error() == ESRCH);
+
+    m_audio_client->shutdown();
 }
 
-void AudioPlayerLoop::enqueue_audio()
+intptr_t AudioPlayerLoop::pipeline_thread_main()
 {
-    m_track_manager.fill_buffer(m_buffer);
-    // FIXME: Handle OOM better.
-    auto audio_buffer = m_resampler->resample(m_buffer);
-    (void)m_audio_client->async_enqueue(audio_buffer);
+    m_audio_client->set_deferred_invoker(make<AudioLoopDeferredInvoker>());
+    auto& deferred_invoker = static_cast<AudioLoopDeferredInvoker&>(m_audio_client->deferred_invoker());
 
-    // FIXME: This should be done somewhere else.
-    if (m_need_to_write_wav) {
-        m_need_to_write_wav = false;
-        m_track_manager.reset();
-        m_track_manager.set_should_loop(false);
-        do {
-            m_track_manager.fill_buffer(m_buffer);
-            m_wav_writer.write_samples(m_buffer.span());
-        } while (m_track_manager.transport()->time());
-        m_track_manager.reset();
-        m_track_manager.set_should_loop(true);
-        m_wav_writer.finalize();
+    m_audio_client->async_start_playback();
+
+    while (!m_exit_requested.load()) {
+        deferred_invoker.run_functions();
+
+        // The track manager guards against allocations itself.
+        m_track_manager.fill_buffer(m_buffer);
+
+        auto result = send_audio_to_server();
+        // Tolerate errors in the audio pipeline; we don't want this thread to crash the program. This might likely happen with OOM.
+        if (result.is_error()) [[unlikely]] {
+            dbgln("Error in audio pipeline: {}", result.error());
+            m_track_manager.reset();
+        }
+
+        write_wav_if_needed();
+    }
+    m_audio_client->async_pause_playback();
+    return static_cast<intptr_t>(0);
+}
+
+ErrorOr<void> AudioPlayerLoop::send_audio_to_server()
+{
+    TRY(m_resampler->try_resample_into_end(m_remaining_samples, m_buffer));
+
+    auto sample_rate = static_cast<double>(m_resampler->target());
+    auto buffer_play_time_ns = 1'000'000'000.0 / (sample_rate / static_cast<double>(Audio::AUDIO_BUFFER_SIZE));
+    auto good_sleep_time = Time::from_nanoseconds(static_cast<unsigned>(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);
+        auto exact_chunk_array = Array<Audio::Sample, Audio::AUDIO_BUFFER_SIZE>::from_span(exact_chunk);
+
+        TRY(m_audio_client->blocking_realtime_enqueue(exact_chunk_array, [&]() {
+            nanosleep(&good_sleep_time, nullptr);
+        }));
+
+        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);
+
+    return {};
+}
+
+void AudioPlayerLoop::write_wav_if_needed()
+{
+    bool _true = true;
+    if (m_need_to_write_wav.compare_exchange_strong(_true, false)) {
+        m_audio_client->async_pause_playback();
+        m_wav_writer.with_locked([this](auto& wav_writer) {
+            m_track_manager.reset();
+            m_track_manager.set_should_loop(false);
+            do {
+                m_track_manager.fill_buffer(m_buffer);
+                wav_writer.write_samples(m_buffer.span());
+            } while (m_track_manager.transport()->time());
+            // FIXME: Make sure that the new TrackManager APIs aren't as bad.
+            m_track_manager.reset();
+            m_track_manager.set_should_loop(true);
+            wav_writer.finalize();
+        });
+        m_audio_client->async_start_playback();
     }
 }
 

Userland/Applications/Piano/AudioPlayerLoop.h

@@ -15,6 +15,7 @@
 #include <LibCore/Event.h>
 #include <LibCore/Object.h>
 #include <LibDSP/Music.h>
+#include <LibThreading/Thread.h>
 
 class TrackManager;
 
@@ -23,23 +24,30 @@ class TrackManager;
 class AudioPlayerLoop final : public Core::Object {
     C_OBJECT(AudioPlayerLoop)
 public:
+    virtual ~AudioPlayerLoop() override;
+
     void enqueue_audio();
 
     void toggle_paused();
     bool is_playing() const { return m_should_play_audio; }
 
 private:
-    AudioPlayerLoop(TrackManager& track_manager, bool& need_to_write_wav, Audio::WavWriter& wav_writer);
+    AudioPlayerLoop(TrackManager& track_manager, Atomic<bool>& need_to_write_wav, Threading::MutexProtected<Audio::WavWriter>& wav_writer);
 
-    virtual void timer_event(Core::TimerEvent&) override;
+    intptr_t pipeline_thread_main();
+    ErrorOr<void> send_audio_to_server();
+    void write_wav_if_needed();
 
     TrackManager& m_track_manager;
     FixedArray<DSP::Sample> m_buffer;
     Optional<Audio::ResampleHelper<DSP::Sample>> m_resampler;
     RefPtr<Audio::ConnectionToServer> m_audio_client;
+    NonnullRefPtr<Threading::Thread> m_pipeline_thread;
+    Vector<Audio::Sample, Audio::AUDIO_BUFFER_SIZE> m_remaining_samples {};
 
-    bool m_should_play_audio = true;
+    Atomic<bool> m_should_play_audio { true };
+    Atomic<bool> m_exit_requested { false };
 
-    bool& m_need_to_write_wav;
-    Audio::WavWriter& m_wav_writer;
+    Atomic<bool>& m_need_to_write_wav;
+    Threading::MutexProtected<Audio::WavWriter>& m_wav_writer;
 };

Userland/Applications/Piano/CMakeLists.txt

@@ -21,4 +21,4 @@ set(SOURCES
 )
 
 serenity_app(Piano ICON app-piano)
-target_link_libraries(Piano PRIVATE LibAudio LibCore LibDSP LibGfx LibGUI LibIPC LibMain)
+target_link_libraries(Piano PRIVATE LibAudio LibCore LibDSP LibGfx LibGUI LibIPC LibMain LibThreading)

Userland/Applications/Piano/main.cpp

@@ -10,6 +10,7 @@
 #include "AudioPlayerLoop.h"
 #include "MainWidget.h"
 #include "TrackManager.h"
+#include <AK/Atomic.h>
 #include <AK/Queue.h>
 #include <LibAudio/ConnectionToServer.h>
 #include <LibAudio/WavWriter.h>
@@ -23,18 +24,19 @@
 #include <LibGUI/MessageBox.h>
 #include <LibGUI/Window.h>
 #include <LibMain/Main.h>
+#include <LibThreading/MutexProtected.h>
 
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
-    TRY(Core::System::pledge("stdio thread rpath cpath wpath recvfd sendfd unix proc"));
+    TRY(Core::System::pledge("stdio thread proc rpath cpath wpath recvfd sendfd unix"));
 
     auto app = TRY(GUI::Application::try_create(arguments));
 
     TrackManager track_manager;
 
-    Audio::WavWriter wav_writer;
+    Threading::MutexProtected<Audio::WavWriter> wav_writer;
     Optional<DeprecatedString> save_path;
-    bool need_to_write_wav = false;
+    Atomic<bool> need_to_write_wav = false;
 
     auto audio_loop = AudioPlayerLoop::construct(track_manager, need_to_write_wav, wav_writer);
 
@@ -45,8 +47,9 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     window->resize(840, 600);
     window->set_icon(app_icon.bitmap_for_size(16));
 
-    auto main_widget_updater = Core::Timer::construct(static_cast<int>((1 / 60.0) * 1000), [&] {
-        Core::EventLoop::current().post_event(main_widget, make<Core::CustomEvent>(0));
+    auto main_widget_updater = Core::Timer::construct(static_cast<int>((1 / 30.0) * 1000), [&] {
+        if (window->is_active())
+            Core::EventLoop::current().post_event(main_widget, make<Core::CustomEvent>(0));
     });
     main_widget_updater->start();
 
@@ -55,10 +58,16 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         save_path = GUI::FilePicker::get_save_filepath(window, "Untitled", "wav");
         if (!save_path.has_value())
             return;
-        wav_writer.set_file(save_path.value());
-        if (wav_writer.has_error()) {
-            GUI::MessageBox::show(window, DeprecatedString::formatted("Failed to export WAV file: {}", wav_writer.error_string()), "Error"sv, GUI::MessageBox::Type::Error);
-            wav_writer.clear_error();
+        DeprecatedString error;
+        wav_writer.with_locked([&](auto& wav_writer) {
+            wav_writer.set_file(save_path.value());
+            if (wav_writer.has_error()) {
+                error = DeprecatedString::formatted("Failed to export WAV file: {}", wav_writer.error_string());
+                wav_writer.clear_error();
+            }
+        });
+        if (!error.is_empty()) {
+            GUI::MessageBox::show_error(window, error);
             return;
         }
         need_to_write_wav = true;