w3c / mediacapture-handle

https://w3c.github.io/mediacapture-handle/
Other
14 stars 10 forks source link

[Identity] .getCaptureHandle() seems misplaced on the track #12

Open jan-ivar opened 2 years ago

jan-ivar commented 2 years ago

MediaStreamTrack is a realtime audio/video consumption API, where consumers apply consumption constraints, such as width, height and noiseSuppression. Implementations satisfy them by processing media, to separate tracks from influencing one another.

JS can clone or transfer tracks to other realms, to delegate them to every corner of their app designed to consume them, be it for local playback, recording, or sending over a peer connection. OverconstrainedError resolves conflicts where processing cannot.

track.getCaptureHandle doesn't fit with this model, because:

  1. It's not clear this control surface should follow the track to all consumers. The top-level JS doing the capturing may wish to retain this control for itself and only send tracks to dumb consumers, without worrying that these consumers may be able to bootstrap and send control messages to the source (this seems true whether we add Actions https://github.com/WICG/capture-handle/issues/6 or not),
  2. Such control messages to the source would affect consumers of all tracks, not just the one track. There's no system to resolve conflicts like we have for constraints.

This suggests the API belongs someplace else.

jan-ivar commented 2 years ago

Possible Solutions

We explored other ways to surface results from getDisplayMedia in w3c/mediacapture-handle#52:

partial interface MediaDevices {
    attribute EventHandler ongetdisplaymediaresult;
}

This seems better organized, even though anyone can register an event handler. At least it doesn't conflate control surfaces meant for different audiences together.

We could also do what fetch did and pass in a controller:

const controller = new CaptureController();
const stream = await navigator.getDisplayMedia({video: true}, {controller});
alvestrand commented 2 years ago

This seems like a complexification of the API. getDisplayMedia has one result, and there seems no reason not to reuse that result.

Again, not something that should meet the bar for not adopting the document.

eladalon1983 commented 2 years ago

The top-level JS doing the capturing may wish to retain this control for itself and only send tracks to dumb consumers

That's a corner case with no concrete examples cited thus far. More importantly, it case can be addressed easily, for example (just spitballing here) by specifying that a transferred track loses access to getCaptureHandle(). Dealing with this hypothetical corner case does not require massive restructuring of getDisplayMedia - which you effectively suggest.

But, @jan-ivar, when you manage to convince the Working Group to add ongetdisplaymediaresult, let's discuss restructuring Capture Handle accordingly. Until that (theoretical) day, let's not block progress by predicating one thing on another.

I suggest we close this issue for now and adopt the document. We can reopen in the future.

jan-ivar commented 2 years ago

getDisplayMedia has one result, and there seems no reason not to reuse that result.

@alvestrand I gave two reasons in the OP. getDisplayMedia produces one result today: a MediaStream for consumption. Trying to add a two-way capturee (bootstrapping) communication/control surface to that, I count as two results.

Redefining well-known types to carry tangential APIs simply because one method (out of the many that returns said type) needs to return more information, seems like an OO anti-pattern we should not adopt.

it ... can be addressed easily, ... by specifying that a transferred track loses access to getCaptureHandle()

@eladalon1983 That sounds hacky, but helps highlight the issue: that this new API probably doesn't belong on that object in the first place. Having clone with side-effects seems like another anti-pattern, but let's discuss that in https://github.com/w3c/mediacapture-main/issues/857.

I think questioning whether a proposed API is misplaced, is a legitimate issue to bring up ahead of adopting said API.

jan-ivar commented 2 years ago

The more I think about this, the more I think the following is the right surface for both Actions (https://github.com/WICG/capture-handle/pull/15) and Identification:

const controller = new CaptureController();
const stream = await navigator.getDisplayMedia({video: true, controller});
const {handle, origin} = controller;
const actions = controller.getSupportedActions();
await controller.sendAction("next");

This requires no "massive restructuring" of MediaStreamTrack.

I suggest we close this issue for now and adopt the document. We can reopen in the future.

I think part of the value of the CfC/CfA process is to surface issues and document them. If and when we adopt this document, I would expect we transfer these issues to the w3c github repo, since they are still valid, not close them. We're at the beginning of standardization here, not the end.

eladalon1983 commented 2 years ago

Trying to add a two-way capturee (bootstrapping) communication/control surface to that, I count as two results.

By this logic, id, kind, label etc. are all additional results, and we have a double-digit of results already. I find this argument unconvincing.

Having clone with side-effects seems like another anti-pattern

I believe intentionally dropping some information when cloning has precedents and makes sense.

This requires no "massive restructuring" of MediaStreamTrack.

It's a big changes to getDisplayMedia. Consensus here will not be easy. I personally find an out-parameter quite awkward. Maybe even "footgunny". For example, what happens to old values of controller if the Promise is rejected? Would an application run the risk of using an old controller then?

These are not discussions I am anxious to have. I am excited by the prospect of extending the capabilities of Web-applications. By extending the Web's reach. I don't think our time will be well-spent lengthily debating minor issues of API shape, that make no functional difference.

I would expect we transfer these issues to the w3c github repo

The issues will be transfered. And I may still advocate for their closure, and you may still object. Progress entails concluding discussions. It is reasonable to disagree about this.

jan-ivar commented 2 years ago

I believe intentionally dropping some information when cloning has precedents ...

What are they?

I personally find an out-parameter quite awkward.

It's technically an in-parameter: an object that will be associated only upon resolve, but I know what you mean, and I agree it's not ideal, but backwards compatible things rarely are.

However, having this capture controller object might be a hub for several capture controls in the hopper:

  1. Capture handle
  2. Capture actions
  3. Conditional focus https://github.com/w3c/mediacapture-screen-share/issues/190

@youennf WDYT?

Maybe even "footgunny". For example, what happens to old values of controller if the Promise is rejected?

The controller would never be associated, which seems fine. We could also prevent reuse if we want.