Bug 226720 - [MSE] Assertion if attempting to perform eviction before an init segment was appended
Summary: [MSE] Assertion if attempting to perform eviction before an init segment was ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2021-06-07 02:17 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-06-08 18:03 PDT (History)
8 users (show)

See Also:


Attachments
Patch (5.84 KB, patch)
2021-06-07 06:06 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Yves Avenard [:jya] 2021-06-07 02:17:01 PDT
Seen while browsing the SourceBufferPrivate::evictCodedFrames code.

In SourceBufferPrivate::evictCodedFrames

we have:

```
    // NOTE: begin by removing data from the beginning of the buffered ranges, 30 seconds at
    // a time, up to 30 seconds before currentTime.
    MediaTime thirtySeconds = MediaTime(30, 1);
    MediaTime maximumRangeEnd = currentTime - thirtySeconds;

#if !RELEASE_LOG_DISABLED
    uint64_t initialBufferedSize = totalTrackBufferSizeInBytes();
    DEBUG_LOG(LOGIDENTIFIER, "currentTime = ", currentTime, ", require ", initialBufferedSize + newDataSize, " bytes, maximum buffer size is ", maximumBufferSize);
#endif

    MediaTime rangeStart = MediaTime::zeroTime();
    MediaTime rangeEnd = rangeStart + thirtySeconds;
    while (rangeStart < maximumRangeEnd) {
        // 4. For each range in removal ranges, run the coded frame removal algorithm with start and
        // end equal to the removal range start and end timestamp respectively.
        removeCodedFrames(rangeStart, std::min(rangeEnd, maximumRangeEnd), currentTime, isEnded);
        if (!isBufferFullFor(newDataSize, maximumBufferSize)) {
            break;
        }

        rangeStart += thirtySeconds;
        rangeEnd += thirtySeconds;
    }
```

removeCodedFrames will immediately assert that `ASSERT(start < end);`

if we haven't started playback, or currentTime = 0
MediaTime maximumRangeEnd = currentTime - thirtySeconds = -30s
MediaTime rangeStart = MediaTime::zeroTime() = 0s
MediaTime rangeEnd = rangeStart + thirtySeconds = 30s

so:
removeCodedFrames(rangeStart, std::min(rangeEnd, maximumRangeEnd), currentTime, isEnded);
is called with
rangeState = 0
std::min(rangeEnd, maximumRangeEnd) = -30s

which will assert.
Comment 1 Jean-Yves Avenard [:jya] 2021-06-07 03:53:01 PDT
This is a regression from bug 225800

It removes the check to determine if the buffer range index were valid. so we attempt to call removeCodedFrames with invalid values.
Comment 2 Jean-Yves Avenard [:jya] 2021-06-07 04:02:39 PDT
My analysis above was wrong, it does assert, but in a different spot.
in the loop that check on what can be removed in the non-contiguous sections after currentTime
Comment 3 Radar WebKit Bug Importer 2021-06-07 04:56:55 PDT
<rdar://problem/78943223>
Comment 4 Jean-Yves Avenard [:jya] 2021-06-07 06:06:48 PDT
Created attachment 430738 [details]
Patch
Comment 5 EWS 2021-06-08 18:03:02 PDT
Committed r278635 (238618@main): <https://commits.webkit.org/238618@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430738 [details].