Bug 230672 - [MSE] appending to the source buffer will not throw when the source buffer is full.
Summary: [MSE] appending to the source buffer will not throw when the source buffer is...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL: https://jyavenard.github.io/htmltests...
Keywords: InRadar, Regression
Depends on: 225800
Blocks:
  Show dependency treegraph
 
Reported: 2021-09-23 00:56 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-09-26 21:40 PDT (History)
8 users (show)

See Also:


Attachments
Patch (10.79 KB, patch)
2021-09-24 08:05 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (16.05 KB, patch)
2021-09-24 20:39 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (16.07 KB, patch)
2021-09-26 19:45 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-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].