Diff
Modified: trunk/LayoutTests/ChangeLog (237270 => 237271)
--- trunk/LayoutTests/ChangeLog 2018-10-18 22:32:13 UTC (rev 237270)
+++ trunk/LayoutTests/ChangeLog 2018-10-18 22:48:29 UTC (rev 237271)
@@ -1,3 +1,14 @@
+2018-10-18 Jer Noble <jer.no...@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 <pvol...@apple.com>
[WebVTT] Region parameter and value should be separated by ':'
Added: trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt (0 => 237271)
--- trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts-expected.txt 2018-10-18 22:48:29 UTC (rev 237271)
@@ -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 = ""
+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
+
Added: trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts.html (0 => 237271)
--- trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts.html (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-append-overlapping-dts.html 2018-10-18 22:48:29 UTC (rev 237271)
@@ -0,0 +1,63 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-source-append-overlapping-dts</title>
+ <script src=""
+ <script src=""
+ <script>
+ var source;
+ var sourceBuffer;
+ var initSegment;
+
+ if (window.internals)
+ internals.initializeMockMediaSource();
+
+ window.addEventListener('load', async event => {
+
+ findMediaElement();
+
+ source = new MediaSource();
+ run('video.src = ""
+ 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>
Modified: trunk/Source/WebCore/ChangeLog (237270 => 237271)
--- trunk/Source/WebCore/ChangeLog 2018-10-18 22:32:13 UTC (rev 237270)
+++ trunk/Source/WebCore/ChangeLog 2018-10-18 22:48:29 UTC (rev 237271)
@@ -1,3 +1,25 @@
+2018-10-18 Jer Noble <jer.no...@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 <pvol...@apple.com>
[WebVTT] Region parameter and value should be separated by ':'
Modified: trunk/Source/WebCore/Modules/mediasource/SampleMap.cpp (237270 => 237271)
--- trunk/Source/WebCore/Modules/mediasource/SampleMap.cpp 2018-10-18 22:32:13 UTC (rev 237270)
+++ trunk/Source/WebCore/Modules/mediasource/SampleMap.cpp 2018-10-18 22:48:29 UTC (rev 237271)
@@ -307,4 +307,18 @@
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 };
}
+
+}
Modified: trunk/Source/WebCore/Modules/mediasource/SampleMap.h (237270 => 237271)
--- trunk/Source/WebCore/Modules/mediasource/SampleMap.h 2018-10-18 22:32:13 UTC (rev 237270)
+++ trunk/Source/WebCore/Modules/mediasource/SampleMap.h 2018-10-18 22:48:29 UTC (rev 237271)
@@ -79,6 +79,7 @@
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 @@
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;
Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (237270 => 237271)
--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2018-10-18 22:32:13 UTC (rev 237270)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2018-10-18 22:48:29 UTC (rev 237271)
@@ -1648,6 +1648,14 @@
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
