givo / lib-kurento

A typescript library for simplifying the use of Kurento in Node.js
24 stars 5 forks source link

ICE candidate race #10

Open khusmann opened 4 years ago

khusmann commented 4 years ago

Hey, thanks for the awesome lib!

I'm recreating the one2many kurento demo tutorial, and running into a simple problem: When I receive a "present" message from a client, I immediately set up a libKurento.WebRtcEndpointWrapper with the Sdp, but before it is constructed I also receive "onIceCandidate" from the client, so I am unable to call webRtcEndpoint.addClientIceCandidate on the endpoint to put it in the queue.

There's two ways I can think of fixing this without modifying lib-kurento: 1) Have the client only start sending onIceCandidates AFTER it receives the sdp answer. 2) queue the messages myself and pass them to the WebRtcEndpointWrapper when it is ready (not utilizing its internal queue)

That said, if I could create a libKurento.WebRtcEndpointWrapper WITHOUT specifying an sdp (so it could have it around to start queuing ice candidates whenever they are received), and then when I receive an sdp from the client I can call WebRtcEndpointWrapper.init(sdpOffer), that would allow me to utilize the internal queue of WebRtcEndpointWrapper.

So here's what I'm proposing:

Of course I'm new to all this so I might be way off the mark -- please let me know if I'm thinking about this correctly!

neilyoung commented 4 years ago

Have the client only start sending onIceCandidates AFTER it receives the sdp answer.

I cannot speak for this particular scenario, but it sounds familiar: Most WebRTC stack don't like to get ICE candidates before an ANSWER is available. You should queue the candidates in your code and fire the candidates, once the ANSWER is ready/processed.

khusmann commented 4 years ago

You should queue the candidates in your code

WebRtcEndpointWrapper has a built-in queue that I want to use for this, but unless we create a constructor that doesn't require an sdp then I think I need to duplicate this effort. I'm proposing a slightly different interface for WebRtcEndpointWrapper:

neilyoung commented 4 years ago

OK, then please ignore 👍

givo commented 4 years ago

Hi!

Thanks for using the library :)

I have two suggestions:

  1. You can create a new instance of WebRtcEndpointWrapper with null sdpOffer and set the "private" member - _sdpOffer just before you call WebRtcEndpointWrapper.init() .
  2. If you have control on the client side, transmit the client ice candidate only after you send the SDP offer

Does it help?

khusmann commented 4 years ago

Awesome, thanks for the reply.

You can create a new instance of WebRtcEndpointWrapper with null sdpOffer and set the "private" member - _sdpOffer just before you call WebRtcEndpointWrapper.init() .

Ah, this makes sense but seems a bit hacky to violate encapsulation.

If you have control on the client side, transmit the client ice candidate only after you send the SDP offer

This is the most appealing, but is this how WebRTC is usually negotiated? According to this it looks like SDP offers and ICE candidates are usually sent pretty simultaneously? As a newbie I'm still trying to understand how things are "usually" done... (is this just an opinionated decision your library makes?)