From dc07fd733f2d8eb2829f8155ad83bb409bd369d1 Mon Sep 17 00:00:00 2001 From: George Politis Date: Mon, 11 Jan 2016 15:51:55 -0600 Subject: [PATCH] Removes the local SSRC replacement hack and fixes video muting/unmuting when simulcast is enabled. --- modules/util/RandomUtil.js | 3 +- modules/xmpp/JingleSessionPC.js | 19 -- modules/xmpp/LocalSSRCReplacement.js | 271 ------------------------ modules/xmpp/TraceablePeerConnection.js | 61 +++++- 4 files changed, 53 insertions(+), 301 deletions(-) delete mode 100644 modules/xmpp/LocalSSRCReplacement.js diff --git a/modules/util/RandomUtil.js b/modules/util/RandomUtil.js index 2b307db66..f82e094ad 100644 --- a/modules/util/RandomUtil.js +++ b/modules/util/RandomUtil.js @@ -66,7 +66,8 @@ var RandomUtil = { return ret; }, randomElement: randomElement, - randomAlphanumStr: randomAlphanumStr + randomAlphanumStr: randomAlphanumStr, + randomInt: randomInt }; module.exports = RandomUtil; diff --git a/modules/xmpp/JingleSessionPC.js b/modules/xmpp/JingleSessionPC.js index 30c833086..7b478d3b2 100644 --- a/modules/xmpp/JingleSessionPC.js +++ b/modules/xmpp/JingleSessionPC.js @@ -10,7 +10,6 @@ var transform = require("sdp-transform"); var XMPPEvents = require("../../service/xmpp/XMPPEvents"); var RTCEvents = require("../../service/RTC/RTCEvents"); var RTCBrowserType = require("../RTC/RTCBrowserType"); -var SSRCReplacement = require("./LocalSSRCReplacement"); // Jingle stuff function JingleSessionPC(me, sid, connection, service, eventEmitter) { @@ -272,8 +271,6 @@ JingleSessionPC.prototype.accept = function () { //console.log('setLocalDescription success'); self.setLocalDescription(); - SSRCReplacement.processSessionInit(accept); - self.connection.sendIQ(accept, function () { var ack = {}; @@ -371,8 +368,6 @@ JingleSessionPC.prototype.sendIceCandidate = function (candidate) { self.initiator == self.me ? 'initiator' : 'responder', ssrc); - SSRCReplacement.processSessionInit(init); - self.connection.sendIQ(init, function () { //console.log('session initiate ack'); @@ -489,8 +484,6 @@ JingleSessionPC.prototype.createdOffer = function (sdp) { init, this.initiator == this.me ? 'initiator' : 'responder'); - SSRCReplacement.processSessionInit(init); - self.connection.sendIQ(init, function () { var ack = {}; @@ -787,8 +780,6 @@ JingleSessionPC.prototype.createdAnswer = function (sdp, provisional) { self.initiator == self.me ? 'initiator' : 'responder', ssrcs); - SSRCReplacement.processSessionInit(accept); - self.connection.sendIQ(accept, function () { var ack = {}; @@ -1227,11 +1218,6 @@ JingleSessionPC.prototype.notifyMySSRCUpdate = function (old_sdp, new_sdp) { ); var removed = sdpDiffer.toJingle(remove); - // Let 'source-remove' IQ through the hack and see if we're allowed to send - // it in the current form - if (removed) - remove = SSRCReplacement.processSourceRemove(remove); - if (removed && remove) { console.info("Sending source-remove", remove); this.connection.sendIQ(remove, @@ -1258,11 +1244,6 @@ JingleSessionPC.prototype.notifyMySSRCUpdate = function (old_sdp, new_sdp) { ); var added = sdpDiffer.toJingle(add); - // Let 'source-add' IQ through the hack and see if we're allowed to send - // it in the current form - if (added) - add = SSRCReplacement.processSourceAdd(add); - if (added && add) { console.info("Sending source-add", add); this.connection.sendIQ(add, diff --git a/modules/xmpp/LocalSSRCReplacement.js b/modules/xmpp/LocalSSRCReplacement.js deleted file mode 100644 index 4bf109c1b..000000000 --- a/modules/xmpp/LocalSSRCReplacement.js +++ /dev/null @@ -1,271 +0,0 @@ -/* global $ */ - -/* - Here we do modifications of local video SSRCs. There are 2 situations we have - to handle: - - 1. We generate SSRC for local recvonly video stream. This is the case when we - have no local camera and it is not generated automatically, but SSRC=1 is - used implicitly. If that happens RTCP packets will be dropped by the JVB - and we won't be able to request video key frames correctly. - - 2. A hack to re-use SSRC of the first video stream for any new stream created - in future. It turned out that Chrome may keep on using the SSRC of removed - video stream in RTCP even though a new one has been created. So we just - want to avoid that by re-using it. Jingle 'source-remove'/'source-add' - notifications are blocked once first video SSRC is advertised to the focus. - - What this hack does: - - 1. Stores the SSRC of the first video stream created by - a) scanning Jingle session-accept/session-invite for existing video SSRC - b) watching for 'source-add' for new video stream if it has not been - created in step a) - 2. Exposes method 'mungeLocalVideoSSRC' which replaces any new video SSRC with - the stored one. It is called by 'TracablePeerConnection' before local SDP is - returned to the other parts of the application. - 3. Scans 'source-remove'/'source-add' notifications for stored video SSRC and - blocks those notifications. This makes Jicofo and all participants think - that it exists all the time even if the video stream has been removed or - replaced locally. Thanks to that there is no additional signaling activity - on video mute or when switching to the desktop stream. - */ - -var SDP = require('./SDP'); -var RTCBrowserType = require('../RTC/RTCBrowserType'); - -/** - * The hack is enabled on all browsers except FF by default - * FIXME finish the hack once removeStream method is implemented in FF - * @type {boolean} - */ -var isEnabled = !RTCBrowserType.isFirefox(); - -/** - * Stored SSRC of local video stream. - */ -var localVideoSSRC; - -/** - * SSRC used for recvonly video stream when we have no local camera. - * This is in order to tell Chrome what SSRC should be used in RTCP requests - * instead of 1. - */ -var localRecvOnlySSRC; - -/** - * cname for localRecvOnlySSRC - */ -var localRecvOnlyCName; - -/** - * Method removes element which describes localVideoSSRC - * from given Jingle IQ. - * @param modifyIq 'source-add' or 'source-remove' Jingle IQ. - * @param actionName display name of the action which will be printed in log - * messages. - * @returns {*} modified Jingle IQ, so that it does not contain element - * corresponding to localVideoSSRC or null if no - * other SSRCs left to be signaled after removing it. - */ -var filterOutSource = function (modifyIq, actionName) { - var modifyIqTree = $(modifyIq.tree()); - - if (!localVideoSSRC) - return modifyIqTree[0]; - - var videoSSRC = modifyIqTree.find( - '>jingle>content[name="video"]' + - '>description>source[ssrc="' + localVideoSSRC + '"]'); - - if (!videoSSRC.length) { - return modifyIqTree[0]; - } - - console.info( - 'Blocking ' + actionName + ' for local video SSRC: ' + localVideoSSRC); - - videoSSRC.remove(); - - // Check if any sources still left to be added/removed - if (modifyIqTree.find('>jingle>content>description>source').length) { - return modifyIqTree[0]; - } else { - return null; - } -}; - -/** - * Scans given Jingle IQ for video SSRC and stores it. - * @param jingleIq the Jingle IQ to be scanned for video SSRC. - */ -var storeLocalVideoSSRC = function (jingleIq) { - var videoSSRCs = - $(jingleIq.tree()) - .find('>jingle>content[name="video"]>description>source'); - - videoSSRCs.each(function (idx, ssrcElem) { - if (localVideoSSRC) - return; - // We consider SSRC real only if it has msid attribute - // recvonly streams in FF do not have it as well as local SSRCs - // we generate for recvonly streams in Chrome - var ssrSel = $(ssrcElem); - var msid = ssrSel.find('>parameter[name="msid"]'); - if (msid.length) { - var ssrcVal = ssrSel.attr('ssrc'); - if (ssrcVal) { - localVideoSSRC = ssrcVal; - console.info('Stored local video SSRC' + - ' for future re-use: ' + localVideoSSRC); - } - } - }); -}; - -/** - * Generates new SSRC for local video recvonly stream. - * FIXME what about eventual SSRC collision ? - */ -function generateRecvonlySSRC() { - // - localRecvOnlySSRC = - localVideoSSRC ? - localVideoSSRC : Math.random().toString(10).substring(2, 11); - - localRecvOnlyCName = - Math.random().toString(36).substring(2); - console.info( - "Generated local recvonly SSRC: " + localRecvOnlySSRC + - ", cname: " + localRecvOnlyCName); -} - -var LocalSSRCReplacement = { - /** - * Method must be called before 'session-initiate' or 'session-invite' is - * sent. Scans the IQ for local video SSRC and stores it if detected. - * - * @param sessionInit our 'session-initiate' or 'session-accept' Jingle IQ - * which will be scanned for local video SSRC. - */ - processSessionInit: function (sessionInit) { - if (!isEnabled) - return; - - if (localVideoSSRC) { - console.error("Local SSRC stored already: " + localVideoSSRC); - return; - } - storeLocalVideoSSRC(sessionInit); - }, - /** - * If we have local video SSRC stored searched given - * localDescription for video SSRC and makes sure it is replaced - * with the stored one. - * @param localDescription local description object that will have local - * video SSRC replaced with the stored one - * @returns modified localDescription object. - */ - mungeLocalVideoSSRC: function (localDescription) { - if (!isEnabled) - return localDescription; - - if (!localDescription) { - console.warn("localDescription is null or undefined"); - return localDescription; - } - - // IF we have local video SSRC stored make sure it is replaced - // with old SSRC - var sdp = new SDP(localDescription.sdp); - if (sdp.media.length < 2) - return; - - if (localVideoSSRC && sdp.media[1].indexOf("a=ssrc:") !== -1 && - !sdp.containsSSRC(localVideoSSRC)) { - - console.info("Does not contain: " - + localVideoSSRC + "---" + sdp.media[1]); - - // Get new video SSRC - var map = sdp.getMediaSsrcMap(); - var videoPart = map[1]; - var videoSSRCs = videoPart.ssrcs; - var newSSRC = Object.keys(videoSSRCs)[0]; - - console.info( - "Replacing new video SSRC: " + newSSRC + - " with " + localVideoSSRC); - - localDescription.sdp = - sdp.raw.replace( - new RegExp('a=ssrc:' + newSSRC, 'g'), - 'a=ssrc:' + localVideoSSRC); - } - else if (sdp.media[1].indexOf('a=ssrc:') === -1 && - sdp.media[1].indexOf('a=recvonly') !== -1) { - // Make sure we have any SSRC for recvonly video stream - if (!localRecvOnlySSRC) { - generateRecvonlySSRC(); - } - - console.info('No SSRC in video recvonly stream' + - ' - adding SSRC: ' + localRecvOnlySSRC); - - sdp.media[1] += 'a=ssrc:' + localRecvOnlySSRC + - ' cname:' + localRecvOnlyCName + '\r\n'; - - localDescription.sdp = sdp.session + sdp.media.join(''); - } - return localDescription; - }, - /** - * Method must be called before 'source-add' notification is sent. In case - * we have local video SSRC advertised already it will be removed from the - * notification. If no other SSRCs are described by given IQ null will be - * returned which means that there is no point in sending the notification. - * @param sourceAdd 'source-add' Jingle IQ to be processed - * @returns modified 'source-add' IQ which can be sent to the focus or - * null if no notification shall be sent. It is no longer - * a Strophe IQ Builder instance, but DOM element tree. - */ - processSourceAdd: function (sourceAdd) { - if (!isEnabled) - return sourceAdd; - - if (!localVideoSSRC) { - // Store local SSRC if available - storeLocalVideoSSRC(sourceAdd); - return sourceAdd; - } else { - return filterOutSource(sourceAdd, 'source-add'); - } - }, - /** - * Method must be called before 'source-remove' notification is sent. - * Removes local video SSRC from the notification. If there are no other - * SSRCs described in the given IQ null will be returned which - * means that there is no point in sending the notification. - * @param sourceRemove 'source-remove' Jingle IQ to be processed - * @returns modified 'source-remove' IQ which can be sent to the focus or - * null if no notification shall be sent. It is no longer - * a Strophe IQ Builder instance, but DOM element tree. - */ - processSourceRemove: function (sourceRemove) { - if (!isEnabled) - return sourceRemove; - - return filterOutSource(sourceRemove, 'source-remove'); - }, - - /** - * Turns the hack on or off - * @param enabled true to enable the hack or false - * to disable it - */ - setEnabled: function (enabled) { - isEnabled = enabled; - } -}; - -module.exports = LocalSSRCReplacement; diff --git a/modules/xmpp/TraceablePeerConnection.js b/modules/xmpp/TraceablePeerConnection.js index 51dd22e89..2eb5d69ca 100644 --- a/modules/xmpp/TraceablePeerConnection.js +++ b/modules/xmpp/TraceablePeerConnection.js @@ -4,7 +4,6 @@ var RTC = require('../RTC/RTC'); var RTCBrowserType = require("../RTC/RTCBrowserType"); var RTCEvents = require("../../service/RTC/RTCEvents"); -var SSRCReplacement = require("./LocalSSRCReplacement"); function TraceablePeerConnection(ice_config, constraints, session) { var self = this; @@ -138,6 +137,47 @@ var dumpSDP = function(description) { return 'type: ' + description.type + '\r\n' + description.sdp; }; +var insertRecvOnlySSRC = function (desc) { + if (typeof desc !== 'object' || desc === null || + typeof desc.sdp !== 'string') { + console.warn('An empty description was passed as an argument.'); + return desc; + } + + var transform = require('sdp-transform'); + var RandomUtil = require('../util/RandomUtil'); + + var session = transform.parse(desc.sdp); + if (!Array.isArray(session.media)) + { + return; + } + + var modded = false; + session.media.forEach(function (bLine) { + if (bLine.direction != 'recvonly') + { + return; + } + + modded = true; + if (!Array.isArray(bLine.ssrcs) || bLine.ssrcs.length === 0) + { + var ssrc = RandomUtil.randomInt(1, 0xffffffff); + bLine.ssrcs = [{ + id: ssrc, + attribute: 'cname', + value: ['recvonly-', ssrc].join(' ') + }]; + } + }); + + return (!modded) ? desc : new RTCSessionDescription({ + type: desc.type, + sdp: transform.write(session), + }); +}; + /** * Takes a SessionDescription object and returns a "normalized" version. * Currently it only takes care of ordering the a=ssrc lines. @@ -218,10 +258,6 @@ if (TraceablePeerConnection.prototype.__defineGetter__ !== undefined) { function() { var desc = this.peerconnection.localDescription; - // FIXME this should probably be after the Unified Plan -> Plan B - // transformation. - desc = SSRCReplacement.mungeLocalVideoSSRC(desc); - this.trace('getLocalDescription::preTransform', dumpSDP(desc)); // if we're running on FF, transform to Plan B first. @@ -367,8 +403,11 @@ TraceablePeerConnection.prototype.createOffer self.trace('createOfferOnSuccess::postTransform (Plan B)', dumpSDP(offer)); } - offer = SSRCReplacement.mungeLocalVideoSSRC(offer); - self.trace('createOfferOnSuccess::mungeLocalVideoSSRC', dumpSDP(offer)); + if (RTCBrowserType.isChrome()) + { + offer = insertRecvOnlySSRC(offer); + self.trace('createOfferOnSuccess::mungeLocalVideoSSRC', dumpSDP(offer)); + } if (config.enableSimulcast && self.simulcast.isSupported()) { offer = self.simulcast.mungeLocalDescription(offer); @@ -398,9 +437,11 @@ TraceablePeerConnection.prototype.createAnswer self.trace('createAnswerOnSuccess::postTransform (Plan B)', dumpSDP(answer)); } - // munge local video SSRC - answer = SSRCReplacement.mungeLocalVideoSSRC(answer); - self.trace('createAnswerOnSuccess::mungeLocalVideoSSRC', dumpSDP(answer)); + if (RTCBrowserType.isChrome()) + { + answer = insertRecvOnlySSRC(answer); + self.trace('createAnswerOnSuccess::mungeLocalVideoSSRC', dumpSDP(answer)); + } if (config.enableSimulcast && self.simulcast.isSupported()) { answer = self.simulcast.mungeLocalDescription(answer);