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

Should RtpSendStream/RtpReceiveStream be transferable? #36

Open
youennf opened this issue May 14, 2024 · 7 comments
Open

Should RtpSendStream/RtpReceiveStream be transferable? #36

youennf opened this issue May 14, 2024 · 7 comments

Comments

@youennf
Copy link

youennf commented May 14, 2024

explainer-use-case-1.md sets the two objects as transferable.
The main benefit is to allow to transfer these objects to workers so that packet processing is done there.
Another approach is to consider how it is being done in webrtc encoded transform or for encoded source.

It would be good to compare the pros and cons of both approaches.

@youennf
Copy link
Author

youennf commented May 14, 2024

Here is an example of how would look like this API without transferable.

// Setup API
partial dictionary RtpSendStreamInit {
    required Worker worker;
}
partial dictionary RtpReceiveStreamInit {
    required Worker worker;
}
partial interface RtpSender {
  Promise<undefined> replaceSendStreams(Worker worker);
}
partial interface RtpReceiver {
  Promise<undefined> replaceReceiveStreams(Worker worker);
}
partial interface RtpTransport {
  Promise<undefined> addRtpSendStream(RtpSendStreamInit);
  Promise<undefined> addRtpReceiveStream(RtpReceiveStreamInit);
}

// Packet API
[Exposed=Worker]
interface RtpSendStream {
    ...
}
[Exposed=Worker]
interface RtpReceiveStream {
    ...
}

// Worker context events
[Exposed=Worker]
interface RTCRtpReceiveStreamEvent: Event {
    readonly attribute RtpReceiveStream stream;
}
[Exposed=Worker]
interface RTCRtpSendStreamEvent: Event {
    readonly attribute RtpSendStream stream;
}
partial interface DedicatedWorkerGlobalScope {
    attribute onrtcrtpsendstream;
    attribute onrtcrtpreceivestream;
}

@youennf
Copy link
Author

youennf commented May 14, 2024

Tentative evaluation of pros and cons:

Pros for transferable approach

  • Potential ability to move objects from one worker to another one, flexibility

Cons for transferable approach

  • API is not worker first but worker second
  • Transferring complex objects is difficult
    • Need to define how transfer works (what happens to events that are about to be fired for instance)
    • Need to define how a transferred object behaves
    • Need to define when object may or may not be transferred

Pros for non transferable approach

  • API is worker first
  • Simpler specification and implementation

Cons for non transferable approach

  • Less flexible, requires the web application to make processing choices at setup time

@tonyherre
Copy link
Contributor

I would add a Pro for transferability / Con for non-transferable that this uses a standard JS pattern with which developers are already familiar, rather than needing to learn the semantics of another api-specific "one off transfer".
Applying the Priority of Constituencies, an easier to use web developer API carries more weight than an easier to spec/implement one which puts a greater burden on the web dev.

@youennf
Copy link
Author

youennf commented May 16, 2024

this uses a standard JS pattern with which developers are already familiar, rather than needing to learn the semantics of another api-specific "one off transfer"

Yes and no.
First, both models are in use. From past experience, web developers had no issue understanding the one off transfer model.

Second, the semantics of the "one off transfer" are simpler to understand. With the transferability approach, web developers may need to understand:

  • where these objects can or cannot be transferred (service workers? iframes? out of process dedicated workers?)
  • when these objects can or cannot be transferred (e.g. can it be transferred if an event handler was set?)
  • what happens when object gets transferred while being in use (will events or packets be forgotten/buffered/fired during transfer)
  • what the state of the 'neutered' object is
    No need to understand any of those things with the "one off transfer" approach, API is self explanatory.

In that same vein, if we want to extend these objects in the future, we would need to design these API extensions with transferability as a constraint.

@alvestrand
Copy link

alvestrand commented May 16, 2024

Pros for the transferable approach:

  • Allows API users to decide on allocation of tasks to threads

Cons for the one-off approach:

  • Doesn't permit applications that make their own decisions on where to allocate tasks to threads

We have already tackled many of the transferable-approach issues when dealing with transfer of tracks.
There's no such thing as a "self-explanatory" API. There is just more or less explanation needed.

@youennf
Copy link
Author

youennf commented May 16, 2024

  • Doesn't permit applications that make their own decisions on where to allocate tasks to threads

How so? in both approaches, the web application is able to decide whether it wants to send/receive packets in a worker/thread A or in a worker/thread B.

@Philipel-WebRTC
Copy link
Contributor

Philipel-WebRTC commented Jun 13, 2024

One thing that came up when thinking about how to implement a JS level pacer.

The app should have control over the order in which packets are put onto the network, and having RtpSendStreams on separate workers would make this difficult.

Lets say for example if you want to send a packet from stream A and stream B within a short interval. Now the pacer can post to woker A and worker B so that RtpSendStream.sendRtp can be called from the respective worker, but now there is no guarantee that worker A will even call sendRtp before packet B has been sent.

AFAICT the solution to this would be to have all RtpSendStreams on the same worker, so the usefulness of making them transferable might be limited.

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

4 participants