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

Duplicate candidates when iceCandidatePoolSize and trickle are enabled. #469

Open
guanzo opened this issue May 17, 2019 · 2 comments
Open
Labels

Comments

@guanzo
Copy link
Contributor

guanzo commented May 17, 2019

Using [email protected]
Demo: https://jsfiddle.net/guanzo/gupdrm1a/5/

Setting the iceCandidatePoolSize option enables candidate prefetching to occur before setLocalDescription is called.

When creating an initiating peer like so:

const p = new SimplePeer({ 
    initiator: true, 
    trickle: true, 
    config: { 
        iceCandidatePoolSize: 5 
    } 
})

The sdp offer in the first signal event contains a prefetched host candidate. Problem is, the next signal event which contains a trickled candidate, is the same candidate as the prefetched one. Is this supposed to happen? It seems redundant to send the same candidate twice to the other peer.

Example:

1st signal

v=0
o=- 965062851818018781 2 IN IP4 127.0.0.1
s=-
t=0 0
a=group:BUNDLE 0
a=msid-semantic: WMS
m=application 39452 DTLS/SCTP 5000
c=IN IP4 192.168.1.92
/// Here's the prefetched candidate ///
a=candidate:1769160098 1 udp 2113937152 192.168.1.92 39452 typ host generation 0 network-cost 999
a=ice-ufrag:4xkg
a=ice-pwd:c2DfHfJZ4J4+K4tuqHVv//bq
a=ice-options:trickle
a=fingerprint:sha-256 32:AE:E1:ED:83:9E:9A:CC:FE:9B:32:D9:7F:6F:C6:61:71:13:3E:84:8C:B2:D6:64:E3:44:D3:1C:4C:B3:DD:85
a=setup:actpass
a=mid:0
a=sctpmap:5000 webrtc-datachannel 1024

2nd signal. It's the same as the prefetched candidate!?
candidate:1769160098 1 udp 2113937152 192.168.1.92 39452 typ host generation 0 ufrag 4xkg network-cost 999

3rd signal
candidate:842163049 1 udp 1677729535 73.189.204.6 39452 typ srflx raddr 192.168.1.92 rport 39452 generation 0 ufrag 4xkg network-cost 999

Is it safe to not send the 2nd signal to the signaling server, seeing how it's already been sent in the initial offer? Trickling candidates puts a lot more load on my server than non-trickle, so I'll take any opportunity to reduce the amount of messaging required.

@t-mullen
Copy link
Collaborator

t-mullen commented May 19, 2019

Yes, it's safe to drop ice candidates that have been sent another way (either by not sending the 2nd message or by filtering out the relevant lines from the offer).

However, based on the candidate type, you may want to consider removing the duplicate line from the SDP instead. reflex and relay candidates should be trickled.

https://tools.ietf.org/html/draft-ietf-mmusic-trickle-ice-02#section-6.1:

   Trickle ICE agents MAY include any set of candidates in an offer.
   This includes the possibility of generating one with no candidates,
   or one that contains all the candidates that the agent is planning on
   using in the following session.

   For optimal performance, it is RECOMMENDED that an initial offer
   contains host candidates only.  This would allow both agents to start
   gathering server reflexive, relayed and other non-host candidates
   simultaneously, and it would also enable them to begin connectivity
   checks.

@t-mullen t-mullen changed the title Duplicate candidates when iceCandidatePoolSize && trickle are enabled. Filter duplicate candidates when iceCandidatePoolSize and trickle are enabled. May 19, 2019
@fippo
Copy link

fippo commented May 26, 2019

at first I thought this was a chrome bug as the spec is pretty clear that the candidates should only be surfaced via the candidate event. As looking at the fiddle in chrome://webrtc-internals shows, the SDP from createOffer and setLocalDescription doesn't contain those candidates however.
What simple-peer does is accessing peerconnection.localDescription which has already been updated with some pre-gathered candidates (before they are emitted even; known issue) instead of signaling the offer that was generated by createOffer.

btw, its unlikely you need a iceCandidatePoolSize of 5. Note that this is not the number of candidates to pre-gather but the number of components (one for sctp, one for audio, one for video, etc; and with bundle you rarely need more than 1 even)

@t-mullen t-mullen changed the title Filter duplicate candidates when iceCandidatePoolSize and trickle are enabled. Duplicate candidates when iceCandidatePoolSize and trickle are enabled. Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants