Add API and algorithm to expose transform to workers #62
Conversation
|
Fixes #18 |
|
Suggest making one PR to add the new API and another one that takes the old API away, so that we can discuss them separately. |
I reverted the changes, PR now only adds stuff! |
There was a problem hiding this comment.
Now that "removing the old way" is removed, this is a completion of the algorithm already landed, and I approve.
I do wonder if we can get the ability to "polyfill" the old behavior by allowing the "worker" argument to the ScriptTransform constructor to be the Javascript equivalent of "this" - that would make same-process processing possible, but not simpler than worker-based processing.
| 5. FIXME: transfer |t1|.`[[readable]]` and |t2|.`[[writable]]` to the dedicated worker. | ||
| 6. FIXME: Create counterpart of |this| in dedicated worker, for instance expose transfered |t1|.`[[readable]]` and |t2|.`[[writable]]`. | ||
| 5. Let |serializedOptions| be the result of [$StructuredSerialize$](|object|). | ||
| 6. Let |serializedReadable| be the result of [$StructuredSerializeWithTransfer$](|t1|.`[[readable]]`, « |t1|.`[[readable]]` »). |
There was a problem hiding this comment.
Should the << marks be square brackets? (I think the second argument to Serialize is a transfer list)
There was a problem hiding this comment.
Not sure what style is best here.
I reused the style used in https://streams.spec.whatwg.org/#ws-transfer.
|
Note: I'd be even happier if the PR also added examples to explainer.md. |
I agree we should do that before landing #64. |
|
Thinking some more - in all cases the transform object is created on the main thread (constructor has exposed=window). If we can then extract the transformer from the transform object that's an attribute of the sender/receiver (hmm.... those are easily confused names, and the difference is importnat ....) we have a very reasonably shimmable interface that gives us the "single thread" functionality. WDYT? |
|
Note: DedicatedWorker is still a link that goes nowhere. |
|
Some examples of how the API is intended to be used in practice would be very beneficial to be able to figure out a plan to implement it. I would prefer that we keep the old API in the spec until implementors have had time to evaluate the new one in practical settings. |
| }; | ||
|
|
||
| [Global=(Worker,DedicatedWorker),Exposed=DedicatedWorker] | ||
| interface RTCRtpScriptTransformer { |
There was a problem hiding this comment.
After having tried to comprehend the overall algorithm several times, I've concluded that we should rename this object to RTCRtpScriptTransformSocket. It's not a transformer itself, it's what you plug a transform (like SframeTransform) into.
| readonly attribute WritableStream writable; | ||
| readonly attribute any options; | ||
| }; | ||
|
|
||
| [Exposed=(Window)] | ||
| interface RTCRtpScriptTransform { |
There was a problem hiding this comment.
similarly, RTCRtpScriptTransform is not a transform. I suggest we call it RTCRtpScriptTransformController.
|
New issues:
|
…form-worker-apiAdd API and algorithm to expose transform to workers SHA: 84e6ef0 Reason: push, by @alvestrand Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Introduced in w3c#62.
| }; | ||
|
|
||
| [Global=(Worker,DedicatedWorker),Exposed=DedicatedWorker] | ||
| partial interface DedicatedWorkerGlobalScope : WorkerGlobalScope { |
There was a problem hiding this comment.
With partial interfaces in Web IDL the parent interface shouldn't be specified. I've sent #71 to fix this.
| @@ -62,54 +62,6 @@ an additional API on {{RTCRtpSender}} and {{RTCRtpReceiver}} to | |||
| insert the processing into the pipeline. | |||
|
|
|||
| <pre class="idl"> | |||
| // New dictionary. | |||
| dictionary RTCInsertableStreams { | |||
There was a problem hiding this comment.
The RTCInsertableStreams dictionary was removed here, but it's still used as the return type of two methods in https://w3c.github.io/webrtc-insertable-streams/#specification.
There was a problem hiding this comment.
Oh, that is a mistake from splitting the PR, I will add it back.
Preview | Diff