fix(TPC): Do not remove ssrcs from remote desc for p2p.

In unified plan, re-use of m-line (i.e., adding an SSRC, removing it and then adding it back) causes the browser to not render the media on Chrome and Safari. The WebRTC spec is not clear as to how browsers have to behave, this doesn't cause any issues on Firefox. As a workaround, only change the media direction and leave the ssrc in the remote desc. This automatically triggers a 'removetrack' event on the associated MediaStream and the track can be removed from the UI.

modules/RTC/MockClasses.js

@@ -4,6 +4,18 @@
  * Mock {@link TraceablePeerConnection} - add things as needed, but only things useful for all tests.
  */
 export class MockPeerConnection {
+
+    /**
+     * Constructor.
+     *
+     * @param {string} id RTC id
+     * @param {boolean} usesUnifiedPlan
+     */
+    constructor(id, usesUnifiedPlan) {
+        this.id = id;
+        this._usesUnifiedPlan = usesUnifiedPlan;
+    }
+
     /**
      * {@link TraceablePeerConnection.localDescription}.
      *
@@ -65,6 +77,13 @@ export class MockPeerConnection {
     setVideoTransferActive() {
         return false;
     }
+
+    /**
+     * {@link TraceablePeerConnection.usesUnifiedPlan}.
+     */
+    usesUnifiedPlan() {
+        return this._usesUnifiedPlan;
+    }
 }
 
 /**

modules/RTC/TraceablePeerConnection.js

@@ -1972,8 +1972,8 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) {
 
         return this.tpcUtils.replaceTrack(oldTrack, newTrack)
 
-            // renegotiate when SDP is used for simulcast munging
-            .then(() => this.isSimulcastOn() && browser.usesSdpMungingForSimulcast());
+            // Renegotiate when SDP is used for simulcast munging or when in p2p mode.
+            .then(() => (this.isSimulcastOn() && browser.usesSdpMungingForSimulcast()) || this.isP2P);
     }
 
     logger.debug(`${this} TPC.replaceTrack using plan B`);
@@ -3043,6 +3043,15 @@ TraceablePeerConnection.prototype.generateNewStreamSSRCInfo = function(track) {
     return ssrcInfo;
 };
 
+/**
+ * Returns if the peer connection uses Unified plan implementation.
+ *
+ * @returns {boolean} True if the pc uses Unified plan, false otherwise.
+ */
+TraceablePeerConnection.prototype.usesUnifiedPlan = function() {
+    return this._usesUnifiedPlan;
+};
+
 /**
  * Creates a text representation of this <tt>TraceablePeerConnection</tt>
  * instance.

modules/sdp/LocalSdpMunger.js

@@ -218,21 +218,23 @@ export default class LocalSdpMunger {
             }
         }
 
+        // Additional transformations related to MSID are applicable to Unified-plan implementation only.
+        if (!this.tpc.usesUnifiedPlan()) {
+            return;
+        }
+
         // If the msid attribute is missing, then remove the ssrc from the transformed description so that a
         // source-remove is signaled to Jicofo. This happens when the direction of the transceiver (or m-line)
         // is set to 'inactive' or 'recvonly' on Firefox, Chrome (unified) and Safari.
-        const msid = mediaSection.ssrcs.find(s => s.attribute === 'msid');
+        const mediaDirection = mediaSection.mLine?.direction;
 
-        if (!this.tpc.isP2P
-            && (!msid
-                || mediaSection.mLine?.direction === MediaDirection.RECVONLY
-                || mediaSection.mLine?.direction === MediaDirection.INACTIVE)) {
+        if (mediaDirection === MediaDirection.RECVONLY || mediaDirection === MediaDirection.INACTIVE) {
             mediaSection.ssrcs = undefined;
             mediaSection.ssrcGroups = undefined;
 
-        // Add the msid attribute if it is missing for p2p sources. Firefox doesn't produce a a=ssrc line
-        // with msid attribute.
-        } else if (this.tpc.isP2P && mediaSection.mLine?.direction === MediaDirection.SENDRECV) {
+        // Add the msid attribute if it is missing when the direction is sendrecv/sendonly. Firefox doesn't produce a
+        // a=ssrc line with msid attribute for p2p connection.
+        } else {
             const msidLine = mediaSection.mLine?.msid;
             const trackId = msidLine && msidLine.split(' ')[1];
             const sources = [ ...new Set(mediaSection.mLine?.ssrcs?.map(s => s.id)) ];

modules/sdp/LocalSdpMunger.spec.js

@@ -1,6 +1,8 @@
 
 import * as transform from 'sdp-transform';
 
+import { MockPeerConnection } from '../RTC/MockClasses';
+
 import LocalSdpMunger from './LocalSdpMunger';
 import { default as SampleSdpStrings } from './SampleSdpStrings.js';
 
@@ -17,9 +19,9 @@ function getSsrcLines(desc, mediaType) {
     return mline.ssrcs ?? [];
 }
 
-describe('TransformRecvOnly', () => {
+describe('TransformSdpsForUnifiedPlan', () => {
     let localSdpMunger;
-    const tpc = { id: '1' };
+    const tpc = new MockPeerConnection('1', true);
     const localEndpointId = 'sRdpsdg';
 
     beforeEach(() => {
@@ -58,7 +60,9 @@ describe('TransformRecvOnly', () => {
             expect(audioSsrcs.length).toEqual(4);
             expect(videoSsrcs.length).toEqual(6);
         });
+    });
 
+    describe('addMsids', () => {
         it('should add endpointId to msid', () => {
             const sdpStr = transform.write(SampleSdpStrings.firefoxSdp);
             const desc = new RTCSessionDescription({
@@ -79,7 +83,7 @@ describe('TransformRecvOnly', () => {
             }
         });
 
-        it('should add msid', () => {
+        it('should add missing msid', () => {
             // P2P case only.
             localSdpMunger.tpc.isP2P = true;
 
@@ -97,3 +101,32 @@ describe('TransformRecvOnly', () => {
         });
     });
 });
+
+describe('DoNotTransformSdpForPlanB', () => {
+    let localSdpMunger;
+    const tpc = new MockPeerConnection('1', false);
+    const localEndpointId = 'sRdpsdg';
+
+    beforeEach(() => {
+        localSdpMunger = new LocalSdpMunger(tpc, localEndpointId);
+    });
+    describe('stripSsrcs', () => {
+        beforeEach(() => { }); // eslint-disable-line no-empty-function
+        it('should not strip ssrcs from an sdp with no msid', () => {
+            localSdpMunger.tpc.isP2P = false;
+
+            const sdpStr = transform.write(SampleSdpStrings.recvOnlySdp);
+            const desc = new RTCSessionDescription({
+                type: 'offer',
+                sdp: sdpStr
+            });
+            const transformedDesc = localSdpMunger.transformStreamIdentifiers(desc);
+            const newSdp = transform.parse(transformedDesc.sdp);
+            const audioSsrcs = getSsrcLines(newSdp, 'audio');
+            const videoSsrcs = getSsrcLines(newSdp, 'video');
+
+            expect(audioSsrcs.length).toEqual(1);
+            expect(videoSsrcs.length).toEqual(1);
+        });
+    });
+});

modules/xmpp/JingleSessionPC.js

@@ -338,7 +338,10 @@ export default class JingleSessionPC extends JingleSession {
             = browser.supportsUnifiedPlan()
                 && (browser.isFirefox()
                     || browser.isWebKitBased()
-                    || (!this.isP2P && browser.isChromiumBased() && options.enableUnifiedOnChrome));
+                    || (browser.isChromiumBased()
+
+                        // Provide a way to control the behavior for jvb and p2p connections independently.
+                        && this.isP2P ? options.p2p?.enableUnifiedOnChrome : options.enableUnifiedOnChrome));
 
         if (this.isP2P) {
             // simulcast needs to be disabled for P2P (121) calls
@@ -1586,6 +1589,7 @@ export default class JingleSessionPC extends JingleSession {
      */
     _parseSsrcInfoFromSourceAdd(sourceAddElem, currentRemoteSdp) {
         const addSsrcInfo = [];
+        const self = this;
 
         $(sourceAddElem).each((i1, content) => {
             const name = $(content).attr('name');
@@ -1605,10 +1609,8 @@ export default class JingleSessionPC extends JingleSession {
                             })
                             .get();
 
-                    if (ssrcs.length) {
-                        lines
-                            += `a=ssrc-group:${semantics} ${
-                                ssrcs.join(' ')}\r\n`;
+                    if (ssrcs.length && !currentRemoteSdp.containsSSRC(ssrcs[0])) {
+                        lines += `a=ssrc-group:${semantics} ${ssrcs.join(' ')}\r\n`;
                     }
                 });
 
@@ -1622,7 +1624,10 @@ export default class JingleSessionPC extends JingleSession {
                 const ssrc = $(this).attr('ssrc');
 
                 if (currentRemoteSdp.containsSSRC(ssrc)) {
-                    logger.warn(`${this} Source-add request for existing SSRC: ${ssrc}`);
+
+                    // Do not print the warning for unified plan p2p case since ssrcs are never removed from the SDP.
+                    !(self.usesUnifiedPlan && self.isP2P)
+                        && logger.warn(`${self} Source-add request for existing SSRC: ${ssrc}`);
 
                     return;
                 }
@@ -1820,7 +1825,17 @@ export default class JingleSessionPC extends JingleSession {
                     const mid = remoteSdp.media.findIndex(mLine => mLine.includes(line));
 
                     if (mid > -1) {
-                        remoteSdp.media[mid] = remoteSdp.media[mid].replace(`${line}\r\n`, '');
+                        // Remove the ssrcs from the m-line in
+                        // 1. Plan-b mode always.
+                        // 2. Unified mode but only for jvb connection. In p2p mode if the ssrc is removed and added
+                        // back to the same m-line, Chrome/Safari do not render the media even if it being received
+                        // and decoded from the remote peer. The webrtc spec is not clear about m-line re-use and
+                        // the browser vendors have implemented this differently. Currently workaround this by changing
+                        // the media direction, that should be enough for the browser to fire the "removetrack" event
+                        // on the associated MediaStream.
+                        if (!this.usesUnifiedPlan || (this.usesUnifiedPlan && !this.isP2P)) {
+                            remoteSdp.media[mid] = remoteSdp.media[mid].replace(`${line}\r\n`, '');
+                        }
 
                         // The current direction of the transceiver for p2p will depend on whether a local sources is
                         // added or not. It will be 'sendrecv' if the local source is present, 'sendonly' otherwise.
@@ -1866,8 +1881,8 @@ export default class JingleSessionPC extends JingleSession {
         addSsrcInfo.forEach((lines, idx) => {
             remoteSdp.media[idx] += lines;
 
-            // Make sure to change the direction to 'sendrecv' only for p2p connections. For jvb connections, a new
-            // m-line is added for the new remote sources.
+            // Make sure to change the direction to 'sendrecv/sendonly' only for p2p connections. For jvb connections,
+            // a new m-line is added for the new remote sources.
             if (this.isP2P && this.usesUnifiedPlan) {
                 const mediaType = SDPUtil.parseMLine(remoteSdp.media[idx].split('\r\n')[0])?.media;
                 const desiredDirection = this.peerconnection.getDesiredMediaDirection(mediaType, true);