Update proposal according to WG directions #66

alvestrand · 2021-11-26T13:40:56Z

This prepares the document for adoption as a WG document.

index.bs

jan-ivar · 2021-11-29T16:17:19Z

index.bs

@@ -190,6 +190,10 @@ dictionary MediaStreamTrackProcessorInit {
 };
 </pre>

+Note: There is WG consensus that the interface should be exposed on DedicatedWorker.
+There is no WG consensus that the interface should not be exposed on Window.


I'm a bit concerned this double-negative is hard to parse. We normally don't require consensus to not do something, so it would seem more correct to say "There is no WG consensus that the interface should be exposed on Window.".

Or doing the same as for audio would be: "There is no WG consensus on whether or not the interface should be exposed on Window."

"on whether or not" adopted.

jan-ivar

Thanks for doing this merge!

I have some concerns around some WebIDL and some audio API surface that remains.

Also, the examples should be updated to show the worker model with transfer of MST. See link to my PR below which has these. But I can also do a separate PR for that if you prefer. LMK.

jan-ivar · 2021-11-30T01:14:11Z

index.bs

@@ -190,6 +190,10 @@ dictionary MediaStreamTrackProcessorInit {
 };
 </pre>

+Note: There is WG consensus that the interface should be exposed on DedicatedWorker.
+There is no WG consensus that the interface should not be exposed on Window.


Suggested change

There is no WG consensus that the interface should not be exposed on Window.

There is no WG consensus on whether or not the interface should be exposed on Window.

index.bs

jan-ivar · 2021-11-30T01:20:03Z

index.bs

+Note: There is consensus in the WG that this interface should be exposed on DedicatedWorker.
+There is no consensus in the WG about whether or not it should be exposed on Window.


Same here

Suggested change

Note: There is consensus in the WG that this interface should be exposed on DedicatedWorker.

There is no consensus in the WG about whether or not it should be exposed on Window.

Note: There is WG consensus that this interface should be exposed on DedicatedWorker.

There is no WG consensus on whether or not it should be exposed on Window.

jan-ivar · 2021-11-30T01:39:16Z

index.bs

+Previous proposals for this interface had an API like this:
+
+<pre class="idl">
+[Exposed=Window,DedicatedWorker]
+interface MediaStreamTrackGenerator : MediaStreamTrack {
+    constructor(MediaStreamTrackGeneratorInit init);
+    attribute WritableStream writable;  // VideoFrame or AudioFrame
+};
+
+dictionary MediaStreamTrackGeneratorInit {
+  required DOMString kind;
+};
+</pre>


My understanding of our agreement was that "We document the parts that have consensus in code", i.e. no WebIDL for non-standard APIs, even in notes.

Provided we remove the WebIDL then I like this section. Once we get the actual JS shim in here, then that should be all we need to document I think.

I agree that we should not try to document non-standard APIs here.

I disagree on this point. WebIDL is a clearer exposition format than prose, and this is an informative section. Leaving as-is.

jan-ivar · 2021-11-30T01:43:28Z

index.bs

+This interface can be shimmed on top of VideoTrackSource like this:
+
+<pre>
+
+Javascript goes here when written
+
+</pre>


I think we should not document this part. I think driving convergence around the spec means:

We should document how to shim standard APIs on top of non-standard ones.
We should not document how to shim non-standard APIs on top of standard ones.

Hm. This can be left to an advice note when shipping the new interface in preference to the older one.
I can leave it out; the reason to include it would be to prove by code that the new interface can do everything the old interface could.

jan-ivar · 2021-11-30T01:51:36Z

index.bs

+The VideoTrackSource can be shimmed on top of MediaStreamTrackGenerator like this:
+
+<pre>
+
+More Javascript goes here
+
+</pre>


This part is good.

jan-ivar · 2021-11-30T02:07:16Z

index.bs

+# Considerations for sourcing audio tracks # {#audio-processor}
+
+These are kept in the document to allow further discussion of audio processing, if
+feasible. They do not indicate that the WG has consensus to add audio.


This section appears to normatively define/alter audio constraints in detail, which I'd consider surface/code. It feels out of place in a document that does not define an API for audio. The fact that this can be done without WebIDL seems like an artifact of our Constrainable pattern and our practice of expressing normative outcomes in tables instead of through algorithms.

Based on our agreement that we "We document the parts that have consensus in code", I would expect this to be removed, unless it can be rewritten to be agnostic to kind. cc @youennf

Agreed this section does not make much sense right now.
It can be revived if there is WG consensus to move on with supporting audio.

This is about preserving previous considerations. This isn't code.

But in the interest of getting this out, replacing it with a note saying "furhter considerations are found in older versions of this spec, pointer will go here".

jan-ivar · 2021-11-30T02:24:17Z

index.bs

+  </tbody>
+</table>
+
+## Example using audio/video co-processing


I think the two code examples dedicated to audio need to stay in alvestrand until we adopt an audio API.

Also, all examples are still main-thread based, and show the old API, which I think needs to be updated.

I provided 3 examples (3.1, 3.2. 3.3) in https://jan-ivar.github.io/mediacapture-transform/#examples I recommend we use to show how to transfer MST to a worker and using the API there (with track.readable rewritten to MediaStreamTrackProcessor of course)

youennf · 2021-11-30T09:10:00Z

index.bs

-and a video {{MediaStreamTrackGenerator}} accepts {{VideoFrame}} objects.
-When a {{VideoFrame}} or {{AudioData}} object is written to
-{{MediaStreamTrackGenerator/writable}},
+MediaStream model</a> in the dedicated worker environment. It has a two readonly


Suggested change

MediaStream model</a> in the dedicated worker environment. It has a two readonly

MediaStream model</a> in the dedicated worker environment. It has two readonly

youennf · 2021-11-30T09:12:01Z

index.bs

+[Exposed=DedicatedWorker]
+interface VideoTrackSource {
+  constructor();
+  readonly attribute WritableStream writable;


We have Promise<XYZ>, could we write it as WritableStream<VideoFrame>?

Don't understand this comment. What are you suggesting?

youennf · 2021-11-30T09:14:02Z

index.bs

-    with |init|.{{MediaStreamTrackGeneratorInit/kind}}.
-4. Return |g|.
+1. Let |source| be a new {{VideoTrackSource}} object.
+1. Let |track| be a new {{MediaStreamTrack}} object, whose {{MediaStreamTrack/kind}} is "video", and whose {{MediaStreamTrack/id}} is a new unique id generated by the user agent.


We could be using https://w3c.github.io/mediacapture-main/#dfn-initialize-the-underlying-source here to tie source and MediaStreamTrack.

youennf · 2021-11-30T09:19:09Z

index.bs

+The <dfn>writeFrame</dfn> algorithm is given a |source| and a |frame| as input. It is defined by running the following steps:
+1. If |frame| is not a {{VideoFrame}} object, return [=a promise rejected with= a {{TypeError}}.
+1. If |source|.`[[isMuted]]` is false, send the media data backing |frame| to all live tracks sourced from |source|.
+1. Invoke the `close` method of |frame|.


Seems good enough for now, I believe there might be cases where the close method will be called too late though so that JS could call close() before running that step for instance. Also, we should handle the case of closed frames.

Let's leave this as a bug. The "close" method of a VideoFrame seems to be idempotent; it sets a bunch of variables, it doesn't read any of them before setting them. But I do wonder if this is consistent with its implementation.
https://w3c.github.io/webcodecs/#close-videoframe

index.bs

youennf · 2021-11-30T09:26:23Z

index.bs

+Previous proposals for this interface had an API like this:
+
+<pre class="idl">
+[Exposed=Window,DedicatedWorker]
+interface MediaStreamTrackGenerator : MediaStreamTrack {
+    constructor(MediaStreamTrackGeneratorInit init);
+    attribute WritableStream writable;  // VideoFrame or AudioFrame
+};
+
+dictionary MediaStreamTrackGeneratorInit {
+  required DOMString kind;
+};
+</pre>


I agree that we should not try to document non-standard APIs here.

youennf · 2021-11-30T09:27:50Z

index.bs

+# Considerations for sourcing audio tracks # {#audio-processor}
+
+These are kept in the document to allow further discussion of audio processing, if
+feasible. They do not indicate that the WG has consensus to add audio.


Agreed this section does not make much sense right now.
It can be revived if there is WG consensus to move on with supporting audio.

alvestrand · 2021-11-30T09:42:27Z

I'm interested in getting this to the WG ASAP for the call for adoption, so would be inclined to minimize changes.
Question: In a couple of places, it's been suggested to "leave this in alvestrand/mediacapture-transform". This can't be done until the move/clone dance has completed, of course (this IS the alvestrand repo). But it makes sense after the move/clone dance has completed.

WDYT about augmenting the backwards compatibility section with an explicit pointer to "older version hosted at..."?

Removed audio examples and constraints. Changed name to VideoTrackGenerator (personal preference). Added aspirational shim JS.

alvestrand

Addressed a number of comments, and changed name from VideoTrackSource to VideoTrackGenerator (it is a special kind of source, not a generic source object).

Leaving the revision of examples to a later CL (Jan-Ivar will do that better than I can).

PTAL.

alvestrand · 2021-11-30T10:09:35Z

index.bs

@@ -190,6 +190,10 @@ dictionary MediaStreamTrackProcessorInit {
 };
 </pre>

+Note: There is WG consensus that the interface should be exposed on DedicatedWorker.
+There is no WG consensus that the interface should not be exposed on Window.


"on whether or not" adopted.

alvestrand · 2021-11-30T10:19:52Z

index.bs

-    with |init|.{{MediaStreamTrackGeneratorInit/kind}}.
-4. Return |g|.
+1. Let |source| be a new {{VideoTrackSource}} object.
+1. Let |track| be a new {{MediaStreamTrack}} object, whose {{MediaStreamTrack/kind}} is "video", and whose {{MediaStreamTrack/id}} is a new unique id generated by the user agent.


alvestrand · 2021-11-30T10:22:35Z

index.bs

+The <dfn>writeFrame</dfn> algorithm is given a |source| and a |frame| as input. It is defined by running the following steps:
+1. If |frame| is not a {{VideoFrame}} object, return [=a promise rejected with= a {{TypeError}}.
+1. If |source|.`[[isMuted]]` is false, send the media data backing |frame| to all live tracks sourced from |source|.
+1. Invoke the `close` method of |frame|.


Let's leave this as a bug. The "close" method of a VideoFrame seems to be idempotent; it sets a bunch of variables, it doesn't read any of them before setting them. But I do wonder if this is consistent with its implementation.
https://w3c.github.io/webcodecs/#close-videoframe

index.bs

alvestrand · 2021-11-30T10:34:08Z

index.bs

+Previous proposals for this interface had an API like this:
+
+<pre class="idl">
+[Exposed=Window,DedicatedWorker]
+interface MediaStreamTrackGenerator : MediaStreamTrack {
+    constructor(MediaStreamTrackGeneratorInit init);
+    attribute WritableStream writable;  // VideoFrame or AudioFrame
+};
+
+dictionary MediaStreamTrackGeneratorInit {
+  required DOMString kind;
+};
+</pre>


I disagree on this point. WebIDL is a clearer exposition format than prose, and this is an informative section. Leaving as-is.

alvestrand · 2021-11-30T10:34:38Z

index.bs

+This interface can be shimmed on top of VideoTrackSource like this:
+
+<pre>
+
+Javascript goes here when written
+
+</pre>


alvestrand · 2021-11-30T10:42:51Z

index.bs

+# Considerations for sourcing audio tracks # {#audio-processor}
+
+These are kept in the document to allow further discussion of audio processing, if
+feasible. They do not indicate that the WG has consensus to add audio.


But in the interest of getting this out, replacing it with a note saying "furhter considerations are found in older versions of this spec, pointer will go here".

youennf · 2021-11-30T12:45:37Z

WDYT about augmenting the backwards compatibility section with an explicit pointer to "older version hosted at..."?

It seems ok for the standard draft to acknowledge the initial non standard document it was based upon, for instance as a link in the introduction.
The non standard document can then document at will how to shim/feature detect/migrate from the non standard API to the standard API.

jan-ivar · 2021-11-30T23:10:09Z

Sorry for the late reply.

WebIDL is a clearer exposition format than prose, and this is an informative section.

I think the WebIDL should at least be enclosed in a <div class="example"> tag like we do in mediacapture, otherwise it gets picked up by various W3C tools that generate all sorts of tests meant for standards track APIs.

But if we link to the non standard document as Youenn suggests, then I'd prefer we move this WebIDL there.

I'll do a PR for examples.

jan-ivar · 2021-12-01T00:19:45Z

I'll do a PR for examples.

Added https://github.com/alvestrand/mediacapture-transform/pull/67

alvestrand · 2021-12-01T13:03:07Z

Sorry for the late reply.

WebIDL is a clearer exposition format than prose, and this is an informative section.

I think the WebIDL should at least be enclosed in a <div class="example"> tag like we do in mediacapture, otherwise it gets picked up by various W3C tools that generate all sorts of tests meant for standards track APIs.

Good point. I added the enclosing "example" div as a direct commit to main.
With the merger of the examples, I'm sending the note to Bernard to ask him to send out the CfC on adoption.

We'll continue changing the document afterwards.

foolip · 2023-01-04T15:23:51Z

I came across this note in https://w3c.github.io/mediacapture-transform/#track-processor-interface:

There is WG consensus that the interface should be exposed on DedicatedWorker. There is no WG consensus on whether or not the interface should not be exposed on Window.

Trying to figure out what that's about I found my way here. But is there an issue or something tracking this issue? That issue in short is that Chrome exposes MediaStreamTrackProcessor only on Window and the spec only on DedicatedWorker.

dontcallmedom · 2023-01-04T16:21:42Z

#23 is where this has been discussed

Update proposal according to WG directions

eec4cea

This prepares the document for adoption as a WG document.

guidou reviewed Nov 26, 2021

View reviewed changes

index.bs Show resolved Hide resolved

Use VideoTrackSource consistently

d73b936

jan-ivar reviewed Nov 29, 2021

View reviewed changes

jan-ivar requested changes Nov 30, 2021

View reviewed changes

youennf reviewed Nov 30, 2021

View reviewed changes

Addressed review comments ++

e88455c

Removed audio examples and constraints. Changed name to VideoTrackGenerator (personal preference). Added aspirational shim JS.

alvestrand commented Nov 30, 2021

View reviewed changes

alvestrand merged commit fe9eafd into main Nov 30, 2021

This was referenced Jan 7, 2022

API shape for creating a WHATWG Stream from a MediaStreamTrack #68

Closed

API shape for creating a MediaStreamTrack from a WHATWG Stream #70

Closed

foolip mentioned this pull request Jan 5, 2023

Update support data for Chrome Android (API's G through M) mdn/browser-compat-data#18360

Closed

jan-ivar mentioned this pull request Jul 2, 2024

Out-of-main-thread processing by default #23

Open

	There is no WG consensus that the interface should not be exposed on Window.
	There is no WG consensus on whether or not the interface should be exposed on Window.

		Note: There is consensus in the WG that this interface should be exposed on DedicatedWorker.
		There is no consensus in the WG about whether or not it should be exposed on Window.

	MediaStream model</a> in the dedicated worker environment. It has a two readonly
	MediaStream model</a> in the dedicated worker environment. It has two readonly

Update proposal according to WG directions #66

Update proposal according to WG directions #66

Uh oh!

Conversation

alvestrand commented Nov 26, 2021

Uh oh!

Uh oh!

jan-ivar Nov 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-ivar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jan-ivar Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

youennf Nov 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvestrand commented Nov 30, 2021

Uh oh!

alvestrand left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

jan-ivar Nov 29, 2021 •

edited

Loading

jan-ivar left a comment •

edited

Loading

jan-ivar Nov 30, 2021 •

edited

Loading

youennf Nov 30, 2021 •

edited

Loading