From 5a120bff33ea14d338675a4e36db2d7a299caeb3 Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Tue, 9 Jun 2026 17:39:16 -0500 Subject: [PATCH] LibMedia: Use a combined status to exit the seeking/buffering states Instead of tracking in-flight seeks across all the sinks in the seeking state handler, move the logic to PlaybackManager to determine the overall status and then notify the state of that status to potentially trigger resumption. The buffering state handler can then share essentially the same logic instead of having the playback manager specifically track the blocked tracks for it. --- Libraries/LibMedia/CMakeLists.txt | 2 + Libraries/LibMedia/PlaybackManager.cpp | 65 ++++++++++++------- Libraries/LibMedia/PlaybackManager.h | 9 +-- .../PlaybackStates/BufferingStateHandler.cpp | 19 ++++++ .../PlaybackStates/BufferingStateHandler.h | 9 +-- .../PlaybackStates/PausedStateHandler.h | 3 - .../PlaybackStates/PlaybackStateHandler.cpp | 5 ++ .../PlaybackStates/PlaybackStateHandler.h | 6 +- .../PlaybackStates/PlayingStateHandler.cpp | 26 ++++++++ .../PlaybackStates/PlayingStateHandler.h | 13 +--- .../PlaybackStates/SeekingStateHandler.h | 44 ++----------- .../PlaybackStates/StartingStateHandler.cpp | 9 +-- .../PlaybackStates/StartingStateHandler.h | 4 +- 13 files changed, 116 insertions(+), 98 deletions(-) create mode 100644 Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp create mode 100644 Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp diff --git a/Libraries/LibMedia/CMakeLists.txt b/Libraries/LibMedia/CMakeLists.txt index e60585fb7a..9cd10e14d0 100644 --- a/Libraries/LibMedia/CMakeLists.txt +++ b/Libraries/LibMedia/CMakeLists.txt @@ -21,6 +21,8 @@ set(SOURCES GenericTimeProvider.cpp IncrementallyPopulatedStream.cpp PlaybackManager.cpp + PlaybackStates/BufferingStateHandler.cpp + PlaybackStates/PlayingStateHandler.cpp PlaybackStates/StartingStateHandler.cpp PlaybackStates/PausedStateHandler.cpp PlaybackStates/PlaybackStateHandler.cpp diff --git a/Libraries/LibMedia/PlaybackManager.cpp b/Libraries/LibMedia/PlaybackManager.cpp index c03e941b17..560d00aa86 100644 --- a/Libraries/LibMedia/PlaybackManager.cpp +++ b/Libraries/LibMedia/PlaybackManager.cpp @@ -251,33 +251,46 @@ AK::Duration PlaybackManager::current_time() const void PlaybackManager::on_audio_sink_state_changed(PipelineStatus status) { - m_audio_buffering = status == PipelineStatus::Blocked; - update_buffering_state(); - m_handler->on_audio_sink_state_changed(status); + m_audio_sink_status = status; + update_pipeline_state(); } void PlaybackManager::on_video_sink_state_changed(Track const& track, PipelineStatus status) { - if (status == PipelineStatus::Blocked) { - if (m_video_tracks_buffering.set(track) == HashSetResult::InsertedNewEntry) - update_buffering_state(); - } else { - if (m_video_tracks_buffering.remove(track)) - update_buffering_state(); - } - m_handler->on_video_sink_state_changed(track, status); + auto& track_data = get_video_data_for_track(track); + track_data.sink_status = status; + update_pipeline_state(); } -void PlaybackManager::update_buffering_state() +PipelineStatus PlaybackManager::combined_pipeline_status() const { - auto is_buffering = m_audio_buffering || !m_video_tracks_buffering.is_empty(); - if (is_buffering == m_was_buffering) - return; - m_was_buffering = is_buffering; - if (is_buffering) - m_handler->enter_buffering(); - else - m_handler->exit_buffering(); + auto status = PipelineStatus::EndOfStream; + + if (m_audio_sink != nullptr) + status = select_combined_pipeline_status(status, m_audio_sink_status); + + for (auto const& track_data : m_video_track_datas) { + if (track_data.display == nullptr) + continue; + status = select_combined_pipeline_status(status, track_data.sink_status); + } + + return status; +} + +void PlaybackManager::update_pipeline_state() +{ + m_handler->on_pipeline_status_changed(combined_pipeline_status()); +} + +void PlaybackManager::reset_pipeline_state() +{ + for (auto& track_data : m_video_track_datas) { + if (track_data.display == nullptr) + continue; + track_data.sink_status = PipelineStatus::Pending; + } + m_audio_sink_status = m_audio_sink != nullptr ? PipelineStatus::Pending : PipelineStatus::HaveData; } void PlaybackManager::check_for_duration_change(AK::Duration duration) @@ -317,7 +330,6 @@ void PlaybackManager::set_time_provider(NonnullRefPtr const& void PlaybackManager::disable_audio() { - m_audio_buffering = false; m_audio_mixer = nullptr; m_audio_time_stretch_processor = nullptr; m_audio_sink = nullptr; @@ -329,6 +341,7 @@ NonnullRefPtr PlaybackManager::get_or_create_the_displaying { auto& track_data = get_video_data_for_track(track); if (track_data.display == nullptr) { + track_data.sink_status = PipelineStatus::HaveData; auto display = MUST(Media::DisplayingVideoSink::try_create(m_time_provider, [self = weak(), track](PipelineStatus status) { if (!self) @@ -337,6 +350,7 @@ NonnullRefPtr PlaybackManager::get_or_create_the_displaying })); MUST(display->connect_input(track_data.producer)); track_data.display = move(display); + update_pipeline_state(); } return *track_data.display; } @@ -347,29 +361,34 @@ void PlaybackManager::remove_the_displaying_video_sink_for_track(Track const& tr VERIFY(track_data.display); track_data.display->disconnect_input(track_data.producer); track_data.display = nullptr; - on_video_sink_state_changed(track, PipelineStatus::EndOfStream); + track_data.sink_status = PipelineStatus::HaveData; + update_pipeline_state(); } void PlaybackManager::enable_an_audio_track(Track const& track) { auto& track_data = get_audio_data_for_track(track); VERIFY(!track_data.enabled); + m_audio_sink_status = PipelineStatus::HaveData; if (m_audio_mixer) { m_audio_mixer->seek(current_time()); MUST(m_audio_mixer->connect_input(track_data.producer)); } track_data.enabled = true; + update_pipeline_state(); } void PlaybackManager::disable_an_audio_track(Track const& track) { auto& track_data = get_audio_data_for_track(track); VERIFY(track_data.enabled); + m_audio_sink_status = PipelineStatus::HaveData; if (m_audio_mixer) { m_audio_mixer->seek(current_time()); m_audio_mixer->disconnect_input(track_data.producer); } track_data.enabled = false; + update_pipeline_state(); } bool PlaybackManager::track_is_enabled(Track const& track) const @@ -401,8 +420,10 @@ void PlaybackManager::pause() void PlaybackManager::seek(AK::Duration timestamp, SeekMode mode) { + reset_pipeline_state(); m_handler->seek(timestamp, mode); m_is_in_error_state = false; + update_pipeline_state(); } bool PlaybackManager::is_playing() diff --git a/Libraries/LibMedia/PlaybackManager.h b/Libraries/LibMedia/PlaybackManager.h index 086e5f5841..0635e50981 100644 --- a/Libraries/LibMedia/PlaybackManager.h +++ b/Libraries/LibMedia/PlaybackManager.h @@ -106,6 +106,7 @@ private: Track track; NonnullRefPtr producer; RefPtr display; + PipelineStatus sink_status { PipelineStatus::HaveData }; }; using VideoTrackDatas = Vector; @@ -126,7 +127,9 @@ private: void set_up_producers(); void on_audio_sink_state_changed(PipelineStatus); void on_video_sink_state_changed(Track const&, PipelineStatus); - void update_buffering_state(); + void update_pipeline_state(); + void reset_pipeline_state(); + PipelineStatus combined_pipeline_status() const; void check_for_duration_change(AK::Duration); void dispatch_error(DecoderError&&); @@ -182,9 +185,7 @@ private: AK::Duration m_duration; Optional m_start_time_realtime; - bool m_audio_buffering { false }; - HashTable m_video_tracks_buffering; - bool m_was_buffering { false }; + PipelineStatus m_audio_sink_status { PipelineStatus::HaveData }; bool m_is_in_error_state { false }; }; diff --git a/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp new file mode 100644 index 0000000000..765e44a9e2 --- /dev/null +++ b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp @@ -0,0 +1,19 @@ +/* + * Copyright (c) 2025-2026, Gregory Bertilson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "BufferingStateHandler.h" + +#include + +namespace Media { + +void BufferingStateHandler::on_pipeline_status_changed(PipelineStatus status) +{ + if (status != PipelineStatus::Blocked) + resume(); +} + +} diff --git a/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.h b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.h index b213f1d4f3..905ff957a7 100644 --- a/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.h @@ -30,14 +30,7 @@ public: return AvailableData::Current; } - virtual void enter_buffering() override - { - } - - virtual void exit_buffering() override - { - resume(); - } + virtual void on_pipeline_status_changed(PipelineStatus) override; }; } diff --git a/Libraries/LibMedia/PlaybackStates/PausedStateHandler.h b/Libraries/LibMedia/PlaybackStates/PausedStateHandler.h index 2d31e36499..ba108ff869 100644 --- a/Libraries/LibMedia/PlaybackStates/PausedStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/PausedStateHandler.h @@ -37,9 +37,6 @@ public: { return AvailableData::Future; } - - virtual void enter_buffering() override { } - virtual void exit_buffering() override { } }; } diff --git a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp index e000c97b2b..cac07d9c1a 100644 --- a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp +++ b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp @@ -21,4 +21,9 @@ void PlaybackStateHandler::seek(AK::Duration timestamp, SeekMode mode) manager().replace_state_handler(manager().is_playing(), timestamp, mode); } +void PlaybackStateHandler::on_pipeline_status_changed(PipelineStatus status) +{ + (void)status; +} + } diff --git a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.h b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.h index 62a43a1af3..518c219e4c 100644 --- a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.h @@ -37,11 +37,7 @@ public: virtual PlaybackState state() = 0; virtual AvailableData available_data() = 0; - virtual void enter_buffering() = 0; - virtual void exit_buffering() = 0; - - virtual void on_audio_sink_state_changed(PipelineStatus) { } - virtual void on_video_sink_state_changed(Track const&, PipelineStatus) { } + virtual void on_pipeline_status_changed(PipelineStatus); protected: PlaybackManager& manager() const { return m_manager; } diff --git a/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp new file mode 100644 index 0000000000..f837ee6dc3 --- /dev/null +++ b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp @@ -0,0 +1,26 @@ +/* + * Copyright (c) 2025-2026, Gregory Bertilson + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "PlayingStateHandler.h" + +#include +#include +#include + +namespace Media { + +void PlayingStateHandler::pause() +{ + manager().replace_state_handler(); +} + +void PlayingStateHandler::on_pipeline_status_changed(PipelineStatus status) +{ + if (status == PipelineStatus::Blocked) + manager().replace_state_handler(true); +} + +} diff --git a/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.h b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.h index d6d65f7ae5..f46ce6d22b 100644 --- a/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.h @@ -7,9 +7,7 @@ #pragma once #include -#include #include -#include namespace Media { @@ -31,10 +29,7 @@ public: } virtual void play() override { } - virtual void pause() override - { - manager().replace_state_handler(); - } + virtual void pause() override; virtual bool is_playing() override { @@ -49,11 +44,7 @@ public: return AvailableData::Future; } - virtual void enter_buffering() override - { - manager().replace_state_handler(true); - } - virtual void exit_buffering() override { } + virtual void on_pipeline_status_changed(PipelineStatus) override; }; } diff --git a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h index be84dea22d..53c0fe50c3 100644 --- a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h @@ -6,7 +6,6 @@ #pragma once -#include #include #include #include @@ -36,11 +35,7 @@ public: begin_seek(); } - virtual void on_exit() override - { - VERIFY(m_video_seeks_pending.is_empty()); - VERIFY(!m_audio_seek_pending); - } + virtual void on_exit() override { } virtual AK::Duration current_time() const override { return m_chosen_timestamp; } @@ -60,23 +55,12 @@ public: return AvailableData::Current; } - virtual void enter_buffering() override { } - virtual void exit_buffering() override { } - - virtual void on_audio_sink_state_changed(PipelineStatus status) override + virtual void on_pipeline_status_changed(PipelineStatus status) override { if (!resolves_seek(status)) return; - m_audio_seek_pending = false; - possibly_complete_seek(); - } - virtual void on_video_sink_state_changed(Track const& track, PipelineStatus status) override - { - if (!resolves_seek(status)) - return; - m_video_seeks_pending.remove(track); - possibly_complete_seek(); + resume(); } private: @@ -98,40 +82,22 @@ private: void begin_seek() { m_chosen_timestamp = choose_timestamp(); - m_video_seeks_pending.clear(); - m_audio_seek_pending = false; for (auto& video_track_data : manager().m_video_track_datas) { if (video_track_data.display == nullptr) continue; - m_video_seeks_pending.set(video_track_data.track); video_track_data.display->seek(m_chosen_timestamp); } - if (manager().m_audio_sink) { - m_audio_seek_pending = true; + if (manager().m_audio_sink) manager().m_audio_sink->seek(m_chosen_timestamp); - } else { + else manager().m_time_provider->seek(m_chosen_timestamp); - } - - possibly_complete_seek(); - } - - void possibly_complete_seek() - { - if (m_audio_seek_pending) - return; - if (!m_video_seeks_pending.is_empty()) - return; - resume(); } AK::Duration m_target_timestamp; SeekMode m_mode { SeekMode::Accurate }; AK::Duration m_chosen_timestamp { AK::Duration::zero() }; - HashTable m_video_seeks_pending; - bool m_audio_seek_pending { false }; }; } diff --git a/Libraries/LibMedia/PlaybackStates/StartingStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/StartingStateHandler.cpp index 6f4caf921d..981628dea4 100644 --- a/Libraries/LibMedia/PlaybackStates/StartingStateHandler.cpp +++ b/Libraries/LibMedia/PlaybackStates/StartingStateHandler.cpp @@ -7,7 +7,6 @@ #include "StartingStateHandler.h" #include -#include namespace Media { @@ -15,13 +14,15 @@ void StartingStateHandler::start() { m_started = true; - if (!manager().m_audio_buffering && manager().m_video_tracks_buffering.is_empty()) + if (!m_pipeline_blocked) resume(); } -void StartingStateHandler::exit_buffering() +void StartingStateHandler::on_pipeline_status_changed(PipelineStatus status) { - if (m_started) + m_pipeline_blocked = status == PipelineStatus::Blocked; + + if (m_started && !m_pipeline_blocked) resume(); } diff --git a/Libraries/LibMedia/PlaybackStates/StartingStateHandler.h b/Libraries/LibMedia/PlaybackStates/StartingStateHandler.h index 230b7e3801..4f0dc7231e 100644 --- a/Libraries/LibMedia/PlaybackStates/StartingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/StartingStateHandler.h @@ -29,11 +29,11 @@ public: return AvailableData::None; } - virtual void enter_buffering() override { } - virtual void exit_buffering() override; + virtual void on_pipeline_status_changed(PipelineStatus) override; private: bool m_started { false }; + bool m_pipeline_blocked { false }; }; }