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

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

Open
henbos opened this issue Jan 25, 2024 · 1 comment · May be fixed by #204
Open

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

henbos opened this issue Jan 25, 2024 · 1 comment · May be fixed by #204
Assignees

Comments

@henbos
Copy link
Collaborator

henbos commented Jan 25, 2024

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
Copy link
Contributor

sam-vi commented Apr 9, 2024

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.

sam-vi added a commit to sam-vi/webrtc-extensions that referenced this issue Apr 18, 2024
Enumerate examples of situations in which `icecandidatepairremove` is
fired, including ICE restart.

This makes it clear that the event is fired and [[CandidatePairs]] is
cleared in the event of an ICE restart.
@sam-vi sam-vi linked a pull request Apr 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants