Browse Source

Refactor presence update 2 (#1814)

* feat: Adds events for session-add, session-remove and session-accept.

* feat: Refactor updating presence for audio/video mute and video type.

This brings few changes and fixes. Thew initial presence will always miss audioMuted and videoMuted value, those will be added on session accept. All updates to presence are done on sessionAccept, on source-add or on source-remove and the only one not signalled when camera track is replaced by video track and vice versa.
This change brings more presence updates when number of participants are below startAudioMuted/startVideoMuted, but when participants are above those numbers we get less presence update. This is important for big meetings.
Fixes wrong videoMuted state, as replace track and mute are both executed in promise, sometimes the replace-track one finishes first and when the mute one is resolved there is no conference object in the track to be able to update the presence (hitting this when we pass the startAudioMuted threshold).

* squash: Put back the tracks for _setTrackMuteStatus and _setNewVideoType.

* squash: Fix line length.

* squash: Fix skip sending presence twice.

* squash: Adds the error to the error callback.

* squash: Fix newJingleErrorHandler.
release-8443
Дамян Минков 3 years ago
parent
commit
579a49b4c6
No account linked to committer's email address
4 changed files with 125 additions and 54 deletions
  1. 65
    17
      JitsiConference.js
  2. 0
    33
      modules/xmpp/ChatRoom.js
  3. 30
    4
      modules/xmpp/JingleSessionPC.js
  4. 30
    0
      service/xmpp/XMPPEvents.js

+ 65
- 17
JitsiConference.js View File

@@ -400,6 +400,11 @@ JitsiConference.prototype._init = function(options = {}) {
400 400
     this._sendConferenceJoinAnalyticsEvent = this._sendConferenceJoinAnalyticsEvent.bind(this);
401 401
     this.room.addListener(XMPPEvents.MEETING_ID_SET, this._sendConferenceJoinAnalyticsEvent);
402 402
 
403
+    this._updateRoomPresence = this._updateRoomPresence.bind(this);
404
+    this.room.addListener(XMPPEvents.SESSION_ACCEPT, this._updateRoomPresence);
405
+    this.room.addListener(XMPPEvents.SOURCE_ADD, this._updateRoomPresence);
406
+    this.room.addListener(XMPPEvents.SOURCE_REMOVE, this._updateRoomPresence);
407
+
403 408
     this.e2eping = new E2ePing(
404 409
         this,
405 410
         config,
@@ -685,6 +690,9 @@ JitsiConference.prototype.leave = async function() {
685 690
         this._updateProperties);
686 691
 
687 692
     room.removeListener(XMPPEvents.MEETING_ID_SET, this._sendConferenceJoinAnalyticsEvent);
693
+    room.removeListener(XMPPEvents.SESSION_ACCEPT, this._updateRoomPresence);
694
+    room.removeListener(XMPPEvents.SOURCE_ADD, this._updateRoomPresence);
695
+    room.removeListener(XMPPEvents.SOURCE_REMOVE, this._updateRoomPresence);
688 696
 
689 697
     this.eventManager.removeXMPPListeners();
690 698
 
@@ -1253,6 +1261,11 @@ JitsiConference.prototype.replaceTrack = function(oldTrack, newTrack) {
1253 1261
                 this._sendBridgeVideoTypeMessage(newTrack);
1254 1262
             }
1255 1263
 
1264
+            // updates presence when we replace the video tracks desktop with screen and screen with desktop
1265
+            if (oldTrackBelongsToConference && oldTrack?.isVideoTrack()) {
1266
+                this._updateRoomPresence(this.p2pJingleSession ? this.p2pJingleSession : this.jvbJingleSession);
1267
+            }
1268
+
1256 1269
             if (newTrack !== null && (this.isMutedByFocus || this.isVideoMutedByFocus)) {
1257 1270
                 this._fireMuteChangeEvent(newTrack);
1258 1271
             }
@@ -1319,19 +1332,8 @@ JitsiConference.prototype._setupNewTrack = function(newTrack) {
1319 1332
         }
1320 1333
     }
1321 1334
 
1322
-    let videoTypeChanged = false;
1323
-
1324
-    if (newTrack.isVideoTrack()) {
1325
-        videoTypeChanged = this._setNewVideoType(newTrack);
1326
-    }
1327 1335
     this.rtc.addLocalTrack(newTrack);
1328 1336
 
1329
-    // ensure that we're sharing proper "is muted" state
1330
-    if (this._setTrackMuteStatus(newTrack, newTrack.isMuted()) || videoTypeChanged) {
1331
-        // send presence if it was changed with vide type or mute status
1332
-        this.room.sendPresence();
1333
-    }
1334
-
1335 1337
     newTrack.muteHandler = this._fireMuteChangeEvent.bind(this, newTrack);
1336 1338
     newTrack.audioLevelHandler = this._fireAudioLevelChangeEvent.bind(this);
1337 1339
     newTrack.addEventListener(
@@ -1353,7 +1355,7 @@ JitsiConference.prototype._setupNewTrack = function(newTrack) {
1353 1355
  * @private
1354 1356
  */
1355 1357
 JitsiConference.prototype._setNewVideoType = function(track) {
1356
-    if (FeatureFlags.isSourceNameSignalingEnabled()) {
1358
+    if (FeatureFlags.isSourceNameSignalingEnabled() && track) {
1357 1359
         // FIXME once legacy signaling using 'sendCommand' is removed, signalingLayer.setTrackVideoType must be adjusted
1358 1360
         // to send the presence (not just modify it).
1359 1361
         this._signalingLayer.setTrackVideoType(
@@ -1370,11 +1372,15 @@ JitsiConference.prototype._setNewVideoType = function(track) {
1370 1372
 
1371 1373
     const videoTypeTagName = 'videoType';
1372 1374
 
1375
+    // if track is missing we revert to default type Camera, the case where we screenshare and
1376
+    // we return to be video muted
1377
+    const trackVideoType = track ? track.videoType : VideoType.CAMERA;
1378
+
1373 1379
     // if video type is camera and there is no videoType in presence, we skip adding it, as this is the default one
1374
-    if (track.videoType !== VideoType.CAMERA || this.room.getFromPresence(videoTypeTagName)) {
1380
+    if (trackVideoType !== VideoType.CAMERA || this.room.getFromPresence(videoTypeTagName)) {
1375 1381
         // we will not use this.sendCommand here to avoid sending the presence immediately, as later we may also set
1376 1382
         // and the mute status
1377
-        return this.room.addOrReplaceInPresence(videoTypeTagName, { value: track.videoType });
1383
+        return this.room.addOrReplaceInPresence(videoTypeTagName, { value: trackVideoType });
1378 1384
     }
1379 1385
 
1380 1386
     return false;
@@ -1382,17 +1388,18 @@ JitsiConference.prototype._setNewVideoType = function(track) {
1382 1388
 
1383 1389
 /**
1384 1390
  * Sets mute status.
1391
+ * @param mediaType
1385 1392
  * @param localTrack
1386 1393
  * @param isMuted
1387 1394
  * @param <tt>true</tt> when presence was changed, <tt>false</tt> otherwise.
1388 1395
  * @private
1389 1396
  */
1390
-JitsiConference.prototype._setTrackMuteStatus = function(localTrack, isMuted) {
1397
+JitsiConference.prototype._setTrackMuteStatus = function(mediaType, localTrack, isMuted) {
1391 1398
     if (FeatureFlags.isSourceNameSignalingEnabled()) {
1392 1399
         // TODO When legacy signaling part is removed, remember to adjust signalingLayer.setTrackMuteStatus, so that
1393 1400
         // it triggers sending the presence (it only updates it for now, because the legacy code below sends).
1394 1401
         this._signalingLayer.setTrackMuteStatus(
1395
-            getSourceNameForJitsiTrack(this.myUserId(), localTrack.getType(), 0),
1402
+            getSourceNameForJitsiTrack(this.myUserId(), mediaType, 0),
1396 1403
             isMuted
1397 1404
         );
1398 1405
     }
@@ -1401,7 +1408,7 @@ JitsiConference.prototype._setTrackMuteStatus = function(localTrack, isMuted) {
1401 1408
         return false;
1402 1409
     }
1403 1410
 
1404
-    if (localTrack.isAudioTrack()) {
1411
+    if (mediaType === MediaType.AUDIO) {
1405 1412
         return this.room.addAudioInfoToPresence(isMuted);
1406 1413
     }
1407 1414
 
@@ -3596,6 +3603,47 @@ JitsiConference.prototype._stopP2PSession = function(options = {}) {
3596 3603
     }
3597 3604
 };
3598 3605
 
3606
+/**
3607
+ * Updates room presence if needed and send the packet in case of a modification.
3608
+ * @param {JingleSessionPC} jingleSession the session firing the event, contains the peer connection which
3609
+ * tracks we will check.
3610
+ * @param {Object|null} ctx a context object we can distinguish multiple calls of the same pass of updating tracks.
3611
+ */
3612
+JitsiConference.prototype._updateRoomPresence = function(jingleSession, ctx) {
3613
+    // skips sending presence twice for the same pass of updating ssrcs
3614
+    if (ctx) {
3615
+        if (ctx.skip) {
3616
+            return;
3617
+        }
3618
+        ctx.skip = true;
3619
+    }
3620
+
3621
+    const localAudioTracks = jingleSession.peerconnection.getLocalTracks(MediaType.AUDIO);
3622
+    const localVideoTracks = jingleSession.peerconnection.getLocalTracks(MediaType.VIDEO);
3623
+    let presenceChanged = false;
3624
+
3625
+    if (localAudioTracks && localAudioTracks.length) {
3626
+        presenceChanged = this._setTrackMuteStatus(MediaType.AUDIO, localAudioTracks[0], localAudioTracks[0].isMuted());
3627
+    } else if (this._setTrackMuteStatus(MediaType.AUDIO, undefined, true)) {
3628
+        presenceChanged = true;
3629
+    }
3630
+
3631
+    if (localVideoTracks && localVideoTracks.length) {
3632
+        const muteStatusChanged = this._setTrackMuteStatus(
3633
+            MediaType.VIDEO, localVideoTracks[0], localVideoTracks[0].isMuted());
3634
+        const videoTypeChanged = this._setNewVideoType(localVideoTracks[0]);
3635
+
3636
+        presenceChanged = presenceChanged || muteStatusChanged || videoTypeChanged;
3637
+    } else {
3638
+        const muteStatusChanged = this._setTrackMuteStatus(MediaType.VIDEO, undefined, true);
3639
+        const videoTypeChanged = this._setNewVideoType(); // set back to default video type
3640
+
3641
+        presenceChanged = presenceChanged || muteStatusChanged || videoTypeChanged;
3642
+    }
3643
+
3644
+    presenceChanged && this.room.sendPresence();
3645
+};
3646
+
3599 3647
 /**
3600 3648
  * Checks whether or not the conference is currently in the peer to peer mode.
3601 3649
  * Being in peer to peer mode means that the direct connection has been

+ 0
- 33
modules/xmpp/ChatRoom.js View File

@@ -1553,22 +1553,6 @@ export default class ChatRoom extends Listenable {
1553 1553
         return null;
1554 1554
     }
1555 1555
 
1556
-    /**
1557
-     *
1558
-     * @param mute
1559
-     */
1560
-    setVideoMute(mute) {
1561
-        this.sendVideoInfoPresence(mute);
1562
-    }
1563
-
1564
-    /**
1565
-     *
1566
-     * @param mute
1567
-     */
1568
-    setAudioMute(mute) {
1569
-        this.sendAudioInfoPresence(mute);
1570
-    }
1571
-
1572 1556
     /**
1573 1557
      *
1574 1558
      * @param mute
@@ -1588,15 +1572,6 @@ export default class ChatRoom extends Listenable {
1588 1572
             });
1589 1573
     }
1590 1574
 
1591
-    /**
1592
-     *
1593
-     * @param mute
1594
-     */
1595
-    sendAudioInfoPresence(mute) {
1596
-        // FIXME resend presence on CONNECTED
1597
-        this.addAudioInfoToPresence(mute) && this.sendPresence();
1598
-    }
1599
-
1600 1575
     /**
1601 1576
      *
1602 1577
      * @param mute
@@ -1616,14 +1591,6 @@ export default class ChatRoom extends Listenable {
1616 1591
             });
1617 1592
     }
1618 1593
 
1619
-    /**
1620
-     *
1621
-     * @param mute
1622
-     */
1623
-    sendVideoInfoPresence(mute) {
1624
-        this.addVideoInfoToPresence(mute) && this.sendPresence();
1625
-    }
1626
-
1627 1594
     /**
1628 1595
      * Obtains the info about given media advertised in the MUC presence of
1629 1596
      * the participant identified by the given endpoint JID.

+ 30
- 4
modules/xmpp/JingleSessionPC.js View File

@@ -954,7 +954,16 @@ export default class JingleSessionPC extends JingleSession {
954 954
                 // FIXME we may not care about RESULT packet for session-accept
955 955
                 // then we should either call 'success' here immediately or
956 956
                 // modify sendSessionAccept method to do that
957
-                this.sendSessionAccept(success, failure);
957
+                this.sendSessionAccept(() => {
958
+                    success();
959
+
960
+                    this.room.eventEmitter.emit(XMPPEvents.SESSION_ACCEPT, this);
961
+                },
962
+                error => {
963
+                    failure(error);
964
+
965
+                    this.room.eventEmitter.emit(XMPPEvents.SESSION_ACCEPT_ERROR, this, error);
966
+                });
958 967
             },
959 968
             failure,
960 969
             localTracks);
@@ -2512,12 +2521,22 @@ export default class JingleSessionPC extends JingleSession {
2512 2521
             );
2513 2522
         const removedAnySSRCs = sdpDiffer.toJingle(remove);
2514 2523
 
2524
+        // context a common object for one run of ssrc update (source-add and source-remove) so we can match them if we
2525
+        // need to
2526
+        const ctx = {};
2527
+
2515 2528
         if (removedAnySSRCs) {
2516 2529
             logger.info(`${this} Sending source-remove`);
2517 2530
             logger.debug(remove.tree());
2518 2531
             this.connection.sendIQ(
2519
-                remove, null,
2520
-                this.newJingleErrorHandler(remove), IQ_TIMEOUT);
2532
+                remove,
2533
+                () => {
2534
+                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_REMOVE, this, ctx);
2535
+                },
2536
+                this.newJingleErrorHandler(remove, error => {
2537
+                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_REMOVE_ERROR, this, error, ctx);
2538
+                }),
2539
+                IQ_TIMEOUT);
2521 2540
         }
2522 2541
 
2523 2542
         // send source-add IQ.
@@ -2538,7 +2557,14 @@ export default class JingleSessionPC extends JingleSession {
2538 2557
             logger.info(`${this} Sending source-add`);
2539 2558
             logger.debug(add.tree());
2540 2559
             this.connection.sendIQ(
2541
-                add, null, this.newJingleErrorHandler(add), IQ_TIMEOUT);
2560
+                add,
2561
+                () => {
2562
+                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_ADD, this, ctx);
2563
+                },
2564
+                this.newJingleErrorHandler(add, error => {
2565
+                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_ADD_ERROR, this, error, ctx);
2566
+                }),
2567
+                IQ_TIMEOUT);
2542 2568
         }
2543 2569
     }
2544 2570
 

+ 30
- 0
service/xmpp/XMPPEvents.js View File

@@ -229,6 +229,16 @@ const XMPPEvents = {
229 229
     // a specific user of the muc.
230 230
     SENDING_PRIVATE_CHAT_MESSAGE: 'xmpp.sending_private_chat_message',
231 231
 
232
+    /**
233
+     * Event fired after receiving the confirmation about session accept.
234
+     */
235
+    SESSION_ACCEPT: 'xmpp.session_accept',
236
+
237
+    /**
238
+     * Event fired if we receive an error after sending the session accept.
239
+     */
240
+    SESSION_ACCEPT_ERROR: 'xmpp.session_accept_error',
241
+
232 242
     /**
233 243
      * Event fired when we do not get our 'session-accept' acknowledged by
234 244
      * Jicofo. It most likely means that there is serious problem with our
@@ -241,6 +251,26 @@ const XMPPEvents = {
241 251
      */
242 252
     SESSION_ACCEPT_TIMEOUT: 'xmpp.session_accept_timeout',
243 253
 
254
+    /**
255
+     * Event fired after successful sending of jingle source-add.
256
+     */
257
+    SOURCE_ADD: 'xmpp.source_add',
258
+
259
+    /**
260
+     * Event fired after receiving an error sending of jingle source-add.
261
+     */
262
+    SOURCE_ADD_ERROR: 'xmpp.source_add_error',
263
+
264
+    /**
265
+     * Event fired after successful sending of jingle source-remove.
266
+     */
267
+    SOURCE_REMOVE: 'xmpp.source_remove',
268
+
269
+    /**
270
+     * Event fired after receiving an error sending of jingle source-remove.
271
+     */
272
+    SOURCE_REMOVE_ERROR: 'xmpp.source_remove_error',
273
+
244 274
     /**
245 275
      * Event fired when speaker stats update message is received.
246 276
      */

Loading…
Cancel
Save