search

pion / ice

A Go implementation of ICE
https://pion.ly/
MIT License
412 stars 155 forks source link

Only switch to failed after all candidates are exhausted? #271

Open jech opened 3 years ago

jech commented 3 years ago

As pointed out in https://github.com/pion/webrtc/issues/1212#issuecomment-686932358 , Pion's handling of the failed state is somewhat naive: Pion will switch to failed if connectivity cannot be established even if it hasn't received an end-of-candidates indication.

There's a tradeoff here: the current behaviour works well enough in practice, and it gracefully handles the case when the peer never sends an end-of-candidates indication. On the other hand, it means that if the peer is performing expensive gathering (e.g. because a TURN server is overloaded), we might spuriously switch into failed.

scorpionknifes commented 3 years ago

@Sean-Der just clarifying, it should only fail if end-of-candidate is detected? if so how do we handle it gracefully if the peer never sends an end-of-candidate indication?

Cause it would break this test (potentially implementation). https://github.com/pion/ice/blob/62d1c40d60d65116202d743cb47bda9b1a78cd9f/agent_test.go#L1307.

Sean-Der commented 3 years ago

Hey @scorpionknifes

Sorry for the confusion, but I don't think we can move forward on this. Behavior between browsers is inconsistent. I am afraid to implement something and then have browsers diverge. I am sorry I didn't do more research before recommending this issue :(

Sean-Der commented 3 years ago

We should love your PR though, and maybe engage with webrtc-pc on what is the proper behavior?

jech commented 3 years ago

Small timeout when eoc is received, larger timeout if it isn't?

scorpionknifes commented 3 years ago

@jech cool idea. idk if this is a feature people want?

jech commented 3 years ago

There's a tradeoff here: on the one hand, we don't want to give up too early, on the other hand, we want to switch to failed as early as possible so as to give the user timely feedback and possibly trigger an ICE restart. Question: would using the end-of-candidates indication allow us to fail faster without breaking connectivity in cases where it currently works? I'm hoping somebody will do the necessary experiments, and I sure hope that won't be me ;-)

jech commented 3 years ago

There is some related discussion at https://tools.ietf.org/html/draft-ietf-ice-trickle-21#section-14.