Bug 226711 - [MSE] Rework handling of SourceBuffer's buffer full.
Summary: [MSE] Rework handling of SourceBuffer's buffer full.
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
Depends on:
Blocks:
 
Reported: 2021-06-07 00:55 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-06-08 05:30 PDT (History)
8 users (show)

See Also:


Attachments
Patch (24.81 KB, patch)
2021-06-07 02:06 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (24.70 KB, patch)
2021-06-07 21:27 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 00:55:04 PDT
Follow-up from https://bugs.webkit.org/show_bug.cgi?id=225630#c9

bug 225630 changed the logic on how we are to reject an append only after one has already been added.

One downside of the change was also that it broke the class separation between SourceBuffer and SourceBufferPrivate ; it is preferable to have all the handling of buffer size in the SourceBufferPrivate which avoid ambiguities as discussed in bug https://bugs.webkit.org/show_bug.cgi?id=226034
Comment 1 Radar WebKit Bug Importer 2021-06-07 00:58:09 PDT
<rdar://problem/78937909>
Comment 2 Jean-Yves Avenard [:jya] 2021-06-07 02:06:34 PDT
Created attachment 430730 [details]
Patch
Comment 3 Jer Noble 2021-06-07 16:43:24 PDT
Comment on attachment 430730 [details]
Patch

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

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:726
> +    if (WTF::sumOverflows<uint64_t>(totalTrackBufferSizeInBytes(), requiredSize))
> +        return true;
> +
> +    return totalTrackBufferSizeInBytes() + requiredSize >= maximumBufferSize;

Irrespective of the overall patch, this performs the addition twice. It would probably be more efficient to do something like:

uint64_t totalRequired = 0;
if (CheckedState::DidOverflow == CheckedSum(totalTrackBufferSizeInBytes(), requiredSize).safeGet(totalRequired);
    return true;
return totalRequired >= maximumBufferSize;
Comment 4 Jean-Yves Avenard [:jya] 2021-06-07 21:27:54 PDT
Created attachment 430806 [details]
Patch
Comment 5 Jean-Yves Avenard [:jya] 2021-06-07 23:43:52 PDT
Comment on attachment 430806 [details]
Patch

test failures unrelated to media's MSE
Comment 6 EWS 2021-06-07 23:44:15 PDT
jean-yves.avenard@apple.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json.

Rejecting attachment 430806 [details] from commit queue.
Comment 7 EWS 2021-06-08 00:41:04 PDT
jean-yves.avenard@apple.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json.

Rejecting attachment 430806 [details] from commit queue.
Comment 8 EWS 2021-06-08 03:52:11 PDT
jean-yves.avenard@apple.com does not have committer permissions according to https://raw.githubusercontent.com/WebKit/WebKit/main/Tools/Scripts/webkitpy/common/config/contributors.json.

Rejecting attachment 430806 [details] from commit queue.
Comment 9 EWS 2021-06-08 05:29:58 PDT
Committed r278603 (238592@main): <https://commits.webkit.org/238592@main>

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