fix(SDPDiffer) Check explicitly for ssrc changes

jitsi · Oct 11, 2024 · d562bbe · d562bbe
1 parent 6466202
commit d562bbe
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 2 deletions.
diff --git a/modules/sdp/SDPDiffer.js b/modules/sdp/SDPDiffer.js
@@ -35,7 +35,15 @@ export class SDPDiffer {
         for (const [ index, othersSource ] of othersSources.entries()) {
             const mySource = mySources.get(index);
 
-            if (!mySource || !isEqual(mySource, othersSource)) {
+            if (!mySource) {
+                diff[index] = othersSource;
+                continue; // eslint-disable-line no-continue
+            }
+
+            const mySsrcs = Object.keys(mySource.ssrcs);
+            const othersSsrcs = Object.keys(othersSource.ssrcs);
+
+            if (othersSsrcs.length && !isEqual([ ...mySsrcs ].sort(), [ ...othersSsrcs ].sort())) {
                 diff[index] = othersSource;
             }
         }

diff --git a/modules/sdp/SDPDiffer.spec.js b/modules/sdp/SDPDiffer.spec.js
@@ -4,6 +4,7 @@ import FeatureFlags from '../flags/FeatureFlags';
 
 import SDP from './SDP';
 import { SDPDiffer } from './SDPDiffer';
+import SampleSdpStrings from './SampleSdpStrings';
 
 /* eslint-disable max-len*/
 
@@ -278,4 +279,51 @@ describe('SDPDiffer', () => {
             expect(addedVideoSourceGroups.length).toBe(1);
         });
     });
+
+    describe('getNewMedia', () => {
+        it(' should generate sources for source-remove when SSCRs are missing from the new SDP', () => {
+            const oldSdp = new SDP(SampleSdpStrings.simulcastSdpStr);
+            const newSdp = new SDP(SampleSdpStrings.recvOnlySdpStrChrome);
+
+            let sdpDiffer = new SDPDiffer(newSdp, oldSdp, false);
+            let diff = sdpDiffer.getNewMedia();
+
+            // There should be 2 sources for source-remove.
+            expect(Object.keys(diff).length).toBe(2);
+
+            sdpDiffer = new SDPDiffer(oldSdp, newSdp, false);
+            diff = sdpDiffer.getNewMedia();
+
+            // There should zero sources for source-add.
+            expect(Object.keys(diff).length).toBe(0);
+        });
+
+        it(' should not generate sources for source-remove or source-add if the SDP does not change', () => {
+            const oldSdp = new SDP(SampleSdpStrings.simulcastSdpStr);
+            const newSdp = new SDP(SampleSdpStrings.simulcastSdpStr);
+
+            const sdpDiffer = new SDPDiffer(newSdp, oldSdp, false);
+            const diff = sdpDiffer.getNewMedia();
+
+            // There should be zero sources in diff.
+            expect(Object.keys(diff).length).toBe(0);
+        });
+
+        it(' should generate sources for source-remove and source-add when SSRC changes', () => {
+            const oldSdp = new SDP(SampleSdpStrings.simulcastSdpStr);
+            const newSdp = new SDP(SampleSdpStrings.simulcastDifferentSsrcSdpStr);
+
+            let sdpDiffer = new SDPDiffer(newSdp, oldSdp, false);
+            let diff = sdpDiffer.getNewMedia();
+
+            // There should be 1 source for source-remove.
+            expect(Object.keys(diff).length).toBe(1);
+
+            sdpDiffer = new SDPDiffer(oldSdp, newSdp, false);
+            diff = sdpDiffer.getNewMedia();
+
+            // There should 1 source for source-add.
+            expect(Object.keys(diff).length).toBe(1);
+        });
+    });
 });
diff --git a/modules/sdp/SampleSdpStrings.js b/modules/sdp/SampleSdpStrings.js
@@ -36,6 +36,31 @@ const baseAudioMLineSdp = ''
 + 'a=ssrc:124723944 label:40abf2d3-a415-4c68-8c17-2a038e8bebcf\r\n'
 + 'a=rtcp-mux\r\n';
 
+const baseAudioMLineSdpDifferentSSRC = ''
++ 'm=audio 54405 RTP/SAVPF 111 103 104 126\r\n'
++ 'c=IN IP4 172.29.32.39\r\n'
++ 'a=rtpmap:111 opus/48000/2\r\n'
++ 'a=rtpmap:103 ISAC/16000\r\n'
++ 'a=rtpmap:104 ISAC/32000\r\n'
++ 'a=rtpmap:126 telephone-event/8000\r\n'
++ 'a=fmtp:111 minptime=10;useinbandfec=1\r\n'
++ 'a=rtcp:9 IN IP4 0.0.0.0\r\n'
++ 'a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level\r\n'
++ 'a=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\n'
++ 'a=setup:passive\r\n'
++ 'a=mid:audio\r\n'
++ 'a=msid:- 40abf2d3-a415-4c68-8c17-2a038e8bebcf\r\n'
++ 'a=sendrecv\r\n'
++ 'a=ice-ufrag:adPg\r\n'
++ 'a=ice-pwd:Xsr05Mq8S7CR44DAnusZE26F\r\n'
++ 'a=fingerprint:sha-256 6A:39:DE:11:24:AD:2E:4E:63:D6:69:D3:85:05:53:C7:3C:38:A4:B7:91:74:C0:91:44:FC:94:63:7F:01:AB:A9\r\n'
++ 'a=candidate:1581043602 1 udp 2122260223 172.29.32.39 54405 typ host generation 0\r\n'
++ 'a=ssrc:1757014965 cname:peDGrDD6WsxUOki/\r\n'
++ 'a=ssrc:1757014965 msid:dcbb0236-cea5-402e-9e9a-595c65ffcc2a 40abf2d3-a415-4c68-8c17-2a038e8bebcf\r\n'
++ 'a=ssrc:1757014965 mslabel:dcbb0236-cea5-402e-9e9a-595c65ffcc2a\r\n'
++ 'a=ssrc:1757014965 label:40abf2d3-a415-4c68-8c17-2a038e8bebcf\r\n'
++ 'a=rtcp-mux\r\n';
+
 // A basic sdp application mline
 const baseDataMLineSdp = ''
 + 'm=application 9 UDP/DTLS/SCTP webrtc-datachannel\r\n'
@@ -324,6 +349,26 @@ const recvOnlyAudioMline = ''
 + 'a=ssrc:124723944 cname:peDGrDD6WsxUOki\r\n'
 + 'a=rtcp-mux\r\n';
 
+const recvOnlyAudioMlineChrome = ''
++ 'm=audio 54405 RTP/SAVPF 111 103 104 126\r\n'
++ 'c=IN IP4 172.29.32.39\r\n'
++ 'a=rtpmap:111 opus/48000/2\r\n'
++ 'a=rtpmap:103 ISAC/16000\r\n'
++ 'a=rtpmap:104 ISAC/32000\r\n'
++ 'a=rtpmap:126 telephone-event/8000\r\n'
++ 'a=fmtp:111 minptime=10;useinbandfec=1\r\n'
++ 'a=rtcp:9 IN IP4 0.0.0.0\r\n'
++ 'a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level\r\n'
++ 'a=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\n'
++ 'a=setup:passive\r\n'
++ 'a=mid:audio\r\n'
++ 'a=recvonly\r\n'
++ 'a=ice-ufrag:adPg\r\n'
++ 'a=ice-pwd:Xsr05Mq8S7CR44DAnusZE26F\r\n'
++ 'a=fingerprint:sha-256 6A:39:DE:11:24:AD:2E:4E:63:D6:69:D3:85:05:53:C7:3C:38:A4:B7:91:74:C0:91:44:FC:94:63:7F:01:AB:A9\r\n'
++ 'a=candidate:1581043602 1 udp 2122260223 172.29.32.39 54405 typ host generation 0\r\n'
++ 'a=rtcp-mux\r\n';
+
 const recvOnlyVideoMline = ''
 + 'm=video 9 RTP/SAVPF 96 97 98 99 102 121 127 120\r\n'
 + 'c=IN IP4 0.0.0.0\r\n'
@@ -372,6 +417,53 @@ const recvOnlyVideoMline = ''
 + 'a=ssrc:1757014965 cname:peDGrDD6WsxUOki\r\n'
 + 'a=rtcp-mux\r\n';
 
+const recvOnlyVideoMlineChrome = ''
++ 'm=video 9 RTP/SAVPF 96 97 98 99 102 121 127 120\r\n'
++ 'c=IN IP4 0.0.0.0\r\n'
++ 'a=rtpmap:96 VP8/90000\r\n'
++ 'a=rtpmap:97 rtx/90000\r\n'
++ 'a=rtpmap:98 VP9/90000\r\n'
++ 'a=rtpmap:99 rtx/90000\r\n'
++ 'a=rtpmap:102 H264/90000\r\n'
++ 'a=rtpmap:121 rtx/90000\r\n'
++ 'a=rtpmap:127 H264/90000\r\n'
++ 'a=rtpmap:120 rtx/90000\r\n'
++ 'a=rtcp:9 IN IP4 0.0.0.0\r\n'
++ 'a=rtcp-fb:96 ccm fir\r\n'
++ 'a=rtcp-fb:96 transport-cc\r\n'
++ 'a=rtcp-fb:96 nack\r\n'
++ 'a=rtcp-fb:96 nack pli\r\n'
++ 'a=rtcp-fb:96 goog-remb\r\n'
++ 'a=rtcp-fb:98 ccm fir\r\n'
++ 'a=rtcp-fb:98 transport-cc\r\n'
++ 'a=rtcp-fb:98 nack\r\n'
++ 'a=rtcp-fb:98 nack pli\r\n'
++ 'a=rtcp-fb:98 goog-remb\r\n'
++ 'a=rtcp-fb:102 ccm fir\r\n'
++ 'a=rtcp-fb:102 transport-cc\r\n'
++ 'a=rtcp-fb:102 nack\r\n'
++ 'a=rtcp-fb:102 nack pli\r\n'
++ 'a=rtcp-fb:102 goog-remb\r\n'
++ 'a=rtcp-fb:127 ccm fir\r\n'
++ 'a=rtcp-fb:127 transport-cc\r\n'
++ 'a=rtcp-fb:127 nack\r\n'
++ 'a=rtcp-fb:127 nack pli\r\n'
++ 'a=rtcp-fb:127 goog-remb\r\n'
++ 'a=fmtp:97 apt=96\r\n'
++ 'a=fmtp:98 profile-id=0\r\n'
++ 'a=fmtp:102 profile-level-id=42e01f;level-asymmetry-allowed=1;packetization-mode=1\r\n'
++ 'a=fmtp:121 apt=102\r\n'
++ 'a=fmtp:127 profile-level-id=42e01f;level-asymmetry-allowed=1:packetization-mode=0\r\n'
++ 'a=fmtp:120 apt=127\r\n'
++ 'a=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\n'
++ 'a=setup:passive\r\n'
++ 'a=mid:video\r\n'
++ 'a=recvonly\r\n'
++ 'a=ice-ufrag:adPg\r\n'
++ 'a=ice-pwd:Xsr05Mq8S7CR44DAnusZE26F\r\n'
++ 'a=fingerprint:sha-256 6A:39:DE:11:24:AD:2E:4E:63:D6:69:D3:85:05:53:C7:3C:38:A4:B7:91:74:C0:91:44:FC:94:63:7F:01:AB:A9\r\n'
++ 'a=rtcp-mux\r\n';
+
 const videoMlineFF = ''
 + 'm=video 9 RTP/SAVPF 100 96\r\n'
 + 'c=IN IP4 0.0.0.0\r\n'
@@ -443,6 +535,8 @@ const simulcastVideoMLineNoRtxSdp = ''
 // A full sdp string representing a client doing simulcast
 const simulcastSdpStr = baseSessionSdp + baseAudioMLineSdp + simulcastVideoMLineSdp + baseDataMLineSdp;
 
+const simulcastDifferentSsrcSdpStr = baseSessionSdp + baseAudioMLineSdpDifferentSSRC + simulcastVideoMLineSdp + baseDataMLineSdp;
+
 // A full sdp string representing a remote client doing simucast when RTX is not negotiated with the jvb.
 const simulcastNoRtxSdpStr = baseSessionSdp + baseAudioMLineSdp + simulcastVideoMLineNoRtxSdp;
 
@@ -465,7 +559,10 @@ const multiCodecVideoSdpStr = baseSessionSdp + baseAudioMLineSdp + multiCodecVid
 const flexFecSdpStr = baseSessionSdp + baseAudioMLineSdp + flexFecVideoMLineSdp + baseDataMLineSdp;
 
 // A full sdp string representing a client that doesn't have local sources added on Firefox.
-const recvOnlySdpStr = baseSessionSdp + recvOnlyAudioMline + recvOnlyVideoMline;
+const recvOnlySdpStr = baseSessionSdp + recvOnlyAudioMline + recvOnlyVideoMline + baseDataMLineSdp;
+
+// A full sdp string representing a client that doesn't have local sources added on Chrome.
+const recvOnlySdpStrChrome = baseSessionSdp + recvOnlyAudioMlineChrome + recvOnlyVideoMlineChrome + baseDataMLineSdp;
 
 // A full sdp string representing a Firefox client with msid set to '-'.
 const sdpFirefoxStr = baseSessionSdp + baseAudioMLineSdp + videoMlineFF;
@@ -474,6 +571,14 @@ const sdpFirefoxStr = baseSessionSdp + baseAudioMLineSdp + videoMlineFF;
 const sdpFirefoxP2pStr = baseSessionSdp + baseAudioMLineSdp + videoLineP2pFF;
 
 export default {
+    get simulcastSdpStr() {
+        return simulcastSdpStr;
+    },
+
+    get simulcastDifferentSsrcSdpStr() {
+        return simulcastDifferentSsrcSdpStr;
+    },
+
     get simulcastSdp() {
         return transform.parse(simulcastSdpStr);
     },
@@ -506,6 +611,14 @@ export default {
         return transform.parse(flexFecSdpStr);
     },
 
+    get recvOnlySdpStr() {
+        return recvOnlySdpStr;
+    },
+
+    get recvOnlySdpStrChrome() {
+        return recvOnlySdpStrChrome;
+    },
+
     get recvOnlySdp() {
         return transform.parse(recvOnlySdpStr);
     },