Title: [279973] trunk
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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to