Safari is not able to adapt between H264 streams with EditList and without EditList
authorjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Oct 2018 22:48:29 +0000 (22:48 +0000)
committerjer.noble@apple.com <jer.noble@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Oct 2018 22:48:29 +0000 (22:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=190638
<rdar://problem/45342208>

Reviewed by Eric Carlson.

Source/WebCore:

Test: media/media-source/media-source-append-overlapping-dts.html

The MSE frame replacement algorithm does not take decode timestamps into account; this can
lead to situations where the replacement algorithm may leave in place frames where the
presentationTimestamp is less than the replacement frame, but whose decodeTimestamp is
after the replacement frame. When re-enqueuing these frames, they may cause a decode error
if they break the group-of-pictures sequence of the replaced range.

* Modules/mediasource/SampleMap.cpp:
(WebCore::DecodeOrderSampleMap::findSamplesBetweenDecodeKeys):
* Modules/mediasource/SampleMap.h:
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):

LayoutTests:

* media/media-source/media-source-append-overlapping-dts-expected.txt: Added.
* media/media-source/media-source-append-overlapping-dts.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt [new file with mode: 0644]
LayoutTests/media/media-source/media-source-append-overlapping-dts.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediasource/SampleMap.cpp
Source/WebCore/Modules/mediasource/SampleMap.h
Source/WebCore/Modules/mediasource/SourceBuffer.cpp

index 39f45d7..6f9ae7f 100644 (file)
@@ -1,3 +1,14 @@
+2018-10-18  Jer Noble  <jer.noble@apple.com>
+
+        Safari is not able to adapt between H264 streams with EditList and without EditList
+        https://bugs.webkit.org/show_bug.cgi?id=190638
+        <rdar://problem/45342208>
+
+        Reviewed by Eric Carlson.
+
+        * media/media-source/media-source-append-overlapping-dts-expected.txt: Added.
+        * media/media-source/media-source-append-overlapping-dts.html: Added.
+
 2018-10-18  Per Arne Vollan  <pvollan@apple.com>
 
         [WebVTT] Region parameter and value should be separated by ':'
diff --git a/LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt b/LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt
new file mode 100644 (file)
index 0000000..2773154
--- /dev/null
@@ -0,0 +1,23 @@
+This tests that an overlapping append of samples with reordered presentation timestamps will correctly remove previously appended non-reordered samples.
+
+RUN(video.src = URL.createObjectURL(source))
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock"))
+RUN(sourceBuffer.appendBuffer(initSegment))
+EVENT(updateend)
+RUN(sourceBuffer.appendBuffer(samples))
+EVENT(updateend)
+RUN(sourceBuffer.appendBuffer(samples))
+EVENT(updateend)
+EXPECTED (bufferedSamples.length == '9') OK
+{PTS({0/1000 = 0.000000}), DTS({0/1000 = 0.000000}), duration({1000/1000 = 1.000000}), flags(1), generation(0)}
+{PTS({1000/1000 = 1.000000}), DTS({1000/1000 = 1.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(0)}
+{PTS({2000/1000 = 2.000000}), DTS({2000/1000 = 2.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(0)}
+{PTS({3000/1000 = 3.000000}), DTS({3000/1000 = 3.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(0)}
+{PTS({4000/1000 = 4.000000}), DTS({4000/1000 = 4.000000}), duration({1000/1000 = 1.000000}), flags(1), generation(0)}
+{PTS({6000/1000 = 6.000000}), DTS({4000/1000 = 4.000000}), duration({1000/1000 = 1.000000}), flags(1), generation(1)}
+{PTS({7000/1000 = 7.000000}), DTS({5000/1000 = 5.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(1)}
+{PTS({4000/1000 = 4.000000}), DTS({6000/1000 = 6.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(1)}
+{PTS({5000/1000 = 5.000000}), DTS({7000/1000 = 7.000000}), duration({1000/1000 = 1.000000}), flags(0), generation(1)}
+END OF TEST
+
diff --git a/LayoutTests/media/media-source/media-source-append-overlapping-dts.html b/LayoutTests/media/media-source/media-source-append-overlapping-dts.html
new file mode 100644 (file)
index 0000000..3be2bbe
--- /dev/null
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>media-source-append-overlapping-dts</title>
+    <script src="mock-media-source.js"></script>
+    <script src="../video-test.js"></script>
+    <script>
+    var source;
+    var sourceBuffer;
+    var initSegment;
+
+    if (window.internals)
+        internals.initializeMockMediaSource();
+
+    window.addEventListener('load', async event => {
+
+        findMediaElement();
+
+        source = new MediaSource();
+        run('video.src = URL.createObjectURL(source)');
+        await waitFor(source, 'sourceopen');
+
+        run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
+        initSegment = makeAInit(8, [makeATrack(1, 'mock', TRACK_KIND.VIDEO)]);
+        run('sourceBuffer.appendBuffer(initSegment)');
+
+        await waitFor(sourceBuffer, 'updateend');
+
+        samples = concatenateSamples([
+            makeASample(0, 0, 1, 1, SAMPLE_FLAG.SYNC, 0),
+            makeASample(1, 1, 1, 1, SAMPLE_FLAG.NONE, 0),
+            makeASample(2, 2, 1, 1, SAMPLE_FLAG.NONE, 0),
+            makeASample(3, 3, 1, 1, SAMPLE_FLAG.NONE, 0),
+            makeASample(4, 4, 1, 1, SAMPLE_FLAG.SYNC, 0),
+            makeASample(5, 5, 1, 1, SAMPLE_FLAG.NONE, 0),
+            makeASample(6, 6, 1, 1, SAMPLE_FLAG.NONE, 0),
+            makeASample(7, 7, 1, 1, SAMPLE_FLAG.NONE, 0),
+        ]);
+        run('sourceBuffer.appendBuffer(samples)');
+        await waitFor(sourceBuffer, 'updateend');
+
+        samples = concatenateSamples([
+            makeASample(6, 4, 1, 1, SAMPLE_FLAG.SYNC, 1),
+            makeASample(7, 5, 1, 1, SAMPLE_FLAG.NONE, 1),
+            makeASample(4, 6, 1, 1, SAMPLE_FLAG.NONE, 1),
+            makeASample(5, 7, 1, 1, SAMPLE_FLAG.NONE, 1),
+        ]);
+        run('sourceBuffer.appendBuffer(samples)');
+        await waitFor(sourceBuffer, 'updateend');
+
+        bufferedSamples = internals.bufferedSamplesForTrackID(sourceBuffer, 1);
+        testExpected("bufferedSamples.length", 9);
+        bufferedSamples.forEach(consoleWrite);
+        endTest();
+
+    }, {once: true});
+    </script>
+</head>
+<body>
+    <div>This tests that an overlapping append of samples with reordered presentation timestamps will correctly remove previously appended non-reordered samples.</div>
+    <video></video>
+</body>
+</html>
index 2c1f73a..0577798 100644 (file)
@@ -1,3 +1,25 @@
+2018-10-18  Jer Noble  <jer.noble@apple.com>
+
+        Safari is not able to adapt between H264 streams with EditList and without EditList
+        https://bugs.webkit.org/show_bug.cgi?id=190638
+        <rdar://problem/45342208>
+
+        Reviewed by Eric Carlson.
+
+        Test: media/media-source/media-source-append-overlapping-dts.html
+
+        The MSE frame replacement algorithm does not take decode timestamps into account; this can
+        lead to situations where the replacement algorithm may leave in place frames where the 
+        presentationTimestamp is less than the replacement frame, but whose decodeTimestamp is
+        after the replacement frame. When re-enqueuing these frames, they may cause a decode error
+        if they break the group-of-pictures sequence of the replaced range.
+
+        * Modules/mediasource/SampleMap.cpp:
+        (WebCore::DecodeOrderSampleMap::findSamplesBetweenDecodeKeys):
+        * Modules/mediasource/SampleMap.h:
+        * Modules/mediasource/SourceBuffer.cpp:
+        (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+
 2018-10-18  Per Arne Vollan  <pvollan@apple.com>
 
         [WebVTT] Region parameter and value should be separated by ':'
index afade7a..07cf5d7 100644 (file)
@@ -307,4 +307,18 @@ DecodeOrderSampleMap::reverse_iterator_range DecodeOrderSampleMap::findDependent
     return reverse_iterator_range(currentDecodeIter, nextSyncSample);
 }
 
+DecodeOrderSampleMap::iterator_range DecodeOrderSampleMap::findSamplesBetweenDecodeKeys(const KeyType& beginKey, const KeyType& endKey)
+{
+    if (beginKey > endKey)
+        return { end(), end() };
+
+    // beginKey is inclusive, so use lower_bound to include samples wich start exactly at beginKey.
+    // endKey is not inclusive, so use lower_bound to exclude samples which start exactly at endKey.
+    auto lower_bound = m_samples.lower_bound(beginKey);
+    auto upper_bound = m_samples.lower_bound(endKey);
+    if (lower_bound == upper_bound)
+        return { end(), end() };
+    return { lower_bound, upper_bound };
+}
+
 }
index f27faad..f783082 100644 (file)
@@ -79,6 +79,7 @@ public:
     typedef MapType::const_iterator const_iterator;
     typedef MapType::reverse_iterator reverse_iterator;
     typedef MapType::const_reverse_iterator const_reverse_iterator;
+    typedef std::pair<iterator, iterator> iterator_range;
     typedef std::pair<reverse_iterator, reverse_iterator> reverse_iterator_range;
     typedef MapType::value_type value_type;
 
@@ -98,6 +99,7 @@ public:
     WEBCORE_EXPORT iterator findSyncSampleAfterPresentationTime(const MediaTime&, const MediaTime& threshold = MediaTime::positiveInfiniteTime());
     WEBCORE_EXPORT iterator findSyncSampleAfterDecodeIterator(iterator);
     WEBCORE_EXPORT reverse_iterator_range findDependentSamples(MediaSample*);
+    WEBCORE_EXPORT iterator_range findSamplesBetweenDecodeKeys(const KeyType&, const KeyType&);
 
 private:
     MapType m_samples;
index 18a4405..4cac52c 100644 (file)
@@ -1648,6 +1648,14 @@ void SourceBuffer::sourceBufferPrivateDidReceiveSample(MediaSample& sample)
             auto nextSyncIter = trackBuffer.samples.decodeOrder().findSyncSampleAfterDecodeIterator(lastDecodeIter);
             dependentSamples.insert(firstDecodeIter, nextSyncIter);
 
+            // NOTE: in the case of b-frames, the previous step may leave in place samples whose presentation
+            // timestamp < presentationTime, but whose decode timestamp >= decodeTime. These will eventually cause
+            // a decode error if left in place, so remove these samples as well.
+            DecodeOrderSampleMap::KeyType decodeKey(sample.decodeTime(), sample.presentationTime());
+            auto samplesWithHigherDecodeTimes = trackBuffer.samples.decodeOrder().findSamplesBetweenDecodeKeys(decodeKey, erasedSamples.decodeOrder().begin()->first);
+            if (samplesWithHigherDecodeTimes.first != samplesWithHigherDecodeTimes.second)
+                dependentSamples.insert(samplesWithHigherDecodeTimes.first, samplesWithHigherDecodeTimes.second);
+
             PlatformTimeRanges erasedRanges = removeSamplesFromTrackBuffer(dependentSamples, trackBuffer, this, "sourceBufferPrivateDidReceiveSample");
 
             // Only force the TrackBuffer to re-enqueue if the removed ranges overlap with enqueued and possibly