Skip to content

Commit 98827e7

Browse files
calvariscalvaris
committed
[MSE][GStreamer] Fix non-seeking flushes
https://bugs.webkit.org/show_bug.cgi?id=262693 Reviewed by Philippe Normand. This reverts https://commits.webkit.org/269358@main . Problem is that you can screw quality changes as they require a non seeking flush with an updated segment. We still need to change the segment to use the proper playback position and we can do it by using the player's position. Besides, as a fly-by-fix, we are solving the problem of position going back to 0 when the sinks can't provide a proper position while it is prerolling. For that, we use the cached position of the player even if it was supposed to be invalidated already as a best effort. * LayoutTests/media/media-source/media-source-audio-switch-expected.txt: Added. * LayoutTests/media/media-source/media-source-audio-switch.html: Added. * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp: (WebCore::MediaPlayerPrivateGStreamer::playbackPosition const): (WebCore::MediaPlayerPrivateGStreamer::didEnd): (WebCore::MediaPlayerPrivateGStreamer::setCachedPosition const): (WebCore::MediaPlayerPrivateGStreamer::invalidateCachedPosition const): * Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h: * Source/WebCore/platform/graphics/gstreamer/mse/MediaPlayerPrivateGStreamerMSE.cpp: (WebCore::MediaPlayerPrivateGStreamerMSE::sourceSetup): * Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp: (webKitMediaSrcPlayer): (webKitMediaSrcSetPlayer): (webKitMediaSrcStreamFlush): * Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.h: Canonical link: https://commits.webkit.org/270764@main
1 parent 51171f8 commit 98827e7

7 files changed

Lines changed: 192 additions & 10 deletions

File tree

‎LayoutTests/media/media-source/media-source-audio-switch-expected.txt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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+
Appended all media segments
8+
RUN(video.play())
9+
EXPECTED (video.paused == 'false') OK
10+
11+
EXPECTED (video.currentTime >= '1') OK
12+
RUN(video.currentTime = 3)
13+
14+
EXPECTED (video.currentTime >= '4') OK
15+
RUN(video.pause())
16+
EXPECTED (video.paused == 'true') OK
17+
RUN(sourceBuffer.remove(0, 15))
18+
EVENT(update)
19+
RUN(sourceBuffer.appendBuffer(loader.initSegment()))
20+
EVENT(update)
21+
Appended all media segments
22+
RUN(video.play())
23+
EXPECTED (video.paused == 'false') OK
24+
25+
EXPECTED (video.currentTime >= '5') OK
26+
END OF TEST
27+

‎LayoutTests/media/media-source/media-source-audio-switch.html‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>media-source-audio-switch</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 oldTime = -1;
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-48khz-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+
for (i = 0; i < loader.mediaSegmentsLength(); i++) {
37+
sourceBuffer.appendBuffer(loader.mediaSegment(i));
38+
await waitFor(sourceBuffer, 'update', true);
39+
}
40+
consoleWrite('Appended all media segments')
41+
42+
consoleWrite('RUN(video.play())')
43+
await video.play();
44+
testExpected('video.paused', false);
45+
46+
async function timeUpdated() {
47+
if (oldTime <= video.currentTime) {
48+
oldTime = video.currentTime;
49+
if (video.currentTime >= 5) {
50+
consoleWrite('');
51+
testExpected('video.currentTime', 5, '>=')
52+
endTest();
53+
}
54+
return;
55+
}
56+
consoleWrite('');
57+
consoleWrite("oldTime " + oldTime);
58+
consoleWrite("video.currentTime " + video.currentTime);
59+
failTest('oldTime <= video.currentTime');
60+
}
61+
62+
async function audioSwitch() {
63+
if (video.currentTime < 4)
64+
return;
65+
video.removeEventListener("timeupdate", audioSwitch);
66+
video.addEventListener("timeupdate", timeUpdated);
67+
68+
consoleWrite('');
69+
testExpected('video.currentTime', 4, '>=');
70+
71+
run('video.pause()');
72+
testExpected('video.paused', true);
73+
74+
run('sourceBuffer.remove(0, 15)');
75+
await waitFor(sourceBuffer, 'update');
76+
77+
run('sourceBuffer.appendBuffer(loader.initSegment())');
78+
await waitFor(sourceBuffer, 'update');
79+
for (i = 0; i < loader.mediaSegmentsLength(); i++) {
80+
sourceBuffer.appendBuffer(loader.mediaSegment(i));
81+
await waitFor(sourceBuffer, 'update', true);
82+
}
83+
consoleWrite('Appended all media segments')
84+
85+
consoleWrite('RUN(video.play())')
86+
await video.play();
87+
testExpected('video.paused', false);
88+
}
89+
90+
function seek() {
91+
if (video.currentTime < 1)
92+
return;
93+
94+
video.removeEventListener("timeupdate", seek);
95+
video.addEventListener("timeupdate", audioSwitch);
96+
consoleWrite('');
97+
testExpected('video.currentTime', 1, '>=');
98+
run('video.currentTime = 3');
99+
}
100+
video.addEventListener("timeupdate", seek);
101+
});
102+
103+
</script>
104+
</head>
105+
<body>
106+
<video controls></video>
107+
</body>
108+
</html>

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,22 +1336,23 @@ MediaTime MediaPlayerPrivateGStreamer::playbackPosition() const
13361336
if (m_isEndReached)
13371337
return m_playbackRate > 0 ? durationMediaTime() : MediaTime::zeroTime();
13381338

1339-
if (m_cachedPosition) {
1340-
GST_TRACE_OBJECT(pipeline(), "Returning cached position: %s", m_cachedPosition.value().toString().utf8().data());
1341-
return m_cachedPosition.value();
1339+
if (m_isCachedPositionValid) {
1340+
GST_TRACE_OBJECT(pipeline(), "Returning cached position: %s", m_cachedPosition.toString().utf8().data());
1341+
return m_cachedPosition;
13421342
}
13431343

13441344
GstClockTime gstreamerPosition = gstreamerPositionFromSinks();
13451345
GST_TRACE_OBJECT(pipeline(), "Position %" GST_TIME_FORMAT ", canFallBackToLastFinishedSeekPosition: %s", GST_TIME_ARGS(gstreamerPosition), boolForPrinting(m_canFallBackToLastFinishedSeekPosition));
13461346

1347-
MediaTime playbackPosition = MediaTime::zeroTime();
1347+
// Cached position is marked as non valid here but we might fail to get a new one so initializing to this as "educated guess".
1348+
MediaTime playbackPosition = m_cachedPosition;
13481349

13491350
if (GST_CLOCK_TIME_IS_VALID(gstreamerPosition))
13501351
playbackPosition = MediaTime(gstreamerPosition, GST_SECOND);
13511352
else if (m_canFallBackToLastFinishedSeekPosition)
13521353
playbackPosition = m_seekTime;
13531354

1354-
m_cachedPosition = playbackPosition;
1355+
setCachedPosition(playbackPosition);
13551356
invalidateCachedPositionOnNextIteration();
13561357
return playbackPosition;
13571358
}
@@ -2720,7 +2721,7 @@ void MediaPlayerPrivateGStreamer::didEnd()
27202721
// HTMLMediaElement. In some cases like reverse playback the
27212722
// position is not always reported as 0 for instance.
27222723
if (!m_isSeeking) {
2723-
m_cachedPosition = m_playbackRate > 0 ? durationMediaTime() : MediaTime::zeroTime();
2724+
setCachedPosition(m_playbackRate > 0 ? durationMediaTime() : MediaTime::zeroTime());
27242725
GST_DEBUG("Position adjusted: %s", currentMediaTime().toString().utf8().data());
27252726
}
27262727
}
@@ -3607,9 +3608,15 @@ void MediaPlayerPrivateGStreamer::updateVideoSizeAndOrientationFromCaps(const Gs
36073608
m_player->sizeChanged();
36083609
}
36093610

3611+
void MediaPlayerPrivateGStreamer::setCachedPosition(const MediaTime& cachedPosition) const
3612+
{
3613+
m_cachedPosition = cachedPosition;
3614+
m_isCachedPositionValid = true;
3615+
}
3616+
36103617
void MediaPlayerPrivateGStreamer::invalidateCachedPosition() const
36113618
{
3612-
m_cachedPosition.reset();
3619+
m_isCachedPositionValid = false;
36133620
}
36143621

36153622
void MediaPlayerPrivateGStreamer::invalidateCachedPositionOnNextIteration() const

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,10 +371,13 @@ class MediaPlayerPrivateGStreamer : public MediaPlayerPrivateInterface
371371
void updateTextureMapperFlags();
372372
#endif
373373

374+
void setCachedPosition(const MediaTime&) const;
375+
374376
Ref<MainThreadNotifier<MainThreadNotification>> m_notifier;
375377
MediaPlayer* m_player;
376378
String m_referrer;
377-
mutable std::optional<MediaTime> m_cachedPosition;
379+
mutable MediaTime m_cachedPosition;
380+
mutable bool m_isCachedPositionValid { false };
378381
mutable MediaTime m_cachedDuration;
379382
bool m_canFallBackToLastFinishedSeekPosition { false };
380383
bool m_isChangingRate { false };

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ void MediaPlayerPrivateGStreamerMSE::sourceSetup(GstElement* sourceElement)
361361
{
362362
ASSERT(WEBKIT_IS_MEDIA_SRC(sourceElement));
363363
GST_DEBUG_OBJECT(pipeline(), "Source %p setup (old was: %p)", sourceElement, m_source.get());
364+
webKitMediaSrcSetPlayer(WEBKIT_MEDIA_SRC(sourceElement), WeakPtr { *this });
364365
m_source = sourceElement;
365366

366367
if (m_mediaSourcePrivate->hasAllTracks())

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

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ struct WebKitMediaSrcPrivate {
8787

8888
// Only used by URI Handler API implementation.
8989
GUniquePtr<char> uri;
90+
91+
WeakPtr<MediaPlayerPrivateGStreamerMSE> player;
9092
};
9193

9294
static void webKitMediaSrcUriHandlerInit(gpointer, gpointer);
@@ -98,6 +100,7 @@ static void webKitMediaSrcTearDownStream(WebKitMediaSrc* source, const AtomStrin
98100
static void webKitMediaSrcGetProperty(GObject*, unsigned propId, GValue*, GParamSpec*);
99101
static void webKitMediaSrcStreamFlush(Stream*, bool isSeekingFlush);
100102
static gboolean webKitMediaSrcSendEvent(GstElement*, GstEvent*);
103+
static MediaPlayerPrivateGStreamerMSE* webKitMediaSrcPlayer(WebKitMediaSrc*);
101104

102105
#define webkit_media_src_parent_class parent_class
103106

@@ -336,6 +339,16 @@ void webKitMediaSrcEmitStreams(WebKitMediaSrc* source, const Vector<RefPtr<Media
336339
GST_DEBUG_OBJECT(source, "All pads added");
337340
}
338341

342+
static MediaPlayerPrivateGStreamerMSE* webKitMediaSrcPlayer(WebKitMediaSrc* source)
343+
{
344+
return source->priv->player.get();
345+
}
346+
347+
void webKitMediaSrcSetPlayer(WebKitMediaSrc* source, WeakPtr<MediaPlayerPrivateGStreamerMSE>&& player)
348+
{
349+
source->priv->player = WTFMove(player);
350+
}
351+
339352
static void webKitMediaSrcTearDownStream(WebKitMediaSrc* source, const AtomString& name)
340353
{
341354
ASSERT(isMainThread());
@@ -623,11 +636,27 @@ static void webKitMediaSrcStreamFlush(Stream* stream, bool isSeekingFlush)
623636
// The resulting segment is brand new, but with a different start time.
624637
WebKitMediaSrcPrivate* priv = stream->source->priv;
625638
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
639+
streamingMembers->segment.base = 0;
626640
streamingMembers->segment.rate = priv->rate;
627641
streamingMembers->segment.start = streamingMembers->segment.time = priv->startTime;
642+
} else {
643+
// In the case of non-seeking flushes we don't reset the timeline, so instead we need to increase the `base` field
644+
// by however running time we're starting after the flush.
645+
MediaPlayerPrivateGStreamerMSE* player = webKitMediaSrcPlayer(stream->source);
646+
if (player) {
647+
MediaTime streamTime = player->currentMediaTime();
648+
GstClockTime pipelineStreamTime = toGstClockTime(streamTime);
649+
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
650+
// We need to increase the base by the running time accumulated during the previous segment.
651+
GstClockTime pipelineRunningTime = gst_segment_to_running_time(&streamingMembers->segment, GST_FORMAT_TIME, pipelineStreamTime);
652+
if ((GST_CLOCK_TIME_IS_VALID(pipelineRunningTime))) {
653+
GST_DEBUG_OBJECT(stream->source, "Resetting segment to current pipeline running time (%" GST_TIME_FORMAT " and stream time (%" GST_TIME_FORMAT " = %s)",
654+
GST_TIME_ARGS(pipelineRunningTime), GST_TIME_ARGS(pipelineStreamTime), streamTime.toString().ascii().data());
655+
streamingMembers->segment.base = pipelineRunningTime;
656+
streamingMembers->segment.start = streamingMembers->segment.time = static_cast<GstClockTime>(pipelineStreamTime);
657+
}
658+
}
628659
}
629-
// In the case of non-seeking flushes we don't reset the timeline, so the buffers will
630-
// have the same running time as before and we don't need to alter the segment.
631660

632661
if (!skipFlush) {
633662
// By taking the stream lock we are waiting for the streaming thread task to stop if it hadn't yet.
@@ -658,6 +687,11 @@ static void webKitMediaSrcStreamFlush(Stream* stream, bool isSeekingFlush)
658687
gst_pad_push_event(stream->pad.get(), gst_event_new_flush_stop(isSeekingFlush));
659688
GST_DEBUG_OBJECT(stream->pad.get(), "FLUSH_STOP sent.");
660689

690+
{
691+
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
692+
streamingMembers->hasPoppedFirstObject = false;
693+
}
694+
661695
GST_DEBUG_OBJECT(stream->pad.get(), "Starting webKitMediaSrcLoop task and releasing the STREAM_LOCK.");
662696
gst_pad_start_task(stream->pad.get(), webKitMediaSrcLoop, stream->pad.get(), nullptr);
663697
}

‎Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ void webKitMediaSrcEmitStreams(WebKitMediaSrc* source, const Vector<RefPtr<WebCo
6565

6666
void webKitMediaSrcFlush(WebKitMediaSrc*, const AtomString& streamName);
6767

68+
void webKitMediaSrcSetPlayer(WebKitMediaSrc*, WeakPtr<WebCore::MediaPlayerPrivateGStreamerMSE>&&);
69+
6870
G_END_DECLS
6971

7072
namespace WTF {

0 commit comments

Comments
 (0)