Fix codec negotiation and other multi-stream issues (#2133)

* fix(codec-selection) Munge all related m-lines for multi-stream.
Munge all the video m-lines to apply the preferred codec settings in multi-stream mode.

* fix(TPC) Cache transceiver mids for all local video tracks.
There will be multiple video tracks in multi-stream, we need to cache the mids for all the the associated transceivers.

* fix(sender-constraints) Do not apply sender constraints if value hasn't changed for a given source.

* fix(TPC) Configure the direction correctly for all m-lines.
Ignore negotiationneeded events for p2p to avoid going into a renegotiation loop.

* squash: Address review comments.

modules/RTC/TraceablePeerConnection.js

@@ -513,6 +513,21 @@ TraceablePeerConnection.prototype.getDesiredMediaDirection = function(mediaType,
     return MediaDirection.INACTIVE;
 };
 
+/**
+ * Returns the MID of the m-line associated with the local desktop track (if it exists).
+ *
+ * @returns {Number|null}
+ */
+TraceablePeerConnection.prototype._getDesktopTrackMid = function() {
+    const desktopTrack = this.getLocalVideoTracks().find(track => track.getVideoType() === VideoType.DESKTOP);
+
+    if (desktopTrack) {
+        return Number(this._localTrackTransceiverMids.get(desktopTrack.rtcId));
+    }
+
+    return null;
+};
+
 /**
  * Returns the list of RTCRtpReceivers created for the source of the given media type associated with
  * the set of remote endpoints specified.
@@ -1639,28 +1654,26 @@ TraceablePeerConnection.prototype._mungeCodecOrder = function(description) {
     }
 
     const parsedSdp = transform.parse(description.sdp);
+    const mLines = parsedSdp.media.filter(m => m.type === this.codecPreference.mediaType);
 
-    // Only the m-line that defines the source the browser will be sending should need to change.
-    // This is typically the first m-line with the matching media type.
-    const mLine = parsedSdp.media.find(m => m.type === this.codecPreference.mediaType);
-
-    if (!mLine) {
+    if (!mLines.length) {
         return description;
     }
 
-    if (this.codecPreference.enable) {
-        SDPUtil.preferCodec(mLine, this.codecPreference.mimeType);
-
-        // Strip the high profile H264 codecs on mobile clients for p2p connection.
-        // High profile codecs give better quality at the expense of higher load which
-        // we do not want on mobile clients.
-        // Jicofo offers only the baseline code for the jvb connection.
-        // TODO - add check for mobile browsers once js-utils provides that check.
-        if (this.codecPreference.mimeType === CodecMimeType.H264 && browser.isReactNative() && this.isP2P) {
-            SDPUtil.stripCodec(mLine, this.codecPreference.mimeType, true /* high profile */);
+    for (const mLine of mLines) {
+        if (this.codecPreference.enable) {
+            SDPUtil.preferCodec(mLine, this.codecPreference.mimeType);
+
+            // Strip the high profile H264 codecs on mobile clients for p2p connection. High profile codecs give better
+            // quality at the expense of higher load which we do not want on mobile clients. Jicofo offers only the
+            // baseline code for the jvb connection and therefore this is not needed for jvb connection.
+            // TODO - add check for mobile browsers once js-utils provides that check.
+            if (this.codecPreference.mimeType === CodecMimeType.H264 && browser.isReactNative() && this.isP2P) {
+                SDPUtil.stripCodec(mLine, this.codecPreference.mimeType, true /* high profile */);
+            }
+        } else {
+            SDPUtil.stripCodec(mLine, this.codecPreference.mimeType);
         }
-    } else {
-        SDPUtil.stripCodec(mLine, this.codecPreference.mimeType);
     }
 
     return new RTCSessionDescription({
@@ -1989,15 +2002,17 @@ TraceablePeerConnection.prototype.processLocalSdpForTransceiverInfo = function(l
         return;
     }
 
-    for (const localTrack of localTracks) {
-        const mediaType = localTrack.getType();
+    [ MediaType.AUDIO, MediaType.VIDEO ].forEach(mediaType => {
+        const tracks = localTracks.filter(t => t.getType() === mediaType);
         const parsedSdp = transform.parse(localSdp);
-        const mLine = parsedSdp.media.find(mline => mline.type === mediaType);
+        const mLines = parsedSdp.media.filter(mline => mline.type === mediaType);
 
-        if (!this._localTrackTransceiverMids.has(localTrack.rtcId)) {
-            this._localTrackTransceiverMids.set(localTrack.rtcId, mLine.mid.toString());
-        }
-    }
+        tracks.forEach((track, idx) => {
+            if (!this._localTrackTransceiverMids.has(track.rtcId)) {
+                this._localTrackTransceiverMids.set(track.rtcId, mLines[idx].mid.toString());
+            }
+        });
+    });
 };
 
 /**
@@ -2083,7 +2098,7 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) {
                     ? Promise.resolve()
                     : this.tpcUtils.setEncodings(newTrack);
 
-                return configureEncodingsPromise.then(() => false);
+                return configureEncodingsPromise.then(() => this.isP2P);
             });
     }
 
@@ -2240,15 +2255,31 @@ TraceablePeerConnection.prototype._adjustRemoteMediaDirection = function(remoteD
     const transformer = new SdpTransformWrap(remoteDescription.sdp);
 
     [ MediaType.AUDIO, MediaType.VIDEO ].forEach(mediaType => {
-        const media = transformer.selectMedia(mediaType)?.[0];
-        const hasLocalSource = this.hasAnyTracksOfType(mediaType);
-        const hasRemoteSource = this.getRemoteTracks(null, mediaType).length > 0;
-
-        media.direction = hasLocalSource && hasRemoteSource
-            ? MediaDirection.SENDRECV
-            : hasLocalSource
-                ? MediaDirection.RECVONLY
-                : hasRemoteSource ? MediaDirection.SENDONLY : MediaDirection.INACTIVE;
+        const media = transformer.selectMedia(mediaType);
+        const localSources = this.getLocalTracks(mediaType).length;
+        const remoteSources = this.getRemoteTracks(null, mediaType).length;
+
+        media.forEach((mLine, idx) => {
+            if (localSources && localSources === remoteSources) {
+                mLine.direction = MediaDirection.SENDRECV;
+            } else if (!localSources && !remoteSources) {
+                mLine.direction = MediaDirection.INACTIVE;
+            } else if (!localSources) {
+                mLine.direction = MediaDirection.SENDONLY;
+            } else if (!remoteSources) {
+                mLine.direction = MediaDirection.RECVONLY;
+
+            // When there are 2 local sources and 1 remote source, the first m-line should be set to 'sendrecv' while
+            // the second one needs to be set to 'recvonly'.
+            } else if (localSources > remoteSources) {
+                mLine.direction = idx ? MediaDirection.RECVONLY : MediaDirection.SENDRECV;
+
+            // When there are 2 remote sources and 1 local source, the first m-line should be set to 'sendrecv' while
+            // the second one needs to be set to 'sendonly'.
+            } else {
+                mLine.direction = idx ? MediaDirection.SENDONLY : MediaDirection.SENDRECV;
+            }
+        });
     });
 
     return new RTCSessionDescription({
@@ -2398,34 +2429,13 @@ TraceablePeerConnection.prototype._setVp9MaxBitrates = function(description, isL
         ? parsedSdp.media.filter(m => m.type === MediaType.VIDEO && m.direction !== direction)
         : [ parsedSdp.media.find(m => m.type === MediaType.VIDEO) ];
 
-    // Find the mid associated with the desktop track so that bitrates can be configured accordingly on the
-    // corresponding m-line.
-    const getDesktopTrackMid = () => {
-        const desktopTrack = this.getLocalVideoTracks().find(track => track.getVideoType() === VideoType.DESKTOP);
-        let mid;
-
-        if (desktopTrack) {
-            const trackIndex = Number(desktopTrack.getSourceName()?.split('-')[1].substring(1));
-
-            if (typeof trackIndex === 'number') {
-                const transceiver = this.peerconnection.getTransceivers()
-                    .filter(t => t.receiver.track.kind === MediaType.VIDEO
-                        && t.direction !== MediaDirection.RECVONLY)[trackIndex];
-
-                mid = transceiver?.mid;
-            }
-        }
-
-        return Number(mid);
-    };
-
     for (const mLine of mLines) {
         if (this.codecPreference.mimeType === CodecMimeType.VP9) {
             const bitrates = this.tpcUtils.videoBitrates.VP9 || this.tpcUtils.videoBitrates;
             const hdBitrate = bitrates.high ? bitrates.high : HD_BITRATE;
             const mid = mLine.mid;
             const isSharingScreen = FeatureFlags.isMultiStreamSupportEnabled()
-                ? mid === getDesktopTrackMid()
+                ? mid === this._getDesktopTrackMid()
                 : this._isSharingScreen();
             const limit = Math.floor((isSharingScreen ? HD_BITRATE : hdBitrate) / 1000);
 
@@ -2635,7 +2645,12 @@ TraceablePeerConnection.prototype.setSenderVideoConstraints = function(frameHeig
     }
 
     if (FeatureFlags.isSourceNameSignalingEnabled()) {
-        this._senderMaxHeights.set(localVideoTrack.getSourceName(), frameHeight);
+        const sourceName = localVideoTrack.getSourceName();
+
+        if (this._senderMaxHeights.get(sourceName) === frameHeight) {
+            return Promise.resolve();
+        }
+        this._senderMaxHeights.set(sourceName, frameHeight);
     } else {
         this._senderVideoMaxHeight = frameHeight;
     }

modules/qualitycontrol/SendVideoController.js

@@ -104,7 +104,11 @@ export default class SendVideoController {
                 if (track.getSourceName() === sourceName
                     && (!this._sourceSenderConstraints.has(sourceName)
                     || this._sourceSenderConstraints.get(sourceName) !== maxHeight)) {
-                    this._sourceSenderConstraints.set(sourceName, maxHeight);
+                    this._sourceSenderConstraints.set(
+                        sourceName,
+                        maxHeight === -1
+                            ? Math.min(MAX_LOCAL_RESOLUTION, this._preferredSendMaxFrameHeight)
+                            : maxHeight);
                     logger.debug(`Sender constraints for source:${sourceName} changed to maxHeight:${maxHeight}`);
                     this._propagateSendMaxFrameHeight(sourceName);
                 }

modules/xmpp/JingleSessionPC.js

@@ -636,6 +636,7 @@ export default class JingleSessionPC extends JingleSession {
             const remoteDescription = this.peerconnection.remoteDescription;
 
             if (this.usesUnifiedPlan
+                && !this.isP2P
                 && state === 'stable'
                 && remoteDescription
                 && typeof remoteDescription.sdp === 'string') {
@@ -1188,7 +1189,7 @@ export default class JingleSessionPC extends JingleSession {
             Promise.all(addTracks)
                 .then(() => this._responderRenegotiate(remoteDescription))
                 .then(() => {
-                    this.peerconnection.processLocalSdpForTransceiverInfo(localTracks);
+                    this.peerconnection.processLocalSdpForTransceiverInfo(tracks);
                     if (this.state === JingleSessionState.PENDING) {
                         this.state = JingleSessionState.ACTIVE;
 
@@ -2571,14 +2572,9 @@ export default class JingleSessionPC extends JingleSession {
     }
 
     /**
-     * Will put and execute on the queue a session modify task. Currently it
-     * only checks the senders attribute of the video content in order to figure
-     * out if the remote peer has video in the inactive state (stored locally
-     * in {@link _remoteVideoActive} - see field description for more info).
-     * @param {jQuery} jingleContents jQuery selector pointing to the jingle
-     * element of the session modify IQ.
-     * @see {@link _remoteVideoActive}
-     * @see {@link _localVideoActive}
+     * Will put and execute on the queue a session modify task. It checks if the sourceMaxFrameHeight (as requested by
+     * the p2p peer) or the senders attribute of the video content has changed and modifies the local video sources
+     * accordingly.
      */
     modifyContents(jingleContents) {
         const newVideoSenders = JingleSessionPC.parseVideoSenders(jingleContents);