Skip to content

Commit 98943f6

Browse files
committed
[MSE] Change canPlayThroughRange to check buffered data at the current position https://bugs.webkit.org/show_bug.cgi?id=265023
Reviewed by Xabier Rodriguez-Calvar. The current SourceBuffer::canPlayThroughRange() implementation is based on average buffering rate. While this approach makes sense in a context of continuous playback, where the JS app is always trying to append data, this isn't always the case in some real life apps. For instance, an app may append a lot of data on page load (enough for sustained playback), then decide to wait for whatever reason, and then start playback. In those circumstances, wait would cause the average buffering rate to be artificially low. There are more examples of the kind of problems that a time-based/average buffering rate-based approach may cause. See: #928 This patch uses the Firefox strategy to this problem[1]: if the ranges to be checked have 3 seconds or more buffered after the current position, we consider that sustained playback is possible. This solves the issues seen in some real world apps. All the logic to monitor and compute the buffering rate has been removed, because it would have remained unused after this change. Based on code from Arnaud Vrac <avrac@freebox.fr> and Jean-Yves Avenard <jyavenard@mozilla.com>. [1] https://github.com/mozilla/gecko-dev/blob/master/dom/media/mediasource/MediaSourceDecoder.cpp#L320 * LayoutTests/media/media-source/media-source-monitor-playing-event-expected.txt: Changed expectation after second append to be HAVE_CURRENT_DATA instead of HAVE_ENOUGH_DATA because the current playback implementation in MockMediaPlayerMediaSource advances playback to the end of the buffered range, so there's no 3s buffered slack after that (needed to get enough data). * LayoutTests/media/media-source/media-source-monitor-playing-event.html: Added some clarification comments. Coalesce multiple 'waiting' events, since they're time dependant and can change between platforms. * LayoutTests/media/media-source/media-managedmse-resume-after-stall-expected.txt: Changed expectation to conform to 3s + 3s buffered segments. * LayoutTests/media/media-source/media-managedmse-resume-after-stall.html: Buffer a minimum of 3s (3 segments) instead of 2, because 3s is the new limit to reach canPlayThrough. * Source/WebCore/Modules/mediasource/SourceBuffer.cpp: (WebCore::SourceBuffer::appendBuffer): Remove call to monitorBufferingRate(). (WebCore::SourceBuffer::sourceBufferPrivateAppendComplete): Ditto. (WebCore::SourceBuffer::canPlayThroughRange): Removed implementation based on m_averageBufferRate. Now return true if the ranges have at least 3 seconds of data after currentTime (with a special case that accounts for the end of the video). Use a tolerance to prevent small errors. (WebCore::SourceBuffer::sourceBufferPrivateDidParseSample): Deleted. (WebCore::SourceBuffer::monitorBufferingRate): Deleted. * Source/WebCore/Modules/mediasource/SourceBuffer.h: Deleted sourceBufferPrivateDidParseSample() and monitorBufferingRate(). * Source/WebCore/platform/graphics/SourceBufferPrivate.cpp: (WebCore::SourceBufferPrivate::processMediaSample): Remove call to sourceBufferPrivateDidParseSample(). Added comment about the tolerance being the same as in SourceBuffer::canPlayThroughRange(). * Source/WebCore/platform/graphics/SourceBufferPrivateClient.h: Deleted sourceBufferPrivateDidParseSample(). * Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp: (WebKit::RemoteSourceBufferProxy::sourceBufferPrivateDidParseSample): Deleted. * Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h: Deleted sourceBufferPrivateDidParseSample(). * Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp: (WebKit::SourceBufferPrivateRemote::sourceBufferPrivateDidParseSample): Deleted. * Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h: Deleted sourceBufferPrivateDidParseSample(). * Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in: Deleted sourceBufferPrivateDidParseSample message. Canonical link: https://commits.webkit.org/272762@main
1 parent f7120b4 commit 98943f6

11 files changed

Lines changed: 26 additions & 75 deletions

LayoutTests/media/media-source/media-source-monitor-playing-event-expected.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ EVENT(canplay)
1616
EVENT(updateend)
1717
EVENT(canplaythrough)
1818
EVENT(playing)
19-
video.readyState : HAVE_ENOUGH_DATA
19+
EXPECTED (video.readyState >= readyStateString.indexOf("HAVE_CURRENT_DATA") == 'true') OK
2020
RUN(sourceBuffer.remove(0,10))
2121
EVENT(updateend)
2222
EVENT(waiting)

LayoutTests/media/media-source/media-source-monitor-playing-event.html

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
var sample;
1212
var handleVideoEvents = [
1313
"loadstart",
14-
"waiting",
1514
"loadedmetadata",
1615
"loadeddata",
1716
"canplay",
@@ -62,10 +61,17 @@
6261
run('sourceBuffer.appendBuffer(sample)');
6362
await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);
6463

65-
consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
64+
// As per the MockMediaPlayerMediaSource implementation, currentTime=10 (the maximum playable time) at
65+
// this point, right after playback has started (at least in WebKitGTK), so we no longer have
66+
// HAVE_ENOUGH_DATA. We have HAVE_CURRENT_DATA instead. We can't test for HAVE_ENOUGH_DATA and for
67+
// canplaythrough at the same time in a single test with the current MockMediaPlayerMediaSource
68+
// implementation. However, we get HAVE_ENOUGH_DATA on some Apple implementations. To avoid problems,
69+
// let's just check for >= HAVE_CURRENT_DATA.
70+
testExpected('video.readyState >= readyStateString.indexOf("HAVE_CURRENT_DATA")', true);
6671
// This remove changes ready state to HAVE_METADATA.
6772
run('sourceBuffer.remove(0,10)');
68-
await waitFor(sourceBuffer, 'updateend');
73+
// Waiting is time-dependant and can happen more than once. We're only interested in at least one occurence.
74+
await Promise.all([waitFor(mediaElement, 'waiting'), waitFor(sourceBuffer, 'updateend')]);
6975

7076
await sleepFor(1000);
7177

@@ -74,8 +80,9 @@
7480
run('sourceBuffer.appendBuffer(sample)');
7581
await waitFor(sourceBuffer, 'updateend');
7682

83+
// Append at least 3s more than currentTime (10) to create an additional playable range that can trigger canPlayThrough.
7784
consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
78-
sample = makeASample(1, 1, 9, 1, 1, SAMPLE_FLAG.SYNC, 1);
85+
sample = makeASample(1, 1, 12, 1, 1, SAMPLE_FLAG.SYNC, 1);
7986
// This append changes the ready state to HAVE_ENOUGH_DATA and fires the playing event.
8087
run('sourceBuffer.appendBuffer(sample)');
8188
await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);

Source/WebCore/Modules/mediasource/SourceBuffer.cpp

Lines changed: 13 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ SourceBuffer::SourceBuffer(Ref<SourceBufferPrivate>&& sourceBufferPrivate, Media
8787
, m_appendWindowStart(MediaTime::zeroTime())
8888
, m_appendWindowEnd(MediaTime::positiveInfiniteTime())
8989
, m_appendState(WaitingForSegment)
90-
, m_timeOfBufferingMonitor(MonotonicTime::fromRawSeconds(0))
9190
, m_pendingRemoveStart(MediaTime::invalidTime())
9291
, m_pendingRemoveEnd(MediaTime::invalidTime())
9392
, m_removeTimer(*this, &SourceBuffer::removeTimerFired)
@@ -227,7 +226,6 @@ ExceptionOr<void> SourceBuffer::setAppendWindowEnd(double newValue)
227226

228227
ExceptionOr<void> SourceBuffer::appendBuffer(const BufferSource& data)
229228
{
230-
monitorBufferingRate();
231229
return appendBufferInternal(static_cast<const unsigned char*>(data.data()), data.length());
232230
}
233231

@@ -593,7 +591,6 @@ void SourceBuffer::sourceBufferPrivateAppendComplete(AppendResult result)
593591
scheduleEvent(eventNames().updateendEvent);
594592

595593
m_source->monitorSourceBuffers();
596-
monitorBufferingRate();
597594
m_private->reenqueueMediaIfNeeded(m_source->currentTime());
598595

599596
DEBUG_LOG(LOGIDENTIFIER, "buffered = ", m_private->buffered()->ranges());
@@ -1174,11 +1171,6 @@ void SourceBuffer::textTrackLanguageChanged(TextTrack& track)
11741171
m_textTracks->scheduleChangeEvent();
11751172
}
11761173

1177-
void SourceBuffer::sourceBufferPrivateDidParseSample(double frameDuration)
1178-
{
1179-
m_bufferedSinceLastMonitor += frameDuration;
1180-
}
1181-
11821174
void SourceBuffer::sourceBufferPrivateDurationChanged(const MediaTime& duration, CompletionHandler<void()>&& completionHandler)
11831175
{
11841176
if (isRemoved()) {
@@ -1209,51 +1201,28 @@ void SourceBuffer::sourceBufferPrivateStreamEndedWithDecodeError()
12091201
m_source->streamEndedWithError(MediaSource::EndOfStreamError::Decode);
12101202
}
12111203

1212-
void SourceBuffer::monitorBufferingRate()
1213-
{
1214-
// We avoid the first update of m_averageBufferRate on purpose, but in exchange we get a more accurate m_timeOfBufferingMonitor initial time.
1215-
if (!m_timeOfBufferingMonitor) {
1216-
m_timeOfBufferingMonitor = MonotonicTime::now();
1217-
return;
1218-
}
1219-
1220-
MonotonicTime now = MonotonicTime::now();
1221-
Seconds interval = now - m_timeOfBufferingMonitor;
1222-
double rateSinceLastMonitor = m_bufferedSinceLastMonitor / interval.seconds();
1223-
1224-
m_timeOfBufferingMonitor = now;
1225-
m_bufferedSinceLastMonitor = 0;
1226-
1227-
m_averageBufferRate += (interval.seconds() * ExponentialMovingAverageCoefficient) * (rateSinceLastMonitor - m_averageBufferRate);
1228-
1229-
DEBUG_LOG(LOGIDENTIFIER, m_averageBufferRate);
1230-
}
1231-
12321204
bool SourceBuffer::canPlayThroughRange(const PlatformTimeRanges& ranges)
12331205
{
12341206
if (isRemoved())
12351207
return false;
12361208

1237-
monitorBufferingRate();
1238-
1239-
// Assuming no fluctuations in the buffering rate, loading 1 second per second or greater
1240-
// means indefinite playback. This could be improved by taking jitter into account.
1241-
if (m_averageBufferRate > 1)
1242-
return true;
1243-
1244-
// Add up all the time yet to be buffered.
1245-
MediaTime currentTime = m_source->currentTime();
12461209
MediaTime duration = m_source->duration();
1210+
if (!duration.isValid())
1211+
return false;
12471212

1248-
PlatformTimeRanges unbufferedRanges = ranges;
1249-
unbufferedRanges.invert();
1250-
unbufferedRanges.intersectWith(PlatformTimeRanges(currentTime, std::max(currentTime, duration)));
1251-
MediaTime unbufferedTime = unbufferedRanges.totalDuration();
1252-
if (!unbufferedTime.isValid())
1213+
MediaTime currentTime = m_source->currentTime();
1214+
if (duration <= currentTime)
12531215
return true;
12541216

1255-
MediaTime timeRemaining = duration - currentTime;
1256-
return unbufferedTime.toDouble() / m_averageBufferRate < timeRemaining.toDouble();
1217+
// If we have data up to the mediasource's duration or 3s ahead, we can
1218+
// assume that we can play without interruption.
1219+
MediaTime bufferedEnd = ranges.maximumBufferedTime();
1220+
// Same tolerance as contiguousFrameTolerance in SourceBufferPrivate::processMediaSample(),
1221+
// to account for small errors.
1222+
const MediaTime tolerance = MediaTime(1, 1000);
1223+
MediaTime timeAhead = std::min(duration, currentTime + MediaTime(3, 1)) - tolerance;
1224+
1225+
return bufferedEnd >= timeAhead;
12571226
}
12581227

12591228
void SourceBuffer::sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory)

Source/WebCore/Modules/mediasource/SourceBuffer.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ class SourceBuffer final
162162
void sourceBufferPrivateAppendComplete(AppendResult) final;
163163
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) final;
164164
void sourceBufferPrivateDurationChanged(const MediaTime& duration, CompletionHandler<void()>&&) final;
165-
void sourceBufferPrivateDidParseSample(double sampleDuration) final;
166165
void sourceBufferPrivateDidDropSample() final;
167166
void sourceBufferPrivateBufferedDirtyChanged(bool) final;
168167
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) final;
@@ -201,8 +200,6 @@ class SourceBuffer final
201200

202201
uint64_t maximumBufferSize() const;
203202

204-
void monitorBufferingRate();
205-
206203
void removeTimerFired();
207204

208205
void reportExtraMemoryAllocated(uint64_t extraMemory);
@@ -243,9 +240,6 @@ class SourceBuffer final
243240
enum AppendStateType { WaitingForSegment, ParsingInitSegment, ParsingMediaSegment };
244241
AppendStateType m_appendState;
245242

246-
MonotonicTime m_timeOfBufferingMonitor;
247-
double m_bufferedSinceLastMonitor { 0 };
248-
double m_averageBufferRate { 0 };
249243
bool m_bufferedDirty { true };
250244

251245
// Can only grow.

Source/WebCore/platform/graphics/SourceBufferPrivate.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,7 @@ void SourceBufferPrivate::didReceiveSample(Ref<MediaSample>&& originalSample)
961961
// For instance, most WebM files are muxed rounded to the millisecond (the default TimecodeScale of the format)
962962
// but their durations use a finer timescale (causing a sub-millisecond overlap). More rarely, there are also
963963
// MP4 files with slightly off tfdt boxes, presenting a similar problem at the beginning of each fragment.
964+
// Same as tolerance in SourceBuffer::canPlayThroughRange().
964965
const MediaTime contiguousFrameTolerance = MediaTime(1, 1000);
965966

966967
// If highest presentation timestamp for track buffer is set and less than or equal to presentation timestamp
@@ -1108,9 +1109,7 @@ void SourceBufferPrivate::didReceiveSample(Ref<MediaSample>&& originalSample)
11081109
presentationEndTime = nearestToPresentationEndTime;
11091110

11101111
trackBuffer.addBufferedRange(presentationTimestamp, presentationEndTime);
1111-
m_client->sourceBufferPrivateDidParseSample(frameDuration.toDouble());
11121112
setBufferedDirty(true);
1113-
11141113
break;
11151114
} while (true);
11161115

Source/WebCore/platform/graphics/SourceBufferPrivateClient.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ class SourceBufferPrivateClient : public CanMakeWeakPtr<SourceBufferPrivateClien
8585
virtual void sourceBufferPrivateAppendComplete(AppendResult) = 0;
8686
virtual void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&) = 0;
8787
virtual void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) = 0;
88-
virtual void sourceBufferPrivateDidParseSample(double frameDuration) = 0;
8988
virtual void sourceBufferPrivateDidDropSample() = 0;
9089
virtual void sourceBufferPrivateBufferedDirtyChanged(bool) = 0;
9190
virtual void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) = 0;

Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,6 @@ void RemoteSourceBufferProxy::sourceBufferPrivateDurationChanged(const MediaTime
147147
m_connectionToWebProcess->connection().sendWithAsyncReply(Messages::SourceBufferPrivateRemote::SourceBufferPrivateDurationChanged(duration), WTFMove(completionHandler), m_identifier);
148148
}
149149

150-
void RemoteSourceBufferProxy::sourceBufferPrivateDidParseSample(double sampleDuration)
151-
{
152-
if (!m_connectionToWebProcess)
153-
return;
154-
155-
m_connectionToWebProcess->connection().send(Messages::SourceBufferPrivateRemote::SourceBufferPrivateDidParseSample(sampleDuration), m_identifier);
156-
}
157-
158150
void RemoteSourceBufferProxy::sourceBufferPrivateDidDropSample()
159151
{
160152
if (!m_connectionToWebProcess)

Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ class RemoteSourceBufferProxy final
7676
void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult) final;
7777
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) final;
7878
void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&) final;
79-
void sourceBufferPrivateDidParseSample(double sampleDuration) final;
8079
void sourceBufferPrivateDidDropSample() final;
8180
void sourceBufferPrivateBufferedDirtyChanged(bool) final;
8281
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) final;

Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -441,12 +441,6 @@ void SourceBufferPrivateRemote::sourceBufferPrivateDurationChanged(const MediaTi
441441
completionHandler();
442442
}
443443

444-
void SourceBufferPrivateRemote::sourceBufferPrivateDidParseSample(double sampleDuration)
445-
{
446-
if (m_client)
447-
m_client->sourceBufferPrivateDidParseSample(sampleDuration);
448-
}
449-
450444
void SourceBufferPrivateRemote::sourceBufferPrivateDidDropSample()
451445
{
452446
if (m_client)

Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ class SourceBufferPrivateRemote final
114114
void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult, const WebCore::PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes, const MediaTime& timestampOffset);
115115
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&);
116116
void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&);
117-
void sourceBufferPrivateDidParseSample(double sampleDuration);
118117
void sourceBufferPrivateDidDropSample();
119118
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode);
120119
void sourceBufferPrivateBufferedDirtyChanged(bool dirty);

0 commit comments

Comments
 (0)