Bug 230672

Summary: [MSE] appending to the source buffer will not throw when the source buffer is full.
Product: WebKit Reporter: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Component: MediaAssignee: Jean-Yves Avenard [:jya] <jean-yves.avenard>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar, Regression
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
URL: https://jyavenard.github.io/htmltests/tests/mse_webm/sizeSourceBuffer.html
Bug Depends on: 225800    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Jean-Yves Avenard [:jya] 2021-09-23 00:56:25 PDT
Consider the following test.

It adds 1MB of content (about 5s of content) to a source buffer. Playback doesn't start so currentTime = 0

On Safari, after adding 318MB worth we see the buffered range going from:
3003: SourceBuffer buffered ranges grew from TimeRanges: [0.007, 677.601) to TimeRanges: [0.007, 679.47)
3003: 318777183 bytes added
3003: Loading buffer: [0, 1000000)
3031: SourceBuffer buffered ranges grew from TimeRanges: [0.007, 679.47) to TimeRanges: [2.001, 668.138)[679.478, 681.872)
3031: 319777183 bytes added

As you can see the first 2 seconds were evicted ; and then the last 10s before the content is appended.

With the first seconds being evicted, the content is no longer playable.

If we continue to append:
 buffered ranges grew from TimeRanges: [2.001, 668.138)[691.778, 698.889) to TimeRanges: [2.001, 668.138)[691.778, 700.641)
3166: 329777183 bytes added
3166: Loading buffer: [0, 1000000)
3189: SourceBuffer buffered ranges grew from TimeRanges: [2.001, 668.138)[691.778, 700.641) to TimeRanges: [2.001, 668.138)[691.778, 698.898)[700.658, 704.795)
3189: 330777183 bytes added
3190: Loading buffer: [0, 1000000)

We end up with gaps here and there:
after appending 1.6GB of data we have:
 buffered ranges grew from TimeRanges: [2.001, 668.138)[691.778, 698.889) to TimeRanges: [2.001, 668.138)[691.778, 700.641)
3166: 329777183 bytes added
3166: Loading buffer: [0, 1000000)
3189: SourceBuffer buffered ranges grew from TimeRanges: [2.001, 668.138)[691.778, 700.641) to TimeRanges: [2.001, 668.138)[691.778, 698.898)[700.658, 704.795)
3189: 330777183 bytes added
3190: Loading buffer: [0, 1000000)

So effectively, we continue to just evict data , playback can't start and QuotaExceededError is never thrown
Comment 1 Jean-Yves Avenard [:jya] 2021-09-23 02:19:32 PDT
Old regression: bug 166620
Comment 2 Jean-Yves Avenard [:jya] 2021-09-23 02:26:53 PDT
My bad, this is another regression from bug 225800 ; another regression was fixed by 226720 but it was not handling all cases.
Comment 3 Jean-Yves Avenard [:jya] 2021-09-23 02:49:13 PDT
what we see here is because the buffered range start is 0.007 which is close to currentTime, but doesn't contain currentTime.

Ideally we should have a fuzzy find that contains performs a search ignoring the gap the player will ignore.
Comment 4 Radar WebKit Bug Importer 2021-09-24 07:49:07 PDT
<rdar://problem/83496195>
Comment 5 Jean-Yves Avenard [:jya] 2021-09-24 08:05:11 PDT
Created attachment 439147 [details]
Patch
Comment 6 Jean-Yves Avenard [:jya] 2021-09-24 20:39:50 PDT
Created attachment 439234 [details]
Patch

Add an extra test
Comment 7 Eric Carlson 2021-09-26 18:26:22 PDT
Comment on attachment 439234 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=439234&action=review

> Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:206
> +size_t PlatformTimeRanges::findWithFudge(const MediaTime& time, const MediaTime& fudge)

I know we call the factor “fudge” in MSE code, but we use “epsilon” in the rest of WebKit so I think it would be better to use that term in this class

> Source/WebCore/platform/graphics/PlatformTimeRanges.cpp:216
> +PlatformTimeRanges PlatformTimeRanges::copyWithFudge(const MediaTime& fudge) const

Ditto
Comment 8 Jean-Yves Avenard [:jya] 2021-09-26 19:45:29 PDT
Created attachment 439302 [details]
Patch

apply comment: rename methods
Comment 9 EWS 2021-09-26 21:40:15 PDT
Committed r283097 (242155@main): <https://commits.webkit.org/242155@main>

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