From df704573a422dc270fd9c9bfb8572b8f3c3a1626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 19 Sep 2020 03:25:55 +0200 Subject: [PATCH 1/9] Specialize "startSendingNick" function to use only the own peer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "startSendingNick" is only used when the HPB is used, and only with the own peer, so there is no need to provide the peer as a parameter. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index c3cb185929e..8bad28e28c1 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -509,21 +509,21 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local // TODO: The name for the avatar should come from the participant list // which already has all information and get rid of using the // DataChannel for this. - function stopSendingNick(peer) { - if (!peer.nickInterval) { + function stopSendingNick() { + if (!ownPeer.nickInterval) { return } - clearInterval(peer.nickInterval) - peer.nickInterval = null + clearInterval(ownPeer.nickInterval) + ownPeer.nickInterval = null } - function startSendingNick(peer) { + function startSendingNick() { if (!signaling.hasFeature('mcu')) { return } - stopSendingNick(peer) - peer.nickInterval = setInterval(function() { + stopSendingNick() + ownPeer.nickInterval = setInterval(function() { let payload if (signaling.settings.userId === null) { payload = store.getters.getDisplayName() @@ -533,7 +533,7 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local 'userid': signaling.settings.userId, } } - peer.sendDirectly('status', 'nickChanged', payload) + ownPeer.sendDirectly('status', 'nickChanged', payload) }, 1000) } @@ -717,7 +717,7 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local if (peer.id === signaling.getSessionId()) { console.debug('Not adding ICE connection state handler for own peer', peer) - startSendingNick(peer) + startSendingNick() } else { setHandlerForIceConnectionStateChange(peer) } @@ -799,7 +799,6 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local function stopPeerCheckMedia(peer) { stopPeerCheckAudioMedia(peer) stopPeerCheckVideoMedia(peer) - stopSendingNick(peer) } function startPeerCheckMedia(peer, stream) { @@ -840,6 +839,10 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local webrtc.on('peerStreamRemoved', function(peer) { stopPeerCheckMedia(peer) + + if (peer.id === signaling.getSessionId()) { + stopSendingNick() + } }) webrtc.webrtc.on('videoOn', function() { From a5ed998229ebb9946dcc02f786141091271d7411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 28 Sep 2020 11:01:10 +0200 Subject: [PATCH 2/9] Do not explicitly send "nickChanged" message on connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the current participant is a guest the nick was always sent when another participant was connected. However, the nick is sent already (without HPB in the offer/answer, with HPB it is periodically sent), so there is no need to do it again. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index 8bad28e28c1..44e719b4e8b 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -546,11 +546,6 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local sendCurrentMediaStateWithRepetition() } - if (signaling.settings.userId === null) { - const currentGuestNick = store.getters.getDisplayName() - sendDataChannelToAll('status', 'nickChanged', currentGuestNick) - } - // Reset ice restart counter for peer if (spreedPeerConnectionTable[peer.id] > 0) { spreedPeerConnectionTable[peer.id] = 0 From d874ccac73a4d01597831df7f56a356c14f6da4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 29 Sep 2020 19:09:42 +0200 Subject: [PATCH 3/9] Unify sending "nickChanged" data channel messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of sending "nickChanged" data channel messages from different places now an internal "nickChanged" event with the name is emitted, and its handler takes care of sending the data channel message as needed. This also fixes trying to send data channel messages to receiving only peers when the nick is changed and the HPB is used. Signed-off-by: Daniel Calviño Sánchez --- .../models/LocalCallParticipantModel.js | 2 +- src/utils/webrtc/webrtc.js | 26 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/utils/webrtc/models/LocalCallParticipantModel.js b/src/utils/webrtc/models/LocalCallParticipantModel.js index 544c5e8d2d8..60bffab1b64 100644 --- a/src/utils/webrtc/models/LocalCallParticipantModel.js +++ b/src/utils/webrtc/models/LocalCallParticipantModel.js @@ -125,7 +125,7 @@ LocalCallParticipantModel.prototype = { this.set('guestName', guestName) - this._webRtc.sendDirectlyToAll('status', 'nickChanged', guestName) + this._webRtc.webrtc.emit('nickChanged', guestName) }, _handleForcedMute: function() { diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index 44e719b4e8b..e9193f6ac72 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -524,16 +524,7 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local stopSendingNick() ownPeer.nickInterval = setInterval(function() { - let payload - if (signaling.settings.userId === null) { - payload = store.getters.getDisplayName() - } else { - payload = { - 'name': store.getters.getDisplayName(), - 'userid': signaling.settings.userId, - } - } - ownPeer.sendDirectly('status', 'nickChanged', payload) + webrtc.webrtc.emit('nickChanged', store.getters.getDisplayName()) }, 1000) } @@ -1010,6 +1001,21 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local sendDataChannelToAll('status', 'videoOff') }) + // Send the nick changed event via data channel + webrtc.on('nickChanged', function(name) { + let payload + if (signaling.settings.userId === null) { + payload = name + } else { + payload = { + 'name': name, + 'userid': signaling.settings.userId, + } + } + + sendDataChannelToAll('status', 'nickChanged', payload) + }) + // Local screen added. webrtc.on('localScreenAdded', function(/* video */) { const currentSessionId = signaling.getSessionId() From 07530c43f9f991fd4c3df4454b58235fe091e8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 28 Sep 2020 11:24:54 +0200 Subject: [PATCH 4/9] Generalize function name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function will be used to send both the media state and the nick, so its name is adjusted accordingly. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index e9193f6ac72..96ff2d5b760 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -47,7 +47,7 @@ const delayedConnectionToPeer = [] let callParticipantCollection = null let localCallParticipantModel = null let showedTURNWarning = false -let sendCurrentMediaStateWithRepetitionTimeout = null +let sendCurrentStateWithRepetitionTimeout = null function arrayDiff(a, b) { return a.filter(function(i) { @@ -159,14 +159,14 @@ function sendCurrentMediaState() { } } -function sendCurrentMediaStateWithRepetition(timeout) { +function sendCurrentStateWithRepetition(timeout) { if (!timeout) { timeout = 0 - clearTimeout(sendCurrentMediaStateWithRepetitionTimeout) + clearTimeout(sendCurrentStateWithRepetitionTimeout) } - sendCurrentMediaStateWithRepetitionTimeout = setTimeout(function() { + sendCurrentStateWithRepetitionTimeout = setTimeout(function() { sendCurrentMediaState() if (!timeout) { @@ -176,11 +176,11 @@ function sendCurrentMediaStateWithRepetition(timeout) { } if (timeout > 16000) { - sendCurrentMediaStateWithRepetitionTimeout = null + sendCurrentStateWithRepetitionTimeout = null return } - sendCurrentMediaStateWithRepetition(timeout) + sendCurrentStateWithRepetition(timeout) }, timeout) } @@ -246,7 +246,7 @@ function usersChanged(signaling, newUsers, disconnectedSessionIds) { // video not available on the other end, so there is no need to send // the media state. if (signaling.hasFeature('mcu')) { - sendCurrentMediaStateWithRepetition() + sendCurrentStateWithRepetition() } } @@ -534,7 +534,7 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local if (!signaling.hasFeature('mcu')) { sendCurrentMediaState() } else { - sendCurrentMediaStateWithRepetition() + sendCurrentStateWithRepetition() } // Reset ice restart counter for peer From fffa58d24e22bba45392c7343dfb8310d4e600e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 19 Sep 2020 03:38:59 +0200 Subject: [PATCH 5/9] Stop sending the nick through data channels after some time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the introduction of the HPB the nick has been sent over and over again every second, as it is not possible to know when other participants have established a data channel connection. However, it is possible to know when other participants join the call, and it is safe to assume that the other participant established a connection with the local one after a "reasonable" amount of time. As sending the nick every second is too aggressive (specially in calls with a lot of users) now the nick is sent when the local participant establishes a connection with the remote one and, as that does not guarantee that the remote one has established a connection with the local one, the nick 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. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 48 +++++++++----------------------------- 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index 96ff2d5b760..c1140525532 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -159,6 +159,13 @@ function sendCurrentMediaState() { } } +// TODO The participant name should be got from the participant list, but it is +// not currently possible to associate a Nextcloud ID with a standalone +// signaling ID for guests. +function sendCurrentNick() { + webrtc.webrtc.emit('nickChanged', store.getters.getDisplayName()) +} + function sendCurrentStateWithRepetition(timeout) { if (!timeout) { timeout = 0 @@ -168,6 +175,7 @@ function sendCurrentStateWithRepetition(timeout) { sendCurrentStateWithRepetitionTimeout = setTimeout(function() { sendCurrentMediaState() + sendCurrentNick() if (!timeout) { timeout = 1000 @@ -499,39 +507,11 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local webrtc.sendDirectlyToAll(channel, message, payload) } - // The nick name below the avatar is distributed through the DataChannel - // of the PeerConnection and only sent once during establishment. For - // the MCU case, the sending PeerConnection is created once and then - // never changed when more participants join. For this, we periodically - // send the nick to all other participants through the sending - // PeerConnection. - // - // TODO: The name for the avatar should come from the participant list - // which already has all information and get rid of using the - // DataChannel for this. - function stopSendingNick() { - if (!ownPeer.nickInterval) { - return - } - - clearInterval(ownPeer.nickInterval) - ownPeer.nickInterval = null - } - function startSendingNick() { - if (!signaling.hasFeature('mcu')) { - return - } - - stopSendingNick() - ownPeer.nickInterval = setInterval(function() { - webrtc.webrtc.emit('nickChanged', store.getters.getDisplayName()) - }, 1000) - } - function handleIceConnectionStateConnected(peer) { - // Send the current information about the video and microphone - // state. + // Send the current information about the state. if (!signaling.hasFeature('mcu')) { + // Only the media state needs to be sent, the nick was already sent + // in the offer/answer. sendCurrentMediaState() } else { sendCurrentStateWithRepetition() @@ -702,8 +682,6 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local if (peer.type === 'video') { if (peer.id === signaling.getSessionId()) { console.debug('Not adding ICE connection state handler for own peer', peer) - - startSendingNick() } else { setHandlerForIceConnectionStateChange(peer) } @@ -825,10 +803,6 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local webrtc.on('peerStreamRemoved', function(peer) { stopPeerCheckMedia(peer) - - if (peer.id === signaling.getSessionId()) { - stopSendingNick() - } }) webrtc.webrtc.on('videoOn', function() { From 351345a4b88f8411c8164cfadb31a1333306357a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 29 Sep 2020 19:14:37 +0200 Subject: [PATCH 6/9] Remove "userId" property from internal "nick" event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "userId" property was used only in CallParticipantModel, but as it is also set from the participant information provided by the signaling in most cases it is not needed. The only case in which it would be needed is if the Peer object is created and connected before the participant is found in the participant information provided by the signaling. However even in that rare case showing a user as a guest would happen very briefly and it will correct itself as soon as the signaling sends the participant information. Due to this, and as it will simplify also a "nickChanged" signaling message to be introduced later, the "userId" property is now ignored. Note that this only affects the internal "nick" event; the "userId" property still needs to be sent in the "nickChanged" data channel message as it is used by the Android app. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/models/CallParticipantModel.js | 1 - src/utils/webrtc/webrtc.js | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/utils/webrtc/models/CallParticipantModel.js b/src/utils/webrtc/models/CallParticipantModel.js index c4c079d452e..63ffa61abc7 100644 --- a/src/utils/webrtc/models/CallParticipantModel.js +++ b/src/utils/webrtc/models/CallParticipantModel.js @@ -174,7 +174,6 @@ CallParticipantModel.prototype = { return } - this.set('userId', data.userid || null) this.set('name', data.name || null) }, diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index c1140525532..356d416d27f 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -937,12 +937,8 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local } else if (data.type === 'videoOff') { webrtc.emit('mute', { id: peer.id, name: 'video' }) } else if (data.type === 'nickChanged') { - const payload = data.payload || '' - if (typeof (payload) === 'string') { - webrtc.emit('nick', { id: peer.id, name: data.payload }) - } else { - webrtc.emit('nick', { id: peer.id, name: payload.name, userid: payload.userid }) - } + const name = typeof (data.payload) === 'string' ? data.payload : data.payload.name + webrtc.emit('nick', { id: peer.id, name: name }) } else if (data.type === 'speaking' || data.type === 'stoppedSpeaking') { // Valid known messages, but handled elsewhere } else { From 8b149ef1f979d2af3ff9364d68a00301497bdedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 29 Sep 2020 19:16:17 +0200 Subject: [PATCH 7/9] Send and handle "nickChanged" message also through signaling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the HPB is used, if the local participant does not send audio nor video there is no Peer for the local participant, so it is not possible to send data channel messages (although they can be received if the other participant sends audio or video and thus has a Peer object). When the HPB is not used, if both the local participant and the remote one do not send audio nor video there is no Peer for their connection, so it is not possible to send or received data channel messages between those participants. The nick was only sent through data channel messages, so in the above cases the nick was not initially shown and neither updated if it was changed later. Now a "nickChanged" signaling message is introduced to ensure that the nick is transmitted even if there is no Peer for a participant. Even if the mobile apps do not currently handle the "nickChanged" event they will just ignore it without breaking havoc. Signed-off-by: Daniel Calviño Sánchez --- .../webrtc/models/CallParticipantModel.js | 3 ++- src/utils/webrtc/simplewebrtc/simplewebrtc.js | 4 ++++ src/utils/webrtc/webrtc.js | 23 ++++++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/utils/webrtc/models/CallParticipantModel.js b/src/utils/webrtc/models/CallParticipantModel.js index 63ffa61abc7..88b7e0f6226 100644 --- a/src/utils/webrtc/models/CallParticipantModel.js +++ b/src/utils/webrtc/models/CallParticipantModel.js @@ -170,7 +170,8 @@ CallParticipantModel.prototype = { }, _handleNick: function(data) { - if (!this.get('peer') || this.get('peer').id !== data.id) { + // The nick could be changed even if there is no Peer object. + if (this.get('peerId') !== data.id) { return } diff --git a/src/utils/webrtc/simplewebrtc/simplewebrtc.js b/src/utils/webrtc/simplewebrtc/simplewebrtc.js index 05e1e0f0f6f..46507a7e143 100644 --- a/src/utils/webrtc/simplewebrtc/simplewebrtc.js +++ b/src/utils/webrtc/simplewebrtc/simplewebrtc.js @@ -119,6 +119,10 @@ function SimpleWebRTC(opts) { self.emit('mute', { id: message.payload.peerId }) } } + } else if (message.type === 'nickChanged') { + // "nickChanged" can be received from a participant without a Peer + // object if that participant is not sending audio nor video. + self.emit('nick', { id: message.from, name: message.payload.name }) } else if (peers.length) { peers.forEach(function(peer) { if (message.sid && !self.connection.hasFeature('mcu')) { diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index 356d416d27f..bc1da0b4c06 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -971,7 +971,7 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local sendDataChannelToAll('status', 'videoOff') }) - // Send the nick changed event via data channel + // Send the nick changed event via data channel and signaling webrtc.on('nickChanged', function(name) { let payload if (signaling.settings.userId === null) { @@ -984,6 +984,27 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local } sendDataChannelToAll('status', 'nickChanged', payload) + + // "webrtc.sendToAll" can not be used, as it only sends the signaling + // message to participants for which there is a Peer object, so the + // message may not be sent to participants without audio and video. + for (const sessionId in usersInCallMapping) { + if (!usersInCallMapping[sessionId].inCall) { + continue + } else if (sessionId === signaling.getSessionId()) { + continue + } + + const message = { + to: sessionId, + roomType: 'video', + type: 'nickChanged', + payload: { + name: name, + }, + } + signaling.emit('message', message) + } }) // Local screen added. From 38e71ceb115ed2ed72fcf2e9f65881087dbef30b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 30 Sep 2020 21:59:46 +0200 Subject: [PATCH 8/9] Fix initial nick for participants without Peer when HPB is not used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the HPB is not used the initial nick is sent along the offer and answer. However if there is no Peer between two participants there will be no offer/answer, so the nick needs to be explicitly sent in that case. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index bc1da0b4c06..007284849b5 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -246,15 +246,17 @@ function usersChanged(signaling, newUsers, disconnectedSessionIds) { || (!signaling.hasFeature('mcu') && user && !userHasStreams(user) && !webrtc.webrtc.localStreams.length)) { callParticipantModel.setPeer(null) - // As there is no Peer for the other participant the current media - // state will not be sent once it is connected, so it needs to be - // sent now. - // When there is no MCU this is not needed; as the local participant - // has no streams it will be automatically marked with audio and - // video not available on the other end, so there is no need to send - // the media state. + // As there is no Peer for the other participant the current state + // will not be sent once it is connected, so it needs to be sent + // now. + // When there is no MCU this is only needed for the nick; as the + // local participant has no streams it will be automatically marked + // with audio and video not available on the other end, so there is + // no need to send the media state. if (signaling.hasFeature('mcu')) { sendCurrentStateWithRepetition() + } else { + sendCurrentNick() } } From 77d2daacd64ee419ff3872ff60c4d23f27e40fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 18 Nov 2020 21:25:16 +0100 Subject: [PATCH 9/9] Add explicit documentation about payload of "nickChanged" message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/webrtc.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/utils/webrtc/webrtc.js b/src/utils/webrtc/webrtc.js index 007284849b5..4e3a9d1dbfa 100644 --- a/src/utils/webrtc/webrtc.js +++ b/src/utils/webrtc/webrtc.js @@ -974,6 +974,14 @@ export default function initWebRTC(signaling, _callParticipantCollection, _local }) // Send the nick changed event via data channel and signaling + // + // The message format is different in each case. Due to historical reasons + // the payload of the data channel message is either a string that contains + // the name (if the participant is a guest) or an object with "name" and + // "userid" string fields (when the participant is a user). + // + // In the newer signaling message, on the other hand, the payload is always + // an object with only a "name" string field. webrtc.on('nickChanged', function(name) { let payload if (signaling.settings.userId === null) {