Skip to content

Commit 52ff406

Browse files
asurdej-comcastcalvaris
authored andcommitted
[GStreamer] Ignore sinks position while seeking
https://bugs.webkit.org/show_bug.cgi?id=272167 Reviewed by Alicia Boya Garcia. After removing MSE data, sinks get flushed and are not able to return valid playback time. Some implementation return invalid time, 0.00 or even some random value. This value may be then reported up to HTMLMediaElement and that may be confusing to web applications. This commit doesn't trust sinks position when pipeline is not prerolled, as behavor is different across devices. The last cached position is used instead. To reflect better what's actually happening, isPipelineSeeking() has been renamed as isPipelineWaitingPreroll() and the condition now also includes pending states higher than paused. Original author: Andrzej Surdej (https://github.com/asurdej-comcast) See: #1302 * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::isPipelineWaitingPreroll const): Renamed from isPipelineSeeking(). (WebCore::MediaPlayerPrivateGStreamer::play): isPipelineSeeking() now named as isPipelineWaitingPreroll(). (WebCore::MediaPlayerPrivateGStreamer::paused const): Ditto. (WebCore::MediaPlayerPrivateGStreamer::changePipelineState): Ditto. (WebCore::MediaPlayerPrivateGStreamer::playbackPosition const): Use GST_CLOCK_TIME_NONE when the pipeline isn't prerolled. This will force the usage of the target seek time (if possible) or the last cached position (in the worst case). (WebCore::MediaPlayerPrivateGStreamer::isPipelineSeeking const): Deleted. Renamed to isPipelineWaitingPreroll(). * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: isPipelineSeeking() now named as isPipelineWaitingPreroll(). * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::updateStates): isPipelineSeeking() now named as isPipelineWaitingPreroll(). Canonical link: https://commits.webkit.org/277541@main
1 parent a4387a2 commit 52ff406

3 files changed

Lines changed: 17 additions & 16 deletions

File tree

Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,16 @@ void MediaPlayerPrivateGStreamer::prepareToPlay()
388388
}
389389
}
390390

391-
bool MediaPlayerPrivateGStreamer::isPipelineSeeking(GstState current, GstState pending, GstStateChangeReturn change) const
391+
bool MediaPlayerPrivateGStreamer::isPipelineWaitingPreroll(GstState current, GstState pending, GstStateChangeReturn change) const
392392
{
393-
return change == GST_STATE_CHANGE_ASYNC && current == GST_STATE_PAUSED && pending == GST_STATE_PAUSED;
393+
return change == GST_STATE_CHANGE_ASYNC && current == GST_STATE_PAUSED && pending >= GST_STATE_PAUSED;
394394
}
395395

396-
bool MediaPlayerPrivateGStreamer::isPipelineSeeking() const
396+
bool MediaPlayerPrivateGStreamer::isPipelineWaitingPreroll() const
397397
{
398398
GstState current, pending;
399399
GstStateChangeReturn change = gst_element_get_state(m_pipeline.get(), &current, &pending, 0);
400-
return isPipelineSeeking(current, pending, change);
400+
return isPipelineWaitingPreroll(current, pending, change);
401401
}
402402

403403
void MediaPlayerPrivateGStreamer::play()
@@ -414,8 +414,8 @@ void MediaPlayerPrivateGStreamer::play()
414414
return;
415415
}
416416

417-
if (isPipelineSeeking()) {
418-
GST_DEBUG_OBJECT(pipeline(), "pipeline is seeking, let's delay moving the pipeline to playing right now");
417+
if (isPipelineWaitingPreroll()) {
418+
GST_DEBUG_OBJECT(pipeline(), "pipeline is waiting preroll (after seek or flush), let's delay moving the pipeline to playing right now");
419419
return;
420420
}
421421

@@ -480,7 +480,7 @@ bool MediaPlayerPrivateGStreamer::paused() const
480480
return isPipelinePaused;
481481

482482
#if !defined(GST_DISABLE_GST_DEBUG) || !defined(NDEBUG)
483-
if (!isPipelineSeeking(state, pending, stateChange) && isPipelinePaused != !m_isPipelinePlaying
483+
if (!isPipelineWaitingPreroll(state, pending, stateChange) && isPipelinePaused != !m_isPipelinePlaying
484484
&& (stateChange == GST_STATE_CHANGE_SUCCESS || stateChange == GST_STATE_CHANGE_NO_PREROLL)) {
485485
GST_WARNING_OBJECT(pipeline(), "states are not synchronized, player paused %s, pipeline paused %s",
486486
boolForPrinting(!m_isPipelinePlaying), boolForPrinting(isPipelinePaused));
@@ -987,8 +987,8 @@ MediaPlayerPrivateGStreamer::ChangePipelineStateResult MediaPlayerPrivateGStream
987987

988988
GstState currentState, pending;
989989
GstStateChangeReturn change = gst_element_get_state(m_pipeline.get(), &currentState, &pending, 0);
990-
if (isPipelineSeeking(currentState, pending, change)) {
991-
GST_DEBUG_OBJECT(pipeline(), "rejected state change during seek");
990+
if (isPipelineWaitingPreroll(currentState, pending, change)) {
991+
GST_DEBUG_OBJECT(pipeline(), "rejected state change during preroll");
992992
return ChangePipelineStateResult::Rejected;
993993
}
994994

@@ -1417,7 +1417,8 @@ MediaTime MediaPlayerPrivateGStreamer::playbackPosition() const
14171417
return m_cachedPosition;
14181418
}
14191419

1420-
GstClockTime gstreamerPosition = gstreamerPositionFromSinks();
1420+
// We can't trust sinks position when pipeline is flushed (e.g. after MSE samples removal).
1421+
GstClockTime gstreamerPosition = isPipelineWaitingPreroll() ? GST_CLOCK_TIME_NONE : gstreamerPositionFromSinks();
14211422
GST_TRACE_OBJECT(pipeline(), "Position %" GST_TIME_FORMAT ", canFallBackToLastFinishedSeekPosition: %s", GST_TIME_ARGS(gstreamerPosition), boolForPrinting(m_canFallBackToLastFinishedSeekPosition));
14221423

14231424
// Cached position is marked as non valid here but we might fail to get a new one so initializing to this as "educated guess".

Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,8 @@ class MediaPlayerPrivateGStreamer
370370

371371
void setCachedPosition(const MediaTime&) const;
372372

373-
bool isPipelineSeeking(GstState current, GstState pending, GstStateChangeReturn) const;
374-
bool isPipelineSeeking() const;
373+
bool isPipelineWaitingPreroll(GstState current, GstState pending, GstStateChangeReturn) const;
374+
bool isPipelineWaitingPreroll() const;
375375

376376
Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
377377
ThreadSafeWeakPtr<MediaPlayer> m_player;

Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,18 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
324324

325325
void MediaPlayerPrivateGStreamerMSE::updateStates()
326326
{
327-
bool isSeeking = isPipelineSeeking();
327+
bool isWaitingPreroll = isPipelineWaitingPreroll();
328328
bool shouldBePlaying = (!m_isPaused && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused)
329329
|| m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying;
330330
GST_DEBUG_OBJECT(pipeline(), "shouldBePlaying = %s, m_isPipelinePlaying = %s, is seeking %s", boolForPrinting(shouldBePlaying),
331-
boolForPrinting(m_isPipelinePlaying), boolForPrinting(isSeeking));
332-
if (!isSeeking && shouldBePlaying && !m_isPipelinePlaying) {
331+
boolForPrinting(m_isPipelinePlaying), boolForPrinting(isWaitingPreroll));
332+
if (!isWaitingPreroll && shouldBePlaying && !m_isPipelinePlaying) {
333333
auto result = changePipelineState(GST_STATE_PLAYING);
334334
if (result == ChangePipelineStateResult::Failed)
335335
GST_ERROR_OBJECT(pipeline(), "Setting the pipeline to PLAYING failed");
336336
else if (result == ChangePipelineStateResult::Ok)
337337
m_playbackRatePausedState = PlaybackRatePausedState::Playing;
338-
} else if (!isSeeking && !shouldBePlaying && m_isPipelinePlaying) {
338+
} else if (!isWaitingPreroll && !shouldBePlaying && m_isPipelinePlaying) {
339339
if (changePipelineState(GST_STATE_PAUSED) == ChangePipelineStateResult::Failed)
340340
GST_ERROR_OBJECT(pipeline(), "Setting the pipeline to PAUSED failed");
341341
}

0 commit comments

Comments
 (0)