Add TURN support #30
Add TURN support #30
Conversation
|
Needs testing together with palavatv/palava-web#59 and palavatv/signaltower#30. |
| username: @room.options.turn.username | ||
| credential: @room.options.turn.password | ||
| urls: [@room.options.turn] | ||
| username: @turnCredentials.user |
There was a problem hiding this comment.
+1 for completely removing static configuration!
coffee/remote_peer.coffee
Outdated
| @@ -29,6 +29,7 @@ class palava.RemotePeer extends palava.Peer | |||
| @setupPeerConnection(offers) | |||
| @setupDistributor() | |||
|
|
|||
| @offers = offers | |||
There was a problem hiding this comment.
If #20 yields that we have to keep this around, we should really rename this flag
coffee/remote_peer.coffee
Outdated
| # | ||
| # @return [Object] true if turn was tried by using the tryTurn function | ||
| # | ||
| tryTurn: (credentials) => |
There was a problem hiding this comment.
+1 for the idea to only use TURN when the peer connection has failed
There was a problem hiding this comment.
I actually don't see the value of this in the way it's currently implemented. The browser chooses the ice candidate pair based on the "cost" of the connection with TURN being lowest priority. The only drawback are the minimal resources used for reserving a TURN connection which does not get used (if the browser even gets to that).
Would be another scenario if the credentials were requested/created when the connection fails. But with the credentials being present from the start I don't see the point.
There was a problem hiding this comment.
But it wasn't always like that right? I can at least remember times when a TURN server was given, it was always turn being used. Is this choosing the lowest cost ice candidate pair documented somewhere or is that your experience and did you try that with all major browsers?
There was a problem hiding this comment.
The process is described in the ICE RFC section about prioritizing candiates,. You can see the priorities in the SDP of the candidates.
There was a problem hiding this comment.
Ok, I changed it back to directly adding turn to the connection
coffee/session.coffee
Outdated
| @@ -29,6 +29,7 @@ class palava.Session extends @EventEmitter | |||
|
|
|||
| @createChannel() | |||
| @createRoom() | |||
| @createTurnAPIClient | |||
There was a problem hiding this comment.
No, I'll remove it (the other case as well)
coffee/session.coffee
Outdated
| @@ -177,6 +184,9 @@ class palava.Session extends @EventEmitter | |||
| @room.on 'signaling_error', (t, e) => @emit 'signaling_error', t, e | |||
| true | |||
|
|
|||
| createTurnAPIClient: => | |||
| @@ -52,11 +53,11 @@ class palava.RemotePeer extends palava.Peer | |||
| options = [] | |||
| if @room.options.stun | |||
| options.push({urls: [@room.options.stun]}) | |||
| if @room.options.turn | |||
| if @room.options.turn && @turnCredentials | |||
| options.push | |||
There was a problem hiding this comment.
Only being able to add one TURN configuration is limiting. A common scenario is adding one TURN server using UDP and one server using TCP (with and/or without TLS). Physically this can be the same TURN server, just a different configuration.
There was a problem hiding this comment.
Thanks, I changed that in the latest commit
The client receives a turn login on room join if the server supports TURN. It first tries a connection via stun. If that fails, it tries again with turn using the received login.
farao
force-pushed
the
turn
branch
2 times, most recently
from
3e0fae6 to
37c3191
Compare
The client receives a turn login on room join if the server supports TURN.
It first tries a connection via stun. If that fails, it tries again with
turn using the received login.