Skip to content

Commit

Permalink
[MSE] Change canPlayThroughRange to check buffered data at the curren…
Browse files Browse the repository at this point in the history
…t 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: WebPlatformForEmbedded#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 <[email protected]> and Jean-Yves Avenard <[email protected]>.

[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
  • Loading branch information
eocanha committed Jan 9, 2024
1 parent f7120b4 commit 98943f6
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ EVENT(canplay)
EVENT(updateend)
EVENT(canplaythrough)
EVENT(playing)
video.readyState : HAVE_ENOUGH_DATA
EXPECTED (video.readyState >= readyStateString.indexOf("HAVE_CURRENT_DATA") == 'true') OK
RUN(sourceBuffer.remove(0,10))
EVENT(updateend)
EVENT(waiting)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
var sample;
var handleVideoEvents = [
"loadstart",
"waiting",
"loadedmetadata",
"loadeddata",
"canplay",
Expand Down Expand Up @@ -62,10 +61,17 @@
run('sourceBuffer.appendBuffer(sample)');
await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);

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

await sleepFor(1000);

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

// Append at least 3s more than currentTime (10) to create an additional playable range that can trigger canPlayThrough.
consoleWrite('video.readyState : ' + readyStateString[video.readyState]);
sample = makeASample(1, 1, 9, 1, 1, SAMPLE_FLAG.SYNC, 1);
sample = makeASample(1, 1, 12, 1, 1, SAMPLE_FLAG.SYNC, 1);
// This append changes the ready state to HAVE_ENOUGH_DATA and fires the playing event.
run('sourceBuffer.appendBuffer(sample)');
await Promise.all([waitFor(mediaElement, 'playing'), waitFor(sourceBuffer, 'updateend')]);
Expand Down
57 changes: 13 additions & 44 deletions Source/WebCore/Modules/mediasource/SourceBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ SourceBuffer::SourceBuffer(Ref<SourceBufferPrivate>&& sourceBufferPrivate, Media
, m_appendWindowStart(MediaTime::zeroTime())
, m_appendWindowEnd(MediaTime::positiveInfiniteTime())
, m_appendState(WaitingForSegment)
, m_timeOfBufferingMonitor(MonotonicTime::fromRawSeconds(0))
, m_pendingRemoveStart(MediaTime::invalidTime())
, m_pendingRemoveEnd(MediaTime::invalidTime())
, m_removeTimer(*this, &SourceBuffer::removeTimerFired)
Expand Down Expand Up @@ -227,7 +226,6 @@ ExceptionOr<void> SourceBuffer::setAppendWindowEnd(double newValue)

ExceptionOr<void> SourceBuffer::appendBuffer(const BufferSource& data)
{
monitorBufferingRate();
return appendBufferInternal(static_cast<const unsigned char*>(data.data()), data.length());
}

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

m_source->monitorSourceBuffers();
monitorBufferingRate();
m_private->reenqueueMediaIfNeeded(m_source->currentTime());

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

void SourceBuffer::sourceBufferPrivateDidParseSample(double frameDuration)
{
m_bufferedSinceLastMonitor += frameDuration;
}

void SourceBuffer::sourceBufferPrivateDurationChanged(const MediaTime& duration, CompletionHandler<void()>&& completionHandler)
{
if (isRemoved()) {
Expand Down Expand Up @@ -1209,51 +1201,28 @@ void SourceBuffer::sourceBufferPrivateStreamEndedWithDecodeError()
m_source->streamEndedWithError(MediaSource::EndOfStreamError::Decode);
}

void SourceBuffer::monitorBufferingRate()
{
// We avoid the first update of m_averageBufferRate on purpose, but in exchange we get a more accurate m_timeOfBufferingMonitor initial time.
if (!m_timeOfBufferingMonitor) {
m_timeOfBufferingMonitor = MonotonicTime::now();
return;
}

MonotonicTime now = MonotonicTime::now();
Seconds interval = now - m_timeOfBufferingMonitor;
double rateSinceLastMonitor = m_bufferedSinceLastMonitor / interval.seconds();

m_timeOfBufferingMonitor = now;
m_bufferedSinceLastMonitor = 0;

m_averageBufferRate += (interval.seconds() * ExponentialMovingAverageCoefficient) * (rateSinceLastMonitor - m_averageBufferRate);

DEBUG_LOG(LOGIDENTIFIER, m_averageBufferRate);
}

bool SourceBuffer::canPlayThroughRange(const PlatformTimeRanges& ranges)
{
if (isRemoved())
return false;

monitorBufferingRate();

// Assuming no fluctuations in the buffering rate, loading 1 second per second or greater
// means indefinite playback. This could be improved by taking jitter into account.
if (m_averageBufferRate > 1)
return true;

// Add up all the time yet to be buffered.
MediaTime currentTime = m_source->currentTime();
MediaTime duration = m_source->duration();
if (!duration.isValid())
return false;

PlatformTimeRanges unbufferedRanges = ranges;
unbufferedRanges.invert();
unbufferedRanges.intersectWith(PlatformTimeRanges(currentTime, std::max(currentTime, duration)));
MediaTime unbufferedTime = unbufferedRanges.totalDuration();
if (!unbufferedTime.isValid())
MediaTime currentTime = m_source->currentTime();
if (duration <= currentTime)
return true;

MediaTime timeRemaining = duration - currentTime;
return unbufferedTime.toDouble() / m_averageBufferRate < timeRemaining.toDouble();
// If we have data up to the mediasource's duration or 3s ahead, we can
// assume that we can play without interruption.
MediaTime bufferedEnd = ranges.maximumBufferedTime();
// Same tolerance as contiguousFrameTolerance in SourceBufferPrivate::processMediaSample(),
// to account for small errors.
const MediaTime tolerance = MediaTime(1, 1000);
MediaTime timeAhead = std::min(duration, currentTime + MediaTime(3, 1)) - tolerance;

return bufferedEnd >= timeAhead;
}

void SourceBuffer::sourceBufferPrivateReportExtraMemoryCost(uint64_t extraMemory)
Expand Down
6 changes: 0 additions & 6 deletions Source/WebCore/Modules/mediasource/SourceBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ class SourceBuffer final
void sourceBufferPrivateAppendComplete(AppendResult) final;
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) final;
void sourceBufferPrivateDurationChanged(const MediaTime& duration, CompletionHandler<void()>&&) final;
void sourceBufferPrivateDidParseSample(double sampleDuration) final;
void sourceBufferPrivateDidDropSample() final;
void sourceBufferPrivateBufferedDirtyChanged(bool) final;
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) final;
Expand Down Expand Up @@ -201,8 +200,6 @@ class SourceBuffer final

uint64_t maximumBufferSize() const;

void monitorBufferingRate();

void removeTimerFired();

void reportExtraMemoryAllocated(uint64_t extraMemory);
Expand Down Expand Up @@ -243,9 +240,6 @@ class SourceBuffer final
enum AppendStateType { WaitingForSegment, ParsingInitSegment, ParsingMediaSegment };
AppendStateType m_appendState;

MonotonicTime m_timeOfBufferingMonitor;
double m_bufferedSinceLastMonitor { 0 };
double m_averageBufferRate { 0 };
bool m_bufferedDirty { true };

// Can only grow.
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/platform/graphics/SourceBufferPrivate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ void SourceBufferPrivate::didReceiveSample(Ref<MediaSample>&& originalSample)
// For instance, most WebM files are muxed rounded to the millisecond (the default TimecodeScale of the format)
// but their durations use a finer timescale (causing a sub-millisecond overlap). More rarely, there are also
// MP4 files with slightly off tfdt boxes, presenting a similar problem at the beginning of each fragment.
// Same as tolerance in SourceBuffer::canPlayThroughRange().
const MediaTime contiguousFrameTolerance = MediaTime(1, 1000);

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

trackBuffer.addBufferedRange(presentationTimestamp, presentationEndTime);
m_client->sourceBufferPrivateDidParseSample(frameDuration.toDouble());
setBufferedDirty(true);

break;
} while (true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ class SourceBufferPrivateClient : public CanMakeWeakPtr<SourceBufferPrivateClien
virtual void sourceBufferPrivateAppendComplete(AppendResult) = 0;
virtual void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&) = 0;
virtual void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) = 0;
virtual void sourceBufferPrivateDidParseSample(double frameDuration) = 0;
virtual void sourceBufferPrivateDidDropSample() = 0;
virtual void sourceBufferPrivateBufferedDirtyChanged(bool) = 0;
virtual void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) = 0;
Expand Down
8 changes: 0 additions & 8 deletions Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,6 @@ void RemoteSourceBufferProxy::sourceBufferPrivateDurationChanged(const MediaTime
m_connectionToWebProcess->connection().sendWithAsyncReply(Messages::SourceBufferPrivateRemote::SourceBufferPrivateDurationChanged(duration), WTFMove(completionHandler), m_identifier);
}

void RemoteSourceBufferProxy::sourceBufferPrivateDidParseSample(double sampleDuration)
{
if (!m_connectionToWebProcess)
return;

m_connectionToWebProcess->connection().send(Messages::SourceBufferPrivateRemote::SourceBufferPrivateDidParseSample(sampleDuration), m_identifier);
}

void RemoteSourceBufferProxy::sourceBufferPrivateDidDropSample()
{
if (!m_connectionToWebProcess)
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class RemoteSourceBufferProxy final
void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult) final;
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&) final;
void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&) final;
void sourceBufferPrivateDidParseSample(double sampleDuration) final;
void sourceBufferPrivateDidDropSample() final;
void sourceBufferPrivateBufferedDirtyChanged(bool) final;
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode) final;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,12 +441,6 @@ void SourceBufferPrivateRemote::sourceBufferPrivateDurationChanged(const MediaTi
completionHandler();
}

void SourceBufferPrivateRemote::sourceBufferPrivateDidParseSample(double sampleDuration)
{
if (m_client)
m_client->sourceBufferPrivateDidParseSample(sampleDuration);
}

void SourceBufferPrivateRemote::sourceBufferPrivateDidDropSample()
{
if (m_client)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ class SourceBufferPrivateRemote final
void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult, const WebCore::PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes, const MediaTime& timestampOffset);
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&);
void sourceBufferPrivateDurationChanged(const MediaTime&, CompletionHandler<void()>&&);
void sourceBufferPrivateDidParseSample(double sampleDuration);
void sourceBufferPrivateDidDropSample();
void sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode);
void sourceBufferPrivateBufferedDirtyChanged(bool dirty);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ messages -> SourceBufferPrivateRemote NotRefCounted {
SourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult appendResult, WebCore::PlatformTimeRanges buffered, uint64_t totalTrackBufferSizeInBytes, MediaTime timeStampOffset)
SourceBufferPrivateHighestPresentationTimestampChanged(MediaTime timestamp)
SourceBufferPrivateDurationChanged(MediaTime duration) -> ()
SourceBufferPrivateDidParseSample(double sampleDuration)
SourceBufferPrivateDidDropSample()
SourceBufferPrivateBufferedDirtyChanged(bool dirty)
SourceBufferPrivateDidReceiveRenderingError(int64_t errorCode)
Expand Down

0 comments on commit 98943f6

Please sign in to comment.