w3c / mediacapture-handle

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

CaptureHandleChangeEvent should just be Event #50

Closed jan-ivar closed 2 years ago

jan-ivar commented 2 years ago

The doc says "Capturing applications who are permitted to observe a track's CaptureHandle have two ways of reading it."

This is one too many. The second way:

let handle;
track.oncapturehandlechange = event => handle = event.captureHandle();

...seems entirely redundant, since this should suffice:

let handle;
track.oncapturehandlechange = ({target}) => handle = target.getCaptureHandle();

I believe the common pattern in situations like this is to just fire a vanilla Event.

eladalon1983 commented 2 years ago

I think that am reading between the lines that you want getCaptureHandle() to return the value set by the latest event. I'll clarify what I mean, then ask if I am reading correctly.

Clarification:

Between the lines, I am reading that you want this to change, so that getCaptureHandle() would always return the value corresponding to the latest event that was popped off of the queue.

  1. Did I read correctly?
  2. If so - is this wise?
eladalon1983 commented 2 years ago

Friendly ping @jan-ivar.

jan-ivar commented 2 years ago

See https://github.com/w3c/mediacapture-handle/issues/56.

... events are still pending on the queue.

JS state updates should be on the same queue, so there's no "latest value" confusion.

eladalon1983 commented 2 years ago

Let's examine a scenario:

The question boils down to - does it make sense for a browser to give C the capture-handle from 4 seconds ago, rather than the current one?

jan-ivar commented 2 years ago

We don't have to ask this question for every new API. This has been solved many times, so we follow established patterns:

const transceiver = new RTCPeerConnection().addTransceiver("video");
transceiver.transport.onstatechange = () => console.log(transceiver.transport.state); // logs all states

The state getter returns "the value of the [[DtlsTransportState]] slot", which is set in the same queued task #56 that fires (synchronously calls) the statechange event (handler), "when the underlying DTLS transport needs to update".

It does not return any underlying "current" value. This is to preserve run-to-completion semantics.

eladalon1983 commented 2 years ago

We don't have to ask this question for every new API.

I have not observed this discussion before. You have the benefit here of having been involved with more APIs than me. I'll do my best to catch up.

that fires (synchronously calls) the statechange event

This is the key part that I was overlooking - that events don't go in a queue (even if they can be called from a queued task).

Shall we close this issue and continue on #56?

jan-ivar commented 2 years ago

Why don't we keep this one open to track removing the custom event, and leave the other one to specify the event firing algorithms?

eladalon1983 commented 2 years ago

That works for me.