[stable15] Fix sharing own screen when other peer is sharing the screen #1740
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 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 <[email protected]>
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 <[email protected]>
This ensures that receiving a screen offer will not abort a delayed reconnection of the video. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The explanation below only applies when no MCU is used; the issue happened both with and without MCU, but the code paths were not modified when a MCU is used, so the bug is still present in that case. When the local screen is shared a new screen peer is created for each of the remote peers. However, when a screen offer is received a new screen peer is created too; as screen sharing is unidirectional there are separate Peer objects for the screen sent to and the screen received from a remote peer. As both Peer objects are for the same remote peer they have the same session ID; the only difference between them is in the "sharemyscreen" property, which is "true" for the screen sent to the remote peer. Due to all this, when sharing the local screen it is not enough to check if there is already a screen Peer object with the remote peer id to prevent creating another one, as that screen Peer object can represent a screen received from the remote peer; it is necessary to ensure that the Peer object represents the screen sent to the remote peer using the "sharemyscreen" property. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When the MCU is used and the local screen is shared the screen is not directly sent to the remote peers, but to the MCU, which then relays it to the desired remote peers. Thus, when the local screen should be shared to a remote peer the MCU has to be notified to send the screen offer to the remote peer. Then, when the remote peer receives the screen offer, it creates a new Peer object for the received screen. The MCU was notified to send the screen offer only if there was no screen Peer object for the remote peer. Due to this, when a remote peer shared the screen with the local peer and the local peer then shared the screen with that remote peer no screen offer was sent, because there was already a screen Peer object for that remote peer. To fix this, now the screen offer is sent whenever the local screen is shared with a remote peer. Note that this should not cause duplicate offers, as the offer will be sent only when the screen sharing starts or when a new user joins the call. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
As screen sharing is unidirectional there are separate Peer objects for the screen sent to and the screen received from a remote Peer. When the MCU is used there is a single Peer object for the screen sent to (as it is sent to the MCU), but when there is no MCU there is one Peer object for each remote peer that the screen is sent to. In that later case, the Peer objects for the screen sent to and the screen received from a remote peer have both the same session ID; the only difference between them is in the "sharemyscreen" property, which is "true" for the screen sent to the remote peer. Due to this, when looking for stale peers the Peer object for the local screen needs to be ignored; otherwise when the screen offer from the remote peer is received the Peer object for the local screen would be seen as stale and removed due to having the same ID and type. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
SimpleWebRTC expects that screen offers from remote peers include the "broadcaster" property set to the ID of the peer; this is used to set the "sharemyscreen" property, which is used to differentiate Peer objects for sent and received screens. Screen offers received from the MCU do not include the "broadcaster" property, so SimpleWebRTC mark the Peer objects created from those offers as local screens. This causes, for example, that the remote screen peers are ended when the local screen is stopped. Due to this, now the "broadcaster" property is added to the screen offers received from the MCU before they are processed by SimpleWebRTC. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
3. to review
bug
regression
feature: WebRTC 🚡
Member
Author
|
I will rebase and create a new pull request (as the branch name matched a protected pattern and thus it could not be force pushed) once #1746 is merged. |
danxuliu
deleted the
stable15-1571-fix-sharing-own-screen-when-other-peer-is-sharing-the-screen
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.
Requires #1739 (please rebase on stable15 once #1739 is merged)
Backport of #1571