Bug 314814 – [MSE] SourceBuffer.remove() incorrectly removes one extra sample, and buffered trims coverage still backed by retained samples

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jean-Yves Avenard [:jya]]

Consider the following:

Sample map m_samples.presentationOrder():

```
┌─────┬──────┬──────┬──────┬──────────────┬──────┐
│  #  │ PTS  │ DTS  │ Dur  │    Range     │ Sync │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 1   │ 0.00 │ 0.00 │ 1.00 │ [0.00, 1.00) │ ✓    │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 2   │ 1.00 │ 1.00 │ 1.00 │ [1.00, 2.00) │      │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 3   │ 2.00 │ 2.00 │ 1.00 │ [2.00, 3.00) │      │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 4   │ 3.00 │ 3.00 │ 1.00 │ [3.00, 4.00) │      │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 5   │ 3.96 │ 3.96 │ 0.33 │ [3.96, 4.29) │ ✓    │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 6   │ 4.29 │ 4.29 │ 0.33 │ [4.29, 4.62) │      │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 7   │ 4.62 │ 4.62 │ 0.33 │ [4.62, 4.95) │      │
├─────┼──────┼──────┼──────┼──────────────┼──────┤
│ 8   │ 4.95 │ 4.95 │ 0.33 │ [4.95, 5.28) │      │
└─────┴──────┴──────┴──────┴──────────────┴──────┘
```

m_buffered = [0.00, 5.28) (ranges merge via AddTimeRangeOption::EliminateSmallGaps in addSample at
TrackBuffer.cpp:150).

Samples #4 and #5 overlap in presentation time at [3.96, 4.00). (We established earlier this is a spec-legal state —
SourceBufferPrivate.cpp:1389 only removes existing samples whose PTS falls in the new sample's range, not whose range
falls in the new sample's range; with new PTS=3.96 and existing PTS=3.00 nothing is removed.)

---
Operation: sourceBuffer.remove(0, 3.96)

Entering TrackBuffer::removeCodedFrames(start=0, end=3.96) at TrackBuffer.cpp:408.

Lines 430-431 — split-at-time attempts. Both return empty (mock samples are non-divisible).

Lines 433-436 — iterator selection:
auto removePresentationStart = m_samples.presentationOrder().findSampleStartingOnOrAfterPresentationTime(start);  //
lower_bound(0) → #1
auto removePresentationEnd   = m_samples.presentationOrder().findSampleStartingOnOrAfterPresentationTime(end);    //
lower_bound(3.96) → #5
Presentation range [#1, #5) = {#1, #2, #3, #4}. Sample #5 is NOT selected (PTS=3.96 is not < 3.96). Spec-correct.

Lines 445-452 — decode-order extraction:
- Min DTS in selection = 0 (#1), Max DTS = 3 (#4).
- findSyncSampleAfterDecodeIterator(#4) → next sync after DTS=3 is DTS=3.96 → #5.
- erasedSamples = decode-order [#1, #5) = {#1, #2, #3, #4}.

Line 454 — call removeSamples(erasedSamples, ...) at TrackBuffer.cpp:319.

Inside removeSamples:

Lines 333-359 — sample removal + erasedRanges build:
- Remove #1: erasedRanges.add(0.00, 1.00, EliminateSmallGaps) → erasedRanges = [0, 1).
- Remove #2: add → [0, 2).
- Remove #3: add → [0, 3).
- Remove #4: add [3.00, 4.00) → erasedRanges = [0.00, 4.00).

After removal, sample map = {#5, #6, #7, #8}.

Lines 365-391 — padding loop, one iteration for the single range [0, 4):

auto erasedStart = 0.00;
auto erasedEnd   = 4.00;

// Front
auto startIterator = m_samples.presentationOrder().reverseFindSampleBeforePresentationTime(0);
if (startIterator == rend())   // true — no retained sample has PTS < 0
    additionalErasedRanges.add(MediaTime::zeroTime(), 0);   // add(0, 0) — no-op

// Back
auto endIterator = m_samples.presentationOrder().findSampleStartingAfterPresentationTime(erasedStart);  //
upper_bound(0) = #5
// endIterator != end()
Ref nextSample = endIterator->second.get();   // #5
if (nextSample->presentationTime() > erasedEnd)         // 3.96 > 4 ? NO — branch skipped
    additionalErasedRanges.add(erasedEnd, nextSample->presentationTime());

Neither branch in the back-side else fires — the > check doesn't cover the < case. additionalErasedRanges stays empty.

erasedRanges returned unchanged: [0.00, 4.00).

Lines 467-468 back in removeCodedFrames:
erasedRanges.invert();                       // (-∞, 0) ∪ [4.00, +∞)
m_buffered.intersectWith(erasedRanges);      // [0, 5.28) ∩ above = [4.00, 5.28)

---
Final state (current code)

- m_samples = {#5, #6, #7, #8} (samples #5's range is still [3.96, 4.29)).
- m_buffered = [4.00, 5.28).

Why this is incorrect

Union of retained samples' ranges:
#5 [3.96, 4.29) ∪ #6 [4.29, 4.62) ∪ #7 [4.62, 4.95) ∪ #8 [4.95, 5.28) = [3.96, 5.28)

m_buffered should equal this union, but instead reports [4.00, 5.28). The [3.96, 4.00) region is missing from
m_buffered even though sample #5 still covers it.

The root cause: erasedRanges is the union of removed samples' full [PT, PT+dur) intervals = [0, 4). That union
includes [3.96, 4.00) (from removed #4's range extending to 4.00), but [3.96, 4.00) is also covered by retained #5 —
so subtracting erasedRanges from m_buffered trims buffered coverage that is still backed by a sample.

The padding loop at lines 381-388 already consults endIterator (the next retained sample after erasedStart) and
handles one case — when its PTS is greater than erasedEnd (gap). The other side of the comparison — PTS less than
erasedEnd (overlap) — never appears. That's the missing code.

This creates gaps that will cause playback to stall sometimes should the JS player remove + re-append samples from [0-3.96] as the buffered range would now be [0, 3.96] [4, 5.28]

[Radar WebKit Bug Importer]

<rdar://problem/177065364>

[Jean-Yves Avenard [:jya]]

Pull request: https://github.com/WebKit/WebKit/pull/64959

[EWS]

Committed 313383@main (21a3e6b45c05): <https://commits.webkit.org/313383@main>

Reviewed commits have been landed. Closing PR #64959 and removing active labels.