Keep the negotiating state until the SDP is ready #558
+18
−4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
giavac
force-pushed
the
gv/missing_ice_candidates
branch
from
32998c4 to
029edc1
Compare
jpsantosbh
previously approved these changes
Jan 7, 2025
iAmmar7
previously approved these changes
Jan 9, 2025
Is it not so straightforward to generate a call from the Peer? |
iAmmar7
approved these changes
Jan 9, 2025
Yes, that was at the moment of raising the PR; I tested the call generation since that comment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When answering a call, when setting the remote description the signaling state moves to
have-remote-offer.Peer's
_negotiatingis set to true, to prevent triggering new negotiations during this phase.However, when setting the local description, while the ICE candidates haven't been gathered yet, the signaling state moves to
stable.A new
onnegotiationneededat that point would trigger a new negotiation, which is not desired, and a loop starts, continuing during the call itself.This is invisible unless debug logging is activated, but has the side effect of causing some ICE candidates to be excluded from the SDP before it's transmitted. This per se doesn't cause the call to fail, which made the bug harder to track.
With the changes in this PR though, the "negotiating" state is kept in the Peer until the final SDP will be ready.
In my tests this proved to fix the issue with the infinite loop and the missing candidates.
Admittedly, I haven't tested the case of the Peer generating a call, which is a scenario that shares some of the change code, but I've only tested the case of the Peer receiving a call.