Re: [whatwg/streams] Explainer for proposed stream changes related to VideoFrame processing (PR #1193)

@MattiasBuelens requested changes on this pull request.

Thanks for kicking this off! Looks pretty good already. 👍

> +      if (!controller.count)
+        controller.count = 0;

It might be a bit confusing to use "expando properties" in these examples. (What if we decide to add `TransformStreamDefaultController.prototype.count` in the future? 😛)

For these examples, it's probably better to use a regular variable:
```javascript
let count = 0;
const frameCountTransform = new TransformStream({
  transform: async (videoFrame, controller) => { /* ... */ }
});
```

> +## Example
+
+Below is an example of JavaScript that shows how this can be used.
+The example creates a processing pipe starting with a camera stream and applying two transforms, one for doing a processing for every 30 frame, and one for doing background blur.
+
+```javascript
+// JS transform
+const frameCountTransform = new TransformStream({
+  transform: async (videoFrame, controller) => {
+    try {
+      // videoFrame is under the responsibility of the script and must be closed when no longer needed
+      controller.enqueue(videoFrame);
+      // At this point, videoFrame has been transfered within controller.enqueue call. frameCountTransform cannot mutate it.
+      if (!controller.count)
+        controller.count = 0;
+      if (!(i++controller.count % 30) && frameCountTransform.onEach30Frame)

`i++controller.count` is a `SyntaxError`. 😬

```suggestion
      if (!(++controller.count % 30) && frameCountTransform.onEach30Frame)
```

> +*   Add a new 'transfer' type to `ReadableStream`, `WritableStream` and `TransformStream`.
+    For streams that do not have the 'transfer' type, nothing changes.

A `TransformStream` may transform one type of chunks to a different type of chunks. For example, a `TextDecoderStream` accepts `ByteSource`s on its writable end and produces `string`s on its readable end. We should try to support such use cases for ownership-transferring streams too. Therefore, we shouldn't "just" have `type: "transfer"` for `TransformStream`.

We have already reserved `readableType` and `writableType` for such use cases, see the [`TransformStream` constructor](https://streams.spec.whatwg.org/#ts-constructor). I suggest we use that:
* If `writableType` is `"transfer"`, then the writable end will accept transferable objects. Ownership is transferred into the stream when calling `writer.write(chunk)`, and transferred out when the stream calls `transformer.transform(chunk, controller)`.
* If `readableType` is `"transfer"`, then the readable end will produce transferable objects. Ownership is transferred into the stream when calling `controller.enqueue(chunk)`, and transferred out when resolving a `reader.reader()` call.

For convenience, we *could* still add `transformer.type`, as a shorthand for setting both `readableType` and `writableType` to the same value. That would make the common case of transforming a `VideoFrame` to another `VideoFrame` easier. Or maybe we want to leave that as a future improvement.

> +*   Performing realtime transformations of `VideoFrame` objects, for instance taking a camera `MediaStreamTrack` and applying
+    a background blur effect as a `TransformStream` on each `VideoFrame` of the `MediaStreamTrack`.
+
+## End-user benefit
+
+*   `VideoFrame` needs specific management and be closed as quickly as possible, without relying on garbage collection.
+    This is important to not create hangs/stutters in the processing pipeline. By building support for safe patterns
+    directly in streams, this will allow web developers to optimize `VideoFrame` management, and allow user experience
+    to be more consistent accross devices.
+
+## Principles
+
+The envisioned changes to the streams specification could look like the following:
+*   Add a new 'transfer' type to `ReadableStream`, `WritableStream` and `TransformStream`.
+    For streams that do not have the 'transfer' type, nothing changes.
+*   Streams of the 'transfer' type can only manipulate Transferable or Serializable objects.

I believe we have to *require* all objects to be `Serializable`. Otherwise, we cannot call `StructuredSerialize`/`StructuredDeserialize` inside `tee()` to make a "real" clone. (A transferred version of an object is not a clone, since you can no longer use the original.)

> +    or `WritableStreamDefaultWriter`, create a copy of the object using StructuredSerializeWithTransfer/StructuredDeserializeWithTransfer.
+    Proceed with the regular stream algorithm by using the copy of the object instead of the object itself.

```suggestion
    or `WritableStreamDefaultWriter`, create a transferred version of the object using StructuredSerializeWithTransfer/StructuredDeserializeWithTransfer.
    Proceed with the regular stream algorithm by using the transferred object instead of the object itself.
```

> +    directly in streams, this will allow web developers to optimize `VideoFrame` management, and allow user experience
+    to be more consistent accross devices.
+
+## Principles
+
+The envisioned changes to the streams specification could look like the following:
+*   Add a new 'transfer' type to `ReadableStream`, `WritableStream` and `TransformStream`.
+    For streams that do not have the 'transfer' type, nothing changes.
+*   Streams of the 'transfer' type can only manipulate Transferable or Serializable objects.
+    If a non Transferable or Serializable object is enqueued or written, the object is ignored as if it was never enqueued/written.
+*   If a Transferable object is enqueueud/written in a 'transfer' type `ReadableStreamDefaultController`, `TransformStreamDefaultController`
+    or `WritableStreamDefaultWriter`, create a copy of the object using StructuredSerializeWithTransfer/StructuredDeserializeWithTransfer.
+    Proceed with the regular stream algorithm by using the copy of the object instead of the object itself.
+*   If a Serializable object is enqueueud/written in a 'transfer' type `ReadableStreamDefaultController`, `TransformStreamDefaultController`
+    or `WritableStreamDefaultWriter`, create a copy of the object using StructuredSerialize/StructuredDeserialize.
+    Proceed with the regular stream algorithm by using the copy of the object instead of the object itself.

We should also close the original object if it supports `Closeable`. (If it doesn't support it, we just let it be garbage collected as normal.)

> +## Principles
+
+The envisioned changes to the streams specification could look like the following:
+*   Add a new 'transfer' type to `ReadableStream`, `WritableStream` and `TransformStream`.
+    For streams that do not have the 'transfer' type, nothing changes.
+*   Streams of the 'transfer' type can only manipulate Transferable or Serializable objects.
+    If a non Transferable or Serializable object is enqueued or written, the object is ignored as if it was never enqueued/written.
+*   If a Transferable object is enqueueud/written in a 'transfer' type `ReadableStreamDefaultController`, `TransformStreamDefaultController`
+    or `WritableStreamDefaultWriter`, create a copy of the object using StructuredSerializeWithTransfer/StructuredDeserializeWithTransfer.
+    Proceed with the regular stream algorithm by using the copy of the object instead of the object itself.
+*   If a Serializable object is enqueueud/written in a 'transfer' type `ReadableStreamDefaultController`, `TransformStreamDefaultController`
+    or `WritableStreamDefaultWriter`, create a copy of the object using StructuredSerialize/StructuredDeserialize.
+    Proceed with the regular stream algorithm by using the copy of the object instead of the object itself.
+*   Introduce a WhatWG streams 'close-able' concept. An object that is 'close-able' defines closing steps.
+    For instance `VideoFrame` closing steps could be defined using https://www.w3.org/TR/webcodecs/#close-videoframe.

+    `ArrayBuffer` closing steps could be defined using https://tc39.es/ecma262/#sec-detacharraybuffer.


`DetachArrayBuffer` doesn't deallocate the underlying Data Block of the `ArrayBuffer`, since it's also used to *transfer* an `ArrayBuffer` (see [step 5.4 of `StructuredSerializeWithTransfer`](https://html.spec.whatwg.org/multipage/structured-data.html#structuredserializewithtransfer)). However, in our case we know that we won't transfer it to a new `ArrayBuffer`, and thus the Data Block *could* be deallocated immediately.

We should make a note that implementations are *allowed* to deallocate the Data Block immediately after closing an `ArrayBuffer` (or they could just wait for garbage collection as normal).

> +    a background blur effect as a `TransformStream` on each `VideoFrame` of the `MediaStreamTrack`.
+
+## End-user benefit
+
+*   `VideoFrame` needs specific management and be closed as quickly as possible, without relying on garbage collection.
+    This is important to not create hangs/stutters in the processing pipeline. By building support for safe patterns
+    directly in streams, this will allow web developers to optimize `VideoFrame` management, and allow user experience
+    to be more consistent accross devices.
+
+## Principles
+
+The envisioned changes to the streams specification could look like the following:
+*   Add a new 'transfer' type to `ReadableStream`, `WritableStream` and `TransformStream`.
+    For streams that do not have the 'transfer' type, nothing changes.
+*   Streams of the 'transfer' type can only manipulate Transferable or Serializable objects.
+    If a non Transferable or Serializable object is enqueued or written, the object is ignored as if it was never enqueued/written.

Hmm, not sure if I like this "silent error". For readable byte streams, we throw a `TypeError` in `controller.enqueue()` if the given chunk is not an `ArrayBufferView`. Ideally, we'd do something similar for ownership-transferring streams.

> +const blurredStream = new MediaStream([getTrackFromReadableStream(blurredVideoFrameStream)]);
+// Make use of blurredStream.
+...
+```
+
+## Goals
+
+*   Permit `ReadableStream`, `WritableStream` and `TransformStream` objects to take ownership of objects they manipulate.
+*   Permit to build a safe and optimal video pipeline using `ReadableStream`, `WritableStream` and `TransformStream` objects that manipulate `VideoFrame` objects.
+*   Permit both native and JavaScript-based streams of type 'transfer'.
+*   Permit to optimize streams pipelines of transferable objects like `ArrayBuffer`, `RTCEncodedVideoFrame` or `RTCEncodedAudioFrame`.
+*   Permit to tee a `ReadableStream` of `VideoFrame` objects without tight coupling between the teed branches.
+
+## Non-goals
+
+*   Add support for supporting transfer and closing of arbitrary JavaScript objects.

```suggestion
*   Add support for transferring and closing of arbitrary JavaScript objects.
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1193#pullrequestreview-821277372

Received on Thursday, 2 December 2021 10:59:01 UTC