[stable15] Fix removal of stale peers when a screen is shared #1746
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
When a screen is shared a new offer is sent, just like when a video (or audio only) connection starts; any offer coming from the same peer has the same id, even if the type of the offer is different and separate Peer objects are created for each offer. Due to this now a peer is stale only if both the id and type of the new offer matches an existing peer; otherwise when the screen offer was received the video peer was seen as stale and removed. Note, however, that even if the id and type of the new offer matches an existing peer its participant should not be marked as disconnected, as that removes all its Peer objects. If that happens then the connection becames unstable with the peers sending offers back and forth due to the difference in how they are started (immediately or delayed for video depending on the ids of both peer, immediately for screens but only for the peer that shares), the removal of Peers when a new offer is received, and the creation of new Peers (and, with them, new offers) when the signaling returns the user list. Moreover, if a stale peer is found it is going to be replaced by a fresh peer, so there it is not really needed to perform the whole disconnection process. Therefore, now the stale peer object is simply ended instead (and in the case of video peers their video and speaker are explicitly removed; this should probably be done automatically when handling the peer end, like done for screen peers, but for the time being an explicit call is used just like in other areas of the file). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When a stale peer reconnects its Peer object, and thus its video view, is removed and created again. The video view always set the screen icon as not available and it was later changed to available if a screen offer was received, so if the peer sent the screen offer before the video offer the screen icon was never set as available. Now whether the screen is shared or not is taken into account when setting the initial state of the video view. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This ensures that receiving a screen offer will not abort a delayed reconnection of the video. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
danxuliu
added
3. to review
bug
regression
feature: WebRTC 🚡
This was referenced Apr 24, 2019
nickvergessen
approved these changes
Apr 29, 2019
nickvergessen
deleted the
backport/1569/stable15-fix-removal-of-stale-peers-when-a-screen-is-shared
branch
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.
Replaces #1739 (as the branch name matched a protected pattern and thus it could not be force pushed to fix an eslint issue).
Backport of #1569
The automatic backport had failed because #1554 was never merged; unfortunately this slipped and currently screen sharing is broken (or, at least, flaky) in stable15.