⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243138 in webkit


Ignore:
Timestamp:
Mar 19, 2019, 7:38:45 AM (7 years ago)
Author:
aboya@igalia.com
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.
Location:
trunk
Files:
2 added
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r243134 r243138  
     12019-03-19  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE] Use tolerance in eraseBeginTime
     4        https://bugs.webkit.org/show_bug.cgi?id=195911
     5
     6        Reviewed by Jer Noble.
     7
     8        * media/media-source/media-source-append-twice-overlapping-sync-frame-expected.txt: Added.
     9        * media/media-source/media-source-append-twice-overlapping-sync-frame.html: Added.
     10
    1112019-03-19  Antti Koivisto  <antti@apple.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r243137 r243138  
     12019-03-19  Alicia Boya García  <aboya@igalia.com>
     2
     3        [MSE] Use tolerance in eraseBeginTime
     4        https://bugs.webkit.org/show_bug.cgi?id=195911
     5
     6        Reviewed by Jer Noble.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=190085 introduced tolerance
     9        when erasing frames during the Coded Frame Processing algorithm in
     10        such a way that, in files with less than perfect timestamps, a frame
     11        existing before after the current append is not erased accidentally
     12        due to small overlaps.
     13
     14        This patch takes care of the opposite problem: we don't want an old
     15        frame being accidentally NOT erased by a new one with the same
     16        timestamps just because these overlaps make
     17        highestPresentationTimestamp very slightly higher than the frame PTS.
     18
     19        This bug in practice causes some frames of the old quality to not be
     20        erased when the new quality is appended, resulting in some seemingly
     21        still frames from a different quality appearing at some points during
     22        WebM video in presence of quality changes.
     23
     24        This bug can be reduced to this minimal test case that illustrates the
     25        timestamp imprecission of a typical WebM file:
     26
     27        function sampleRun(generation) {
     28            return concatenateSamples([
     29                makeASample(     0,      0, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation),
     30                makeASample(167000, 167000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
     31                makeASample(333000, 333000, 166667, 1000000, 1, SAMPLE_FLAG.SYNC, generation), // overlaps previous frame
     32                makeASample(500000, 500000, 166667, 1000000, 1, SAMPLE_FLAG.NONE, generation),
     33            ]);
     34        }
     35
     36        After appending this twice it would be expected that the second
     37        generation takes fully over the first, since the timestamps are
     38        completely the same. Due to the bug, sync frames with an overlap, like
     39        the third one in that list, actually persist from the first
     40        generation, due to lack of tolerance when comparing the start of a new
     41        frame with highestPresentationTimestamp.
     42
     43        This patch introduces the tolerance in that case too to fix this
     44        problem.
     45
     46        Test: media/media-source/media-source-append-twice-overlapping-sync-frame.html
     47
     48        * Modules/mediasource/SourceBuffer.cpp:
     49        (WebCore::SourceBuffer::sourceBufferPrivateDidReceiveSample):
     50
    1512019-03-19  Michael Catanzaro  <mcatanzaro@igalia.com>
    252

  • trunk/Source/WebCore/Modules/mediasource/SourceBuffer.cpp

    r243135 r243138  
    16951695
    16961696        // If highest presentation timestamp for track buffer is set and less than or equal to presentation timestamp
    1697         if (trackBuffer.highestPresentationTimestamp.isValid() && trackBuffer.highestPresentationTimestamp <= presentationTimestamp) {
     1697        if (trackBuffer.highestPresentationTimestamp.isValid() && trackBuffer.highestPresentationTimestamp - contiguousFrameTolerance <= presentationTimestamp) {
    16981698            // Remove all coded frames from track buffer that have a presentation timestamp greater than highest
    16991699            // presentation timestamp and less than or equal to frame end timestamp.
    … …  
    17071707
    17081708                MediaTime highestBufferedTime = trackBuffer.buffered.maximumBufferedTime();
    1709                 MediaTime eraseBeginTime = trackBuffer.highestPresentationTimestamp;
     1709                MediaTime eraseBeginTime = trackBuffer.highestPresentationTimestamp - contiguousFrameTolerance;
    17101710                MediaTime eraseEndTime = frameEndTimestamp - contiguousFrameTolerance;
    17111711
Note: See TracChangeset for help on using the changeset viewer.