REGRESSION (macOS 10.13.2): imported/w3c/web-platform-tests/media-source/mediasource...
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 02:38:51 +0000 (02:38 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 02:38:51 +0000 (02:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181891

Reviewed by Eric Carlson.

Source/WebCore:

In macOS 10.13.2, CoreMedia changed the definition of CMSampleBufferGetDuration() to return
the presentation duration rather than the decode duration. For media streams where those two
durations are identical (or at least, closely similar), this isn't a problem. But the media
file used in the WPT tests have an unusual frame cadence: decode durations go {3000, 1, 5999,
1, 5999,...} and presentation durations go {3000, 2999, 3000, 2999}. This caused one check in
the "Coded Frame Processing" algorithm to begin failing, where it checks that the delta
between the last sample's decode time and the new decode time is no more than 2x as far as
the last sample's duration. That's not a problem as long as the "duration" is the "decode
duration" and the samples are all adjacent. Once the "duration" is "presentation duration",
all the assumptions in the algorithm are invalidated. In the WPT test case, the delta between
decode times is 5999, and 2 * the presentation duration is 5998, causing all samples up to
the next sync sample to be dropped.

To work around this change in behavior, we'll adopt the same technique used by Mozilla's MSE
implementation, which was done for similar reasons. Rather than track the "last frame duration",
we'll record the "greatest frame duration", and use actual decode timestamps to derive this
duration. The "greatest frame duration" field will be reset at the same times as "last frame
duration", and will be used only in the part of the algorithm that checks for large decode
timestamp gaps.

* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::TrackBuffer::TrackBuffer):
(WebCore::SourceBuffer::resetParserState):
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):

LayoutTests:

* platform/mac/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227278 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SourceBuffer.cpp

index f71e902..0934a0d 100644 (file)
@@ -1,3 +1,12 @@
+2018-01-21  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (macOS 10.13.2): imported/w3c/web-platform-tests/media-source/mediasource-* LayoutTests failing
+        https://bugs.webkit.org/show_bug.cgi?id=181891
+
+        Reviewed by Eric Carlson.
+
+        * platform/mac/TestExpectations:
+
 2018-01-21  Andy Estes  <aestes@apple.com>
 
         [ios] LayoutTest imported/w3c/web-platform-tests/payment-request/rejects_if_not_active.https.html is crashing in JSC::JSONParse
index 72ad51c..d650001 100644 (file)
@@ -1746,11 +1746,6 @@ webkit.org/b/175998 http/tests/security/mixedContent/insecure-css-with-secure-co
 [ ElCapitan Sierra ] platform/mac/media/encrypted-media/fps-generateRequest.html [ Skip ]
 [ ElCapitan Sierra ] platform/mac/media/encrypted-media/fps-encrypted-event.html [ Skip ]
 
-# rdar://problem/35395437
-[ HighSierra+ ] imported/w3c/web-platform-tests/media-source/mediasource-play.html [ Failure ]
-[ HighSierra+ ] imported/w3c/web-platform-tests/media-source/mediasource-redundant-seek.html [ Failure ]
-[ HighSierra+ ] imported/w3c/web-platform-tests/media-source/mediasource-remove.html [ Failure ]
-
 webkit.org/b/165311 [ Sierra+ ] media/modern-media-controls/pip-support/pip-support-click.html [ Pass Failure ]
 
 webkit.org/b/176693 storage/indexeddb/modern/idbtransaction-objectstore-failures-private.html [ Pass Failure ]
index 2119631..a91ec46 100644 (file)
@@ -1,3 +1,35 @@
+2018-01-21  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION (macOS 10.13.2): imported/w3c/web-platform-tests/media-source/mediasource-* LayoutTests failing
+        https://bugs.webkit.org/show_bug.cgi?id=181891
+
+        Reviewed by Eric Carlson.
+
+        In macOS 10.13.2, CoreMedia changed the definition of CMSampleBufferGetDuration() to return
+        the presentation duration rather than the decode duration. For media streams where those two
+        durations are identical (or at least, closely similar), this isn't a problem. But the media
+        file used in the WPT tests have an unusual frame cadence: decode durations go {3000, 1, 5999,
+        1, 5999,...} and presentation durations go {3000, 2999, 3000, 2999}. This caused one check in
+        the "Coded Frame Processing" algorithm to begin failing, where it checks that the delta
+        between the last sample's decode time and the new decode time is no more than 2x as far as
+        the last sample's duration. That's not a problem as long as the "duration" is the "decode
+        duration" and the samples are all adjacent. Once the "duration" is "presentation duration",
+        all the assumptions in the algorithm are invalidated. In the WPT test case, the delta between
+        decode times is 5999, and 2 * the presentation duration is 5998, causing all samples up to
+        the next sync sample to be dropped.
+
+        To work around this change in behavior, we'll adopt the same technique used by Mozilla's MSE
+        implementation, which was done for similar reasons. Rather than track the "last frame duration",
+        we'll record the "greatest frame duration", and use actual decode timestamps to derive this
+        duration. The "greatest frame duration" field will be reset at the same times as "last frame
+        duration", and will be used only in the part of the algorithm that checks for large decode
+        timestamp gaps.
+
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::TrackBuffer::TrackBuffer):
+        (WebCore::SourceBuffer::resetParserState):
+        (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+
 2018-01-21  Andy Estes  <aestes@apple.com>
 
         [ios] LayoutTest imported/w3c/web-platform-tests/payment-request/rejects_if_not_active.https.html is crashing in JSC::JSONParse
index afb9fe8..c79757a 100644 (file)
@@ -64,6 +64,7 @@ static const double ExponentialMovingAverageCoefficient = 0.1;
 
 struct SourceBuffer::TrackBuffer {
     MediaTime lastDecodeTimestamp;
+    MediaTime greatestDecodeDuration;
     MediaTime lastFrameDuration;
     MediaTime highestPresentationTimestamp;
     MediaTime lastEnqueuedPresentationTime;
@@ -78,6 +79,7 @@ struct SourceBuffer::TrackBuffer {
 
     TrackBuffer()
         : lastDecodeTimestamp(MediaTime::invalidTime())
+        , greatestDecodeDuration(MediaTime::invalidTime())
         , lastFrameDuration(MediaTime::invalidTime())
         , highestPresentationTimestamp(MediaTime::invalidTime())
         , lastEnqueuedPresentationTime(MediaTime::invalidTime())
@@ -247,6 +249,7 @@ void SourceBuffer::resetParserState()
     // 5. Set the need random access point flag on all track buffers to true.
     for (auto& trackBufferPair : m_trackBufferMap.values()) {
         trackBufferPair.lastDecodeTimestamp = MediaTime::invalidTime();
+        trackBufferPair.greatestDecodeDuration = MediaTime::invalidTime();
         trackBufferPair.lastFrameDuration = MediaTime::invalidTime();
         trackBufferPair.highestPresentationTimestamp = MediaTime::invalidTime();
         trackBufferPair.needRandomAccessFlag = true;
@@ -1392,7 +1395,7 @@ void SourceBuffer::sourceBufferPrivateDidReceiveSample(MediaSample& sample)
         // ↳ If last decode timestamp for track buffer is set and the difference between decode timestamp and
         // last decode timestamp is greater than 2 times last frame duration:
         if (trackBuffer.lastDecodeTimestamp.isValid() && (decodeTimestamp < trackBuffer.lastDecodeTimestamp
-            || abs(decodeTimestamp - trackBuffer.lastDecodeTimestamp) > (trackBuffer.lastFrameDuration * 2))) {
+            || (trackBuffer.greatestDecodeDuration.isValid() && abs(decodeTimestamp - trackBuffer.lastDecodeTimestamp) > (trackBuffer.greatestDecodeDuration * 2)))) {
 
             // 1.6.1:
             if (m_mode == AppendMode::Segments) {
@@ -1409,6 +1412,7 @@ void SourceBuffer::sourceBufferPrivateDidReceiveSample(MediaSample& sample)
                 // 1.6.2 Unset the last decode timestamp on all track buffers.
                 trackBuffer.lastDecodeTimestamp = MediaTime::invalidTime();
                 // 1.6.3 Unset the last frame duration on all track buffers.
+                trackBuffer.greatestDecodeDuration = MediaTime::invalidTime();
                 trackBuffer.lastFrameDuration = MediaTime::invalidTime();
                 // 1.6.4 Unset the highest presentation timestamp on all track buffers.
                 trackBuffer.highestPresentationTimestamp = MediaTime::invalidTime();
@@ -1593,6 +1597,14 @@ void SourceBuffer::sourceBufferPrivateDidReceiveSample(MediaSample& sample)
             trackBuffer.decodeQueue.insert(DecodeOrderSampleMap::MapType::value_type(decodeKey, &sample));
         }
 
+        // NOTE: the spec considers "Coded Frame Duration" to be the presentation duration, but this is not necessarily equal
+        // to the decoded duration. When comparing deltas between decode timestamps, the decode duration, not the presentation.
+        if (trackBuffer.lastDecodeTimestamp.isValid()) {
+            MediaTime lastDecodeDuration = decodeTimestamp - trackBuffer.lastDecodeTimestamp;
+            if (lastDecodeDuration > trackBuffer.greatestDecodeDuration)
+                trackBuffer.greatestDecodeDuration = lastDecodeDuration;
+        }
+
         // 1.18 Set last decode timestamp for track buffer to decode timestamp.
         trackBuffer.lastDecodeTimestamp = decodeTimestamp;