Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Current steps are inefficient and difficult to build on. Cleanup needed #229

Open
jan-ivar opened this issue May 21, 2024 · 0 comments
Open

Comments

@jan-ivar
Copy link
Member

RTCRtpScriptTransformer holds the only JS-exposed readable + writable pair, which is what the JS app calls pipeTo on:

// worker.js
onrtctransform = async ({transformer: {readable, writable, options}}) =>
    await readable.pipeThrough(new TransformStream({transform})).pipeTo(writable);

Most of the implementation can be described by specifying how the underlying source and the underlying sink MUST work.

This would help trim a lot of prose from the spec.

In contrast, the current prose describes a JS-shim-like reference implementation, which was helpful at one point. But it prescribes objects that aren't necessary in a C++ implementation. This is proving difficult to build on:

  1. The stream creation steps say to create the following objects on main thread:

    • rtcRtpSender.[[readable]]
    • rtcRtpSender.[[writable]]
    • rtcRtpReceiver.[[readable]]
    • rtcRtpReceiver.[[writable]]
    • And then queues a task to pipeTo with this.[[readable]], this.[[writable]]

    These internal objects don't need to exist, and pipeTo is a JS API, not infra. Main thread is also not where media encoders/decoders or RTP packetizers are.

  2. The new RTCRtpScriptTransform(worker, options, transfer) steps set up

    Here, rtcRtpSender/rtcRtpReceiver and rtcRtpScriptTransform have separate internal readable/writables, and the sender/receiver.transform setter steps ties them together.

    This relies on transferable streams (a special tunnel-like construct) to carry the weight of the cross-threading part of the implementation. That is AN implementation choice, but hardly the only one, nor the most likely in C++. There's no reason for a C++ implementation to work this way, or use streams internally this way, or rely on transferable streams like this, when we probably don’t want to involve the main thread with this data piping at all.

It seems simpler to express implementation requirements for the one readable and one writable this specification defines directly. Both live and die on the dedicated worker and the UA needs to dispatch data to and from the worker to service the app-created pipe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant