From 393023d226053bfc433189e4c8e635af811ef3a3 Mon Sep 17 00:00:00 2001 From: Alicia Boya Garcia Date: Tue, 14 Jan 2025 17:27:41 -0800 Subject: [PATCH 1/4] [GStreamer] Fix "pipeline and player states are not synchronized" assertion crash https://bugs.webkit.org/show_bug.cgi?id=285850 Reviewed by Philippe Normand. WebKit pauses muted videos when scrolling to save resources. However, when running in Debug, an assertion was failing inside MediaPlayerPrivateGStreamer::paused() in the GStreamer ports: > ASSERTION FAILED: pipeline and player states are not synchronized After some debugging, I found two problems that were causing the assertion to fail: (1) MediaPlayerPrivateGStreamerMSE::updateStates() didn't account for this behavior, which could trigger unexpected state changes. The patch adds it to the `shouldBePlaying` check. (2) The assertion checks m_isPipelinePlaying against the actual state of the pipeline. However, the code setting the pipeline to PAUSED when scrolling away (see setVisibleInViewport()) didn't update this field. Normally you use setPipelineState() instead which updates this field. As drive-by fixes this patch also adds new logs and renames two fields to have more useful names. * m_isVisibleInViewport is negated and renamed m_isPausedByViewport, reflecting its actual meaning (`m_isVisibleInViewport` was often true while the HTMLMediaElement was not visible in the viewport. * m_isVisible is renamed to m_pageIsVisible to both match other code and to reflect its actual meaning. This patch adds back media-source-muted-scroll-and-seek-crash.html from 276798@main, which can be used to test the crash does not happen. Note however the same caveat from back then applies: > Currently, part of the code to trigger the crash isn't executed > due to WEBKIT_GST_ALLOW_PLAYBACK_OF_INVISIBLE_VIDEOS=1 being set in the test driver in > Tools/Scripts/webkitpy/port/glib.py (264017@main), which needs to be commented manually. > We should find a way to add a test preference for this code path to be enabled. * LayoutTests/media/media-source/media-source-muted-scroll-and-seek-crash.html: Added. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::paused const): (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): (WebCore::MediaPlayerPrivateGStreamer::setVisibleInViewport): (WebCore::MediaPlayerPrivateGStreamer::paint): * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::updateStates): Canonical link: https://commits.webkit.org/288906@main --- .../gstreamer/MediaPlayerPrivateGStreamer.cpp | 12 ++++++------ .../graphics/gstreamer/MediaPlayerPrivateGStreamer.h | 6 ++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp index 273c3c9c4afe9..34a743b42287a 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp @@ -4315,7 +4315,7 @@ void MediaPlayerPrivateGStreamer::paint(GraphicsContext& context, const FloatRec if (context.paintingDisabled()) return; - if (!m_visible || m_isPausedByViewport) + if (!m_pageIsVisible || m_isPausedByViewport) return; // Keep a reference to the sample to avoid keeping the sampleMutex locked, which would be prone @@ -4934,7 +4934,7 @@ void MediaPlayerPrivateGStreamer::setVideoRectangle(const IntRect& rect) Locker locker { m_holePunchLock }; - if (!m_visible || m_suspended) + if (!m_pageIsVisible || m_suspended) return; if (m_quirksManagerForTesting) { @@ -4948,18 +4948,18 @@ void MediaPlayerPrivateGStreamer::setVideoRectangle(const IntRect& rect) void MediaPlayerPrivateGStreamer::setPageIsVisible(bool visible) { - if (m_visible == visible) + if (m_pageIsVisible == visible) return; if (!isHolePunchRenderingEnabled() || !m_videoSink) { - m_visible = visible; + m_pageIsVisible = visible; return; } Locker locker { m_holePunchLock }; - m_visible = visible; + m_pageIsVisible = visible; - if (!m_visible) { + if (!m_pageIsVisible) { if (m_quirksManagerForTesting) { m_quirksManagerForTesting->setHolePunchVideoRectangle(m_videoSink.get(), IntRect()); return; diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h index 5dfff9dd95165..854ed3d9aeb5f 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h @@ -666,7 +666,10 @@ class MediaPlayerPrivateGStreamer #endif bool m_isMuted { false }; - bool m_visible { false }; + + // Whether the page containing the HTMLMediaElement is visible, reflects: setPageIsVisible() + bool m_pageIsVisible { false }; + bool m_suspended { false }; // playbin3 only: @@ -721,7 +724,6 @@ class MediaPlayerPrivateGStreamer bool m_didTryToRecoverPlayingState { false }; - // The state the pipeline should be set back to after the player becomes visible in the viewport again. GstState m_invisiblePlayerState { GST_STATE_VOID_PENDING }; // Specific to MediaStream playback. From e2e9a16b3ec5a4eebb31cc1081a81833fb55f85f Mon Sep 17 00:00:00 2001 From: Alicia Boya Garcia Date: Fri, 7 Feb 2025 05:35:40 -0800 Subject: [PATCH 2/4] [GStreamer] Refactor away MediaPlayerPrivateGStreamer::m_isPausedByViewport https://bugs.webkit.org/show_bug.cgi?id=287162 Reviewed by Philippe Normand. This is a follow-up of the cleanups discussed in https://github.com/WebKit/WebKit/pull/39765. This patch combines m_isPausedByViewport and m_invisiblePlayerState into one single field and renames it to m_stateToRestoreWhenVisible. The field is set to VOID_PENDING when we're not paused by viewport. Additionally, safeguards have been added to setVisibleInViewport() to prevent the suspension code from accidentally running more than once. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): (WebCore::MediaPlayerPrivateGStreamer::setVisibleInViewport): (WebCore::MediaPlayerPrivateGStreamer::paint): * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: (WebCore::MediaPlayerPrivateGStreamer::isPausedByViewport): * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::updateStates): Canonical link: https://commits.webkit.org/289997@main --- .../gstreamer/MediaPlayerPrivateGStreamer.cpp | 30 +++++++++---------- .../gstreamer/MediaPlayerPrivateGStreamer.h | 5 ++-- .../mse/MediaPlayerPrivateGStreamerMSE.cpp | 2 +- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp index 34a743b42287a..154550f9b79cb 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp @@ -1088,9 +1088,9 @@ MediaPlayerPrivateGStreamer::ChangePipelineStateResult MediaPlayerPrivateGStream { ASSERT(m_pipeline); - if (m_isPausedByViewport && newState > GST_STATE_PAUSED) { + if (isPausedByViewport() && newState > GST_STATE_PAUSED) { GST_DEBUG_OBJECT(pipeline(), "Saving state for when player becomes visible: %s", gst_element_state_get_name(newState)); - m_invisiblePlayerState = newState; + m_stateToRestoreWhenVisible = newState; return ChangePipelineStateResult::Ok; } @@ -4282,26 +4282,26 @@ void MediaPlayerPrivateGStreamer::setVisibleInViewport(bool isVisible) if ((player && !player->isVideoPlayer()) || !m_isMuted) return; - if (!isVisible) { + if (!isVisible && !isPausedByViewport()) { GstState currentState, pendingState; gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GstState targetState = (pendingState != GST_STATE_VOID_PENDING ? pendingState : currentState); - if (targetState > GST_STATE_NULL) - m_invisiblePlayerState = targetState; - m_isPausedByViewport = true; + if (targetState == GST_STATE_NULL) { + GST_DEBUG_OBJECT(pipeline(), "Pipeline is already in NULL state, no point in suspending the player."); + return; + } + m_stateToRestoreWhenVisible = targetState; GST_DEBUG_OBJECT(pipeline(), "Media element is muted and not visible in viewport, pausing it to save resources. Will resume afterwards to %s state.", - gst_element_state_get_name(m_invisiblePlayerState)); + gst_element_state_get_name(m_stateToRestoreWhenVisible)); gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED); gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GST_DEBUG_OBJECT(pipeline(), "Now pipeline is in %s state with %s pending", gst_element_state_get_name(currentState), gst_element_state_get_name(pendingState)); m_isPipelinePlaying = false; - } else { - m_isPausedByViewport = false; - if (m_invisiblePlayerState != GST_STATE_VOID_PENDING) { - GST_DEBUG_OBJECT(pipeline(), "Element in viewport again, resuming playback via state change to %s.", - gst_element_state_get_name(m_invisiblePlayerState)); - changePipelineState(m_invisiblePlayerState); - } + } else if (isVisible && isPausedByViewport()) { + GST_DEBUG_OBJECT(pipeline(), "Element in viewport again, resuming playback via state change to %s.", + gst_element_state_get_name(m_stateToRestoreWhenVisible)); + changePipelineState(m_stateToRestoreWhenVisible); + m_stateToRestoreWhenVisible = GST_STATE_VOID_PENDING; } } @@ -4315,7 +4315,7 @@ void MediaPlayerPrivateGStreamer::paint(GraphicsContext& context, const FloatRec if (context.paintingDisabled()) return; - if (!m_pageIsVisible || m_isPausedByViewport) + if (!m_pageIsVisible || isPausedByViewport()) return; // Keep a reference to the sample to avoid keeping the sampleMutex locked, which would be prone diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h index 854ed3d9aeb5f..ee93563b1361f 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h @@ -451,7 +451,7 @@ class MediaPlayerPrivateGStreamer bool m_areVolumeAndMuteInitialized { false }; // Reflects whether the pipeline was paused due to the HTMLMediaElement being both muted and invisible in the viewport. - bool m_isPausedByViewport { false }; + bool isPausedByViewport() const { return m_stateToRestoreWhenVisible != GST_STATE_VOID_PENDING; }; #if USE(TEXTURE_MAPPER) OptionSet m_textureMapperFlags; @@ -724,7 +724,8 @@ class MediaPlayerPrivateGStreamer bool m_didTryToRecoverPlayingState { false }; - GstState m_invisiblePlayerState { GST_STATE_VOID_PENDING }; + // The state the pipeline should be set back to after the player becomes visible in the viewport again. + GstState m_stateToRestoreWhenVisible { GST_STATE_VOID_PENDING }; // Specific to MediaStream playback. MediaTime m_startTime; diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp index 18a99580198e6..3588d7a8464a6 100644 --- a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp @@ -459,7 +459,7 @@ void MediaPlayerPrivateGStreamerMSE::updateStates() { bool isWaitingPreroll = isPipelineWaitingPreroll(); bool shouldUpdatePlaybackState = false; - bool shouldBePlaying = (!m_isPaused && !m_isPausedByViewport && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused) + bool shouldBePlaying = (!m_isPaused && !isPausedByViewport() && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused) || m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying; GST_DEBUG_OBJECT(pipeline(), "shouldBePlaying = %s, m_isPipelinePlaying = %s, is seeking %s", boolForPrinting(shouldBePlaying), boolForPrinting(m_isPipelinePlaying), boolForPrinting(isWaitingPreroll)); From 538b5a32acd81cbf5165e4d2ebf6de5e2e797ec4 Mon Sep 17 00:00:00 2001 From: Vivienne Watermeier Date: Mon, 25 May 2026 13:12:21 +0200 Subject: [PATCH 3/4] [GStreamer] Rework suspending the player when muted and hidden https://bugs.webkit.org/show_bug.cgi?id=315755 Reviewed by NOBODY (OOPS!). When the player is both muted and invisible, we try to suspend it (by just pausing the playback pipeline), to save on resources. However so far this is happens quite inconsistently, with multiple issues: First, suspends are tied too much to visibility changes - (un)muting can't suspend/resume, so this refactors them out into `managePlayerSuspend()`, and also renames the relevant variables/methods to reflect that they're about suspends, not player visibility. Second, we are currently not able to suspend the player if it's already muted and hidden at creation: Either we ignore it because we don't have a pipeline yet, or because it's in NULL. In this scenario, we would also not yet have a target state to resume to, until e.g. a play() request later on, so we need reintroduce a separate boolean flag (m_isSuspended) to make sure we know to delay that state change until we resume. Finally, `m_isMuted` is currently updated inconsistently, and in some scenarios only from a notify::mute callback. This presents a race condition, if we try to suspend after muting the player, but read m_isMuted before the callback updates it. To avoid this, `isMuted()` now directly checks with the pipeline so we have a single source of truth. Only `notifyPlayerOfMute()` still uses the flag to avoid sending redundant notifications. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::play): (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): Clear saved state if a change < PLAYING is requested; so a sequence like (suspend -> play() -> pause() -> unsuspend) doesn't end up in the wrong state. (WebCore::MediaPlayerPrivateGStreamer::isMuted const): (WebCore::MediaPlayerPrivateGStreamer::setMuted): Also check here if we need to suspend. (WebCore::MediaPlayerPrivateGStreamer::createGSTPlayBin): After creating the pipeline, also check here if we need to suspend. (WebCore::MediaPlayerPrivateGStreamer::setViewportVisibility): (WebCore::MediaPlayerPrivateGStreamer::managePlayerSuspend): Added. Handles suspending/resuming the pipeline when needed. (WebCore::MediaPlayerPrivateGStreamer::paint): * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: (WebCore::MediaPlayerPrivateGStreamer::isSuspended const): (WebCore::MediaPlayerPrivateGStreamer::isPausedByViewport const): Deleted. * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::updateStates): Go forward with the state change even when suspended; changePipelineState() needs to save the new target state --- .../gstreamer/MediaPlayerPrivateGStreamer.cpp | 79 +++++++++++++------ .../gstreamer/MediaPlayerPrivateGStreamer.h | 18 ++++- .../mse/MediaPlayerPrivateGStreamerMSE.cpp | 4 +- 3 files changed, 69 insertions(+), 32 deletions(-) diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp index 154550f9b79cb..89fb9b57b57ce 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp @@ -495,11 +495,12 @@ void MediaPlayerPrivateGStreamer::play() return; } - if (changePipelineState(GST_STATE_PLAYING) == ChangePipelineStateResult::Ok) { + auto res = changePipelineState(GST_STATE_PLAYING); + if (res == ChangePipelineStateResult::Ok || res == ChangePipelineStateResult::SavedUntilResume) { m_isEndReached = false; m_isDelayingLoad = false; m_preload = MediaPlayer::Preload::Auto; - GST_INFO_OBJECT(pipeline(), "Play"); + GST_INFO_OBJECT(pipeline(), "Play%s", (res == ChangePipelineStateResult::Ok) ? "" : " (state change saved due to suspend)"); #if ENABLE(MEDIA_TELEMETRY) MediaTelemetryReport::singleton().reportPlaybackState(MediaTelemetryReport::AVPipelineState::Play); #endif @@ -1088,10 +1089,16 @@ MediaPlayerPrivateGStreamer::ChangePipelineStateResult MediaPlayerPrivateGStream { ASSERT(m_pipeline); - if (isPausedByViewport() && newState > GST_STATE_PAUSED) { - GST_DEBUG_OBJECT(pipeline(), "Saving state for when player becomes visible: %s", gst_element_state_get_name(newState)); - m_stateToRestoreWhenVisible = newState; - return ChangePipelineStateResult::Ok; + if (isSuspended()) { + // Save requests to play until we resume. + if (newState > GST_STATE_PAUSED) { + GST_DEBUG_OBJECT(pipeline(), "Saving state for when player is resumed: %s", gst_element_state_get_name(newState)); + m_stateToResume = newState; + return ChangePipelineStateResult::SavedUntilResume; + } + + // Any other state changes are ok to execute right away. + m_stateToResume = GST_STATE_VOID_PENDING; } GstState currentState, pending; @@ -1363,8 +1370,14 @@ MediaTime MediaPlayerPrivateGStreamer::platformDuration() const bool MediaPlayerPrivateGStreamer::isMuted() const { - GST_INFO_OBJECT(pipeline(), "Player is muted: %s", boolForPrinting(m_isMuted)); - return m_isMuted; + bool isMuted = false; + + auto streamVolume = GST_STREAM_VOLUME(m_pipeline.get()); + if (streamVolume) + isMuted = gst_stream_volume_get_mute(streamVolume); + + GST_INFO_OBJECT(pipeline(), "Player is muted: %s", boolForPrinting(isMuted)); + return isMuted; } void MediaPlayerPrivateGStreamer::commitLoad() @@ -1964,6 +1977,7 @@ void MediaPlayerPrivateGStreamer::setMuted(bool shouldMute) GST_INFO_OBJECT(pipeline(), "Setting muted state to %s", boolForPrinting(shouldMute)); g_object_set(m_volumeElement.get(), "mute", static_cast(shouldMute), nullptr); configureMediaStreamAudioTracks(); + managePlayerSuspend(); } void MediaPlayerPrivateGStreamer::notifyPlayerOfMute() @@ -3441,6 +3455,7 @@ void MediaPlayerPrivateGStreamer::createGSTPlayBin(const URL& url) player->videoSinkCapsChanged(videoSinkPad); }), this); + managePlayerSuspend(); #if ENABLE(MEDIA_TELEMETRY) MediaTelemetryReport::singleton().reportDrmInfo(getDrm()); MediaTelemetryReport::singleton().reportPlaybackState(MediaTelemetryReport::AVPipelineState::Create); @@ -4264,44 +4279,56 @@ void MediaPlayerPrivateGStreamer::flushCurrentBuffer() void MediaPlayerPrivateGStreamer::setVisibleInViewport(bool isVisible) { - if (isMediaStreamPlayer()) - return; + GST_DEBUG_OBJECT(pipeline(), "Player is now %svisible in the viewport", isVisible ? "" : "not "); - // Some layout tests (webgl) expect playback of invisible videos to not be suspended, so allow - // this using an environment variable, set from the webkitpy glib port sub-classes. - const char* allowPlaybackOfInvisibleVideos = g_getenv("WEBKIT_GST_ALLOW_PLAYBACK_OF_INVISIBLE_VIDEOS"); - if (!isVisible && allowPlaybackOfInvisibleVideos && !strcmp(allowPlaybackOfInvisibleVideos, "1")) + if (isMediaStreamPlayer() || isVisible == m_isVisibleInViewport) return; + m_isVisibleInViewport = isVisible; + managePlayerSuspend(); +} + +void MediaPlayerPrivateGStreamer::managePlayerSuspend() +{ if (!m_pipeline) return; RefPtr player = m_player.get(); - GST_INFO_OBJECT(m_pipeline.get(), "%s %s player %svisible in viewport", m_isMuted ? "Muted" : "Un-muted", (player && player->isVideoPlayer()) ? "video" : "audio", isVisible ? "" : "no longer "); - if ((player && !player->isVideoPlayer()) || !m_isMuted) - return; + // Some layout tests (webgl) expect playback of invisible videos to not be suspended, so allow + // this using an environment variable, set from the webkitpy glib port sub-classes. + const char* allowPlaybackOfInvisibleVideos = g_getenv("WEBKIT_GST_ALLOW_PLAYBACK_OF_INVISIBLE_VIDEOS"); + bool muted = isMuted(); + bool shouldBeSuspended = (player && player->isVideoPlayer()) && muted && !m_isVisibleInViewport && allowPlaybackOfInvisibleVideos && !strcmp(allowPlaybackOfInvisibleVideos, "1"); + GST_INFO_OBJECT(m_pipeline.get(), "%s %s player %svisible in viewport", muted ? "Muted" : "Un-muted", (player && player->isVideoPlayer()) ? "video" : "audio", m_isVisibleInViewport ? "" : "not "); - if (!isVisible && !isPausedByViewport()) { + if (shouldBeSuspended && !isSuspended()) { GstState currentState, pendingState; gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GstState targetState = (pendingState != GST_STATE_VOID_PENDING ? pendingState : currentState); + m_isSuspended = true; if (targetState == GST_STATE_NULL) { - GST_DEBUG_OBJECT(pipeline(), "Pipeline is already in NULL state, no point in suspending the player."); + GST_DEBUG_OBJECT(pipeline(), "Pipeline is already in NULL state, no point in pausing the player."); return; } - m_stateToRestoreWhenVisible = targetState; + m_stateToResume = targetState; GST_DEBUG_OBJECT(pipeline(), "Media element is muted and not visible in viewport, pausing it to save resources. Will resume afterwards to %s state.", gst_element_state_get_name(m_stateToRestoreWhenVisible)); gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED); gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GST_DEBUG_OBJECT(pipeline(), "Now pipeline is in %s state with %s pending", gst_element_state_get_name(currentState), gst_element_state_get_name(pendingState)); m_isPipelinePlaying = false; - } else if (isVisible && isPausedByViewport()) { - GST_DEBUG_OBJECT(pipeline(), "Element in viewport again, resuming playback via state change to %s.", - gst_element_state_get_name(m_stateToRestoreWhenVisible)); - changePipelineState(m_stateToRestoreWhenVisible); - m_stateToRestoreWhenVisible = GST_STATE_VOID_PENDING; + } else if (!shouldBeSuspended && isSuspended()) { + m_isSuspended = false; + + if (m_stateToResume == GST_STATE_VOID_PENDING) + return; + + GstState resumeState = m_stateToResume; + m_stateToResume = GST_STATE_VOID_PENDING; + GST_DEBUG_OBJECT(pipeline(), "Element is either unmuted or in viewport again, resuming playback via state change to %s.", + gst_element_state_get_name(resumeState)); + changePipelineState(resumeState); } } @@ -4315,7 +4342,7 @@ void MediaPlayerPrivateGStreamer::paint(GraphicsContext& context, const FloatRec if (context.paintingDisabled()) return; - if (!m_pageIsVisible || isPausedByViewport()) + if (!m_pageIsVisible || isSuspended()) return; // Keep a reference to the sample to avoid keeping the sampleMutex locked, which would be prone diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h index ee93563b1361f..4eb4d498adbd0 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h @@ -332,6 +332,8 @@ class MediaPlayerPrivateGStreamer Ok, Rejected, Failed, + // Pipeline is suspended, and the requested state change was saved to be executed when it resumes. + SavedUntilResume, }; ChangePipelineStateResult changePipelineState(GstState); @@ -450,8 +452,8 @@ class MediaPlayerPrivateGStreamer GRefPtr m_source { nullptr }; bool m_areVolumeAndMuteInitialized { false }; - // Reflects whether the pipeline was paused due to the HTMLMediaElement being both muted and invisible in the viewport. - bool isPausedByViewport() const { return m_stateToRestoreWhenVisible != GST_STATE_VOID_PENDING; }; + // Reflects whether the pipeline was suspended due to the HTMLMediaElement being both muted and invisible in the viewport. + bool isSuspended() const { return m_isSuspended; }; #if USE(TEXTURE_MAPPER) OptionSet m_textureMapperFlags; @@ -564,6 +566,8 @@ class MediaPlayerPrivateGStreamer void finishSeek(); virtual void didPreroll() { } + void managePlayerSuspend(); + void createGSTPlayBin(const URL&); bool loadNextLocation(); @@ -665,8 +669,13 @@ class MediaPlayerPrivateGStreamer RefPtr m_streamPrivate; #endif + // Only notifyPlayerOfMute uses this to avoid sending redundant notifications. + // Since it's updated by a callback, this will be incorrect right after un/muting the player, + // use isMuted() instead. bool m_isMuted { false }; + bool m_isVisibleInViewport { true }; + // Whether the page containing the HTMLMediaElement is visible, reflects: setPageIsVisible() bool m_pageIsVisible { false }; @@ -724,8 +733,9 @@ class MediaPlayerPrivateGStreamer bool m_didTryToRecoverPlayingState { false }; - // The state the pipeline should be set back to after the player becomes visible in the viewport again. - GstState m_stateToRestoreWhenVisible { GST_STATE_VOID_PENDING }; + bool m_isSuspended { false }; + // The state the pipeline should be set back to after the player is resumed. + GstState m_stateToResume { GST_STATE_VOID_PENDING }; // Specific to MediaStream playback. MediaTime m_startTime; diff --git a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp index 3588d7a8464a6..e93b67bed0697 100644 --- a/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp @@ -459,7 +459,7 @@ void MediaPlayerPrivateGStreamerMSE::updateStates() { bool isWaitingPreroll = isPipelineWaitingPreroll(); bool shouldUpdatePlaybackState = false; - bool shouldBePlaying = (!m_isPaused && !isPausedByViewport() && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused) + bool shouldBePlaying = (!m_isPaused && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused) || m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying; GST_DEBUG_OBJECT(pipeline(), "shouldBePlaying = %s, m_isPipelinePlaying = %s, is seeking %s", boolForPrinting(shouldBePlaying), boolForPrinting(m_isPipelinePlaying), boolForPrinting(isWaitingPreroll)); @@ -467,7 +467,7 @@ void MediaPlayerPrivateGStreamerMSE::updateStates() auto result = changePipelineState(GST_STATE_PLAYING); if (result == ChangePipelineStateResult::Failed) GST_ERROR_OBJECT(pipeline(), "Setting the pipeline to PLAYING failed"); - else if (result == ChangePipelineStateResult::Ok) { + else if (result == ChangePipelineStateResult::Ok || result == ChangePipelineStateResult::SavedUntilResume) { m_playbackRatePausedState = PlaybackRatePausedState::Playing; shouldUpdatePlaybackState = true; } From 6fb77b7897a385167fcba642220d84f9e8871cd7 Mon Sep 17 00:00:00 2001 From: Vivienne Watermeier Date: Wed, 3 Jun 2026 18:33:22 +0200 Subject: [PATCH 4/4] [GStreamer] Rename m_suspended and m_isSuspended to disambiguate --- .../gstreamer/MediaPlayerPrivateGStreamer.cpp | 24 +++++++++---------- .../gstreamer/MediaPlayerPrivateGStreamer.h | 6 ++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp index 89fb9b57b57ce..b6bf68daa0cf8 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp @@ -1089,7 +1089,7 @@ MediaPlayerPrivateGStreamer::ChangePipelineStateResult MediaPlayerPrivateGStream { ASSERT(m_pipeline); - if (isSuspended()) { + if (playerIsSuspended()) { // Save requests to play until we resume. if (newState > GST_STATE_PAUSED) { GST_DEBUG_OBJECT(pipeline(), "Saving state for when player is resumed: %s", gst_element_state_get_name(newState)); @@ -4302,24 +4302,24 @@ void MediaPlayerPrivateGStreamer::managePlayerSuspend() bool shouldBeSuspended = (player && player->isVideoPlayer()) && muted && !m_isVisibleInViewport && allowPlaybackOfInvisibleVideos && !strcmp(allowPlaybackOfInvisibleVideos, "1"); GST_INFO_OBJECT(m_pipeline.get(), "%s %s player %svisible in viewport", muted ? "Muted" : "Un-muted", (player && player->isVideoPlayer()) ? "video" : "audio", m_isVisibleInViewport ? "" : "not "); - if (shouldBeSuspended && !isSuspended()) { + if (shouldBeSuspended && !playerIsSuspended()) { GstState currentState, pendingState; gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GstState targetState = (pendingState != GST_STATE_VOID_PENDING ? pendingState : currentState); - m_isSuspended = true; + m_playerIsSuspended = true; if (targetState == GST_STATE_NULL) { GST_DEBUG_OBJECT(pipeline(), "Pipeline is already in NULL state, no point in pausing the player."); return; } m_stateToResume = targetState; GST_DEBUG_OBJECT(pipeline(), "Media element is muted and not visible in viewport, pausing it to save resources. Will resume afterwards to %s state.", - gst_element_state_get_name(m_stateToRestoreWhenVisible)); + gst_element_state_get_name(m_stateToResume)); gst_element_set_state(m_pipeline.get(), GST_STATE_PAUSED); gst_element_get_state(m_pipeline.get(), ¤tState, &pendingState, 0); GST_DEBUG_OBJECT(pipeline(), "Now pipeline is in %s state with %s pending", gst_element_state_get_name(currentState), gst_element_state_get_name(pendingState)); m_isPipelinePlaying = false; - } else if (!shouldBeSuspended && isSuspended()) { - m_isSuspended = false; + } else if (!shouldBeSuspended && playerIsSuspended()) { + m_playerIsSuspended = false; if (m_stateToResume == GST_STATE_VOID_PENDING) return; @@ -4342,7 +4342,7 @@ void MediaPlayerPrivateGStreamer::paint(GraphicsContext& context, const FloatRec if (context.paintingDisabled()) return; - if (!m_pageIsVisible || isSuspended()) + if (!m_pageIsVisible || playerIsSuspended()) return; // Keep a reference to the sample to avoid keeping the sampleMutex locked, which would be prone @@ -4961,7 +4961,7 @@ void MediaPlayerPrivateGStreamer::setVideoRectangle(const IntRect& rect) Locker locker { m_holePunchLock }; - if (!m_pageIsVisible || m_suspended) + if (!m_pageIsVisible || m_pageIsSuspended) return; if (m_quirksManagerForTesting) { @@ -4999,18 +4999,18 @@ void MediaPlayerPrivateGStreamer::setPageIsVisible(bool visible) void MediaPlayerPrivateGStreamer::setPageIsSuspended(bool suspended) { - if (m_suspended == suspended) + if (m_pageIsSuspended == suspended) return; if (!isHolePunchRenderingEnabled() || !m_videoSink) { - m_suspended = suspended; + m_pageIsSuspended = suspended; return; } Locker locker { m_holePunchLock }; - m_suspended = suspended; + m_pageIsSuspended = suspended; - if (m_suspended) { + if (m_pageIsSuspended) { if (m_quirksManagerForTesting) { m_quirksManagerForTesting->setHolePunchVideoRectangle(m_videoSink.get(), IntRect()); return; diff --git a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h index 4eb4d498adbd0..8f065e4e95f97 100644 --- a/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h +++ b/Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h @@ -453,7 +453,7 @@ class MediaPlayerPrivateGStreamer bool m_areVolumeAndMuteInitialized { false }; // Reflects whether the pipeline was suspended due to the HTMLMediaElement being both muted and invisible in the viewport. - bool isSuspended() const { return m_isSuspended; }; + bool playerIsSuspended() const { return m_playerIsSuspended; }; #if USE(TEXTURE_MAPPER) OptionSet m_textureMapperFlags; @@ -679,7 +679,7 @@ class MediaPlayerPrivateGStreamer // Whether the page containing the HTMLMediaElement is visible, reflects: setPageIsVisible() bool m_pageIsVisible { false }; - bool m_suspended { false }; + bool m_pageIsSuspended { false }; // playbin3 only: bool m_waitingForStreamsSelectedEvent { true }; @@ -733,7 +733,7 @@ class MediaPlayerPrivateGStreamer bool m_didTryToRecoverPlayingState { false }; - bool m_isSuspended { false }; + bool m_playerIsSuspended { false }; // The state the pipeline should be set back to after the player is resumed. GstState m_stateToResume { GST_STATE_VOID_PENDING };