Re: [mediacapture-screen-share-extensions] Auto-pause for Captured Surface Switching (2nd edition) (#15)

Replying to @youennf 

> * Why put the event in controller instead of a MediaStreamTrack? The plan seems to set enabled to false/true at track level. There is the case of two MediaStreamTrack clones one in a worker and the other in a window and it might be more handy for each track to be notified of surface switching.

I put it on the controller as the default place for screen-capture-related functionality, but the track might be a better place for it. I think it should be manageable for applications centralize the control of when tracks should be re-enabled to one of the event-listeners as you propose when it is needed.

> * Why do we have to set `enabled` to false in step 2? Why not simply asking the web page to set `enabled` to false synchronously for each MediaStreamTrack within the event handler (or call `replaceTrack` to null) or whatever most appropriate.

Yes, that might be better. That would also allow applications to not disable tracks where it is not needed.

> * If we have `autopause`, why not using `configurationchange` which will allow having the right information via `getSettings`: the surface type, maybe deviceId...

Yes, that might be better. If going with `configurationchange`, what mechanism would you suggest to detect that a source-switch has occurred?

> I would tend to go with the following approach:
> 
> * Add `autopause` to `getDisplayMedia`. (Or maybe `pauseOnSurfaceSwitching`?)

I like the more explicit `pauseOnSurfaceSwitching`.

> * When `autopause` is true, and capture switches to a surface S, sinks of a track are guaranteed to not receive video frames from S until the `configurationchange` event is fired on that particular track.

Yes, I think this should work.

Replying to @jan-ivar 

> Let me comment on the OP first to narrow down what is being proposed.
> 
> > 1. An event is added to the CaptureController that is emitted each time a user has switched to a new captured surface:
> > 
> > ```js
> > controller.addEventHandler('surfaceswitch', ...);
> > ```
> 
> Do you mean [addEventListener](https://dom.spec.whatwg.org/#ref-for-dom-eventtarget-addeventlistener%E2%91%A2)? ([EventHandler](https://html.spec.whatwg.org/multipage/webappapis.html#eventhandler)s are attributes the [guidelines recommend adding as well](https://w3ctag.github.io/design-principles/#always-add-event-handlers), `controller.onsurfaceswitch` in our case.)

Yes, sorry, I meant `addEventListener`

> > 2. Set enabled = false for all tracks associated with the capture.
> 
> This seems like a foot gun, and violates [Media Capture and Streams](https://w3c.github.io/mediacapture-main/getusermedia.html#track-enabled) which designates `muted` for UA control, and leaves `enabled` _"available to the application to control"_.
> 
> Having the browser set this attribute can race with the application. Worst case, mistakenly inverted application logic might cause others to see the user when they shouldn't.

What do you think about Youenn’s suggestion of letting the application set `enabled = false`? I think that should solve this issue?

> Other ideas we discussed (and I expressed support for), like [postSurfaceSwitchingHandler](https://docs.google.com/document/d/16CUOJeuXimNPi4kZHOS9rF-WhMuVvOqOg9P--Dvqi_w/edit#bookmark=id.3e5aj0t5ghqi) does not have this problem. In that model, no state is touched, the default behavior is to inject (which I prefer), with late JS action required to pause. Was it considered?

Yes, it was considered but as I replied to you in that comment thread, I think an event-based model using the enabled-state might be a better option:
* It uses events rather than callbacks, which I believe is preferable.
* I think it’s simpler to let the application change the enabled-state rather than manipulating promises.

> Step 2 and 3 seem to invite confusion over whether a source or a track feature is being proposed.

Yes, it might be better and more consistent to have the event on the MediaStreamTrack as Youenn suggests.


-- 
GitHub Notification of comment by tovepet
Please view or discuss this issue at https://github.com/w3c/mediacapture-screen-share-extensions/issues/15#issuecomment-2432852331 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Wednesday, 23 October 2024 16:54:59 UTC