Fix endless offer requests with the SIP bridge #7257
Merged
Conversation
This file contains hidden or 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
When the HPB is used the data channel messages need to be sent through the publisher peer. Due to a wrong condition if there was no publisher peer they tried to be sent using the data channels of the subscribers, but those messages were simply ignored. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
"enableDataChannels" prevented the messages to be sent when the data channels were disabled, but it did not prevent the data channels from being created. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The SIP bridge publisher does not have data channels, so they need to be explicitly disabled in the subscriber. Otherwise it would try to open them, which would cause an endless loop of renegotiations, as after a negotiation the data channels will still not be opened, which will trigger a negotiation again. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
3. to review
bug
regression
feature: SFU & SIP ☎️
feature: call 📹
Member
Author
|
/backport to stable24 |
fancycode
approved these changes
May 3, 2022
Member
fancycode
left a comment
fancycode
left a comment
There was a problem hiding this comment.
Indeed the SIP bridge doesn't create datachannels, so this could have been the problem.
👍 I can no longer reproduce the loop with this change.
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.
Follow up to #6862
Since 48a74da offers are requested again and again in an endless loop for the peer of the SIP bridge. If the block that requests an offer when a negotiation is needed for a subscriber is moved into the if condition that checks that the ICE connection state is not
newnorcheckingthen only the original offer is requested and that is it.My guess is that this is caused by the lack of data channels in the SIP bridge (but as mentioned it is just a guess, I do not know if the SIP bridge has data channels or not, and I can not test it either). If a publisher does not have data channels then Janus will not open data channels for its subscribers. However, when a subscriber is created the
statusdata channel was always tried to be opened. As the offer sent by Janus for the SIP bridge subscriber does not have data channels opening them will trigger anegotiationneededevent. This will cause a new offer to be requested, but that offer will still not have data channels, sonegotiationneededwill be triggered again, another offer will be requested, the offer will not have data channels... and so on.Then, why does it work when moving the block inside the if condition? In that case opening the data channel will still trigger the
negotiationneededevent. However, as it is done even before the offer was set the ICE connection state will benew, so it will not enter the if condition. That is, the subscriber is left forever in thenegotiationneededstate, and as there are no further offers received for that subscriber the endless loop does not occur.Due to all that this pull request prevents the SIP bridge subscriber from creating data channels. It also fixes another minor bug found while investigating that (trying to send data channels messages through the subscribers if there is no publisher peer).
@fancycode Could you test this pull request and confirm (or deny :-) ) my suspicions? Thanks!