w3c / webcodecs

WebCodecs is a flexible web API for encoding and decoding audio and video.
https://w3c.github.io/webcodecs/
Other
953 stars 135 forks source link

SharedWorker support? #635

Closed dalecurtis closed 1 year ago

dalecurtis commented 1 year ago

I just noticed we have a WPT test checking for SharedWorker support: https://wpt.fyi/results/webcodecs/videoFrame-serialization.crossAgentCluster.https.html?label=experimental&label=master&aligned

It looks like it was added by Firefox here: https://hg.mozilla.org/integration/autoland/rev/5a2fc97cac78

@padenot @youennf should we enable SharedWorker support? No objections off hand on the Chromium side. I'm not very familiar with the technical limitations around SharedWorkers though.

youennf commented 1 year ago

Ah, VideoFrame is interesting since it is transferable/serializable but is only exposed in window/DedicatedWorker. I am wondering what should happen if a VideoFrame is transferred to a SharedWorker then. cc @jan-ivar with whom we were speaking of such kind of objects.

I do not see anything particularly wrong in having VideoFrames in all workers, except the use cases are probably smaller in non dedicated workers. Technically speaking, this is no different in WebKit than in DedicatedWorker so should be straightforward.

should we enable SharedWorker support?

For VideoFrame or for all WebCodecs, i.e. encoders/decoders?

dalecurtis commented 1 year ago

I think it would make sense to expose everything if we can for consistency. I'd be fine with just VideoFrame if there's reasons we shouldn't for the codecs though.

jan-ivar commented 1 year ago

I am wondering what should happen if a VideoFrame is transferred to a SharedWorker then.

You'll get a DataCloneError when you deserialize:

I think it would make sense to expose everything if we can for consistency.

A more conservative approach would be to only expose what we need for the use cases we know of. I'd favor that since exposing more later is cheap, but once we've exposed something, it's much harder if not impossible to put back in the bottle.

For instance, the wpt mentioned verifies that "frames cannot be transferred to serviceworker".

jan-ivar commented 1 year ago

For instance, VideoFrames may hold on to physical resources. So preventing their spread to realms that outlast where they came from may have consequences we haven't thought of.

dalecurtis commented 1 year ago

Yeah, ServiceWorker exposure seems questionable, but SharedWorker seems like a logical extension and doesn't seem to have any consequences that two DedicatedWorker instances wouldn't have.

jan-ivar commented 1 year ago

Well, DedicatedWorkers go away with the document, whereas a SharedWorker might outlive it. I also don't know for sure if it adds out of process requirements or not sometimes? Or are SharedWorkers implicitly same process?

That said, since the WPT from Mozilla allowed it, maybe @padenot has thoughts on it?

dalecurtis commented 1 year ago

That's fair. Out of process requirements might be why we didn't add it originally since not all frames a backed in a way that's shareable without readback/copy.

dalecurtis commented 1 year ago

If we decide we shouldn't have SharedWorker (or at least not yet) I'll land https://chromium-review.googlesource.com/c/chromium/src/+/4216102 to fix the wpt test.

youennf commented 1 year ago

Thanks for the link, this is good to know!

For instance, VideoFrames may hold on to physical resources. So preventing their spread to realms that outlast where they came from may have consequences we haven't thought of.

It is already a problem without workers. You can postMessage a VideoFrame from one window to another and you have to deal with the fact that the other window might hook to it forever.

I also don't know for sure if it adds out of process requirements or not sometimes? Or are SharedWorkers implicitly same process?

Given you can post message a video frame to another tab, or a third-party iframe, the spec already mandates out of process support (implementations may lag though, Safari does not yet support this).

So far, I haven't seen any new issue/requirement related to SharedWorker/ServiceWorker exposure. But I did not really hear anybody asking for it either, I would tend to wait for people asking for it.

I'll land https://chromium-review.googlesource.com/c/chromium/src/+/4216102 to fix the wpt test.

That sounds good to me. I do not think one can expect SharedWorker to be in another process. I would tend to use different tabs instead, maybe process isolation as well to be extra sure.

dalecurtis commented 1 year ago

We've had only a single request for SharedWorker: https://bugs.chromium.org/p/chromium/issues/detail?id=1410412

I think the use case can be handled in other ways though. I'll wait to hear from @padenot before landing the test change.

padenot commented 1 year ago

I think this is a mistake on our side. This shouldn't work in SharedWorker, it's not exposed there.

dalecurtis commented 1 year ago

Thanks Paul. I'll go ahead and land the test fix then. I'll close this for now and devs can +1 or chime in on it if this is something they'd like to see added.