[webkit-changes] [WebKit/WebKit] bbf768: [GPUP][MSE] RemoteSourceBufferProxy::EvictCodedFra...

Tue Feb 27 06:31:00 PST 2024

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: bbf768df7ba799771f882dc739c09034f7a92934
      https://github.com/WebKit/WebKit/commit/bbf768df7ba799771f882dc739c09034f7a92934
  Author: Jean-Yves Avenard <jya at apple.com>
  Date:   2024-02-27 (Tue, 27 Feb 2024)

  Changed paths:
    M LayoutTests/media/media-source/live-rewind-seek-and-evict.html
    M LayoutTests/media/media-source/media-source-append-before-last-range-no-quota-exceeded.html
    M LayoutTests/media/media-source/media-source-append-buffer-full-evict-prior-to-end.html
    M LayoutTests/media/media-source/media-source-append-buffer-full-quota-exceeded-error-onstart.html
    M LayoutTests/media/media-source/media-source-append-buffer-full-quota-exceeded-error.html
    M LayoutTests/media/media-source/media-source-evict-codedframe-after-seek.html
    M LayoutTests/media/media-source/media-source-evict-codedframe-large-currenttime.html
    M LayoutTests/media/media-source/media-source-monitor-playing-event.html
    M Source/WebCore/Modules/mediasource/SourceBuffer.cpp
    M Source/WebCore/Modules/mediasource/SourceBuffer.h
    M Source/WebCore/platform/graphics/SourceBufferPrivate.cpp
    M Source/WebCore/platform/graphics/SourceBufferPrivate.h
    M Source/WebCore/platform/graphics/SourceBufferPrivateClient.h
    M Source/WebCore/platform/graphics/TrackBuffer.cpp
    M Source/WebCore/platform/graphics/TrackBuffer.h
    M Source/WebCore/testing/Internals.cpp
    M Source/WebCore/testing/Internals.h
    M Source/WebCore/testing/Internals.idl
    M Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp
    M Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h
    M Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.messages.in
    M Source/WebKit/Scripts/webkit/messages.py
    M Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
    M Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp
    M Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h
    M Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemoteMessageReceiver.messages.in

  Log Message:
  -----------
  [GPUP][MSE] RemoteSourceBufferProxy::EvictCodedFrames shouldn't be a sync call
https://bugs.webkit.org/show_bug.cgi?id=265079
rdar://118783504

Reviewed by Youenn Fablet.

Sync calls should be avoided under most circumstances, however with MSE, the prepare append
algorithm is only made of synchronous steps and the synchronous call was made into a
sendSync call to the GPU process for expediency.
The only reason this call had to be synchronous was to determine if a new appendBuffer
call was going to succeed even if we were running out of space and after an eviction.

Instead, we calculate following each source buffer operation the amount of evictable data
based on the currentTime.
When the SourceBuffer needs to append a new buffer, we will then now immediately if:
1- there's enough space to accept the new buffer as-is
2- if there's not enough space and we evict content then the data would fit.
If either 1 or 2 fails, then we are to throw a QuotaExceededError.

In the case of 1) there's nothing to do, we can avoid the call to EvictCodedFrame altogher.
If 2) we can run the EvictCodedFrame algorithm asynchronously and don't need to wait for
the result of the operation: we know the next append will succeed.

For 100% spec-compliance, should we fire QuotaExceededError, we want the eviction to run and
be completed. Some browsers (Firefox in particular), do not fullfill this requirement: the SourceBuffer
will throw, and eviction will still be done asynchronously; which is arguably a better and simpler
behaviour from an implementer point of view.

While the evictable amount of data would change over time as playback progress, for our case
we don't need to update it at regular interval as we will conservatively run the eviction step
which will always then free more memory than we expected we could.

In practice, it means that the only time we will fire a sync call to the GPU process is if
the append buffer operation will fail as we have run out of space, and we will not be
able to make sufficient room.

Covered by existing tests, no change in observable behaviour.

* LayoutTests/media/media-source/live-rewind-seek-and-evict.html:
* LayoutTests/media/media-source/media-source-append-before-last-range-no-quota-exceeded.html:
* LayoutTests/media/media-source/media-source-append-buffer-full-evict-prior-to-end.html:
* LayoutTests/media/media-source/media-source-append-buffer-full-quota-exceeded-error-onstart.html:
* LayoutTests/media/media-source/media-source-append-buffer-full-quota-exceeded-error.html:
* LayoutTests/media/media-source/media-source-evict-codedframe-after-seek.html:
* LayoutTests/media/media-source/media-source-monitor-playing-event.html:
* Source/WebCore/Modules/mediasource/SourceBuffer.cpp:
(WebCore::m_logIdentifier):
(WebCore::SourceBuffer::appendBufferInternal):
(WebCore::SourceBuffer::maximumBufferSize const):
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveInitializationSegment):
(WebCore::SourceBuffer::setMaximumSourceBufferSize):
(WebCore::SourceBuffer::sourceBufferPrivateBufferedChanged):
(WebCore::SourceBuffer::memoryPressure):
* Source/WebCore/Modules/mediasource/SourceBuffer.h:
* Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:
(WebCore::SourceBufferPrivate::updateBuffered):
(WebCore::SourceBufferPrivate::computeSeekTime):
(WebCore::SourceBufferPrivate::clearTrackBuffers):
(WebCore::SourceBufferPrivate::removeCodedFrames):
(WebCore::SourceBufferPrivate::removeCodedFramesInternal):
(WebCore::SourceBufferPrivate::setMaximumBufferSize):
(WebCore::SourceBufferPrivate::computeEvictionData):
(WebCore::SourceBufferPrivate::hasTooManySamples const):
(WebCore::SourceBufferPrivate::asyncEvictCodedFrames): Add asynchronous task that will run
evictCodedFrame after all the currently pending tasks.
(WebCore::SourceBufferPrivate::evictCodedFrames): Return a boolean to indicate if the operation
succeeded, instead of having to query isBufferFullFor.
(WebCore::SourceBufferPrivate::isBufferFullFor const):
(WebCore::SourceBufferPrivate::canAppend const):
(WebCore::SourceBufferPrivate::append):
(WebCore::SourceBufferPrivate::memoryPressure):
(WebCore::SourceBufferPrivate::evictFrames):
(WebCore::SourceBufferPrivate::isBufferFullFor): Deleted.
* Source/WebCore/platform/graphics/SourceBufferPrivate.h:
(WebCore::SourceBufferPrivate::evictionData const):
* Source/WebCore/platform/graphics/SourceBufferPrivateClient.h:
(WebCore::SourceBufferEvictionData::operator!=):
(WebCore::SourceBufferEvictionData::clear):
(WebCore::SourceBufferPrivateClient::sourceBufferPrivateEvictionDataChanged):
(WTF::LogArgument<WebCore::SourceBufferEvictionData>::toString):
* Source/WebCore/platform/graphics/TrackBuffer.cpp:
(WebCore::TrackBuffer::removeSamples): Instead of returning a boolean to determine if the
operatiation suceeded (returned value that was unused). We return the amount of bytes we
removed from the SampleMap. It allows to incrementally update the known SourceBuffer size
without having to recalculate it all the time.
(WebCore::TrackBuffer::removeCodedFrames): Simplify code by only calculating the size removed
(for logging purposes) outside the critical loop.
(WebCore::TrackBuffer::codedFramesIntervalSize): Add method to determine approximately how
much bytes would be removed should we remove the frames. This is used for calculating the evictable
size, without actually removing the data.
* Source/WebCore/platform/graphics/TrackBuffer.h:
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::setMaximumSourceBufferSize):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl: Add a way to asynchronously set the SourceBuffer's maximumBufferSize
and wait for the operation to complete. Simply changing the settings wasn't sufficient as it provided no guarantee
that the GPU process would receive the new value in time.
* Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp:
(WebKit::RemoteSourceBufferProxy::sourceBufferPrivateBufferedChanged):
(WebKit::RemoteSourceBufferProxy::sourceBufferPrivateEvictionDataChanged):
(WebKit::RemoteSourceBufferProxy::evictCodedFrames):
(WebKit::RemoteSourceBufferProxy::asyncEvictCodedFrames):
(WebKit::RemoteSourceBufferProxy::setMaximumBufferSize):
(WebKit::RemoteSourceBufferProxy::memoryPressure):
* Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.h:
* Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.messages.in:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
(WebKit::SourceBufferPrivateRemote::evictCodedFrames):
(WebKit::SourceBufferPrivateRemote::clearTrackBuffers):
(WebKit::SourceBufferPrivateRemote::setMaximumBufferSize):
(WebKit::SourceBufferPrivateRemote::MessageReceiver::sourceBufferPrivateEvictionDataChanged):
(WebKit::SourceBufferPrivateRemote::MessageReceiver::sourceBufferPrivateBufferedChanged):
(WebKit::SourceBufferPrivateRemote::totalTrackBufferSizeInBytes const):
(WebKit::SourceBufferPrivateRemote::memoryPressure):
(WebKit::SourceBufferPrivateRemote::isBufferFullFor const):
(WebKit::SourceBufferPrivateRemote::canAppend const):
* Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h:
* Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemoteMessageReceiver.messages.in:

Canonical link: https://commits.webkit.org/275380@main

To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications