Skip to content

Commit 6e84da2

Browse files
committed
[Media] Avoid play() call during seek flow before the finishSeek()
https://bugs.webkit.org/show_bug.cgi?id=283172 Reviewed by Xabier Rodriguez-Calvar. During the video playback states should ideally flow as play -> pause -> seek_start -> seek_done -> play, but randomly it is observed that the play event is being sent before finishing the seek (seek_done) i.e, play -> pause -> seek_start -> play -> seek_done. Ideally play call might be triggered continuously by the webapp when we are playing a video, so in our scenario between seek_start and seek_done we should avoid the play call. This commit adds a new condition to skip the playPlayer() call during updatePlayState() when the seek is ongoing. This would avoid the play event to happen. There's also some special cases to not skip playPlayer() on first play, because initial seeks aren't fully supported on WebKitGTK/WPE and that play call might be required for them to work: - First time playing - Looping - Resuming playback - Seek after playback ended (either as part of a loop or a manual seek) See: #1423 Co-authored with: gowthami <gchikkadasarahallilo.ext@libertyglobal.com> * Source/WebCore/html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::seekWithTolerance): Set seek after playback ended flag when the current position is equal to duration. (WebCore::HTMLMediaElement::finishSeek): Reset the seek after playback ended flag. (WebCore::HTMLMediaElement::playInternal): Set seek after playback ended flag when needed. (WebCore::HTMLMediaElement::updatePlayState): Add the new conditions and exceptions to disallow playback when seeking. (WebCore::HTMLMediaElement::mayResumePlayback): Set the isResumingPlayback condition and reset it after the call to play(). * Source/WebCore/html/HTMLMediaElement.h: Added m_isResumingPlayback and m_seekAfterPlaybackEnded flags. Canonical link: https://commits.webkit.org/287274@main
1 parent d893a3e commit 6e84da2

2 files changed

Lines changed: 26 additions & 3 deletions

File tree

Source/WebCore/html/HTMLMediaElement.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3212,6 +3212,10 @@ void HTMLMediaElement::seekWithTolerance(const MediaTime& inTime, const MediaTim
32123212
refreshCachedTime();
32133213
MediaTime now = currentMediaTime();
32143214

3215+
// Needed to detect a special case in updatePlayState().
3216+
if (now >= durationMediaTime())
3217+
m_seekAfterPlaybackEnded = true;
3218+
32153219
// 3 - If the element's seeking IDL attribute is true, then another instance of this algorithm is
32163220
// already running. Abort that other instance of the algorithm without waiting for the step that
32173221
// it is running to complete.
@@ -3383,6 +3387,8 @@ void HTMLMediaElement::finishSeek()
33833387
#endif
33843388
if (wasPlayingBeforeSeeking)
33853389
playInternal();
3390+
3391+
m_seekAfterPlaybackEnded = false;
33863392
}
33873393

33883394
HTMLMediaElement::ReadyState HTMLMediaElement::readyState() const
@@ -3804,8 +3810,10 @@ void HTMLMediaElement::playInternal()
38043810
if (!m_player || m_networkState == NETWORK_EMPTY)
38053811
selectMediaResource();
38063812

3807-
if (endedPlayback())
3813+
if (endedPlayback()) {
3814+
m_seekAfterPlaybackEnded = true;
38083815
seekInternal(MediaTime::zeroTime());
3816+
}
38093817

38103818
if (m_mediaController)
38113819
m_mediaController->bringElementUpToSpeed(*this);
@@ -5768,7 +5776,17 @@ void HTMLMediaElement::updatePlayState()
57685776
if (shouldBePlaying) {
57695777
invalidateCachedTime();
57705778

5771-
if (playerPaused) {
5779+
// Play is always allowed, except when seeking (to avoid unpausing the video by mistake until the
5780+
// target time is reached). However, there are some exceptional situations when we allow playback
5781+
// during seek. This is because GStreamer-based implementation have a design limitation that doesn't
5782+
// allow initial seeks (seeking before going to playing state), and these exceptions make things
5783+
// work for those platforms.
5784+
bool isLooping = loop() && m_lastSeekTime == MediaTime::zeroTime();
5785+
bool playExceptionsWhenSeeking = m_seeking && (m_firstTimePlaying
5786+
|| isLooping || m_isResumingPlayback || m_seekAfterPlaybackEnded);
5787+
bool allowPlay = !m_seeking || playExceptionsWhenSeeking;
5788+
5789+
if (playerPaused && allowPlay) {
57725790
mediaSession().clientWillBeginPlayback();
57735791

57745792
// Set rate, muted and volume before calling play in case they were set before the media engine was set up.
@@ -8113,8 +8131,11 @@ void HTMLMediaElement::resumeAutoplaying()
81138131
void HTMLMediaElement::mayResumePlayback(bool shouldResume)
81148132
{
81158133
ALWAYS_LOG(LOGIDENTIFIER, "paused = ", paused());
8116-
if (paused() && shouldResume)
8134+
if (paused() && shouldResume) {
8135+
m_isResumingPlayback = true;
81178136
play();
8137+
m_isResumingPlayback = false;
8138+
}
81188139
}
81198140

81208141
String HTMLMediaElement::mediaSessionTitle() const

Source/WebCore/html/HTMLMediaElement.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,8 @@ class HTMLMediaElement
11621162
bool m_shouldAudioPlaybackRequireUserGesture : 1;
11631163
bool m_shouldVideoPlaybackRequireUserGesture : 1;
11641164
bool m_volumeLocked : 1;
1165+
bool m_isResumingPlayback : 1 { false };
1166+
bool m_seekAfterPlaybackEnded : 1 { false };
11651167

11661168
AutoplayEventPlaybackState m_autoplayEventPlaybackState { AutoplayEventPlaybackState::None };
11671169

0 commit comments

Comments
 (0)