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

Implement ICE restarts #579

Open
t-mullen opened this issue Dec 19, 2019 · 13 comments
Open

Implement ICE restarts #579

t-mullen opened this issue Dec 19, 2019 · 13 comments
Assignees

Comments

@t-mullen
Copy link
Collaborator

When network conditions change or the current route is congested, it's possible to restart ICE and gather/select a new candidate pair. Apparently this happens very frequently with cellular networks or with certain types of NATs.

Restart is done by calling createOffer with the iceRestart option when iceConnectionState == 'failed'.

As an optimization, ICE restart can be done preemptively when iceConnectionState == 'disconnected' (before 'failed') and it won't immediately bounce back to connected.

@holtwick
Copy link
Contributor

holtwick commented Apr 6, 2020

@feross
Copy link
Owner

feross commented May 15, 2020

I would love to have this feature for a new project I'm working on. @t-mullen Any idea how you'd like the API design for this to work?

@t-mullen
Copy link
Collaborator Author

t-mullen commented May 15, 2020

simple-peer should detect the ice connection state 'failed' and do this transparently (no public API, except maybe a max number of restart attempts).

Restart is just renegotiation with iceRestart: true in createOffer (or by callingrestartIce() if that's implemented in the browser).

https://developer.mozilla.org/en-US/docs/Web/API/RTCOfferOptions/iceRestart
https://medium.com/the-making-of-whereby/ice-restarts-5d759caceda6

@feross
Copy link
Owner

feross commented May 15, 2020

A few questions:

  • Can either side call restartIce() when they detect the ice connection state is failed? Or do we need to implement the Perfect Negotiation Example to avoid glare?

  • Once we call restartIce(), is the rest of the negotiation logic the same as a normal offer/answer? We currently don't listen for negotiationneeded and just call our own _needsNegotiation() method. Do we just call that after restartIce()?

  • When trickle is disabled, should we disable ice restarts since the user is expecting exactly one signal event from simple-peer?

@t-mullen
Copy link
Collaborator Author

t-mullen commented May 15, 2020

  • We currently avoid glare by sending a { renegotiate: true } message from the non-initiator to the initiator. We could expand this message to be { renegotiate: { iceRestart: true/false }. Offers from the non-initiator were not well-supported when I implemented renegotiation.

  • I don't think we need to listen for negotiationeeded. This wasn't required for changing media, but it's worth a test.

  • That makes sense to me. Developers disabling trickle usually have very simple signalling setups (ie copy-paste) and we don't want to break that.

@samuelmaddock
Copy link
Contributor

We currently avoid glare by sending a { renegotiate: true } message from the non-initiator to the initiator. We could expand this message to be { renegotiate: { iceRestart: true/false }.

In my application, I configure peers in a star topology where clients connect to a central host. As an optimization for this approach, clients disconnect from the signal server after establishing a connection to the host.

When the client switches to a failed ICE state, they'd need to reconnect to the signal server and send the initial ICE restart offer. I'd prefer to see a flexible approach for whether initiator or non-initiator starts an ICE restart.


Some changes I'd like to propose:

  • When a connection's ICE state changes to failed, create a timer which closes the connection after a configurable timeout. Currently failed connections are immediately destroyed.

  • Implement a SimplePeer.restartIce() method which will call RTCPeerConnection.restartIce() then SimplePeer._needsNegotiation() to create the offer.

With these changes, it would be up to the simple-peer consumer to decide whether to handle ICE restarts and if it should start with the initiator or non-initiator.

@t-mullen
Copy link
Collaborator Author

t-mullen commented Aug 9, 2020

The non-initiator isn't actually capable of initiating renegotiation or ICE restart, it's only capable of requesting it from the initiator. At least with the way we currently handle glare. Maybe if we add rollbacks this can be a little simpler.

If you need to reconnect both peers to the signalling server, you should queue signals until that happens instead of delaying the ICE restart - otherwise you'll slow down gathering (and reconnection).

I think the best option is where only the initiator watches for iceConnectionState === 'failed' and begins a renegotiation/iceRestart.

@soup-in-boots
Copy link

Where does this feature stand? Do you need someone to work on it? I have a very strong interest in the feature, and would be able to commit working hours to it.

@feross
Copy link
Owner

feross commented Oct 30, 2020

@fauxsoup Are you still interested in committing working hours to this? I'd love to get this feature in.

@soup-in-boots
Copy link

Absolutely. Just want to know where to start. I saw a pull request with an example implementation, but it was out of date. Should I start there, or has other development commenced on this elsewhere?

@feross
Copy link
Owner

feross commented Nov 10, 2020

@fauxsoup Feel free to start with the example implementation that you found. We'll take a look at your PR.

@t-mullen @nazar-pc – do you have any other pointers for @fauxsoup?

@draeder
Copy link

draeder commented Feb 8, 2022

I am facing this very issue among multiple libraries (torrent-discovery, bittorrent-tracker/client, @geut/discovery-swarm-webrtc) on ^9.11.0. If there is a workaround, I would love to know about it. This greatly affects the reliability of simple-peer to the point that I am going to abandon using it altogether, which will be unfortunate.

@4www
Copy link

4www commented Feb 21, 2022

Hello! Same here as @draeder ; and trying to figure out what's the expected (webrtc) way to restart a connection after the browser error logs WebRTC: ICE failed, add a TURN server and see about:webrtc for more details.

From my investigation it seems that there might be browser specific behaviors concerning the ICE restart, and the iceConnecitonState.

chromium as initiator seems to be the only way to make a peer connection that can "await and restart".

chromium seems to be able to have a RTCPeerConnection stay in iceConnectionState === "checking", after an offer was sent by the initiator, added as remote descriptor by the non-initiator, and and answer has been generated (but not signaled).

This iceConnectionState event pops on both peers, as soon as the offer is added on the non-initiator (chromium only).

This behavior is not the same on firefox; where iceConnectionState goes to "failed", on the non-initiator, a few seconds after the offer has been added as remote descriptor, and answer to local descriptor (but not yet signaled).

chromium is able to stay in the checking state, without failing, for a long duration of time (does not seem to ever fail); this allows "snail signaling", like the copy-paste-dance in my case.

Not sure if this can help, be maybe an insight in debugging.

Trying myself to make web-components for webrtc https://gitlab.com/sctlib/rtc, used for these tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants