- From: henbos via GitHub <sysbot+gh@w3.org>
- Date: Wed, 10 Jul 2019 08:34:07 +0000
- To: public-webrtc-logs@w3.org
> To me, this mirrors how e.g. direction works: we don't trigger on every set, only on value change. That seems most conservative to me. The alternative would be to have calling pc.restartIce() 10 times in a row trigger 10 renegotiations restarting ICE 10 times, which seems undesirable.
Hmm that is true. There is one difference though: with direction, you're happy as long as you end up with the most up-to-date direction having been negotiated. With restartIce(), are you happy that the ICE credentials are relatively new, even if they are older than the call to restartIce()? (Maybe? I don't understand some details here!)
Example:
1. createOffer() such that new ICE credentials are generated. This could be due to {iceRestart:true} or because it was the initial offer or new m= sections or whatever. Or because of restartIce().
2. restartIce(). Let's say that the network interface changed and all existing candidates are invalid, so you call this now. (Depending on what caused new ICE credentials at 1), this function may return true or it may return false!)
3. setLocalDescription() with the offer from 1); i.e. the ICE credentials are new compared to before this O/A exchange, but they are old compared to when the restartIce() at 2) was called. Perform the rest of the O/A, and at setRemoteDescription(answer) we get [[RestartIce]] = false.
In this example, ICE was in restarted, but it was not restarted as a consequence of 2), and no onnegotiationneeded event was ever fired because of restartIce().
My question is this: Did we just restart ICE with obsolete credentials or obsolete candidates, or does the relevant candidate gathering happen at 3) and we are happy anyway?
[Here](https://webrtc-review.googlesource.com/c/src/+/144941/1#message-a5cb61ee5f4a02097f00edea898e40824449c9e6) you say:
> Web sites can use the boolean return value from pc.restartIce() to learn whether it triggered anything. E.g. to force an additional ICE restart even if a restart is in progress:
>
> (async () => {
> while (!pc.restartIce()) await new Promise(r => setTimeout(r, 100));
> })();
> Or to avoid polling, replace setTimeout with
>
> pc.onsignalingstatechange = () => pc.signalingState == "stable" && r()
Firstly, if the ICE restart was not caused by restartIce() but by being the first offer, then restartIce() would still return true even though the ICE-restart-in-progress did not correspond to the restartIce() call.
Secondly, it is not intuitive that you would have to wrap your restartIce() calls in logic like this. This looks similar to me to the way that setLocalDescription() is not glare-proof today so you have to add additional logic around it for it to work (e.g. [slide](https://docs.google.com/presentation/d/1xcvf0udNeSH7s1FOY7RRqr1dEFvokZjn-MZPjwy3iXQ/edit#slide=id.g5c2f3df65b_11_66)).
It would be nice if our API "_just works_", no ifs-and-buts.
If this is a real problem, one way to solve it would be to say that [[RestartIce]] is not a boolean, but an enum with value "not-needed", "needed-when-stable" and "needed". restartIce() would cause its value to change to "needed" except if an O/A is already pending, in which case it becomes "needed-when-stable". If "needed-when-stable", whenever we reach the stable signaling state we set it to "needed". Is this needed? Does it fix the problem?
--
GitHub Notification of comment by henbos
Please view or discuss this issue at https://github.com/w3c/webrtc-pc/pull/2169#issuecomment-509966404 using your GitHub account
Received on Wednesday, 10 July 2019 08:34:09 UTC