search

w3c / webrtc-extensions

A repository for experimental additions to the WebRTC API
https://w3c.github.io/webrtc-extensions/
Other
57 stars 19 forks source link

Add steps to clear [[CandidatePairs]] in response to ICE restart #196

Open henbos opened 5 months ago

henbos commented 5 months ago

Filed as a follow-up to this comment. The reason why I filed it is I'm not sure whether or not this is already covered by existing ICE pair removal steps.

If it is already covered, then restarting ICE would trigger removing all candidate pairs, and icecandidatepairremoved fires for all of them. This might also mean that the app has an opportunity to cancel the removal, depending on if they are cancelable?

If it is not covered, then we should probably say to queue a task to remove all ICE candidate pairs, in which case we need to decide whether or not icecandidatepairremoved should fire and whether or not they are cancelable.

sam-vi commented 2 months ago

Clearing of [[CandidatePairs]] during an ICE restart is not explicitly covered by the existing steps, but it aught to be.

An ICE restart causes the ICE agent to discard existing candidate pairs, regather candidates and re-form check lists. From RFC 8445 section 9:

An ICE restart causes all previous states of the data streams, excluding the roles of the agents, to be flushed.

Or even more detailed from RFC 5245 section 9.3.1.1:

...the agent MUST flush the valid and check lists, and then recompute the check list and its states...

When candidates are regathered and candidate pairs are formed again after an ICE restart, [[CandidatePairs]] will be populated with the new pairs and icecandidatepairadd will be fired for each of them.

So [[CandidatePairs]] should certainly be cleared beforehand. But also for the sake of consistency and continuity in the application, icecandidatepairremove should be fired for every active candidate pair removed by the ICE agent during the ICE restart.

The current language in the spec for firing of icecandidatepairremove can be improved to make it clear that the event is indeed fired in the event of an ICE restart.

The spec should also make it clear that the event is not cancelable in the event of an ICE restart.

On a related note, icecandidatepairremove should not be fired when the peer connection is closed. The algorithm for closing the connection suppresses the firing of events (except where the events are connected with the closing of a media source), and there is no reason to make an exception for icecandidatepairremove.