Skip to content

Commit efc4d3d

Browse files
asurdej-comcasteocanha
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 e1da4bf commit efc4d3d

3 files changed

Lines changed: 12 additions & 11 deletions

File tree

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -410,16 +410,16 @@ void MediaPlayerPrivateGStreamer::prepareToPlay()
410410
}
411411
}
412412

413-
bool MediaPlayerPrivateGStreamer::isPipelineSeeking(GstState current, GstState pending, GstStateChangeReturn change) const
413+
bool MediaPlayerPrivateGStreamer::isPipelineWaitingPreroll(GstState current, GstState pending, GstStateChangeReturn change) const
414414
{
415-
return change == GST_STATE_CHANGE_ASYNC && current == GST_STATE_PAUSED && pending == GST_STATE_PAUSED;
415+
return change == GST_STATE_CHANGE_ASYNC && current == GST_STATE_PAUSED && pending >= GST_STATE_PAUSED;
416416
}
417417

418-
bool MediaPlayerPrivateGStreamer::isPipelineSeeking() const
418+
bool MediaPlayerPrivateGStreamer::isPipelineWaitingPreroll() const
419419
{
420420
GstState current, pending;
421421
GstStateChangeReturn change = gst_element_get_state(m_pipeline.get(), &current, &pending, 0);
422-
return isPipelineSeeking(current, pending, change);
422+
return isPipelineWaitingPreroll(current, pending, change);
423423
}
424424

425425
void MediaPlayerPrivateGStreamer::play()
@@ -436,7 +436,7 @@ void MediaPlayerPrivateGStreamer::play()
436436
return;
437437
}
438438

439-
if (isPipelineSeeking()) {
439+
if (isPipelineWaitingPreroll()) {
440440
GST_DEBUG_OBJECT(pipeline(), "pipeline is seeking, let's delay moving the pipeline to playing right now");
441441
return;
442442
}
@@ -497,7 +497,7 @@ bool MediaPlayerPrivateGStreamer::paused() const
497497
return isPipelinePaused;
498498

499499
#if !defined(GST_DISABLE_GST_DEBUG) || !defined(NDEBUG)
500-
if (!isPipelineSeeking(state, pending, stateChange) && isPipelinePaused != !m_isPipelinePlaying
500+
if (!isPipelineWaitingPreroll(state, pending, stateChange) && isPipelinePaused != !m_isPipelinePlaying
501501
&& (stateChange == GST_STATE_CHANGE_SUCCESS || stateChange == GST_STATE_CHANGE_NO_PREROLL)) {
502502
GST_WARNING_OBJECT(pipeline(), "states are not synchronized, player paused %s, pipeline paused %s",
503503
boolForPrinting(!m_isPipelinePlaying), boolForPrinting(isPipelinePaused));
@@ -978,7 +978,7 @@ MediaPlayerPrivateGStreamer::ChangePipelineStateResult MediaPlayerPrivateGStream
978978

979979
GstState currentState, pending;
980980
GstStateChangeReturn change = gst_element_get_state(m_pipeline.get(), &currentState, &pending, 0);
981-
if (isPipelineSeeking(currentState, pending, change)) {
981+
if (isPipelineWaitingPreroll(currentState, pending, change)) {
982982
GST_DEBUG_OBJECT(pipeline(), "rejected state change during seek");
983983
return ChangePipelineStateResult::Rejected;
984984
}
@@ -1402,7 +1402,8 @@ MediaTime MediaPlayerPrivateGStreamer::playbackPosition() const
14021402
return m_cachedPosition;
14031403
}
14041404

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

14081409
// 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
@@ -377,8 +377,8 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
377377

378378
void setCachedPosition(const MediaTime&) const;
379379

380-
bool isPipelineSeeking(GstState current, GstState pending, GstStateChangeReturn) const;
381-
bool isPipelineSeeking() const;
380+
bool isPipelineWaitingPreroll(GstState current, GstState pending, GstStateChangeReturn) const;
381+
bool isPipelineWaitingPreroll() const;
382382

383383
Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
384384
MediaPlayer* m_player;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
386386

387387
void MediaPlayerPrivateGStreamerMSE::updateStates()
388388
{
389-
bool isSeeking = isPipelineSeeking();
389+
bool isSeeking = isPipelineWaitingPreroll();
390390
bool shouldBePlaying = (!m_isPaused && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused)
391391
|| m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying;
392392
GST_DEBUG_OBJECT(pipeline(), "shouldBePlaying = %s, m_isPipelinePlaying = %s, is seeking %s", boolForPrinting(shouldBePlaying),

0 commit comments

Comments
 (0)