From 27742ef26d81421be0131d98fafb03571569c05b Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Thu, 25 Sep 2025 13:44:38 -0500 Subject: [PATCH] LibMedia+LibWeb: Never return errors when getting PlaybackStream time We should be fine to just fall back to zero or the last returned value if we encounter an error in PlaybackStream::total_time_played(), and this also simplifies the using code. --- Libraries/LibMedia/Audio/PlaybackStream.h | 2 +- .../LibMedia/Audio/PlaybackStreamAudioUnit.cpp | 2 +- .../LibMedia/Audio/PlaybackStreamAudioUnit.h | 2 +- Libraries/LibMedia/Audio/PlaybackStreamOboe.cpp | 4 ++-- Libraries/LibMedia/Audio/PlaybackStreamOboe.h | 2 +- .../LibMedia/Audio/PlaybackStreamPulseAudio.cpp | 4 ++-- .../LibMedia/Audio/PlaybackStreamPulseAudio.h | 2 +- Libraries/LibMedia/Audio/PulseAudioWrappers.cpp | 12 +++++++----- Libraries/LibMedia/Audio/PulseAudioWrappers.h | 2 +- .../Platform/AudioCodecPluginAgnostic.cpp | 17 ++++++----------- .../LibWeb/Platform/AudioCodecPluginAgnostic.h | 1 - 11 files changed, 23 insertions(+), 27 deletions(-) diff --git a/Libraries/LibMedia/Audio/PlaybackStream.h b/Libraries/LibMedia/Audio/PlaybackStream.h index d84a253a0cf..84398b6361f 100644 --- a/Libraries/LibMedia/Audio/PlaybackStream.h +++ b/Libraries/LibMedia/Audio/PlaybackStream.h @@ -63,7 +63,7 @@ public: // whenever possible. // // This function should be able to run from any thread safely. - virtual ErrorOr total_time_played() = 0; + virtual AK::Duration total_time_played() const = 0; virtual NonnullRefPtr> set_volume(double volume) = 0; }; diff --git a/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.cpp b/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.cpp index 99451a800b4..4dd5865e6b0 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.cpp +++ b/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.cpp @@ -281,7 +281,7 @@ NonnullRefPtr> PlaybackStreamAudioUnit::discard_buff return promise; } -ErrorOr PlaybackStreamAudioUnit::total_time_played() +AK::Duration PlaybackStreamAudioUnit::total_time_played() const { return m_state->last_sample_time(); } diff --git a/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.h b/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.h index 96650787289..f949e3179d7 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.h +++ b/Libraries/LibMedia/Audio/PlaybackStreamAudioUnit.h @@ -25,7 +25,7 @@ public: virtual NonnullRefPtr> drain_buffer_and_suspend() override; virtual NonnullRefPtr> discard_buffer_and_suspend() override; - virtual ErrorOr total_time_played() override; + virtual AK::Duration total_time_played() const override; virtual NonnullRefPtr> set_volume(double) override; diff --git a/Libraries/LibMedia/Audio/PlaybackStreamOboe.cpp b/Libraries/LibMedia/Audio/PlaybackStreamOboe.cpp index aa119c51174..cf4c4c6d8e0 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamOboe.cpp +++ b/Libraries/LibMedia/Audio/PlaybackStreamOboe.cpp @@ -123,7 +123,7 @@ void PlaybackStreamOboe::set_underrun_callback(Function) NonnullRefPtr> PlaybackStreamOboe::resume() { auto promise = Core::ThreadedPromise::create(); - auto time = MUST(total_time_played()); + auto time = total_time_played(); m_storage->stream()->start(); promise->resolve(move(time)); return promise; @@ -146,7 +146,7 @@ NonnullRefPtr> PlaybackStreamOboe::discard_buffer_an return promise; } -ErrorOr PlaybackStreamOboe::total_time_played() +AK::Duration PlaybackStreamOboe::total_time_played() const { return m_storage->oboe_callback()->last_sample_time(); } diff --git a/Libraries/LibMedia/Audio/PlaybackStreamOboe.h b/Libraries/LibMedia/Audio/PlaybackStreamOboe.h index 697ad417316..0547f7ebc36 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamOboe.h +++ b/Libraries/LibMedia/Audio/PlaybackStreamOboe.h @@ -22,7 +22,7 @@ public: virtual NonnullRefPtr> drain_buffer_and_suspend() override; virtual NonnullRefPtr> discard_buffer_and_suspend() override; - virtual ErrorOr total_time_played() override; + virtual AK::Duration total_time_played() const override; virtual NonnullRefPtr> set_volume(double) override; diff --git a/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.cpp b/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.cpp index 3f9e5017654..4a0a3d77bf1 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.cpp +++ b/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.cpp @@ -89,7 +89,7 @@ NonnullRefPtr> PlaybackStreamPulseAudio::res TRY_OR_REJECT(m_state->check_is_running(), promise); m_state->enqueue([this, promise]() { TRY_OR_REJECT(m_state->stream()->resume()); - promise->resolve(TRY_OR_REJECT(m_state->stream()->total_time_played())); + promise->resolve(m_state->stream()->total_time_played()); }); return promise; } @@ -116,7 +116,7 @@ NonnullRefPtr> PlaybackStreamPulseAudio::discard_buf return promise; } -ErrorOr PlaybackStreamPulseAudio::total_time_played() +AK::Duration PlaybackStreamPulseAudio::total_time_played() const { if (m_state->stream() != nullptr) return m_state->stream()->total_time_played(); diff --git a/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.h b/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.h index 1d537558566..b8876af5cfe 100644 --- a/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.h +++ b/Libraries/LibMedia/Audio/PlaybackStreamPulseAudio.h @@ -25,7 +25,7 @@ public: virtual NonnullRefPtr> drain_buffer_and_suspend() override; virtual NonnullRefPtr> discard_buffer_and_suspend() override; - virtual ErrorOr total_time_played() override; + virtual AK::Duration total_time_played() const override; virtual NonnullRefPtr> set_volume(double) override; diff --git a/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp b/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp index 3c7ca20bee1..18ac01eba2f 100644 --- a/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp +++ b/Libraries/LibMedia/Audio/PulseAudioWrappers.cpp @@ -232,7 +232,7 @@ ErrorOr> PulseAudioContext::create_stream(Output flags = static_cast(static_cast(flags) | PA_STREAM_START_CORKED); } - // This is a workaround for an issue with starting the stream corked, see PulseAudioPlaybackStream::total_time_played(). + // This is a workaround for an issue with starting the stream corked, see PulseAudioStream::total_time_played(). pa_stream_set_started_callback( stream, [](pa_stream* stream, void* user_data) { static_cast(user_data)->m_started_playback = true; @@ -455,7 +455,7 @@ ErrorOr PulseAudioStream::resume() return {}; } -ErrorOr PulseAudioStream::total_time_played() +AK::Duration PulseAudioStream::total_time_played() const { auto locker = m_context->main_loop_locker(); @@ -472,10 +472,12 @@ ErrorOr PulseAudioStream::total_time_played() pa_usec_t time = 0; auto error = pa_stream_get_time(m_stream, &time); - if (error == -PA_ERR_NODATA) + if (error) return AK::Duration::zero(); - if (error != 0) - return Error::from_string_literal("Failed to get time from PulseAudio stream"); + if (error != 0) { + warnln("Unexpected error in pa_stream_get_time(): {}", error); + return AK::Duration::zero(); + } if (time > NumericLimits::max()) { warnln("WARNING: Audio time is too large!"); time -= NumericLimits::max(); diff --git a/Libraries/LibMedia/Audio/PulseAudioWrappers.h b/Libraries/LibMedia/Audio/PulseAudioWrappers.h index 8e8a4954dfd..930900c57cd 100644 --- a/Libraries/LibMedia/Audio/PulseAudioWrappers.h +++ b/Libraries/LibMedia/Audio/PulseAudioWrappers.h @@ -117,7 +117,7 @@ public: // Uncorks the stream and forces data to be written to the buffers to force playback to // resume as soon as possible. ErrorOr resume(); - ErrorOr total_time_played(); + AK::Duration total_time_played() const; ErrorOr set_volume(double volume); diff --git a/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.cpp b/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.cpp index 7845425c38a..1631432efff 100644 --- a/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.cpp +++ b/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.cpp @@ -68,7 +68,7 @@ ErrorOr> AudioCodecPluginAgnostic::creat })); output->set_underrun_callback([&plugin = *plugin, loader, output]() { - auto new_device_time = output->total_time_played().release_value_but_fixme_should_propagate_errors(); + auto new_device_time = output->total_time_played(); auto new_media_time = timestamp_from_samples(loader->loaded_samples(), loader->sample_rate()); plugin.m_main_thread_event_loop.deferred_invoke([&plugin, new_device_time, new_media_time]() { plugin.m_last_resume_in_device_time = new_device_time; @@ -118,12 +118,12 @@ void AudioCodecPluginAgnostic::pause_playback() { m_paused = true; m_output->drain_buffer_and_suspend() - ->when_resolved([self = make_weak_ptr()]() -> ErrorOr { + ->when_resolved([self = make_weak_ptr()]() { if (!self) - return {}; + return; auto new_media_time = timestamp_from_samples(self->m_loader->loaded_samples(), self->m_loader->sample_rate()); - auto new_device_time = TRY(self->m_output->total_time_played()); + auto new_device_time = self->m_output->total_time_played(); self->m_main_thread_event_loop.deferred_invoke([self, new_media_time, new_device_time]() { if (!self) @@ -134,8 +134,6 @@ void AudioCodecPluginAgnostic::pause_playback() self->m_update_timer->stop(); self->update_timestamp(); }); - - return {}; }) .when_rejected([](Error&&) { // FIXME: Propagate errors. @@ -162,7 +160,7 @@ void AudioCodecPluginAgnostic::seek(double position) return Error::from_string_literal("Seeking in audio loader failed"); auto new_media_time = get_loader_timestamp(self->m_loader); - auto new_device_time = self->m_output->total_time_played().release_value_but_fixme_should_propagate_errors(); + auto new_device_time = self->m_output->total_time_played(); self->m_main_thread_event_loop.deferred_invoke([self, was_paused, new_device_time, new_media_time]() { if (!self) @@ -194,10 +192,7 @@ AK::Duration AudioCodecPluginAgnostic::duration() void AudioCodecPluginAgnostic::update_timestamp() { - auto current_device_time_result = m_output->total_time_played(); - if (!current_device_time_result.is_error()) - m_last_good_device_time = current_device_time_result.release_value(); - auto current_device_time_delta = m_last_good_device_time - m_last_resume_in_device_time; + auto current_device_time_delta = m_output->total_time_played() - m_last_resume_in_device_time; auto current_media_time = m_last_resume_in_media_time + current_device_time_delta; current_media_time = min(current_media_time, m_duration); diff --git a/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.h b/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.h index c416e05eb0c..e4b0ec7e8bb 100644 --- a/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.h +++ b/Libraries/LibWeb/Platform/AudioCodecPluginAgnostic.h @@ -34,7 +34,6 @@ private: AK::Duration m_duration { AK::Duration::zero() }; AK::Duration m_last_resume_in_media_time { AK::Duration::zero() }; AK::Duration m_last_resume_in_device_time { AK::Duration::zero() }; - AK::Duration m_last_good_device_time { AK::Duration::zero() }; Core::EventLoop& m_main_thread_event_loop; NonnullRefPtr m_update_timer; bool m_paused { true };