search

w3c / webrtc-rtptransport

Repository for the RTPTransport specification of the WebRTC Working Group
Other
18 stars 6 forks source link

Should `RtpSendStream` and `RtpReceiveStream` be removed? #47

Open Philipel-WebRTC opened 3 months ago

Philipel-WebRTC commented 3 months ago

AFAICT RtpSendStream and RtpReceiveStream provides no functionality other than sendRtp and readReceivedRtp/receiveRtp respectively, which could be moved to the RtpTransport instead.

One benefit of this is when there are a large amount of receivers, in that case readReceivedRtp would only have to be called once instead of once for every receiver.

tonyherre commented 3 months ago

They do provide a nice handle to return from RTCRtpSender.replaceSendStreams() / RTCRtpReceiver.replaceReceiveStreams() which naturally associates calls to sendRtp() with the right sender etc. I like that part.

Performance-wise, I wouldn't expect much of a difference between N calls to synchronous methods vs one call that returns the union of all results. JS ergonomics-wise, I'd think being able to split separate receivers into their own instances of a MyReceiver class would be a nice architectural option? Potentially an app could even run them concurrently from separate workers?

Oh actually, talking of workers, as it stands the RtpXStreams are the Transferable things, and RtpTransport stays on the main thread with the PeerConnection itself - and I think it might always have to stay there to synchronously interact with the PC. We'd need to find a solution for transferring some other intermediate object.

tonyherre commented 3 months ago

If we have numbers on the overhead of the additional calls, that could definitely motivate a combined version of readReceivedRtp on some central object.

Philipel-WebRTC commented 3 months ago

I don't have any numbers, but here is my reasoning:

A client receiving say 50 video streams would probably receive them at a low resolution and bitrate. In that case there is a good chance every frame is packetized into a single packet, so even with the batched RTCRtpReceiver.readReceivedRtp you would still end up calling readReceivedRtp for basically every RTP packet.

If they can help solving threading issues then we should probably keep them.

tonyherre commented 3 months ago

you would still end up calling readReceivedRtp for basically every RTP packet.

Ah, true. That would still satisfy #20 at least, as we could have a single JS task invocation with the scheduling overhead etc (maybe triggered on vsync, via requestAnimationFrame()?) and run through all 50 synchronously.