Skip to content

Commit 674ac36

Browse files
calvariscalvaris
committed
[GStreamer] Rework playback rates handling
https://bugs.webkit.org/show_bug.cgi?id=259732 Reviewed by Alicia Boya Garcia. There were some use cases in MSE where playback rate change was not working properly and it was decided that a better idea would be to do a rework of the playback rate handling in the base class. The idea was to not do playback rate state changes out of the updateStates functions (both regular and MSE) because calling that function should do what is expected at any moment. In order to do that, we needed to switch the boolean variable representing if pipeline was paused because of rate into something more reflecting the different states we could be in. We created a self explanatory enum class (that also was comments to make it even more clear) and set that state when needed for updateStates to move the pipeline if needed. A fly-by fix was to make the paused function to return true when we are also moving to paused during an async state change because the test we are introducing here arose this flaky bug more clearly. The bug was that the media element went bananas could not realize the player was paused (or on its way to pause) and it was skipping state changes and playback rate changes. * LayoutTests/TestExpectations: * LayoutTests/media/media-source/media-source-play-zero-playbackrate-expected.txt: Added. * LayoutTests/media/media-source/media-source-play-zero-playbackrate.html: Added. * LayoutTests/platform/glib/TestExpectations: * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::play): (WebCore::MediaPlayerPrivateGStreamer::pause): (WebCore::MediaPlayerPrivateGStreamer::paused const): (WebCore::MediaPlayerPrivateGStreamer::updatePlaybackRate): (WebCore::MediaPlayerPrivateGStreamer::setRate): (WebCore::MediaPlayerPrivateGStreamer::updateStates): * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::play): (WebCore::MediaPlayerPrivateGStreamerMSE::pause): (WebCore::MediaPlayerPrivateGStreamerMSE::updateStates): Canonical link: https://commits.webkit.org/268518@main
1 parent 2ec764e commit 674ac36

7 files changed

Lines changed: 188 additions & 28 deletions

File tree

‎LayoutTests/TestExpectations‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1198,6 +1198,9 @@ media/video-supports-fullscreen.html [ Skip ]
11981198
# Support for VP9 encoded videos with transparency is only available on ports that use GStreamer.
11991199
media/video-with-alpha.html [ Skip ]
12001200

1201+
# Support to pause with 0 rate playback seems to work only in GStreamer ports
1202+
media/media-source/media-source-play-zero-playbackrate.html [ Skip ]
1203+
12011204
# accent-color is currently only supported on Cocoa platforms
12021205
fast/css/accent-color/button.html [ ImageOnlyFailure ]
12031206
fast/css/accent-color/checkbox.html [ ImageOnlyFailure ]

‎LayoutTests/media/media-source/media-source-play-zero-playbackrate-expected.txt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
2+
RUN(video.src = URL.createObjectURL(source))
3+
EVENT(sourceopen)
4+
RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
5+
RUN(sourceBuffer.appendBuffer(loader.initSegment()))
6+
EVENT(update)
7+
8+
Append all media segments
9+
10+
RUN(video.play())
11+
EXPECTED (video.paused == 'false') OK
12+
RUN(video.pause())
13+
EXPECTED (video.paused == 'true') OK
14+
RUN(video.playbackRate = 0)
15+
EXPECTED (video.playbackRate == '0') OK
16+
RUN(video.play())
17+
EXPECTED (video.paused == 'false') OK
18+
EXPECTED (video.currentTime == oldPosition == 'true') OK
19+
20+
RUN(video.playbackRate = 1)
21+
EXPECTED (video.playbackRate == '1') OK
22+
EXPECTED (video.paused == 'false') OK
23+
EXPECTED (video.currentTime > oldPosition == 'true') OK
24+
RUN(video.playbackRate = 0)
25+
EXPECTED (video.playbackRate == '0') OK
26+
RUN(video.pause())
27+
EXPECTED (video.currentTime == oldPosition == 'true') OK
28+
RUN(video.playbackRate = 1)
29+
EXPECTED (video.playbackRate == '1') OK
30+
RUN(video.play())
31+
EXPECTED (video.currentTime > oldPosition == 'true') OK
32+
33+
RUN(video.currentTime = 5)
34+
EXPECTED (video.currentTime >= '5') OK
35+
RUN(video.playbackRate = 0)
36+
EXPECTED (video.playbackRate == '0') OK
37+
EXPECTED (video.currentTime == oldPosition == 'true') OK
38+
END OF TEST
39+

‎LayoutTests/media/media-source/media-source-play-zero-playbackrate.html‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>media-source-play-zero-playbackrate</title>
5+
<meta name="timeout" content="long">
6+
<script src="media-source-loader.js"></script>
7+
<script src="../video-test.js"></script>
8+
<script>
9+
var loader;
10+
var source;
11+
var sourceBuffer;
12+
var oldPosition;
13+
14+
function loaderPromise(loader) {
15+
return new Promise((resolve, reject) => {
16+
loader.onload = resolve;
17+
loader.onerror = reject;
18+
});
19+
}
20+
21+
window.addEventListener('load', async event => {
22+
findMediaElement();
23+
24+
loader = new MediaSourceLoader('content/test-fragmented-manifest.json');
25+
await loaderPromise(loader);
26+
27+
source = new MediaSource();
28+
run('video.src = URL.createObjectURL(source)');
29+
await waitFor(source, 'sourceopen');
30+
waitFor(video, 'error').then(failTest);
31+
32+
run('sourceBuffer = source.addSourceBuffer(loader.type())');
33+
run('sourceBuffer.appendBuffer(loader.initSegment())');
34+
await waitFor(sourceBuffer, 'update');
35+
36+
consoleWrite('<br>Append all media segments')
37+
for (i = 0; i < loader.mediaSegmentsLength(); i++) {
38+
sourceBuffer.appendBuffer(loader.mediaSegment(i));
39+
await waitFor(sourceBuffer, 'update', true);
40+
}
41+
42+
consoleWrite('<br>RUN(video.play())')
43+
await video.play();
44+
testExpected('video.paused', false);
45+
run('video.pause()');
46+
testExpected('video.paused', true);
47+
run('video.playbackRate = 0');
48+
testExpected('video.playbackRate', 0);
49+
consoleWrite('RUN(video.play())')
50+
await video.play();
51+
testExpected('video.paused', false);
52+
// To deal with cached position shaenanigans.
53+
await sleepFor(500);
54+
oldPosition = video.currentTime;
55+
await sleepFor(500);
56+
testExpected('video.currentTime == oldPosition', true);
57+
58+
consoleWrite('')
59+
run('video.playbackRate = 1');
60+
testExpected('video.playbackRate', 1);
61+
testExpected('video.paused', false);
62+
await sleepFor(500);
63+
testExpected('video.currentTime > oldPosition', true);
64+
oldPosition = video.currentTime;
65+
run('video.playbackRate = 0');
66+
testExpected('video.playbackRate', 0);
67+
run('video.pause()');
68+
await sleepFor(500);
69+
testExpected('video.currentTime == oldPosition', true);
70+
run('video.playbackRate = 1');
71+
testExpected('video.playbackRate', 1);
72+
consoleWrite('RUN(video.play())')
73+
await video.play();
74+
await sleepFor(500);
75+
testExpected('video.currentTime > oldPosition', true);
76+
77+
consoleWrite('')
78+
run('video.currentTime = 5');
79+
testExpected('video.currentTime', 5, '>=');
80+
await sleepFor(5);
81+
run('video.playbackRate = 0');
82+
testExpected('video.playbackRate', 0);
83+
await sleepFor(500);
84+
oldPosition = video.currentTime;
85+
await sleepFor(500);
86+
testExpected('video.currentTime == oldPosition', true);
87+
88+
endTest();
89+
});
90+
91+
</script>
92+
</head>
93+
<body>
94+
<video controls></video>
95+
</body>
96+
</html>

‎LayoutTests/platform/glib/TestExpectations‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,15 @@ media/video-with-alpha.html [ Pass ]
322322
# Only glib platforms fail (detected by this test) on corrupt aac encapsulated in MP4 because of GStreamer fdkaacdec.
323323
media/media-source/media-source-append-play-corrupt-aac.html [ Pass ]
324324

325+
# Only GStreamer is honoring the CodecDelay field
326+
webkit.org/b/254222 media/media-source/media-webm-opus-codecdelay.html [ Pass ]
327+
328+
# GStreamer seeks to the wrong frame
329+
webkit.org/b/261335 media/media-source/media-managedmse-seek.html [ Failure ]
330+
331+
# Support to pause with 0 rate playback seems to work only in GStreamer ports
332+
media/media-source/media-source-play-zero-playbackrate.html [ Pass ]
333+
325334
# NOTIFICATION_EVENT tests
326335
http/tests/workers/service/shownotification-allowed-document.html [ Pass ]
327336
http/tests/workers/service/shownotification-allowed.html [ Pass ]

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

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,8 @@ void MediaPlayerPrivateGStreamer::play()
409409
}
410410

411411
if (!m_playbackRate) {
412-
m_isPlaybackRatePaused = true;
412+
if (m_playbackRatePausedState == PlaybackRatePausedState::ManuallyPaused)
413+
m_playbackRatePausedState = PlaybackRatePausedState::RatePaused;
413414
return;
414415
}
415416

@@ -428,7 +429,7 @@ void MediaPlayerPrivateGStreamer::pause()
428429
if (isMediaStreamPlayer())
429430
m_pausedTime = currentMediaTime();
430431

431-
m_isPlaybackRatePaused = false;
432+
m_playbackRatePausedState = PlaybackRatePausedState::ManuallyPaused;
432433
GstState currentState, pendingState;
433434
gst_element_get_state(m_pipeline.get(), &currentState, &pendingState, 0);
434435
if (currentState < GST_STATE_PAUSED && pendingState <= GST_STATE_PAUSED)
@@ -450,15 +451,17 @@ bool MediaPlayerPrivateGStreamer::paused() const
450451
return true;
451452
}
452453

453-
if (m_isPlaybackRatePaused) {
454+
if (m_playbackRatePausedState == PlaybackRatePausedState::RatePaused
455+
|| m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying) {
454456
GST_DEBUG_OBJECT(pipeline(), "Playback rate is 0, simulating PAUSED state");
455457
return false;
456458
}
457459

458-
GstState state;
459-
gst_element_get_state(m_pipeline.get(), &state, nullptr, 0);
460-
bool paused = state <= GST_STATE_PAUSED;
461-
GST_LOG_OBJECT(pipeline(), "Paused: %s", toString(paused).utf8().data());
460+
GstState state, pending;
461+
auto stateChange = gst_element_get_state(m_pipeline.get(), &state, &pending, 0);
462+
bool paused = state <= GST_STATE_PAUSED || (stateChange == GST_STATE_CHANGE_ASYNC && pending == GST_STATE_PAUSED);
463+
GST_LOG_OBJECT(pipeline(), "Paused: %s (state %s, pending %s, state change %s)", boolForPrinting(paused),
464+
gst_element_state_get_name(state), gst_element_state_get_name(pending), gst_element_state_change_return_get_name(stateChange));
462465
return paused;
463466
}
464467

@@ -564,15 +567,6 @@ void MediaPlayerPrivateGStreamer::updatePlaybackRate()
564567
}
565568
}
566569

567-
if (m_isPlaybackRatePaused) {
568-
GstState state, pending;
569-
570-
gst_element_get_state(m_pipeline.get(), &state, &pending, 0);
571-
if (state != GST_STATE_PLAYING && pending != GST_STATE_PLAYING)
572-
changePipelineState(GST_STATE_PLAYING);
573-
m_isPlaybackRatePaused = false;
574-
}
575-
576570
m_isChangingRate = false;
577571
m_player->rateChanged();
578572
}
@@ -637,21 +631,23 @@ void MediaPlayerPrivateGStreamer::setRate(float rate)
637631
return;
638632
}
639633

640-
GstState state, pending;
641-
642634
m_playbackRate = rateClamped;
643635
m_isChangingRate = true;
644636

645-
gst_element_get_state(m_pipeline.get(), &state, &pending, 0);
646-
647637
if (!rateClamped) {
648638
m_isChangingRate = false;
649-
m_isPlaybackRatePaused = true;
650-
if (state != GST_STATE_PAUSED && pending != GST_STATE_PAUSED)
651-
changePipelineState(GST_STATE_PAUSED);
639+
if (m_playbackRatePausedState == PlaybackRatePausedState::Playing || m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying) {
640+
m_playbackRatePausedState = PlaybackRatePausedState::RatePaused;
641+
updateStates();
642+
}
652643
return;
644+
} else if (m_playbackRatePausedState == PlaybackRatePausedState::RatePaused) {
645+
m_playbackRatePausedState = PlaybackRatePausedState::ShouldMoveToPlaying;
646+
updateStates();
653647
}
654648

649+
GstState state, pending;
650+
gst_element_get_state(m_pipeline.get(), &state, &pending, 0);
655651
if ((state != GST_STATE_PLAYING && state != GST_STATE_PAUSED)
656652
|| (pending == GST_STATE_PAUSED))
657653
return;
@@ -2497,16 +2493,18 @@ void MediaPlayerPrivateGStreamer::updateStates()
24972493
m_areVolumeAndMuteInitialized = true;
24982494
}
24992495

2500-
if (didBuffering && !m_isBuffering && !m_isPaused && m_playbackRate) {
2501-
GST_INFO_OBJECT(pipeline(), "[Buffering] Restarting playback.");
2496+
if ((didBuffering && !m_isBuffering && !m_isPaused && m_playbackRate)
2497+
|| m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying) {
2498+
m_playbackRatePausedState = PlaybackRatePausedState::Playing;
2499+
GST_INFO_OBJECT(pipeline(), "[Buffering] Restarting playback (because of buffering or resuming from zero playback rate)");
25022500
changePipelineState(GST_STATE_PLAYING);
25032501
}
25042502
} else if (m_currentState == GST_STATE_PLAYING) {
25052503
m_isPaused = false;
25062504

25072505
shouldPauseForBuffering = (m_isBuffering && !m_isLiveStream.value_or(false));
25082506
if (shouldPauseForBuffering || !m_playbackRate) {
2509-
GST_INFO_OBJECT(pipeline(), "[Buffering] Pausing stream for buffering.");
2507+
GST_INFO_OBJECT(pipeline(), "[Buffering] Pausing stream for buffering or because of zero playback rate.");
25102508
changePipelineState(GST_STATE_PAUSED);
25112509
}
25122510
} else

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,16 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
290290
StreamCollectionChanged = 1 << 7
291291
};
292292

293+
enum class PlaybackRatePausedState {
294+
ManuallyPaused, // Initialization or user explicitly paused. This takes preference over RatePaused. You don't
295+
// transition from Manually to Rate Paused unless there is a play while rate == 0.
296+
RatePaused, // Pipeline was playing and rate was set to zero.
297+
ShouldMoveToPlaying, // Pipeline was paused because of zero rate and it should be playing. This is not a
298+
// definitive state, just an operational transition from RatePaused to Playing to keep the
299+
// pipeline state changes contained in updateStates.
300+
Playing, // Pipeline is playing and it should be.
301+
};
302+
293303
static bool isAvailable();
294304

295305
virtual void durationChanged();
@@ -374,6 +384,7 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
374384
mutable std::optional<bool> m_isLiveStream;
375385
bool m_isPaused { true };
376386
float m_playbackRate { 1 };
387+
PlaybackRatePausedState m_playbackRatePausedState { PlaybackRatePausedState::ManuallyPaused };
377388
GstState m_currentState { GST_STATE_NULL };
378389
GstState m_oldState { GST_STATE_NULL };
379390
GstState m_requestedState { GST_STATE_VOID_PENDING };
@@ -545,7 +556,6 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
545556
GRefPtr<GstElement> m_textSink;
546557
GUniquePtr<GstStructure> m_mediaLocations;
547558
int m_mediaLocationCurrentIndex { 0 };
548-
bool m_isPlaybackRatePaused { false };
549559
MediaTime m_timeOfOverlappingSeek;
550560
// Last playback rate sent through a GStreamer seek.
551561
float m_lastPlaybackRate { 1 };

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,13 +206,16 @@ void MediaPlayerPrivateGStreamerMSE::play()
206206
{
207207
GST_DEBUG_OBJECT(pipeline(), "Play requested");
208208
m_isPaused = false;
209+
if (!m_playbackRate && m_playbackRatePausedState == PlaybackRatePausedState::ManuallyPaused)
210+
m_playbackRatePausedState = PlaybackRatePausedState::RatePaused;
209211
updateStates();
210212
}
211213

212214
void MediaPlayerPrivateGStreamerMSE::pause()
213215
{
214216
GST_DEBUG_OBJECT(pipeline(), "Pause requested");
215217
m_isPaused = true;
218+
m_playbackRatePausedState = PlaybackRatePausedState::ManuallyPaused;
216219
updateStates();
217220
}
218221

@@ -354,12 +357,14 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
354357

355358
void MediaPlayerPrivateGStreamerMSE::updateStates()
356359
{
357-
bool shouldBePlaying = !m_isPaused && readyState() >= MediaPlayer::ReadyState::HaveFutureData;
360+
bool shouldBePlaying = (!m_isPaused && readyState() >= MediaPlayer::ReadyState::HaveFutureData && m_playbackRatePausedState != PlaybackRatePausedState::RatePaused)
361+
|| m_playbackRatePausedState == PlaybackRatePausedState::ShouldMoveToPlaying;
358362
GST_DEBUG_OBJECT(pipeline(), "shouldBePlaying = %s, m_isPipelinePlaying = %s", boolForPrinting(shouldBePlaying), boolForPrinting(m_isPipelinePlaying));
359363
if (shouldBePlaying && !m_isPipelinePlaying) {
360364
if (!changePipelineState(GST_STATE_PLAYING))
361365
GST_ERROR_OBJECT(pipeline(), "Setting the pipeline to PLAYING failed");
362366
m_isPipelinePlaying = true;
367+
m_playbackRatePausedState = PlaybackRatePausedState::Playing;
363368
} else if (!shouldBePlaying && m_isPipelinePlaying) {
364369
if (!changePipelineState(GST_STATE_PAUSED))
365370
GST_ERROR_OBJECT(pipeline(), "Setting the pipeline to PAUSED failed");

0 commit comments

Comments
 (0)