Bug 233495 – [MSE][WPE] Parameterize maximum buffer size using the MSE_MAX_BUFFER_SIZE env var

• Component: Media
• Status: RESOLVED FIXED

[Enrique Ocaña]

The current MSE policy of SourceBuffers taking all the available system memory until the MemoryPressureHandler complains may work fine for a typical desktop system, when caching is preferred over memory footprint. However, memory is scarce on embedded systems and WebKit may compete with other software for memory resources. In that scenario, the embedder may prefer to restrict the amount of memory available for each kind of SourceBuffer (audio, video, text).

Metrological has been successfully using an environment variable for that purpose downstream since 2016. If that implementation is upstreamed, other WPE users can benefit from it.

[Enrique Ocaña]

Created attachment 445206 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 445206 [details]
Patch

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

I think we should move this logic to a virtual method, maybe in SourceBufferPrivate.

> Source/WebCore/html/MediaElementSession.cpp:946
> +    String s(getenv("MSE_MAX_BUFFER_SIZE"));

std::

[Enrique Ocaña]

Created attachment 445411 [details]
Patch

[Enrique Ocaña]

I discussed with Alicia the best place to have the logic and we realized that  MediaSessionManagerGLib might also be a good place for the platform logic. SourceBuffer asks HTMLMediaElement, which asks MediaElementSession, a subclass of PlatformMediaSession, which can talk to PlatformMediaSessionManager, which is subclassed by MediaSessionManagerGLib. 

However, all this Session stuff is related to attributes that can change and be set at runtime, while this platformMaximumBufferSize() is something more static, related to the platform. That's why I still think that SourceBufferPrivate should be its proper place to be.

[Xabier Rodríguez Calvar]

Comment on attachment 445411 [details]
Patch

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

> Source/WebCore/ChangeLog:24
> +        * platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp: Read the MSE_MAX_BUFFER_SIZE environment variable and compute the maximum buffer size if the size is specified in the variable for all the track types present in the SourceBufferPrivate. Otherwise, use the default size.

This line is too long.

> Source/WebCore/platform/graphics/gstreamer/mse/SourceBufferPrivateGStreamer.cpp:285
> +static void maximumBufferSizeDefaults(size_t& maxBufferSizeVideo, size_t& maxBufferSizeAudio, size_t& maxBufferSizeText)

If this function is going to operate on statics, I don't think you need the parameters, just use the statics.

Besides, I think considering this function is only called inside de std::once call, you could just inline the code there. You would even save a #ifdef.

[Enrique Ocaña]

Created attachment 445543 [details]
Patch

[EWS]

Committed r286358 (244717@main): <https://commits.webkit.org/244717@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/85914688>