Video suspend abtest (#634)

* revert: Reverts be665cbff7.

* ref: Renames "peerjid".

* ref: Refactors the initialization of a peer connection.

* feat: Re-implements the A/B test for the "suspend video" feature.

* squash: Deep copy.

* ref: Renames forceSuspendVideo to abtestSuspendVideo.

JitsiConference.js

@@ -1370,7 +1370,7 @@ JitsiConference.prototype.onIncomingCall = function(
         now) {
     // Handle incoming P2P call
     if (jingleSession.isP2P) {
-        const role = this.room.getMemberRole(jingleSession.peerjid);
+        const role = this.room.getMemberRole(jingleSession.remoteJid);
 
         if (role !== 'moderator') {
             // Reject incoming P2P call
@@ -1404,7 +1404,7 @@ JitsiConference.prototype.onIncomingCall = function(
         }
 
         return;
-    } else if (!this.room.isFocus(jingleSession.peerjid)) {
+    } else if (!this.room.isFocus(jingleSession.remoteJid)) {
         this._rejectIncomingCall(jingleSession);
 
         return;
@@ -1535,7 +1535,7 @@ JitsiConference.prototype._rejectIncomingCallNonModerator = function(
             reasonTag: 'security-error',
             reasonMsg: 'Only focus can start new sessions',
             errorMsg: 'Rejecting session-initiate from non-focus and'
-                        + `non-moderator user: ${jingleSession.peerjid}`
+                        + `non-moderator user: ${jingleSession.remoteJid}`
         });
 };
 
@@ -1625,7 +1625,7 @@ JitsiConference.prototype.onCallEnded = function(
         logger.error(
             'Received onCallEnded for invalid session',
             jingleSession.sid,
-            jingleSession.peerjid,
+            jingleSession.remoteJid,
             reasonCondition,
             reasonText);
     }
@@ -2120,7 +2120,7 @@ JitsiConference.prototype._acceptP2PIncomingCall = function(
 
     logger.info('Starting CallStats for P2P connection...');
 
-    let remoteID = Strophe.getResourceFromJid(this.p2pJingleSession.peerjid);
+    let remoteID = Strophe.getResourceFromJid(this.p2pJingleSession.remoteJid);
 
     if (this.options.config.enableStatsID) {
         const participant = this.participants[remoteID];
@@ -2394,10 +2394,10 @@ JitsiConference.prototype._setP2PStatus = function(newStatus) {
 
 /**
  * Starts new P2P session.
- * @param {string} peerJid the JID of the remote participant
+ * @param {string} remoteJid the JID of the remote participant
  * @private
  */
-JitsiConference.prototype._startP2PSession = function(peerJid) {
+JitsiConference.prototype._startP2PSession = function(remoteJid) {
     this._maybeClearDeferredStartP2P();
     if (this.p2pJingleSession) {
         logger.error('P2P session already started!');
@@ -2409,14 +2409,15 @@ JitsiConference.prototype._startP2PSession = function(peerJid) {
     this.p2pJingleSession
         = this.xmpp.connection.jingle.newP2PJingleSession(
             this.room.myroomjid,
-            peerJid);
-    logger.info('Created new P2P JingleSession', this.room.myroomjid, peerJid);
+            remoteJid);
+    logger.info(
+        'Created new P2P JingleSession', this.room.myroomjid, remoteJid);
 
     this.p2pJingleSession.initialize(true /* initiator */, this.room, this.rtc);
 
     logger.info('Starting CallStats for P2P connection...');
 
-    let remoteID = Strophe.getResourceFromJid(this.p2pJingleSession.peerjid);
+    let remoteID = Strophe.getResourceFromJid(this.p2pJingleSession.remoteJid);
 
     if (this.options.config.enableStatsID) {
         const participant = this.participants[remoteID];
@@ -2526,7 +2527,7 @@ JitsiConference.prototype._maybeStartOrStopP2P = function(userLeftEvent) {
             this._startP2PSession(jid);
         }
     } else if (this.p2pJingleSession && !shouldBeInP2P) {
-        logger.info(`Will stop P2P with: ${this.p2pJingleSession.peerjid}`);
+        logger.info(`Will stop P2P with: ${this.p2pJingleSession.remoteJid}`);
 
         // Log that there will be a switch back to the JVB connection
         if (this.p2pJingleSession.isInitiator && peerCount > 1) {
@@ -2600,7 +2601,7 @@ JitsiConference.prototype._stopP2PSession = function(
                 ? reasonDescription : 'Turing off P2P session',
             sendSessionTerminate: this.room
                 && this.getParticipantById(
-                    Strophe.getResourceFromJid(this.p2pJingleSession.peerjid))
+                    Strophe.getResourceFromJid(this.p2pJingleSession.remoteJid))
         });
 
     this.p2pJingleSession = null;

modules/RTC/RTC.js

@@ -13,6 +13,7 @@ import * as MediaType from '../../service/RTC/MediaType';
 import RTCBrowserType from './RTCBrowserType';
 import RTCEvents from '../../service/RTC/RTCEvents';
 import RTCUtils from './RTCUtils';
+import Statistics from '../statistics/statistics';
 import TraceablePeerConnection from './TraceablePeerConnection';
 import VideoType from '../../service/RTC/VideoType';
 
@@ -433,12 +434,22 @@ export default class RTC extends Listenable {
      * @return {TraceablePeerConnection}
      */
     createPeerConnection(signaling, iceConfig, isP2P, options) {
+        const pcConstraints = RTC.getPCConstraints(isP2P);
+
+        if (typeof options.abtestSuspendVideo !== 'undefined') {
+            RTCUtils.setSuspendVideo(pcConstraints, options.abtestSuspendVideo);
+
+            Statistics.analytics.addPermanentProperties(
+                { abtestSuspendVideo: options.abtestSuspendVideo });
+        }
+
         const newConnection
             = new TraceablePeerConnection(
                 this,
                 this.peerConnectionIdCounter,
                 signaling,
-                iceConfig, RTC.getPCConstraints(isP2P), isP2P, options);
+                iceConfig, pcConstraints,
+                isP2P, options);
 
         this.peerConnections.set(newConnection.id, newConnection);
         this.peerConnectionIdCounter += 1;
@@ -610,7 +621,10 @@ export default class RTC extends Listenable {
      *
      */
     static getPCConstraints(isP2P) {
-        return isP2P ? RTCUtils.p2pPcConstraints : RTCUtils.pcConstraints;
+        const pcConstraints
+            = isP2P ? RTCUtils.p2pPcConstraints : RTCUtils.pcConstraints;
+
+        return JSON.parse(JSON.stringify(pcConstraints));
     }
 
     /**

modules/RTC/RTCUtils.js

@@ -1117,36 +1117,10 @@ class RTCUtils extends Listenable {
                     { googSuspendBelowMinBitrate: true });
             }
 
-            /**
-             * This option is used to enable the suspend video only for
-             * part of the users on the P2P peer connection. The value of
-             * the option is the ratio:
-             * (users with suspended video enabled)/(all users).
-             *
-             * Note: The option is not documented because it is temporary
-             * and only for internal testing purpose.
-             *
-             * @type {number}
-             */
-            const forceP2PSuspendVideoRatio
-                = (options.testing || {}).forceP2PSuspendVideoRatio;
-
-            // If <tt>forceP2PSuspendVideoRatio</tt> is invalid (not a
-            // number) fallback to the default behavior (enabled for every
-            // user).
-            if (typeof forceP2PSuspendVideoRatio !== 'number'
-                    || Math.random() < forceP2PSuspendVideoRatio) {
-                logger.info(`Enable suspend video mode for p2p (ratio=${
-                    forceP2PSuspendVideoRatio})`);
-
-                Statistics.analytics.addPermanentProperties({
-                    forceP2PSuspendVideo: true
-                });
-
-                this.p2pPcConstraints.optional.push({
-                    googSuspendBelowMinBitrate: true
-                });
-            }
+            // There's no reason not to use this for p2p
+            this.p2pPcConstraints.optional.push({
+                googSuspendBelowMinBitrate: true
+            });
         }
 
         this.p2pPcConstraints = this.p2pPcConstraints || this.pcConstraints;
@@ -1781,6 +1755,30 @@ class RTCUtils extends Listenable {
 
         return { deviceList };
     }
+
+    /**
+     * Configures the given PeerConnection constraints to either enable or
+     * disable (according to the value of the 'enable' parameter) the
+     * 'googSuspendBelowMinBitrate' option.
+     * @param constraints the constraints on which to operate.
+     * @param enable {boolean} whether to enable or disable the suspend video
+     * option.
+     */
+    setSuspendVideo(constraints, enable) {
+        if (!constraints.optional) {
+            constraints.optional = [];
+        }
+
+        // Get rid of all "googSuspendBelowMinBitrate" constraints (we assume
+        // that the elements of constraints.optional contain a single property).
+        constraints.optional
+            = constraints.optional.filter(
+                c => !c.hasOwnProperty('googSuspendBelowMinBitrate'));
+
+        if (enable) {
+            constraints.optional.push({ googSuspendBelowMinBitrate: 'true' });
+        }
+    }
 }
 
 /**

modules/util/StringUtils.js

@@ -0,0 +1,24 @@
+/**
+ * Implements a simple hash code for a string (see
+ * https://en.wikipedia.org/wiki/Java_hashCode()).
+ *
+ * @param {string} The string to return a hash of.
+ * @return {Number} the integer hash code of the string.
+ */
+function integerHash(string) {
+    if (!string) {
+        return 0;
+    }
+
+    let char, hash = 0, i;
+
+    for (i = 0; i < string.length; i++) {
+        char = string.charCodeAt(i);
+        hash += char * Math.pow(31, string.length - 1 - i);
+        hash = Math.abs(hash | 0); // eslint-disable-line no-bitwise
+    }
+
+    return hash;
+}
+
+module.exports = { integerHash };

modules/xmpp/JingleSession.js

@@ -17,7 +17,7 @@ export default class JingleSession {
      * Creates new <tt>JingleSession</tt>.
      * @param {string} sid the Jingle session identifier
      * @param {string} localJid our JID
-     * @param {string} peerjid the JID of the remote peer
+     * @param {string} remoteJid the JID of the remote peer
      * @param {Strophe.Connection} connection the XMPP connection
      * @param {Object} mediaConstraints the media constraints object passed to
      * the PeerConnection onCreateAnswer/Offer as defined by the WebRTC.
@@ -27,13 +27,13 @@ export default class JingleSession {
     constructor(
             sid,
             localJid,
-            peerjid,
+            remoteJid,
             connection,
             mediaConstraints,
             iceConfig) {
         this.sid = sid;
         this.localJid = localJid;
-        this.peerjid = peerjid;
+        this.remoteJid = remoteJid;
         this.connection = connection;
         this.mediaConstraints = mediaConstraints;
         this.iceConfig = iceConfig;
@@ -89,8 +89,8 @@ export default class JingleSession {
         this.room = room;
         this.rtc = rtc;
         this.state = JingleSessionState.PENDING;
-        this.initiator = isInitiator ? this.localJid : this.peerjid;
-        this.responder = isInitiator ? this.peerjid : this.localJid;
+        this.initiator = isInitiator ? this.localJid : this.remoteJid;
+        this.responder = isInitiator ? this.remoteJid : this.localJid;
         this.doInitialize();
     }
 
@@ -164,5 +164,12 @@ export default class JingleSession {
      */
     acceptOffer(jingle, success, failure) {}
 
+    /**
+     * Returns the JID of the initiator of the jingle session.
+     */
+    _getInitiatorJid() {
+        return this.isInitiator ? this.localJid : this.remoteJid;
+    }
+
     /* eslint-enable no-unused-vars, no-empty-function */
 }

modules/xmpp/JingleSessionPC.js

@@ -3,6 +3,7 @@
 import async from 'async';
 import { getLogger } from 'jitsi-meet-logger';
 import { $iq, Strophe } from 'strophe.js';
+import { integerHash } from '../util/StringUtils';
 
 import JingleSession from './JingleSession';
 import * as JingleSessionState from './JingleSessionState';
@@ -59,8 +60,8 @@ export default class JingleSessionPC extends JingleSession {
      * Creates new <tt>JingleSessionPC</tt>
      * @param {string} sid the Jingle Session ID - random string which
      * identifies the session
-     * @param {string} me our JID
-     * @param {string} peerjid remote peer JID
+     * @param {string} localJid our JID
+     * @param {string} remoteJid remote peer JID
      * @param {Strophe.Connection} connection Strophe XMPP connection instance
      * used to send packets.
      * @param mediaConstraints the media constraints object passed to
@@ -71,7 +72,7 @@ export default class JingleSessionPC extends JingleSession {
      * meant to be used in a direct, peer to peer connection or <tt>false</tt>
      * if it's a JVB connection.
      * @param {boolean} isInitiator indicates whether or not we are the side
-     * which sends the 'session-intiate'.
+     * which sends the 'session-initiate'.
      * @param {object} options a set of config options
      * @param {boolean} options.webrtcIceUdpDisable <tt>true</tt> to block UDP
      * candidates.
@@ -86,15 +87,16 @@ export default class JingleSessionPC extends JingleSession {
      */
     constructor(
             sid,
-            me,
-            peerjid,
+            localJid,
+            remoteJid,
             connection,
             mediaConstraints,
             iceConfig,
             isP2P,
             isInitiator,
             options) {
-        super(sid, me, peerjid, connection, mediaConstraints, iceConfig);
+        super(
+            sid, localJid, remoteJid, connection, mediaConstraints, iceConfig);
 
         /**
          * Stores result of {@link window.performance.now()} at the time when
@@ -240,40 +242,40 @@ export default class JingleSessionPC extends JingleSession {
         // Set to true if the connection was ever stable
         this.wasstable = false;
 
-        // Create new peer connection instance
+        const pcOptions = { disableRtx: this.room.options.disableRtx };
+
         if (this.isP2P) {
-            this.peerconnection = this.rtc.createPeerConnection(
-                this.signalingLayer,
-                this.iceConfig,
-                this.isP2P,
-                {
-                    // simulcast needs to be disabled for P2P (121) calls
-                    disableSimulcast: true,
-                    disableRtx: this.room.options.disableRtx,
-                    disableH264: this.room.options.p2p
-                        && this.room.options.p2p.disableH264,
-                    preferH264: this.room.options.p2p
-                        && this.room.options.p2p.preferH264
-                });
+            // simulcast needs to be disabled for P2P (121) calls
+            pcOptions.disableSimulcast = true;
+            pcOptions.disableH264
+                = this.room.options.p2p && this.room.options.p2p.disableH264;
+            pcOptions.preferH264
+                = this.room.options.p2p && this.room.options.p2p.preferH264;
+
+            const abtestSuspendVideo = this._abtestSuspendVideoEnabled();
+
+            if (typeof abtestSuspendVideo !== 'undefined') {
+                pcOptions.abtestSuspendVideo = abtestSuspendVideo;
+            }
         } else {
-            this.peerconnection = this.rtc.createPeerConnection(
-                this.signalingLayer,
-                this.iceConfig,
-                this.isP2P,
-                {
-                    // H264 does not support simulcast, so it needs to be
-                    // disabled.
-                    disableSimulcast: this.room.options.disableSimulcast
-                        || (this.room.options.preferH264
-                            && !this.room.options.disableH264),
-                    disableRtx: this.room.options.disableRtx,
-                    disableH264: this.room.options.disableH264,
-                    preferH264: this.room.options.preferH264,
-                    enableFirefoxSimulcast: this.room.options.testing
-                        && this.room.options.testing.enableFirefoxSimulcast
-                });
+            // H264 does not support simulcast, so it needs to be disabled.
+            pcOptions.disableSimulcast
+                = this.room.options.disableSimulcast
+                    || (this.room.options.preferH264
+                            && !this.room.options.disableH264);
+            pcOptions.preferH264 = this.room.options.preferH264;
+            pcOptions.enableFirefoxSimulcast
+                = this.room.options.testing
+                    && this.room.options.testing.enableFirefoxSimulcast;
         }
 
+        this.peerconnection
+            = this.rtc.createPeerConnection(
+                    this.signalingLayer,
+                    this.iceConfig,
+                    this.isP2P,
+                    pcOptions);
+
         this.peerconnection.onicecandidate = ev => {
             if (!ev) {
                 // There was an incomplete check for ev before which left
@@ -509,7 +511,7 @@ export default class JingleSessionPC extends JingleSession {
         }
 
         logger.log('sendIceCandidates', candidates);
-        const cand = $iq({ to: this.peerjid,
+        const cand = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', { xmlns: 'urn:xmpp:jingle:1',
                 action: 'transport-info',
@@ -657,7 +659,7 @@ export default class JingleSessionPC extends JingleSession {
             if (this.isP2P) {
                 // In P2P all SSRCs are owner by the remote peer
                 this.signalingLayer.setSSRCOwner(
-                    ssrc, Strophe.getResourceFromJid(this.peerjid));
+                    ssrc, Strophe.getResourceFromJid(this.remoteJid));
             } else {
                 $(ssrcElement)
                     .find('>ssrc-info[xmlns="http://jitsi.org/jitmeet"]')
@@ -794,7 +796,7 @@ export default class JingleSessionPC extends JingleSession {
      */
     sendSessionInitiate(offerSdp) {
         let init = $iq({
-            to: this.peerjid,
+            to: this.remoteJid,
             type: 'set'
         }).c('jingle', {
             xmlns: 'urn:xmpp:jingle:1',
@@ -967,7 +969,7 @@ export default class JingleSessionPC extends JingleSession {
         // NOTE: since we're just reading from it, we don't need to be within
         //  the modification queue to access the local description
         const localSDP = new SDP(this.peerconnection.localDescription.sdp);
-        let accept = $iq({ to: this.peerjid,
+        let accept = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', { xmlns: 'urn:xmpp:jingle:1',
                 action: 'session-accept',
@@ -1039,7 +1041,7 @@ export default class JingleSessionPC extends JingleSession {
 
         const sessionModify
             = $iq({
-                to: this.peerjid,
+                to: this.remoteJid,
                 type: 'set'
             })
                 .c('jingle', {
@@ -1074,7 +1076,7 @@ export default class JingleSessionPC extends JingleSession {
      * @private
      */
     sendTransportAccept(localSDP, success, failure) {
-        let transportAccept = $iq({ to: this.peerjid,
+        let transportAccept = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', {
                 xmlns: 'urn:xmpp:jingle:1',
@@ -1123,7 +1125,7 @@ export default class JingleSessionPC extends JingleSession {
     sendTransportReject(success, failure) {
         // Send 'transport-reject', so that the focus will
         // know that we've failed
-        let transportReject = $iq({ to: this.peerjid,
+        let transportReject = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', {
                 xmlns: 'urn:xmpp:jingle:1',
@@ -1152,7 +1154,7 @@ export default class JingleSessionPC extends JingleSession {
         if (!options || Boolean(options.sendSessionTerminate)) {
             let sessionTerminate
                 = $iq({
-                    to: this.peerjid,
+                    to: this.remoteJid,
                     type: 'set'
                 })
                     .c('jingle', {
@@ -2025,7 +2027,7 @@ export default class JingleSessionPC extends JingleSession {
 
         // send source-remove IQ.
         let sdpDiffer = new SDPDiffer(newSDP, oldSDP);
-        const remove = $iq({ to: this.peerjid,
+        const remove = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', {
                 xmlns: 'urn:xmpp:jingle:1',
@@ -2047,7 +2049,7 @@ export default class JingleSessionPC extends JingleSession {
 
         // send source-add IQ.
         sdpDiffer = new SDPDiffer(oldSDP, newSDP);
-        const add = $iq({ to: this.peerjid,
+        const add = $iq({ to: this.remoteJid,
             type: 'set' })
             .c('jingle', {
                 xmlns: 'urn:xmpp:jingle:1',
@@ -2181,4 +2183,23 @@ export default class JingleSessionPC extends JingleSession {
         return `JingleSessionPC[p2p=${this.isP2P},`
                     + `initiator=${this.isInitiator},sid=${this.sid}]`;
     }
+
+    /**
+     * If the A/B test for suspend video is disabled according to the room's
+     * configuration, returns undefined. Otherwise returns a boolean which
+     * indicates whether the suspend video option should be enabled or disabled.
+     */
+    _abtestSuspendVideoEnabled() {
+        if (!this.room.options.abTesting
+            || !this.room.options.abTesting.enableSuspendVideoTest) {
+            return;
+        }
+
+        // We want the two participants in a P2P call to agree on the value of
+        // the "suspend" option. We use the JID of the initiator, because it is
+        // both randomly selected and agreed upon by both participants.
+        const jid = this._getInitiatorJid();
+
+        return integerHash(jid) % 2 === 0;
+    }
 }

modules/xmpp/strophe.jingle.js

@@ -96,9 +96,9 @@ class JingleConnectionPlugin extends ConnectionPlugin {
             }
 
             // local jid is not checked
-            if (fromJid !== sess.peerjid) {
+            if (fromJid !== sess.remoteJid) {
                 logger.warn(
-                    'jid mismatch for session id', sid, sess.peerjid, iq);
+                    'jid mismatch for session id', sid, sess.remoteJid, iq);
                 ack.attrs({ type: 'error' });
                 ack.c('error', { type: 'cancel' })
                     .c('item-not-found', {
@@ -114,7 +114,7 @@ class JingleConnectionPlugin extends ConnectionPlugin {
             }
         } else if (sess !== undefined) {
             // Existing session with same session id. This might be out-of-order
-            // if the sess.peerjid is the same as from.
+            // if the sess.remoteJid is the same as from.
             ack.attrs({ type: 'error' });
             ack.c('error', { type: 'cancel' })
                 .c('service-unavailable', {