Intent to Ship: MSE in Workers

[Matthew Wolenetz]

Contact emails

wole...@chromium.org

Explainer

https://github.com/wicg/media-source/blob/mse-in-workers-using-handle/mse-in-workers-using-handle-explainer.md

Specification

https://www.w3.org/TR/media-source-2

Design docs

https://github.com/wicg/media-source/blob/mse-in-workers-using-handle/mse-in-workers-using-handle-explainer.md
https://goto.google.com/mse-in-workers

Summary

Enable Media Source Extensions (MSE) API usage from DedicatedWorker contexts to enable improved performance of buffering media for playback by an HTMLMediaElement on the main Window context. By creating a MediaSource object on a DedicatedWorker context, an application may then obtain a MediaSourceHandle from it and transfer that handle to the main thread for use in attaching to an HTMLMediaElement. The context that created the MediaSource object may then use it to buffer media.

Blink component

Blink>Media

Search tags

MSE, MediaSource, MediaSourceExtensions

TAG review

https://github.com/w3ctag/design-reviews/issues/656

TAG review status

Issues addressed

Link to origin trial feedback summary

https://github.com/w3c/media-source/issues/175#issuecomment-1045018594

Risks

Interoperability and Compatibility

Main interoperability risk is that other browsers may not implement it. Compatibility risk is mitigated by unchanged same-thread MSE API support and proactive feature-detectability of MSE-in-Workers with a new canConstructInDedicatedWorker attribute on the MediaSource interface.

Gecko: Positive (https://github.com/mozilla/standards-positions/issues/547)

WebKit: No signal (https://lists.webkit.org/pipermail/webkit-dev/2021-June/031916.html) @jernoble has been involved in feature specification discussions in the Media Workgroup and has provided generally positive feedback.

Web developers: Strongly positive (https://github.com/w3c/media-source/issues/175#issuecomment-1045018594) Twitch reported "large positive impact on our topline metrics such as buffering and time to video. We'd love for all browsers to adopt MSE-in-Workers." More (positive) details have been shared and are currently pending them posting them at a public location.

Other signals:

Ergonomics

DedicatedWorker, WorkerGlobalScope and postMessage/onmessage availability is integral to using this feature.

Activation

The primary risk is the potential difficulty in refactoring existing MSE web applications to (conditionally, depending on browser support) perform buffering in a different execution context from the Window context that hosts the DOM and the media element. The benefit of potentially improved buffering performance by offloading the MSE API usage to a worker context provides motivation to overcome this challenge.

Security

Unpredictability of racing context destruction of either the main thread hosting the media element (and owning the underlying MSE media demuxer and player) or the dedicated worker thread hosting the interface to the MSE demuxer when using MSE from a worker context motivated careful design of a refcounted, outside-of-Oilpan, attachment abstraction to ensure safety of operations using locks and having a lifetime that exceeds those execution contexts. Preventing use-after-free and similar was a primary complexity of implementation and a focus area of added tests.

WebView application risks

Does this intent deprecate or change behavior of existing APIs, such that it has potentially high risk for Android WebView-based applications?

Not potentially high risk.

Debuggability

Feature consists of exposing existing interfaces also on a DedicatedWorker, and addition of a WebIDL method for proactively detecting support for this feature, and are covered by basic tooling.

Will this feature be supported on all six Blink platforms (Windows, Mac, Linux, Chrome OS, Android, and Android WebView)?

Yes

MSE and DedicatedWorker already exist on all six Blink platforms. This feature lets MSE work from DedicatedWorker contexts.

Is this feature fully tested by web-platform-tests?

Yes

DevTrial instructions

https://github.com/w3c/media-source/issues/175#issuecomment-721395481

Flag name

MediaSourceInWorkers

Requires code in //chrome?

False

Tracking bug

https://crbug.com/878133

Measurement

Prior to M105 (when worker MediaSource was attachable only by object URL): Blink.UseCounter.Features.CreateObjectURLMediaSourceFromWorker As of 105.0.5180.0 (when attachment of worker MediaSource was changed to be done using transferable MediaSourceHandle): Blink.UseCounter.Features.MediaSourceGetHandle

Non-OSS dependencies

Does the feature depend on any code or APIs outside the Chromium open source repository and its open-source dependencies to function?

No.

Sample links

https://tinyurl.com/mse-in-workers-demo

Estimated milestones

OriginTrial desktop last	103
OriginTrial desktop first	95

OriginTrial Android last	103
OriginTrial Android first	95

NOTE: Requested desktop and Android ship milestone: 105 (The 'edit estimated milestones' link from chromestatus didn't result in such ability to edit, so I'm adding this request explicitly here.)

The feature is spec complete as of this request with corresponding implementation and tests complete as of 105.0.5180.0. OT ends in 103 (and Twitch has reported strongly positive results of their use of it during OT.)

Anticipated spec changes

None currently for this feature.

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/5177263249162240

Links to previous Intent discussions

Intent to prototype: https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/CNRywDqgKjY/m/F0nnA4tTAwAJ
Intent to Experiment: https://groups.google.com/a/chromium.org/g/blink-dev/c/XZMyanniH9E
Intent to Extend Experiment: https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/WETrrUYLrTM

This intent message was generated by Chrome Platform Status.

[Matthew Wolenetz]

Further note, this feature's shipping will entail switching two blink RuntimeEnabled features from experimental to stable simultaneously:

• MediaSourceInWorkers
• MediaSourceInWorkersUsingHandle

A CL [1] is already prepared to do this switch once this intent is approved. The plan is, once that CL has landed and baked in trunk, to request and merge it to the branch for M105.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/3781750

[Matthew Wolenetz]

I've filed a new request-for-position using the new WebKit process for this today.

The previous request was: https://lists.webkit.org/pipermail/webkit-dev/2021-June/031916.html

The new one is: https://github.com/WebKit/standards-positions/issues/40

I've also updated chromestatus' WebKit signals citation to use the new request.

[slightlyoff via Chromestatus]

LGTM1

[Yoav Weiss]

LGTM2

On Fri, Jul 29, 2022 at 12:40 AM slightlyoff via Chromestatus <admin+sl...@cr-status.appspotmail.com> wrote:

LGTM1

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/000000000000eea04c05e4e53819%40google.com.

[Mike Taylor]

LGTM3

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfWsqYRjB%3DqCuu4UxHzEa%2B%2B2SqkDJdE8W8sUNCnS%2BwoH5w%40mail.gmail.com.

[Joe Medley]

Gang,

NOTE: Requested desktop and Android ship milestone: 105 (The 'edit estimated milestones' link from chromestatus didn't result in such ability to edit, so I'm adding this request explicitly here.)

I'm looking into this and will report back. Has anyone else noticed this problem?

Joe 

Joe Medley | Technical Writer, Chrome DevRel | jme...@google.com | 816-678-7195

If an API's not documented it doesn't exist.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/91d78e87-3241-0719-eb2a-a7e8d21afd0b%40chromium.org.

[Matthew Wolenetz]

Per previous LGTMs on this, the feature launched in M105: https://chromium-review.googlesource.com/c/chromium/src/+/3795560

Unfortunately, both the MSE spec and Chromium implementation for this feature included logic which regressed some media playback sites, and the regression was found late after the feature reached stable in M105.

The launch was reverted from 105 (and 106-108 also have had this feature reverted explicitly back to experimental).

The regression was due to the MediaSource object's new handle attribute being:

1. Visible on both main/Window and on Dedicated Worker contexts (not just the latter), and
2. Throwing NotSupportedError exception if read from on a context (main/Window, in this case) where the implementation does not support using a MediaSourceHandle for attaching the MediaSource object owned by that context.

The specific MSE spec language involved in this regression (from https://github.com/w3c/media-source/pull/306):

"If the implementation does not support creating a handle for this MediaSource, then throw a NotSupportedError exception and abort these steps.NOTE Implementations MAY choose to only allow handle creation for MediaSource objects in a DedicatedWorkerGlobalScope, as a minimum requirement for enabling attachment of such a MediaSource object to an HTMLMediaElement."

Uncaught by myself nor multiple reviewers (except partially by cas...@chromium.org, who asked about precedent for throwing exceptions in attribute getters), there was a pre-existing behavior pattern in at least one older version of a media playback javascript library (video.js) that enumerated all MediaSource object attributes, including reading from them and obtaining an unexpected exception on reading this new handle attribute on the main/Window context.

Also uncaught by myself nor reviewers was the lack of "flag-guarding" of this feature (it only had blink runtime enabled features, not corresponding base::Features), so the revert in M105 stable of the previous launch required a binary respin.

The MSE spec has been updated to replace this regressing logic, and the corresponding Chromium implementation update, addition of flag-guards, and relaunch to stable (in current M108 milestone) is in code-review now and is pending further LGTMs on this thread.

Please review for relaunch. I expect I'll need 3 LGTMs to proceed with landing the relaunch CL.

Thank you,

Matt

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJUhtG8EH2E1y33qLPJr3xdp8ZT1z1T98mCQ9bJ4WeBn19BOsQ%40mail.gmail.com.

[Matthew Wolenetz]

Regarding the scope of the fix involved in this relaunch, it removes visibility of the new handle attribute from the main/Window context (and updates test expectations, accordingly.)

While that change alone would fix the regression, it risks potential for unexpected exception in future if that attribute were ever exposed again on the main/Window context.

Therefore, both the spec and implementation involved in this relaunch also remove the ability to throw NotSupportedError exception by this attribute's getter.

With the net change versus the previous launch being specific to fixing this regression, and with the relaunch CL also including base::Feature guards for the two blink RuntimeEnabled features for MSE-in-Workers, I believe the risk of relaunch causing respin to be small even if some similar scope of regression were found after the feature reached stable.

Matt

[Yoav Weiss]

LGTM1 to relaunch

This looks perfectly reasonable. Thanks for doing everything necessary to minimize the damage through what I'm guessing wasn't what one may call a "fun experience".

On Wed, Sep 28, 2022 at 9:51 PM Matthew Wolenetz <wole...@chromium.org> wrote:

Regarding the scope of the fix involved in this relaunch, it removes visibility of the new handle attribute from the main/Window context (and updates test expectations, accordingly.)

While that change alone would fix the regression, it risks potential for unexpected exception in future if that attribute were ever exposed again on the main/Window context.

Therefore, both the spec and implementation involved in this relaunch also remove the ability to throw NotSupportedError exception by this attribute's getter.

With the net change versus the previous launch being specific to fixing this regression, and with the relaunch CL also including base::Feature guards for the two blink RuntimeEnabled features for MSE-in-Workers, I believe the risk of relaunch causing respin to be small even if some similar scope of regression were found after the feature reached stable.

Matt

On Wed, Sep 28, 2022 at 12:40 PM Matthew Wolenetz <wole...@chromium.org> wrote:

Per previous LGTMs on this, the feature launched in M105: https://chromium-review.googlesource.com/c/chromium/src/+/3795560

Unfortunately, both the MSE spec and Chromium implementation for this feature included logic which regressed some media playback sites, and the regression was found late after the feature reached stable in M105.

The launch was reverted from 105 (and 106-108 also have had this feature reverted explicitly back to experimental).

The regression was due to the MediaSource object's new handle attribute being:

1. Visible on both main/Window and on Dedicated Worker contexts (not just the latter), and
2. Throwing NotSupportedError exception if read from on a context (main/Window, in this case) where the implementation does not support using a MediaSourceHandle for attaching the MediaSource object owned by that context.

The specific MSE spec language involved in this regression (from https://github.com/w3c/media-source/pull/306):

"If the implementation does not support creating a handle for this MediaSource, then throw a NotSupportedError exception and abort these steps.NOTE Implementations MAY choose to only allow handle creation for MediaSource objects in a DedicatedWorkerGlobalScope, as a minimum requirement for enabling attachment of such a MediaSource object to an HTMLMediaElement."

Uncaught by myself nor multiple reviewers (except partially by cas...@chromium.org, who asked about precedent for throwing exceptions in attribute getters), there was a pre-existing behavior pattern in at least one older version of a media playback javascript library (video.js) that enumerated all MediaSource object attributes, including reading from them and obtaining an unexpected exception on reading this new handle attribute on the main/Window context.

Huh! Might be worthwhile to file an issue on the TAG's design principles to warn against making getters throw exceptions or have any other side effects for that matter.

 

Also uncaught by myself nor reviewers was the lack of "flag-guarding" of this feature (it only had blink runtime enabled features, not corresponding base::Features), so the revert in M105 stable of the previous launch required a binary respin.

Maybe our process should strongly-recommend/require adding a base_feature flag for anything that changes the API shape, as it's very hard to predict such breakage..

[Mike Taylor]

LGTM2

[Chris Harrelson]

LGTM3

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ea179069-c57f-0b83-0377-dfe394e892fc%40chromium.org.

[Matthew Wolenetz]

Thank you for the relaunch approvals. Once the relaunch CL is fully reviewed, I'll land it.

 

Huh! Might be worthwhile to file an issue on the TAG's design principles to warn against making getters throw exceptions or have any other side effects for that matter.

Indeed, I am tracking this as a potential follow-up and will update here with a TAG issue once filed.

I'm also considering adding a PRESUBMIT warning for any "RaisesException" that is added to a blink IDL attribute getter. It would help prevent similar regression.

Maybe our process should strongly-recommend/require adding a base_feature flag for anything that changes the API shape, as it's very hard to predict such breakage..

I'll re-read the recent PSA about flag-guarding, and if it's unclear, will request this be added.

Matt 

 

[Matthew Wolenetz]

Relaunch landed and is in Chrome >= 108.0.5334.0.

Huh! Might be worthwhile to file an issue on the TAG's design principles to warn against making getters throw exceptions or have any other side effects for that matter.

Indeed, I am tracking this as a potential follow-up and will update here with a TAG issue once filed.

I've filed TAG issue: Request for design principle clarification: Warn explicitly against getters throwing exceptions #400

Maybe our process should strongly-recommend/require adding a base_feature flag for anything that changes the API shape, as it's very hard to predict such breakage..

I'll re-read the recent PSA about flag-guarding, and if it's unclear, will request this be added.

I've replied to the internal PSA thread on this, minus the previously bcc segment.

I'm also considering adding a PRESUBMIT warning ...

I'll be including this in follow-up work.

Matt