- Revision
- 243138
- Author
- [email protected]
- Date
- 2019-03-19 07:38:45 -0700 (Tue, 19 Mar 2019)
Log Message
[MSE] Use tolerance in eraseBeginTime
https://bugs.webkit.org/show_bug.cgi?id=195911
Reviewed by Jer Noble.
Source/WebCore:
https://bugs.webkit.org/show_bug.cgi?id=190085 introduced tolerance
when erasing frames during the Coded Frame Processing algorithm in
such a way that, in files with less than perfect timestamps, a frame
existing before after the current append is not erased accidentally
due to small overlaps.
This patch takes care of the opposite problem: we don't want an old
frame being accidentally NOT erased by a new one with the same
timestamps just because these overlaps make
highestPresentationTimestamp very slightly higher than the frame PTS.
This bug in practice causes some frames of the old quality to not be
erased when the new quality is appended, resulting in some seemingly
still frames from a different quality appearing at some points during
WebM video in presence of quality changes.
This bug can be reduced to this minimal test case that illustrates the
timestamp imprecission of a typical WebM file:
function sampleRun(generation) {
return concatenateSamples([
makeASample( 0, 0, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation),
makeASample(167000, 167000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
makeASample(333000, 333000, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation), // overlaps previous frame
makeASample(500000, 500000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
]);
}
After appending this twice it would be expected that the second
generation takes fully over the first, since the timestamps are
completely the same. Due to the bug, sync frames with an overlap, like
the third one in that list, actually persist from the first
generation, due to lack of tolerance when comparing the start of a new
frame with highestPresentationTimestamp.
This patch introduces the tolerance in that case too to fix this
problem.
Test: media/media-source/media-source-append-twice-overlapping-sync-frame.html
* Modules/mediasource/SourceBuffer.cpp:
(WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
LayoutTests:
* media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt: Added.
* media/media-source/media-source-append-twice-overlapping-sync-frame.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (243137 => 243138)
--- trunk/LayoutTests/ChangeLog 2019-03-19 13:46:25 UTC (rev 243137)
+++ trunk/LayoutTests/ChangeLog 2019-03-19 14:38:45 UTC (rev 243138)
@@ -1,3 +1,13 @@
+2019-03-19 Alicia Boya García <[email protected]>
+
+ [MSE] Use tolerance in eraseBeginTime
+ https://bugs.webkit.org/show_bug.cgi?id=195911
+
+ Reviewed by Jer Noble.
+
+ * media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt: Added.
+ * media/media-source/media-source-append-twice-overlapping-sync-frame.html: Added.
+
2019-03-19 Antti Koivisto <[email protected]>
Layer with no backing store should still hit-test over a scroller
Added: trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt (0 => 243138)
--- trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt 2019-03-19 14:38:45 UTC (rev 243138)
@@ -0,0 +1,16 @@
+
+EXPECTED (source.readyState == 'closed') OK
+EVENT(sourceopen)
+RUN(sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock"))
+RUN(sourceBuffer.appendBuffer(initSegment))
+EVENT(updateend)
+RUN(sourceBuffer.appendBuffer(sampleRun(1)))
+EVENT(updateend)
+RUN(sourceBuffer.appendBuffer(sampleRun(2)))
+EVENT(updateend)
+{PTS({0/1000000 = 0.000000}), DTS({0/1000000 = 0.000000}), duration({166667/1000000 = 0.166667}), flags(1), generation(2)}
+{PTS({167000/1000000 = 0.167000}), DTS({167000/1000000 = 0.167000}), duration({166667/1000000 = 0.166667}), flags(0), generation(2)}
+{PTS({333000/1000000 = 0.333000}), DTS({333000/1000000 = 0.333000}), duration({166667/1000000 = 0.166667}), flags(1), generation(2)}
+{PTS({500000/1000000 = 0.500000}), DTS({500000/1000000 = 0.500000}), duration({166667/1000000 = 0.166667}), flags(0), generation(2)}
+END OF TEST
+
Added: trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame.html (0 => 243138)
--- trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame.html (rev 0)
+++ trunk/LayoutTests/media/media-source/media-source-append-twice-overlapping-sync-frame.html 2019-03-19 14:38:45 UTC (rev 243138)
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-source-append-twice-overlapping-sync-frame</title>
+ <script src=""
+ <script src=""
+ <script src=""
+ <script>
+ let source;
+ let sourceBuffer;
+ let initSegment;
+ let bufferedSamples;
+
+ if (window.internals)
+ internals.initializeMockMediaSource();
+
+ function sampleRun(generation) {
+ return concatenateSamples([
+ makeASample( 0, 0, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation),
+ makeASample(167000, 167000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
+ makeASample(333000, 333000, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation), // overlaps previous frame
+ makeASample(500000, 500000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
+ ]);
+ }
+
+ window.addEventListener('load', async () => {
+ findMediaElement();
+ source = new MediaSource();
+ testExpected('source.readyState', 'closed');
+ const sourceOpened = waitFor(source, 'sourceopen');
+
+ const videoSource = document.createElement('source');
+ videoSource.type = 'video/mock; codecs=mock';
+ videoSource.src = ""
+ video.appendChild(videoSource);
+
+ await sourceOpened;
+ run('sourceBuffer = source.addSourceBuffer("video/mock; codecs=mock")');
+
+ initSegment = makeAInit(5, [makeATrack(1, 'mock', TRACK_KIND.VIDEO)]);
+ run('sourceBuffer.appendBuffer(initSegment)');
+ await waitFor(sourceBuffer, 'updateend');
+
+ run('sourceBuffer.appendBuffer(sampleRun(1))');
+ await waitFor(sourceBuffer, 'updateend');
+
+ run('sourceBuffer.appendBuffer(sampleRun(2))');
+ await waitFor(sourceBuffer, 'updateend');
+
+ bufferedSamples = internals.bufferedSamplesForTrackID(sourceBuffer, 1);
+ consoleWrite(bufferedSamples.join("<br/>"));
+
+ endTest();
+ });
+ </script>
+</head>
+<body>
+ <video controls></video>
+</body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (243137 => 243138)
--- trunk/Source/WebCore/ChangeLog 2019-03-19 13:46:25 UTC (rev 243137)
+++ trunk/Source/WebCore/ChangeLog 2019-03-19 14:38:45 UTC (rev 243138)
@@ -1,3 +1,53 @@
+2019-03-19 Alicia Boya García <[email protected]>
+
+ [MSE] Use tolerance in eraseBeginTime
+ https://bugs.webkit.org/show_bug.cgi?id=195911
+
+ Reviewed by Jer Noble.
+
+ https://bugs.webkit.org/show_bug.cgi?id=190085 introduced tolerance
+ when erasing frames during the Coded Frame Processing algorithm in
+ such a way that, in files with less than perfect timestamps, a frame
+ existing before after the current append is not erased accidentally
+ due to small overlaps.
+
+ This patch takes care of the opposite problem: we don't want an old
+ frame being accidentally NOT erased by a new one with the same
+ timestamps just because these overlaps make
+ highestPresentationTimestamp very slightly higher than the frame PTS.
+
+ This bug in practice causes some frames of the old quality to not be
+ erased when the new quality is appended, resulting in some seemingly
+ still frames from a different quality appearing at some points during
+ WebM video in presence of quality changes.
+
+ This bug can be reduced to this minimal test case that illustrates the
+ timestamp imprecission of a typical WebM file:
+
+ function sampleRun(generation) {
+ return concatenateSamples([
+ makeASample( 0, 0, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation),
+ makeASample(167000, 167000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
+ makeASample(333000, 333000, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation), // overlaps previous frame
+ makeASample(500000, 500000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
+ ]);
+ }
+
+ After appending this twice it would be expected that the second
+ generation takes fully over the first, since the timestamps are
+ completely the same. Due to the bug, sync frames with an overlap, like
+ the third one in that list, actually persist from the first
+ generation, due to lack of tolerance when comparing the start of a new
+ frame with highestPresentationTimestamp.
+
+ This patch introduces the tolerance in that case too to fix this
+ problem.
+
+ Test: media/media-source/media-source-append-twice-overlapping-sync-frame.html
+
+ * Modules/mediasource/SourceBuffer.cpp:
+ (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
+
2019-03-19 Michael Catanzaro <[email protected]>
Unreviewed GTK build fix
Modified: trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp (243137 => 243138)
--- trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2019-03-19 13:46:25 UTC (rev 243137)
+++ trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp 2019-03-19 14:38:45 UTC (rev 243138)
@@ -1694,7 +1694,7 @@
const MediaTime contiguousFrameTolerance = MediaTime(1, 1000);
// If highest presentation timestamp for track buffer is set and less than or equal to presentation timestamp
- if (trackBuffer.highestPresentationTimestamp.isValid() && trackBuffer.highestPresentationTimestamp <= presentationTimestamp) {
+ if (trackBuffer.highestPresentationTimestamp.isValid() && trackBuffer.highestPresentationTimestamp - contiguousFrameTolerance <= presentationTimestamp) {
// Remove all coded frames from track buffer that have a presentation timestamp greater than highest
// presentation timestamp and less than or equal to frame end timestamp.
do {
@@ -1706,7 +1706,7 @@
break;
MediaTime highestBufferedTime = trackBuffer.buffered.maximumBufferedTime();
- MediaTime eraseBeginTime = trackBuffer.highestPresentationTimestamp;
+ MediaTime eraseBeginTime = trackBuffer.highestPresentationTimestamp - contiguousFrameTolerance;
MediaTime eraseEndTime = frameEndTimestamp - contiguousFrameTolerance;
PresentationOrderSampleMap::iterator_range range;
