From 9e2a82088440fdc17878052af2dc17fbb133d95e Mon Sep 17 00:00:00 2001 From: Zaggy1024 Date: Tue, 9 Jun 2026 17:44:01 -0500 Subject: [PATCH] LibMedia+LibWeb: End media element playback based on the pipeline EOS Instead of comparing the current time to the duration, the playback manager now has an explicit Ended state that jumps to the duration. The element simply reacts to that to trigger the ended event and attribute, along with all the other steps involved. This moves the ended event to fire after the seeked event, which matches other browsers' behavior. The spec doesn't explicitly say which order they should fire in. --- .../PlaybackStates/BufferingStateHandler.cpp | 6 +++ .../PlaybackStates/EndedStateHandler.h | 51 +++++++++++++++++++ Libraries/LibMedia/PlaybackStates/Forward.h | 3 +- .../LibMedia/PlaybackStates/PlaybackState.h | 3 ++ .../PlaybackStates/PlaybackStateHandler.cpp | 4 +- .../PlaybackStates/PlayingStateHandler.cpp | 8 ++- .../PlaybackStates/SeekingStateHandler.h | 5 ++ Libraries/LibWeb/HTML/HTMLMediaElement.cpp | 16 +++--- ...eoElement-resize-event-during-playback.txt | 2 +- 9 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 Libraries/LibMedia/PlaybackStates/EndedStateHandler.h diff --git a/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp index 765e44a9e2..3aed98eb74 100644 --- a/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp +++ b/Libraries/LibMedia/PlaybackStates/BufferingStateHandler.cpp @@ -7,11 +7,17 @@ #include "BufferingStateHandler.h" #include +#include namespace Media { void BufferingStateHandler::on_pipeline_status_changed(PipelineStatus status) { + if (status == PipelineStatus::EndOfStream) { + manager().replace_state_handler(); + return; + } + if (status != PipelineStatus::Blocked) resume(); } diff --git a/Libraries/LibMedia/PlaybackStates/EndedStateHandler.h b/Libraries/LibMedia/PlaybackStates/EndedStateHandler.h new file mode 100644 index 0000000000..e17b654bad --- /dev/null +++ b/Libraries/LibMedia/PlaybackStates/EndedStateHandler.h @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2026-present, the Ladybird developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Media { + +class EndedStateHandler final : public PlaybackStateHandler { +public: + EndedStateHandler(PlaybackManager& manager) + : PlaybackStateHandler(manager) + { + } + virtual ~EndedStateHandler() override = default; + + virtual void on_enter() override + { + manager().m_time_provider->pause(); + } + virtual void on_exit() override { } + + virtual AK::Duration current_time() const override + { + return manager().duration(); + } + + virtual void play() override { } + virtual void pause() override { } + + virtual bool is_playing() override + { + return false; + } + virtual PlaybackState state() override + { + return PlaybackState::Ended; + } + virtual AvailableData available_data() override + { + return AvailableData::Current; + } + + virtual void on_pipeline_status_changed(PipelineStatus) override { } +}; + +} diff --git a/Libraries/LibMedia/PlaybackStates/Forward.h b/Libraries/LibMedia/PlaybackStates/Forward.h index d215f4a039..a871fbb737 100644 --- a/Libraries/LibMedia/PlaybackStates/Forward.h +++ b/Libraries/LibMedia/PlaybackStates/Forward.h @@ -15,7 +15,8 @@ X(PlayingStateHandler) \ X(PausedStateHandler) \ X(ResumingStateHandler) \ - X(SeekingStateHandler) + X(SeekingStateHandler) \ + X(EndedStateHandler) namespace Media { diff --git a/Libraries/LibMedia/PlaybackStates/PlaybackState.h b/Libraries/LibMedia/PlaybackStates/PlaybackState.h index fea0c89a50..7465a61ef3 100644 --- a/Libraries/LibMedia/PlaybackStates/PlaybackState.h +++ b/Libraries/LibMedia/PlaybackStates/PlaybackState.h @@ -17,6 +17,7 @@ enum class PlaybackState : u8 { Playing, Paused, Seeking, + Ended, }; constexpr StringView playback_state_to_string(PlaybackState state) @@ -32,6 +33,8 @@ constexpr StringView playback_state_to_string(PlaybackState state) return "Paused"sv; case PlaybackState::Seeking: return "Seeking"sv; + case PlaybackState::Ended: + return "Ended"sv; } return "Invalid"sv; } diff --git a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp index cac07d9c1a..f616ec5a86 100644 --- a/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp +++ b/Libraries/LibMedia/PlaybackStates/PlaybackStateHandler.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include "PlaybackStateHandler.h" @@ -23,7 +24,8 @@ void PlaybackStateHandler::seek(AK::Duration timestamp, SeekMode mode) void PlaybackStateHandler::on_pipeline_status_changed(PipelineStatus status) { - (void)status; + if (status == PipelineStatus::EndOfStream) + manager().replace_state_handler(); } } diff --git a/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp index f837ee6dc3..a804475d64 100644 --- a/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp +++ b/Libraries/LibMedia/PlaybackStates/PlayingStateHandler.cpp @@ -8,6 +8,7 @@ #include #include +#include #include namespace Media { @@ -19,8 +20,13 @@ void PlayingStateHandler::pause() void PlayingStateHandler::on_pipeline_status_changed(PipelineStatus status) { - if (status == PipelineStatus::Blocked) + if (status == PipelineStatus::Blocked) { manager().replace_state_handler(true); + return; + } + + if (status == PipelineStatus::EndOfStream) + manager().replace_state_handler(); } } diff --git a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h index 53c0fe50c3..36f926f2bf 100644 --- a/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h +++ b/Libraries/LibMedia/PlaybackStates/SeekingStateHandler.h @@ -60,6 +60,11 @@ public: if (!resolves_seek(status)) return; + if (status == PipelineStatus::EndOfStream) { + PlaybackStateHandler::on_pipeline_status_changed(status); + return; + } + resume(); } diff --git a/Libraries/LibWeb/HTML/HTMLMediaElement.cpp b/Libraries/LibWeb/HTML/HTMLMediaElement.cpp index 4dee8efe7d..8c586b7423 100644 --- a/Libraries/LibWeb/HTML/HTMLMediaElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLMediaElement.cpp @@ -459,11 +459,6 @@ void HTMLMediaElement::set_current_playback_position(double playback_position) time_marches_on(); - // NOTE: Invoking the following steps is not listed in the spec. Rather, the spec just describes the scenario in - // which these steps should be invoked, which is when we've reached the end of the media playback. - if (m_current_playback_position == m_duration) - reached_end_of_media_playback(); - upon_has_ended_playback_possibly_changed(); update_natural_dimensions(); @@ -2043,6 +2038,8 @@ void HTMLMediaElement::forget_media_resource_specific_tracks() // of text tracks all the media-resource-specific text tracks, then empty the media element's audioTracks attribute's AudioTrackList object, then // empty the media element's videoTracks attribute's VideoTrackList object. No events (in particular, no removetrack events) are fired as part of // this; the error and emptied events, fired by the algorithms that invoke this one, can be used instead. + if (m_playback_manager) + m_playback_manager->on_playback_state_change = nullptr; m_audio_tracks->remove_all_tracks(); m_video_tracks->remove_all_tracks(); m_playback_manager.clear(); @@ -2269,6 +2266,10 @@ void HTMLMediaElement::on_playback_manager_state_change() auto state = m_playback_manager->state(); if (seeking() && state != Media::PlaybackState::Seeking) finish_seeking_element(); + if (state == Media::PlaybackState::Ended && !m_error) { + set_current_playback_position(m_duration); + reached_end_of_media_playback(); + } // NB: Queue the readyState update as a task so that it will never run before the durationchange and loadedmetadata // events are fired. This ensures that readyState has a deterministic value in those events. @@ -2699,10 +2700,13 @@ bool HTMLMediaElement::has_ended_playback() const if (m_ready_state < ReadyState::HaveMetadata) return false; + VERIFY(m_playback_manager != nullptr); // Either: if ( // The current playback position is the end of the media resource, and - m_current_playback_position == m_duration && + // NB: This is represented by the playback manager's Ended state, which is only entered once the pipeline has + // consumed all real media data. + m_playback_manager->state() == Media::PlaybackState::Ended && // The direction of playback is forwards, and direction_of_playback() == PlaybackDirection::Forwards && diff --git a/Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-resize-event-during-playback.txt b/Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-resize-event-during-playback.txt index 670c49a1c4..1b608bcd60 100644 --- a/Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-resize-event-during-playback.txt +++ b/Tests/LibWeb/Text/expected/HTML/HTMLVideoElement-resize-event-during-playback.txt @@ -8,5 +8,5 @@ resize: videoWidth=320 videoHeight=240 seeked: videoWidth=320 videoHeight=240 --- seek to end --- resize: videoWidth=640 videoHeight=480 -ended: videoWidth=640 videoHeight=480 seeked: videoWidth=640 videoHeight=480 +ended: videoWidth=640 videoHeight=480