Improve setting initial audio and video status when the HPB is used #4181
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
|
/backport to stable20 |
e23d483 to
77b541f
Compare
Besides the session ID the SimpleWebRTC code adds a "sid" property to
identify Peers, and signaling messages sent by those Peers include the
"sid" property. When the standalone signaling server is not used this
property makes possible to know if a message from certain participant is
being sent by a "stale" Peer object (a Peer object that has been
replaced with a new one, for example due to reconnection), in which case
it is ignored.
However, when the standalone signaling server is used the "sid" property
is not relayed in offer messages. In this case, when the remote Peer
object is created an arbitrary "sid" is assigned, which does not match
with the "sid" of the local Peer object. As the signaling messages sent
with the local Peer object have one "sid" but the Peer object for that
participant in the remote end has a different "sid" those signaling
messages ("mute", "unmute" and the currently unused "connectivityError")
were ignored.
Due to this now the "sid" property is taken into account only when the
standalone signaling server is not used.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When "audioOn", "audioOff", "videoOn" or "videoOff" events are triggered in the SimpleWebRTC object the message is sent to all the participants through the data channels. When those events are triggered in the WebRTC object (for example, from the LocalMedia object) "mute" or "unmute" messages are sent to all the participants through signaling. The SimpleWebRTC object proxies all the events from its WebRTC object, so triggering those events in the WebRTC objects causes both data channel and signaling messages to be sent. However, when a connection is established with another participant the events were triggered in the SimpleWebRTC object, so the messages were sent only through the data channel. For consistency with the rest of cases, and as signaling messages are preferred over data channel ones, now when a connection with another Peer is established the events are triggered on the WebRTC object instead. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the HPB is used it is possible to know when the local participant has established a connection with a remote participant, but it is not possible to know when a remote participant established a connection with the local one. The initial state (audio and video on or off) was sent just once when the local participant establishes a connection with the remote one, which caused that, in some cases, the message was received by the remote participant before a connection was established with the local one and thus ignored. Even if there is code to try to locally detect the initial media state of the remote participant it has shown to not be as reliable as desirable, which causes a wrong media state to be sometimes shown for remote participants until the participant changes it again (or another user joins and the messages are sent again to everyone). However, even if it is not possible to know when a remote participant established a connection with the local one it is safe to assume that it happened in a "reasonable" amount of time since the local participant has established a connection with the remote one. Due to this now the initial media state is first sent when that happens, and then it is sent again several times (with an increasing interval) in the next 31 seconds. If a connection is established with another participant in the meantime the cycle starts again. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the HPB is used if a remote participant has no audio or video no Peer will be created for that participant. Until now no initial media state was sent for those participants, so a wrong initial media state of the local participant could be seen by those remote participants. Now the current media state is sent to those participants when they join the conversation. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When the HPB is used now the initial state is repeated several times after a connection with another participant is established, so the little wait here is no longer needed. When the HPB is not used the initial state is send just once, but as in that case both participants are already aware of each other the little wait is not needed either. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
If an invalid interval ID is given to "clearInterval()" the function does nothing, so there is no need to check it beforehand. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
As until now the initial media state message was sent just once and it could be missed the received media was locally checked to enable the media if needed. Unfortunately those checks were not as reliable as desirable, so now they are stopped as soon as a media state message from from the other peer is received. This implicitly fixes things like never stopping checking the video if the other peer has no camera enabled. The checks can not be fully removed yet, as the repeated sending of initial media state is currently implemented only in the WebUI, but not in the mobile apps. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
77b541f to
1440652
Compare
| @@ -121,7 +121,7 @@ function SimpleWebRTC(opts) { | |||
| } | |||
| } else if (peers.length) { | |||
| peers.forEach(function(peer) { | |||
| if (message.sid) { | |||
| if (message.sid && !self.connection.hasFeature('mcu')) { | |||
There was a problem hiding this comment.
but now it still (always) runs peer.handleMessage(message) or is that what was missing?
There was a problem hiding this comment.
Now peer.handleMessage(message) is always run; before peer.handleMessage(message) was not run when the HPB was used due to the peer.sid === message.sid check. For the full explanation please refer to the message of the first commit ;-)
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 the HPB is used it is possible to know when the local participant has established a connection with a remote participant, but it is not possible to know when a remote participant established a connection with the local one. The initial state (audio and video on or off) was sent just once when the local participant establishes a connection with the remote one, which caused that, in some cases, the message was received by the remote participant before a connection was established with the local one and thus ignored.
Even if there is code to try to locally detect the initial media state of the remote participant it has shown to not be as reliable as desirable, which causes a wrong media state to be sometimes shown for remote participants until the participant changes it again (or another user joins and the messages are sent again to everyone).
However, even if it is not possible to know when a remote participant established a connection with the local one it is safe to assume that it happened in a "reasonable" amount of time since the local participant has established a connection with the remote one.
Due to this now the initial media state is first sent when that happens, and then it is sent again several times (with an increasing interval) in the next 30 seconds. If a connection is established with another participant in the meantime the cycle starts again.
This pull request also fixes getting the initial media state of other participants when a participant has no audio nor video, as in that case there will be no peer for that participant and the initial media state was sent only when a connection was established with the remote peer.
Additionally the initial media state is now sent also through signaling (all the media state changes were sent both through signaling and data channels except for the initial ones). Signaling media state messages are also correctly handled now (they were ignored due to a bug), although the data channel messages still need to be sent for now for compatibility with the mobile apps.
In a similar way locally checking the initial media state still needs to be done until the mobile apps implement the repetition of the messages, although the check is now stopped as soon as the media state was received from the other participant (until now for example the video check never stopped if the other peer had no camera).