Browse Source

fix(multi-stream) fix p2p issues with secondary video sources.

* fix(multi-stream) add new m-line when remote adds a second video source in p2p.
A new m-line needs to be added in the remote description when a new secondary video source is signaled by the peer.

* fix(TPC) Suppress lower layers only for screenshare tracks.
With multi-stream support enabled, an extra check for videoType for local track is needed to dtermine if the lower layers need to be suppressed.

* fix(TPC) Mark the direction inactive on inactive media connection.
If a new secondary video source is added on the jvb connection while p2p is active, make sure the direction is set to inactive on the jvb connection, so that no media is being sent out on the jvb connection.

* squash: Address review comments.
tags/v0.0.2
Jaya Allamsetty 3 years ago
parent
commit
7e997fb71b
No account linked to committer's email address
3 changed files with 40 additions and 16 deletions
  1. 4
    2
      modules/RTC/TPCUtils.js
  2. 5
    1
      modules/RTC/TraceablePeerConnection.js
  3. 31
    13
      modules/xmpp/JingleSessionPC.js

+ 4
- 2
modules/RTC/TPCUtils.js View File

@@ -298,6 +298,7 @@ export class TPCUtils {
298 298
             // that the highest resolution stream is available always. Safari is an exception here since it does not
299 299
             // send the desktop stream at all if only the high resolution stream is enabled.
300 300
             if (this.pc.isSharingLowFpsScreen()
301
+                && localVideoTrack.getVideoType() === VideoType.DESKTOP
301 302
                 && this.pc.usesUnifiedPlan()
302 303
                 && !browser.isWebKitBased()
303 304
                 && this.localStreamEncodingsConfig[idx].scaleResolutionDownBy !== HD_SCALE_FACTOR) {
@@ -451,8 +452,9 @@ export class TPCUtils {
451 452
         logger.info(`${this.pc} ${active ? 'Enabling' : 'Suspending'} ${mediaType} media transfer.`);
452 453
         transceivers.forEach((transceiver, idx) => {
453 454
             if (active) {
454
-                // The first transceiver is for the local track and only this one can be set to 'sendrecv'
455
-                if (idx === 0 && localTracks.length) {
455
+                // The first transceiver is for the local track and only this one can be set to 'sendrecv'.
456
+                // When multi-stream is enabled, there can be multiple transceivers with outbound streams.
457
+                if (idx < localTracks.length) {
456 458
                     transceiver.direction = MediaDirection.SENDRECV;
457 459
                 } else {
458 460
                     transceiver.direction = MediaDirection.RECVONLY;

+ 5
- 1
modules/RTC/TraceablePeerConnection.js View File

@@ -1977,7 +1977,9 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) {
1977 1977
     // Send the presence before signaling for a new screenshare source. This is needed for multi-stream support since
1978 1978
     // videoType needs to be availble at remote track creation time so that a fake tile for screenshare can be added.
1979 1979
     // FIXME - This check needs to be removed when the client switches to the bridge based signaling for tracks.
1980
-    const isNewTrackScreenshare = !oldTrack && newTrack?.getVideoType() === VideoType.DESKTOP;
1980
+    const isNewTrackScreenshare = !oldTrack
1981
+        && newTrack?.getVideoType() === VideoType.DESKTOP
1982
+        && FeatureFlags.isMultiStreamSupportEnabled();
1981 1983
     const negotiationNeeded = !isNewTrackScreenshare && Boolean(!oldTrack || !this.localTracks.has(oldTrack?.rtcId));
1982 1984
 
1983 1985
     if (this._usesUnifiedPlan) {
@@ -2015,6 +2017,8 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) {
2015 2017
                 // this connection again.
2016 2018
                 if (transceiver && mediaActive) {
2017 2019
                     transceiver.direction = newTrack ? MediaDirection.SENDRECV : MediaDirection.RECVONLY;
2020
+                } else if (transceiver) {
2021
+                    transceiver.direction = MediaDirection.INACTIVE;
2018 2022
                 }
2019 2023
 
2020 2024
                 // Avoid configuring the encodings on Chromium/Safari until simulcast is configured

+ 31
- 13
modules/xmpp/JingleSessionPC.js View File

@@ -1697,6 +1697,8 @@ export default class JingleSessionPC extends JingleSession {
1697 1697
                 });
1698 1698
             });
1699 1699
 
1700
+            let midFound = false;
1701
+
1700 1702
             /* eslint-enable no-invalid-this */
1701 1703
             currentRemoteSdp.media.forEach((media, i2) => {
1702 1704
                 if (!SDPUtil.findLine(media, `a=mid:${name}`)) {
@@ -1706,7 +1708,14 @@ export default class JingleSessionPC extends JingleSession {
1706 1708
                     addSsrcInfo[i2] = '';
1707 1709
                 }
1708 1710
                 addSsrcInfo[i2] += lines;
1711
+                midFound = true;
1709 1712
             });
1713
+
1714
+            // In p2p unified mode with multi-stream enabled, the new sources will have content name that doesn't exist
1715
+            // in the current remote description. Add a new m-line for this newly signaled source.
1716
+            if (!midFound && this.isP2P && FeatureFlags.isSourceNameSignalingEnabled()) {
1717
+                addSsrcInfo[name] = lines;
1718
+            }
1710 1719
         });
1711 1720
 
1712 1721
         return addSsrcInfo;
@@ -1929,8 +1938,17 @@ export default class JingleSessionPC extends JingleSession {
1929 1938
      *  in removeSsrcInfo
1930 1939
      */
1931 1940
     _processRemoteAddSource(addSsrcInfo) {
1932
-        const remoteSdp = new SDP(this.peerconnection.remoteDescription.sdp);
1933
-
1941
+        let remoteSdp = new SDP(this.peerconnection.remoteDescription.sdp);
1942
+
1943
+        // Add a new m-line in the remote description if the source info for a secondary video source is recceived from
1944
+        // the remote p2p peer when multi-stream support is enabled.
1945
+        if (addSsrcInfo.length > remoteSdp.media.length
1946
+            && FeatureFlags.isSourceNameSignalingEnabled()
1947
+            && this.isP2P
1948
+            && this.usesUnifiedPlan) {
1949
+            remoteSdp.addMlineForNewLocalSource(MediaType.VIDEO);
1950
+            remoteSdp = new SDP(remoteSdp.raw);
1951
+        }
1934 1952
         addSsrcInfo.forEach((lines, idx) => {
1935 1953
             remoteSdp.media[idx] += lines;
1936 1954
 
@@ -2635,17 +2653,18 @@ export default class JingleSessionPC extends JingleSession {
2635 2653
                 sid: this.sid
2636 2654
             }
2637 2655
             );
2638
-        const removedAnySSRCs = sdpDiffer.toJingle(remove);
2656
+
2657
+        sdpDiffer.toJingle(remove);
2639 2658
 
2640 2659
         // context a common object for one run of ssrc update (source-add and source-remove) so we can match them if we
2641 2660
         // need to
2642 2661
         const ctx = {};
2662
+        const removedSsrcInfo = getSignaledSourceInfo(sdpDiffer);
2643 2663
 
2644
-        if (removedAnySSRCs) {
2645
-            const sourceInfo = getSignaledSourceInfo(sdpDiffer);
2646
-
2664
+        if (removedSsrcInfo.ssrcs.length) {
2647 2665
             // Log only the SSRCs instead of the full IQ.
2648
-            logger.info(`${this} Sending source-remove for ${sourceInfo.mediaType} ssrcs=${sourceInfo.ssrcs}`);
2666
+            logger.info(`${this} Sending source-remove for ${removedSsrcInfo.mediaType}`
2667
+                + ` ssrcs=${removedSsrcInfo.ssrcs}`);
2649 2668
             this.connection.sendIQ(
2650 2669
                 remove,
2651 2670
                 () => {
@@ -2669,20 +2688,19 @@ export default class JingleSessionPC extends JingleSession {
2669 2688
             }
2670 2689
             );
2671 2690
 
2672
-        const containsNewSSRCs = sdpDiffer.toJingle(add);
2673
-
2674
-        if (containsNewSSRCs) {
2675
-            const sourceInfo = getSignaledSourceInfo(sdpDiffer);
2691
+        sdpDiffer.toJingle(add);
2692
+        const addedSsrcInfo = getSignaledSourceInfo(sdpDiffer);
2676 2693
 
2694
+        if (addedSsrcInfo.ssrcs.length) {
2677 2695
             // Log only the SSRCs instead of the full IQ.
2678
-            logger.info(`${this} Sending source-add for ${sourceInfo.mediaType} ssrcs=${sourceInfo.ssrcs}`);
2696
+            logger.info(`${this} Sending source-add for ${addedSsrcInfo.mediaType} ssrcs=${addedSsrcInfo.ssrcs}`);
2679 2697
             this.connection.sendIQ(
2680 2698
                 add,
2681 2699
                 () => {
2682 2700
                     this.room.eventEmitter.emit(XMPPEvents.SOURCE_ADD, this, ctx);
2683 2701
                 },
2684 2702
                 this.newJingleErrorHandler(add, error => {
2685
-                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_ADD_ERROR, this, error, sourceInfo.mediaType, ctx);
2703
+                    this.room.eventEmitter.emit(XMPPEvents.SOURCE_ADD_ERROR, this, error, addedSsrcInfo.mediaType, ctx);
2686 2704
                 }),
2687 2705
                 IQ_TIMEOUT);
2688 2706
         }

Loading…
Cancel
Save