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

Examples pipe multiple streams into a single writable #15

Open
jan-ivar opened this issue Dec 1, 2023 · 2 comments
Open

Examples pipe multiple streams into a single writable #15

jan-ivar opened this issue Dec 1, 2023 · 2 comments
Assignees
Labels
PR Exists A PR has been submitted

Comments

@jan-ivar
Copy link
Member

jan-ivar commented Dec 1, 2023

The examples won't work as written for more than one sender:

const rtpTransport = pc.createRtpTransport();
pc.getSenders().forEach((sender) => {
  pc.createEncodedStreams().readable.
      pipeThrough(createPacketizingTransformer()).pipeTo(rtpTransport.writable);
});

...because multiple streams cannot be piped into a single writable.

It's not super-clear what a single "transport" is meant to be scoped to.

  1. Is there one for the entire peer connection?
  2. Is there one per transceiver?
  3. Is there one per sender and receiver?
  4. Is there one per transform?
  5. Can apps create as many as they wish?

Is it just a packetizer sink? — In #14 I end up with number 4.

Conceptually, is this just be some option on a transform to change expected inputs and outputs? E.g. on the sender:

  1. encodedFrame → encodedFrame (default, e.g. e2ee)
  2. encodedFrame → packet (e.g adding metadata)
  3. Frame → encodedFrame (JS encoder)
  4. Frame → packet (hold my beer)

If so, this proposal may be closer to @alvestrand's than I previously appreciated.

@aboba
Copy link
Contributor

aboba commented Dec 2, 2023

The example has multiple problems:

  • Multiple streams are piped into a single writable.
  • The examples uses the legacy accessor API instead of the standardized API.
  • The example uses the legacy accessor API incorrectly.

So there is a lot to fix.

aboba added a commit that referenced this issue Dec 4, 2023
@aboba aboba self-assigned this Dec 4, 2023
@aboba aboba added the PR Exists A PR has been submitted label Dec 4, 2023
@dontcallmedom-bot
Copy link

This issue was mentioned in WEBRTCWG-2023-12-05 (Page 62)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Exists A PR has been submitted
Projects
None yet
Development

No branches or pull requests

3 participants