WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
211 stars 135 forks source link

[MSE] Ignore SourceBuffer size limit before init segment received #1261

Closed asurdej-comcast closed 7 months ago

asurdej-comcast commented 8 months ago

SourceBuffer has separate max size limits for video, audio and text. But it has to parse init segment first to know if there are any video, audio or text tracks. At the very first appendBuffer() there are not tracks parsed yet so platformMaximumBufferSize() returns 0 (as no tracks are ther) and the limit is taken from MediaElementSession as audio limit 15MB as no video track is present.

Ignore buffer overflow at the very first appendBuffer() before tracks are parsed from init segment.

The issue is visible only with high resolution start - 4K - that is rather unlikely as most of apps starts from lower resolution. Still, if first appendBuffer() is big enough it can overflow 15MB default limit.

As a test case we can use YT cert test "72. VP9.Profile2.10Bit.PQ.2160p60" from "Format Support Tests" https://ytlr-cert.appspot.com/

eocanha commented 7 months ago

Patch submitted upstream as https://bugs.webkit.org/show_bug.cgi?id=267417 / https://github.com/WebKit/WebKit/pull/22671.

eocanha commented 7 months ago

I'm not sure if this is the right approach.

Test https://github.com/WebKit/WebKit/blob/main/LayoutTests/media/media-source/media-source-append-buffer-full-quota-exceeded-error-onstart.html is failing upstream with this patch (and also others on some platforms). That test is ensuring that there's a QuotaExceptionError (memory thresholds have been surpassed) during an init segment append, so I feel "canceling" that test would be wrong.

I've coded a different alternative (see https://github.com/WebKit/WebKit/pull/22671) that temporarily increases the native buffer size limit by assuming that a SourceBuffer with no tracks (with no init segment processed) may generate all of a video, audio and text tracks at the same time. I've run the test case with these changes and this approach also solves the original reported problem (hopefully without upstream layout test regressions).

eocanha commented 7 months ago

https://bugs.webkit.org/show_bug.cgi?id=267417 / https://github.com/WebKit/WebKit/pull/22671 landed upstream as https://github.com/WebKit/WebKit/commit/71e98e40411520b4980a0b1d9f7aeccb52b369f4 and was backported to wpe-2.38 as https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/00a56e0b7db69d1a7a1025c9f01ebaa06ac0e4ac and to wpe-2.42 as https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/0a32321c31b91a82f685fa4efedde06e489962cf. Closing PR.