- Revision
- 279973
- Author
- [email protected]
- Date
- 2021-07-15 19:07:47 -0700 (Thu, 15 Jul 2021)
Log Message
[MSE] sequence mode is broken if GPU Process is enabled
https://bugs.webkit.org/show_bug.cgi?id=227864
<rdar://problem/80445041>
Reviewed by Jer Noble.
Source/WebCore:
When the source buffer's mode is set to sequence, the timestampOffset attribute
should be updated after each appendBuffer operation. However, when the GPU process
is enabled, the timestampOffset calculations are all done in the GPU process and
its result wasn't communicated back to the WebContent process leading its value
to always be 0 on read.
Fly-by fix: the timestamp offset calculation was incorrect, per spec
https://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing
step 3.5.8.3.1 we read:
// Set timestampOffset equal to group start timestamp - presentation timestamp.
but the code did otherwise.
Test: media/media-source/media-mp4-h264-sequence-mode.html
* platform/graphics/SourceBufferPrivate.cpp:
(WebCore::SourceBufferPrivate::didReceiveSample): Offset m_timestampOffset by
the sample's presentation timestamp
Source/WebKit:
When the source buffer's mode is set to sequence, the timestampOffset attribute
should be updated after each appendBuffer operation. However, when the GPU process
is enabled, the timestampOffset calculations are all done in the GPU process and
its result wasn't communicated back to the WebContent process leading its value
to always be 0 on read.
* GPUProcess/media/RemoteSourceBufferProxy.cpp:
(WebKit::RemoteSourceBufferProxy::sourceBufferPrivateAppendComplete): Add value of
the potentially updated timestamp offset back to the content process.
* WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
(WebKit::SourceBufferPrivateRemote::sourceBufferPrivateAppendComplete):
* WebProcess/GPU/media/SourceBufferPrivateRemote.h:
* WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in: Add extra timestampOffset
parameter.
LayoutTests:
Note: we had tests checking on the correctness of the operation; however it's
using a mock SourceBuffer which doesn't run in the GPU process. Doing bug 225367
should be a priority so we get better code coverage and such bugs don't skip
under the radar.
To get around this issue, we write a new test that uses a mp4 and a h264 video track.
* media/media-source/media-mp4-h264-sequence-mode-expected.txt: Added.
* media/media-source/media-mp4-h264-sequence-mode.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (279972 => 279973)
--- trunk/LayoutTests/ChangeLog 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/LayoutTests/ChangeLog 2021-07-16 02:07:47 UTC (rev 279973)
@@ -1,3 +1,20 @@
+2021-07-15 Jean-Yves Avenard <[email protected]>
+
+ [MSE] sequence mode is broken if GPU Process is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=227864
+ <rdar://problem/80445041>
+
+ Reviewed by Jer Noble.
+
+ Note: we had tests checking on the correctness of the operation; however it's
+ using a mock SourceBuffer which doesn't run in the GPU process. Doing bug 225367
+ should be a priority so we get better code coverage and such bugs don't skip
+ under the radar.
+ To get around this issue, we write a new test that uses a mp4 and a h264 video track.
+
+ * media/media-source/media-mp4-h264-sequence-mode-expected.txt: Added.
+ * media/media-source/media-mp4-h264-sequence-mode.html: Added.
+
2021-07-15 Chris Dumez <[email protected]>
Add initial support for BroadcastChannel behind a runtime flag
Added: trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode-expected.txt (0 => 279973)
--- trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode-expected.txt (rev 0)
+++ trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode-expected.txt 2021-07-16 02:07:47 UTC (rev 279973)
@@ -0,0 +1,22 @@
+
+RUN(video.src = ""
+EVENT(sourceopen)
+Append the init segment
+RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
+RUN(sourceBuffer.appendBuffer(loader.initSegment()))
+EVENT(update)
+EXPECTED (sourceBuffer.timestampOffset == '0') OK
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(1)))
+EVENT(update)
+EXPECTED (sourceBuffer.timestampOffset == '0') OK
+EXPECTED (sourceBuffer.buffered.length == '1') OK
+EXPECTED (sourceBuffer.buffered.start(0) == '1') OK
+EXPECTED (sourceBuffer.buffered.end(0) == '2') OK
+RUN(sourceBuffer.mode="sequence")
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(1)))
+EVENT(update)
+EXPECTED (sourceBuffer.timestampOffset == '1') OK
+EXPECTED (sourceBuffer.buffered.length == '1') OK
+EXPECTED (sourceBuffer.buffered.end(0) == '3') OK
+END OF TEST
+
Added: trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode.html (0 => 279973)
--- trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode.html (rev 0)
+++ trunk/LayoutTests/media/media-source/media-mp4-h264-sequence-mode.html 2021-07-16 02:07:47 UTC (rev 279973)
@@ -0,0 +1,65 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-mp4-h264-sequence-mode</title>
+ <script src=""
+ <script src=""
+ <script>
+ var loader;
+ var source;
+ var sourceBuffer;
+ var random;
+
+ function loaderPromise(loader) {
+ return new Promise((resolve, reject) => {
+ loader._onload_ = resolve;
+ loader._onerror_ = reject;
+ });
+ }
+
+ function randomNumber(min, max) {
+ return Math.floor(Math.random() * (max - min) + min);
+ }
+
+ window.addEventListener('load', async event => {
+ try {
+ findMediaElement();
+ loader = new MediaSourceLoader('content/test-fragmented-video-manifest.json');
+ await loaderPromise(loader);
+
+ source = new MediaSource();
+ run('video.src = ""
+ await waitFor(source, 'sourceopen');
+ waitFor(video, 'error').then(failTest);
+
+ consoleWrite('Append the init segment');
+ run('sourceBuffer = source.addSourceBuffer(loader.type())');
+ run('sourceBuffer.appendBuffer(loader.initSegment())');
+ await waitFor(sourceBuffer, 'update');
+
+ testExpected('sourceBuffer.timestampOffset', '0');
+ run('sourceBuffer.appendBuffer(loader.mediaSegment(1))');
+ await waitFor(sourceBuffer, 'update');
+ testExpected('sourceBuffer.timestampOffset', '0');
+ testExpected('sourceBuffer.buffered.length', '1');
+ testExpected('sourceBuffer.buffered.start(0)', '1');
+ testExpected('sourceBuffer.buffered.end(0)', '2');
+
+ run('sourceBuffer.mode="sequence"');
+ run('sourceBuffer.appendBuffer(loader.mediaSegment(1))');
+ await waitFor(sourceBuffer, 'update');
+ testExpected('sourceBuffer.timestampOffset', '1');
+ testExpected('sourceBuffer.buffered.length', '1');
+ testExpected('sourceBuffer.buffered.end(0)', '3');
+
+ endTest();
+ } catch (e) {
+ failTest(`Caught exception: "${e}"`);
+ }
+ });
+ </script>
+</head>
+<body>
+ <video controls></video>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (279972 => 279973)
--- trunk/Source/WebCore/ChangeLog 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebCore/ChangeLog 2021-07-16 02:07:47 UTC (rev 279973)
@@ -1,3 +1,28 @@
+2021-07-15 Jean-Yves Avenard <[email protected]>
+
+ [MSE] sequence mode is broken if GPU Process is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=227864
+ <rdar://problem/80445041>
+
+ Reviewed by Jer Noble.
+
+ When the source buffer's mode is set to sequence, the timestampOffset attribute
+ should be updated after each appendBuffer operation. However, when the GPU process
+ is enabled, the timestampOffset calculations are all done in the GPU process and
+ its result wasn't communicated back to the WebContent process leading its value
+ to always be 0 on read.
+ Fly-by fix: the timestamp offset calculation was incorrect, per spec
+ https://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing
+ step 3.5.8.3.1 we read:
+ // Set timestampOffset equal to group start timestamp - presentation timestamp.
+ but the code did otherwise.
+
+ Test: media/media-source/media-mp4-h264-sequence-mode.html
+
+ * platform/graphics/SourceBufferPrivate.cpp:
+ (WebCore::SourceBufferPrivate::didReceiveSample): Offset m_timestampOffset by
+ the sample's presentation timestamp
+
2021-07-15 Chris Dumez <[email protected]>
Add initial support for BroadcastChannel behind a runtime flag
Modified: trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp (279972 => 279973)
--- trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebCore/platform/graphics/SourceBufferPrivate.cpp 2021-07-16 02:07:47 UTC (rev 279973)
@@ -891,7 +891,7 @@
// 1.3 If mode equals "sequence" and group start timestamp is set, then run the following steps:
if (m_appendMode == SourceBufferAppendMode::Sequence && m_groupStartTimestamp.isValid()) {
// 1.3.1 Set timestampOffset equal to group start timestamp - presentation timestamp.
- m_timestampOffset = m_groupStartTimestamp;
+ m_timestampOffset = m_groupStartTimestamp - presentationTimestamp;
for (auto& trackBuffer : m_trackBufferMap.values()) {
trackBuffer.get().lastFrameTimescale = 0;
Modified: trunk/Source/WebKit/ChangeLog (279972 => 279973)
--- trunk/Source/WebKit/ChangeLog 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebKit/ChangeLog 2021-07-16 02:07:47 UTC (rev 279973)
@@ -1,3 +1,26 @@
+2021-07-15 Jean-Yves Avenard <[email protected]>
+
+ [MSE] sequence mode is broken if GPU Process is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=227864
+ <rdar://problem/80445041>
+
+ Reviewed by Jer Noble.
+
+ When the source buffer's mode is set to sequence, the timestampOffset attribute
+ should be updated after each appendBuffer operation. However, when the GPU process
+ is enabled, the timestampOffset calculations are all done in the GPU process and
+ its result wasn't communicated back to the WebContent process leading its value
+ to always be 0 on read.
+
+ * GPUProcess/media/RemoteSourceBufferProxy.cpp:
+ (WebKit::RemoteSourceBufferProxy::sourceBufferPrivateAppendComplete): Add value of
+ the potentially updated timestamp offset back to the content process.
+ * WebProcess/GPU/media/SourceBufferPrivateRemote.cpp:
+ (WebKit::SourceBufferPrivateRemote::sourceBufferPrivateAppendComplete):
+ * WebProcess/GPU/media/SourceBufferPrivateRemote.h:
+ * WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in: Add extra timestampOffset
+ parameter.
+
2021-07-15 Chris Dumez <[email protected]>
Add initial support for BroadcastChannel behind a runtime flag
Modified: trunk/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp (279972 => 279973)
--- trunk/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebKit/GPUProcess/media/RemoteSourceBufferProxy.cpp 2021-07-16 02:07:47 UTC (rev 279973)
@@ -165,7 +165,7 @@
return;
auto buffered = m_sourceBufferPrivate->buffered()->ranges();
- m_connectionToWebProcess->connection().send(Messages::SourceBufferPrivateRemote::SourceBufferPrivateAppendComplete(appendResult, WTFMove(buffered), m_sourceBufferPrivate->totalTrackBufferSizeInBytes()), m_identifier);
+ m_connectionToWebProcess->connection().send(Messages::SourceBufferPrivateRemote::SourceBufferPrivateAppendComplete(appendResult, WTFMove(buffered), m_sourceBufferPrivate->totalTrackBufferSizeInBytes(), m_sourceBufferPrivate->timestampOffset()), m_identifier);
}
void RemoteSourceBufferProxy::sourceBufferPrivateDidReceiveRenderingError(int64_t errorCode)
Modified: trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp (279972 => 279973)
--- trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.cpp 2021-07-16 02:07:47 UTC (rev 279973)
@@ -414,12 +414,14 @@
m_client->sourceBufferPrivateAppendError(decodeError);
}
-void SourceBufferPrivateRemote::sourceBufferPrivateAppendComplete(SourceBufferPrivateClient::AppendResult appendResult, const PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes)
+void SourceBufferPrivateRemote::sourceBufferPrivateAppendComplete(SourceBufferPrivateClient::AppendResult appendResult, const PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes, const MediaTime& timestampOffset)
{
setBufferedRanges(buffered);
m_totalTrackBufferSizeInBytes = totalTrackBufferSizeInBytes;
- if (m_client)
+ if (m_client) {
+ setTimestampOffset(timestampOffset);
m_client->sourceBufferPrivateAppendComplete(appendResult);
+ }
}
void SourceBufferPrivateRemote::sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime& timestamp)
Modified: trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h (279972 => 279973)
--- trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.h 2021-07-16 02:07:47 UTC (rev 279973)
@@ -111,7 +111,7 @@
void sourceBufferPrivateDidReceiveInitializationSegment(InitializationSegmentInfo&&, CompletionHandler<void()>&&);
void sourceBufferPrivateStreamEndedWithDecodeError();
void sourceBufferPrivateAppendError(bool decodeError);
- void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult, const WebCore::PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes);
+ void sourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult, const WebCore::PlatformTimeRanges& buffered, uint64_t totalTrackBufferSizeInBytes, const MediaTime& timestampOffset);
void sourceBufferPrivateHighestPresentationTimestampChanged(const MediaTime&);
void sourceBufferPrivateDurationChanged(const MediaTime&);
void sourceBufferPrivateDidParseSample(double sampleDuration);
Modified: trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in (279972 => 279973)
--- trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in 2021-07-16 01:25:20 UTC (rev 279972)
+++ trunk/Source/WebKit/WebProcess/GPU/media/SourceBufferPrivateRemote.messages.in 2021-07-16 02:07:47 UTC (rev 279973)
@@ -29,7 +29,7 @@
SourceBufferPrivateDidReceiveInitializationSegment(struct WebKit::InitializationSegmentInfo segmentConfiguration) -> () Async
SourceBufferPrivateStreamEndedWithDecodeError()
SourceBufferPrivateAppendError(bool decodeError)
- SourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult appendResult, WebCore::PlatformTimeRanges buffered, uint64_t totalTrackBufferSizeInBytes)
+ SourceBufferPrivateAppendComplete(WebCore::SourceBufferPrivateClient::AppendResult appendResult, WebCore::PlatformTimeRanges buffered, uint64_t totalTrackBufferSizeInBytes, MediaTime timeStampOffset)
SourceBufferPrivateHighestPresentationTimestampChanged(MediaTime timestamp)
SourceBufferPrivateDurationChanged(MediaTime duration)
SourceBufferPrivateDidParseSample(double sampleDuration)