fix(SignalingLayer) Update SSRC owners on leave. (#2184)

* fix(SignalingLayer) Update SSRC owners on leave.
Update the SSRC owners in the following cases:
1. When a remote endpoint leaves the call.
2. When a source-remove is received.
3. When a source is remapped (with ssrc-rewriting enabled).
Create the remote track even if presence is not yet received. The ssrc owner check prevents the client from creating a dummy track when the call switches over from p2p to jvb when the last remote endpoint leaves the call.

* ref(SignalingLayer) alpha sort methods.
Clean up unused methods, _findEndpointSourceInfoForMediaType is not used anymore.

* squash: Address review comments.

JitsiConference.js

@@ -1910,6 +1910,9 @@ JitsiConference.prototype.onMemberLeft = function(jid, reason) {
 
             remoteTracks && (tracksToBeRemoved = [ ...tracksToBeRemoved, ...remoteTracks ]);
 
+            // Update the SSRC owners list.
+            session._signalingLayer.updateSsrcOwnersOnLeave(id);
+
             // Remove the ssrcs from the remote description and renegotiate.
             session.removeRemoteStreamsOnLeave(id);
         }

modules/RTC/TraceablePeerConnection.js

@@ -958,7 +958,9 @@ TraceablePeerConnection.prototype._remoteTrackAdded = function(stream, track, tr
                 + 'track creation failed!'));
 
         return;
-    } else if (!ownerEndpointId) {
+    }
+
+    if (!ownerEndpointId) {
         GlobalOnErrorHandler.callErrorHandler(
             new Error(`No SSRC owner known for remote stream[ssrc=${trackSsrc},id=${streamId},type=${mediaType}]`
             + 'track creation failed!'));
@@ -969,16 +971,18 @@ TraceablePeerConnection.prototype._remoteTrackAdded = function(stream, track, tr
     const sourceName = this.signalingLayer.getTrackSourceName(trackSsrc);
     const peerMediaInfo = this.signalingLayer.getPeerMediaInfo(ownerEndpointId, mediaType, sourceName);
 
-    if (!peerMediaInfo) {
-        GlobalOnErrorHandler.callErrorHandler(
-            new Error(`${this}: no sourceInfo available for ${ownerEndpointId}:${sourceName} track creation failed!`));
+    // Assume default presence state for remote source. Presence can be received after source signaling. This shouldn't
+    // prevent the endpoint from creating a remote track for the source.
+    let muted = true;
+    let videoType = VideoType.CAMERA;
 
-        return;
+    if (peerMediaInfo) {
+        muted = peerMediaInfo.muted;
+        videoType = peerMediaInfo.videoType; // can be undefined
+    } else {
+        logger.info(`${this}: no source-info available for ${ownerEndpointId}:${sourceName}, assuming default state`);
     }
 
-    const muted = peerMediaInfo.muted;
-    const videoType = peerMediaInfo.videoType; // can be undefined
-
     this._createRemoteTrack(ownerEndpointId, stream, track, mediaType, videoType, trackSsrc, muted, sourceName);
 };
 
@@ -1082,9 +1086,8 @@ TraceablePeerConnection.prototype._remoteTrackRemoved = function(stream, track)
     const streamId = stream.id;
     const trackId = track?.id;
 
+    // Ignore stream removed events for JVB "mixed" sources (used for RTCP termination).
     if (!RTC.isUserStreamById(streamId)) {
-        logger.info(`${this} ignored remote 'stream removed' event for non-user stream[id=${streamId}]`);
-
         return;
     }
 
@@ -1101,8 +1104,7 @@ TraceablePeerConnection.prototype._remoteTrackRemoved = function(stream, track)
     }
 
     const toBeRemoved = this.getRemoteTracks().find(
-        remoteTrack => remoteTrack.getStreamId() === streamId
-        && remoteTrack.getTrackId() === trackId);
+        remoteTrack => remoteTrack.getStreamId() === streamId && remoteTrack.getTrackId() === trackId);
 
     if (!toBeRemoved) {
         GlobalOnErrorHandler.callErrorHandler(new Error(`${this} remote track removal failed - track not found`));
@@ -1110,7 +1112,6 @@ TraceablePeerConnection.prototype._remoteTrackRemoved = function(stream, track)
         return;
     }
 
-    logger.info(`${this} remote track removed stream[id=${streamId},trackId=${trackId}]`);
     this._removeRemoteTrack(toBeRemoved);
 };
 
@@ -1141,6 +1142,9 @@ TraceablePeerConnection.prototype.removeRemoteTracks = function(owner) {
  * @returns {void}
  */
 TraceablePeerConnection.prototype._removeRemoteTrack = function(toBeRemoved) {
+    logger.info(`${this} Removing remote track stream[id=${toBeRemoved.getStreamId()},`
+        + `trackId=${toBeRemoved.getTrackId()}]`);
+
     toBeRemoved.dispose();
     const participantId = toBeRemoved.getParticipantId();
     const userTracksByMediaType = this.remoteTracks.get(participantId);

modules/proxyconnection/CustomSignalingLayer.js

@@ -2,10 +2,8 @@ import { getLogger } from '@jitsi/logger';
 
 import SignalingLayer from '../../service/RTC/SignalingLayer';
 
-
 const logger = getLogger(__filename);
 
-
 /**
  * Custom semi-mock implementation for the Proxy connection service.
  */
@@ -29,14 +27,6 @@ export default class CustomSignalingLayer extends SignalingLayer {
         this.chatRoom = null;
     }
 
-    /**
-     * Sets the <tt>ChatRoom</tt> instance used.
-     * @param {ChatRoom} room
-     */
-    setChatRoom(room) {
-        this.chatRoom = room;
-    }
-
     /**
      * @inheritDoc
      */
@@ -58,6 +48,34 @@ export default class CustomSignalingLayer extends SignalingLayer {
         return this.ssrcOwners.get(ssrc);
     }
 
+    /**
+     * @inheritDoc
+     */
+    getTrackSourceName(ssrc) { // eslint-disable-line no-unused-vars
+        return undefined;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    removeSSRCOwners(ssrcList) {
+        if (!ssrcList?.length) {
+            return;
+        }
+
+        for (const ssrc of ssrcList) {
+            this.ssrcOwners.delete(ssrc);
+        }
+    }
+
+    /**
+     * Sets the <tt>ChatRoom</tt> instance used.
+     * @param {ChatRoom} room
+     */
+    setChatRoom(room) {
+        this.chatRoom = room;
+    }
+
     /**
      * @inheritDoc
      */
@@ -93,13 +111,21 @@ export default class CustomSignalingLayer extends SignalingLayer {
     /**
      * @inheritDoc
      */
-    getTrackSourceName(ssrc) { // eslint-disable-line no-unused-vars
-        return undefined;
+    setTrackSourceName(ssrc, sourceName) { // eslint-disable-line no-unused-vars
     }
 
     /**
      * @inheritDoc
      */
-    setTrackSourceName(ssrc, sourceName) { // eslint-disable-line no-unused-vars
+    updateSsrcOwnersOnLeave(id) {
+        const ssrcs = Array.from(this.ssrcOwners)
+            .filter(entry => entry[1] === id)
+            .map(entry => entry[0]);
+
+        if (!ssrcs?.length) {
+            return;
+        }
+
+        this.removeSSRCOwners(ssrcs);
     }
 }

modules/xmpp/JingleSessionPC.js

@@ -1810,6 +1810,9 @@ export default class JingleSessionPC extends JingleSession {
                 if (track) {
                     logger.debug(`Existing SSRC ${s.ssrc}: new owner ${s.owner}. name=${s.source}`);
 
+                    // Update the SSRC owner.
+                    this._signalingLayer.setSSRCOwner(s.ssrc, s.owner);
+
                     if (s.videoType === 'CAMERA') {
                         track._setVideoType('camera');
                     } else if (s.videoType === 'DESKTOP') {
@@ -2042,11 +2045,14 @@ export default class JingleSessionPC extends JingleSession {
         const remoteSdp = this.usesUnifiedPlan
             ? new SDP(this.peerconnection.peerconnection.remoteDescription.sdp)
             : new SDP(this.peerconnection.remoteDescription.sdp);
+        let ssrcs;
 
         removeSsrcInfo.forEach((lines, idx) => {
             // eslint-disable-next-line no-param-reassign
             lines = lines.split('\r\n');
             lines.pop(); // remove empty last element;
+            ssrcs = lines.map(line => Number(line.split('a=ssrc:')[1]?.split(' ')[0]));
+
             if (this.usesUnifiedPlan) {
                 let mid;
 
@@ -2085,6 +2091,10 @@ export default class JingleSessionPC extends JingleSession {
                 });
             }
         });
+
+        // Update the ssrc owners list.
+        ssrcs?.length && this._signalingLayer.removeSSRCOwners(ssrcs);
+
         remoteSdp.raw = remoteSdp.session + remoteSdp.media.join('');
 
         return remoteSdp;

modules/xmpp/SignalingLayerImpl.js

@@ -6,6 +6,7 @@ import * as SignalingEvents from '../../service/RTC/SignalingEvents';
 import SignalingLayer, { getMediaTypeFromSourceName } from '../../service/RTC/SignalingLayer';
 import { VideoType } from '../../service/RTC/VideoType';
 import { XMPPEvents } from '../../service/xmpp/XMPPEvents';
+import FeatureFlags from '../flags/FeatureFlags';
 
 import { filterNodeFromPresenceJSON } from './ChatRoom';
 
@@ -79,45 +80,6 @@ export default class SignalingLayerImpl extends SignalingLayer {
         return false;
     }
 
-    /**
-     * Check is given endpoint has advertised <SourceInfo/> in it's presence which means that the source name signaling
-     * is used by this endpoint.
-     *
-     * @param {EndpointId} endpointId
-     * @returns {boolean}
-     */
-    _doesEndpointSendNewSourceInfo(endpointId) {
-        const presence = this.chatRoom?.getLastPresence(endpointId);
-
-        return Boolean(presence && presence.find(node => node.tagName === SOURCE_INFO_PRESENCE_ELEMENT));
-    }
-
-    /**
-     * Sets the <tt>ChatRoom</tt> instance used and binds presence listeners.
-     * @param {ChatRoom} room
-     */
-    setChatRoom(room) {
-        const oldChatRoom = this.chatRoom;
-
-        this.chatRoom = room;
-        if (oldChatRoom) {
-            oldChatRoom.removePresenceListener(
-                'audiomuted', this._audioMuteHandler);
-            oldChatRoom.removePresenceListener(
-                'videomuted', this._videoMuteHandler);
-            oldChatRoom.removePresenceListener(
-                'videoType', this._videoTypeHandler);
-            this._sourceInfoHandler
-                && oldChatRoom.removePresenceListener(SOURCE_INFO_PRESENCE_ELEMENT, this._sourceInfoHandler);
-            this._memberLeftHandler
-                && oldChatRoom.removeEventListener(XMPPEvents.MUC_MEMBER_LEFT, this._memberLeftHandler);
-        }
-        if (room) {
-            this._bindChatRoomEventHandlers(room);
-            this._addLocalSourceInfoToPresence();
-        }
-    }
-
     /**
      * Binds event listeners to the chat room instance.
      * @param {ChatRoom} room
@@ -235,28 +197,31 @@ export default class SignalingLayerImpl extends SignalingLayer {
     }
 
     /**
-     * Finds the first source of given media type for the given endpoint.
-     * @param endpointId
-     * @param mediaType
-     * @returns {SourceInfo|null}
-     * @private
+     * Check is given endpoint has advertised <SourceInfo/> in it's presence which means that the source name signaling
+     * is used by this endpoint.
+     *
+     * @param {EndpointId} endpointId
+     * @returns {boolean}
      */
-    _findEndpointSourceInfoForMediaType(endpointId, mediaType) {
-        const remoteSourceState = this._remoteSourceState[endpointId];
-
-        if (!remoteSourceState) {
-            return null;
-        }
+    _doesEndpointSendNewSourceInfo(endpointId) {
+        const presence = this.chatRoom?.getLastPresence(endpointId);
 
-        for (const sourceInfo of Object.values(remoteSourceState)) {
-            const _mediaType = getMediaTypeFromSourceName(sourceInfo.sourceName);
+        return Boolean(presence && presence.find(node => node.tagName === SOURCE_INFO_PRESENCE_ELEMENT));
+    }
 
-            if (_mediaType === mediaType) {
-                return sourceInfo;
-            }
+    /**
+     * Logs a debug or error message to console depending on whether SSRC rewriting is enabled or not.
+     * Owner changes are permitted only when SSRC rewriting is enabled.
+     *
+     * @param {string} message - The message to be logged.
+     * @returns {void}
+     */
+    _logOwnerChangedMessage(message) {
+        if (FeatureFlags.isSsrcRewritingSupported()) {
+            logger.debug(message);
+        } else {
+            logger.error(message);
         }
-
-        return null;
     }
 
     /**
@@ -322,6 +287,52 @@ export default class SignalingLayerImpl extends SignalingLayer {
         return this.ssrcOwners.get(ssrc);
     }
 
+    /**
+     * @inheritDoc
+     */
+    getTrackSourceName(ssrc) {
+        return this._sourceNames.get(ssrc);
+    }
+
+    /**
+     * @inheritDoc
+     */
+    removeSSRCOwners(ssrcList) {
+        if (!ssrcList?.length) {
+            return;
+        }
+
+        for (const ssrc of ssrcList) {
+            this.ssrcOwners.delete(ssrc);
+        }
+    }
+
+    /**
+     * Sets the <tt>ChatRoom</tt> instance used and binds presence listeners.
+     * @param {ChatRoom} room
+     */
+    setChatRoom(room) {
+        const oldChatRoom = this.chatRoom;
+
+        this.chatRoom = room;
+        if (oldChatRoom) {
+            oldChatRoom.removePresenceListener(
+                'audiomuted', this._audioMuteHandler);
+            oldChatRoom.removePresenceListener(
+                'videomuted', this._videoMuteHandler);
+            oldChatRoom.removePresenceListener(
+                'videoType', this._videoTypeHandler);
+            this._sourceInfoHandler
+                && oldChatRoom.removePresenceListener(SOURCE_INFO_PRESENCE_ELEMENT, this._sourceInfoHandler);
+            this._memberLeftHandler
+                && oldChatRoom.removeEventListener(XMPPEvents.MUC_MEMBER_LEFT, this._memberLeftHandler);
+        }
+        if (room) {
+            this._bindChatRoomEventHandlers(room);
+            this._addLocalSourceInfoToPresence();
+        }
+    }
+
     /**
      * @inheritDoc
      */
@@ -335,7 +346,7 @@ export default class SignalingLayerImpl extends SignalingLayer {
         const existingOwner = this.ssrcOwners.get(ssrc);
 
         if (existingOwner && existingOwner !== endpointId) {
-            logger.error(`SSRC owner re-assigned from ${existingOwner} to ${endpointId}`);
+            this._logOwnerChangedMessage(`SSRC owner re-assigned from ${existingOwner} to ${endpointId}`);
         }
         this.ssrcOwners.set(ssrc, endpointId);
     }
@@ -357,6 +368,25 @@ export default class SignalingLayerImpl extends SignalingLayer {
         return false;
     }
 
+    /**
+     * @inheritDoc
+     */
+    setTrackSourceName(ssrc, sourceName) {
+        if (typeof ssrc !== 'number') {
+            throw new TypeError(`SSRC(${ssrc}) must be a number`);
+        }
+
+        // Now signaling layer instance is shared between different JingleSessionPC instances, so although very unlikely
+        // an SSRC conflict could potentially occur. Log a message to make debugging easier.
+        const existingName = this._sourceNames.get(ssrc);
+
+        if (existingName && existingName !== sourceName) {
+            this._logOwnerChangedMessage(`SSRC(${ssrc}) sourceName re-assigned from ${existingName} to ${sourceName}`);
+        }
+
+        this._sourceNames.set(ssrc, sourceName);
+    }
+
     /**
      * @inheritDoc
      */
@@ -378,27 +408,16 @@ export default class SignalingLayerImpl extends SignalingLayer {
     /**
      * @inheritDoc
      */
-    getTrackSourceName(ssrc) {
-        return this._sourceNames.get(ssrc);
-    }
-
-    /**
-     * @inheritDoc
-     */
-    setTrackSourceName(ssrc, sourceName) {
-        if (typeof ssrc !== 'number') {
-            throw new TypeError(`SSRC(${ssrc}) must be a number`);
-        }
-
-        // Now signaling layer instance is shared between different JingleSessionPC instances, so although very unlikely
-        // an SSRC conflict could potentially occur. Log a message to make debugging easier.
-        const existingName = this._sourceNames.get(ssrc);
+    updateSsrcOwnersOnLeave(id) {
+        const ssrcs = Array.from(this.ssrcOwners)
+            .filter(entry => entry[1] === id)
+            .map(entry => entry[0]);
 
-        if (existingName && existingName !== sourceName) {
-            logger.error(`SSRC(${ssrc}) sourceName re-assigned from ${existingName} to ${sourceName}`);
+        if (!ssrcs?.length) {
+            return;
         }
 
-        this._sourceNames.set(ssrc, sourceName);
+        this.removeSSRCOwners(ssrcs);
     }
 
 }

service/RTC/SignalingLayer.js

@@ -88,16 +88,6 @@ export function getSourceIndexFromSourceName(sourceName) {
  * @interface SignalingLayer
  */
 export default class SignalingLayer extends Listenable {
-
-    /**
-     * Obtains the endpoint ID for given SSRC.
-     * @param {number} ssrc the SSRC number.
-     * @return {string|null} the endpoint ID for given media SSRC.
-     */
-    getSSRCOwner(ssrc) { // eslint-disable-line no-unused-vars
-        throw new Error('not implemented');
-    }
-
     /**
      * Obtains the info about given media advertised in the MUC presence of
      * the participant identified by the given MUC JID.
@@ -126,6 +116,15 @@ export default class SignalingLayer extends Listenable {
         throw new Error('not implemented');
     }
 
+    /**
+     * Obtains the endpoint ID for given SSRC.
+     * @param {number} ssrc the SSRC number.
+     * @return {string|null} the endpoint ID for given media SSRC.
+     */
+    getSSRCOwner(ssrc) { // eslint-disable-line no-unused-vars
+        throw new Error('not implemented');
+    }
+
     /**
      * Obtains the source name for given SSRC.
      * @param {number} ssrc the track's SSRC identifier.
@@ -135,6 +134,14 @@ export default class SignalingLayer extends Listenable {
         throw new Error('not implemented');
     }
 
+    /**
+     * Removes the association between a given SSRC and its current owner so that it can re-used when the SSRC gets
+     * remapped to another source from a different endpoint.
+     * @param {number} ssrc a list of SSRCs.
+     */
+    removeSSRCOwners(ssrcList) { // eslint-disable-line no-unused-vars
+    }
+
     /**
      * Set an SSRC owner.
      * @param {number} ssrc an SSRC to be owned
@@ -144,7 +151,6 @@ export default class SignalingLayer extends Listenable {
     setSSRCOwner(ssrc, endpointId) { // eslint-disable-line no-unused-vars
     }
 
-
     /**
      * Adjusts muted status of given track.
      *
@@ -172,4 +178,13 @@ export default class SignalingLayer extends Listenable {
      */
     setTrackVideoType(sourceName, videoType) { // eslint-disable-line no-unused-vars
     }
+
+    /**
+     * Removes the SSRCs associated with a given endpoint from the SSRC owners.
+     *
+     * @param {string} id endpoint id of the participant leaving the call.
+     * @returns {void}
+     */
+    updateSsrcOwnersOnLeave(id) { // eslint-disable-line no-unused-vars
+    }
 }

types/hand-crafted/modules/xmpp/SignalingLayerImpl.d.ts

@@ -7,5 +7,7 @@ declare class SignalingLayerImpl extends SignalingLayer {
   setChatRoom: ( room: ChatRoom ) => void;
   getPeerMediaInfo: ( owner: string, mediaType: MediaType ) => PeerMediaInfo | null;
   getSSRCOwner: ( ssrc: number ) => string | null;
+  removeSSRCOwners: (ssrcList: Array<number> ) => void;
   setSSRCOwner: ( ssrc: number, endpointId: string ) => void;
+  updateSsrcOwnersOnLeave: ( id: string ) => void;
 }