From bbe475cb4eea577725b91d3bdd00c56312998fce Mon Sep 17 00:00:00 2001 From: damencho Date: Thu, 13 Oct 2016 14:57:54 -0500 Subject: [PATCH] Fixes review comments. Uses some ES6 syntax. Also removes inputHeight key for resolution as it makes no sence to have dictionary with one key. Removes some code duplication. Makes code consistent: method params for updateLocalStats and updateRemoteStats. --- conference.js | 19 ++-- modules/UI/UI.js | 9 -- modules/UI/videolayout/VideoLayout.js | 10 -- .../connectionquality/connectionquality.js | 106 ++++++++++-------- 4 files changed, 70 insertions(+), 74 deletions(-) diff --git a/conference.js b/conference.js index 4aa5fff23..4244e536e 100644 --- a/conference.js +++ b/conference.js @@ -1373,7 +1373,11 @@ export default { room.on(ConferenceEvents.CONNECTION_STATS, function (stats) { ConnectionQuality.updateLocalStats( - stats, connectionIsInterrupted, localVideo); + stats, + connectionIsInterrupted, + localVideo.videoType, + localVideo.isMuted(), + localVideo.resolution); }); ConnectionQuality.addListener(CQEvents.LOCALSTATS_UPDATED, @@ -1384,9 +1388,7 @@ export default { bitrate: stats.bitrate, packetLoss: stats.packetLoss}; if (localVideo && localVideo.resolution) { - data.resolution = { - inputHeight: localVideo.resolution - }; + data.resolution = localVideo.resolution; } try { @@ -1402,12 +1404,13 @@ export default { (participant, payload) => { switch(payload.type) { case this.commands.defaults.CONNECTION_QUALITY: { - let id = participant.getId(); + let remoteVideo = participant.getTracks() + .find(tr => tr.isVideoTrack()); ConnectionQuality.updateRemoteStats( - id, + participant.getId(), payload.values, - APP.UI.getRemoteVideoType(id), - APP.UI.isRemoteVideoMuted(id)); + remoteVideo ? remoteVideo.videoType : undefined, + remoteVideo ? remoteVideo.isMuted() : undefined); break; } default: diff --git a/modules/UI/UI.js b/modules/UI/UI.js index e05c95486..70678b97a 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -779,15 +779,6 @@ UI.getRemoteVideoType = function (jid) { return VideoLayout.getRemoteVideoType(jid); }; -/** - * Return the mute state of the remote video. - * @param jid the jid for the remote video - * @returns the video mute state. - */ -UI.isRemoteVideoMuted = function (jid) { - return VideoLayout.isRemoteVideoMuted(jid); -}; - UI.connectionIndicatorShowMore = function(id) { VideoLayout.showMore(id); }; diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index 44cce4704..749dd23ec 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -327,16 +327,6 @@ var VideoLayout = { return smallVideo ? smallVideo.getVideoType() : null; }, - /** - * Return the mute state of the remote video. - * @param id the id for the remote video - * @returns {boolean} the video mute state. - */ - isRemoteVideoMuted (id) { - let smallVideo = VideoLayout.getSmallVideo(id); - return smallVideo ? smallVideo.isVideoMuted : null; - }, - isPinned (id) { return (pinnedId) ? (id === pinnedId) : false; }, diff --git a/modules/connectionquality/connectionquality.js b/modules/connectionquality/connectionquality.js index 124d58463..443a0492e 100644 --- a/modules/connectionquality/connectionquality.js +++ b/modules/connectionquality/connectionquality.js @@ -38,7 +38,7 @@ function calculateQuality(newVal, oldVal) { // webrtc table describing simulcast resolutions and used bandwidth // https://chromium.googlesource.com/external/webrtc/+/master/webrtc/media/engine/simulcast.cc#42 -var _bandwidthMap = [ +const _bandwidthMap = [ { width: 1920, height: 1080, layers:3, max: 5000, min: 800 }, { width: 1280, height: 720, layers:3, max: 2500, min: 600 }, { width: 960, height: 540, layers:3, max: 900, min: 450 }, @@ -52,7 +52,7 @@ var _bandwidthMap = [ * or enable it in case of no simulcast and we force it. * @type {boolean} */ -var disableQualityBasedOnBandwidth = +const disableQualityBasedOnBandwidth = config.forceQualityBasedOnBandwidth ? false : config.disableSimulcast; /** @@ -61,23 +61,17 @@ var disableQualityBasedOnBandwidth = * _bandwidthMap. * @param inputHeight the resolution used to open the camera. * @param upload the upload rate reported by client. - * @returns {*} the percent of upload based on _bandwidthMap and maximum value + * @returns {int} the percent of upload based on _bandwidthMap and maximum value * of 100, as values of the map are approximate and clients can stream above - * those values. + * those values. Returns undefined if no result is found. */ function calculateQualityUsingUpload(inputHeight, upload) { - let foundResolution = null; - - for (let i in _bandwidthMap) { - let r = _bandwidthMap[i]; - if (r.height <= inputHeight) { - foundResolution = r; - break; - } - } + // found resolution from _bandwidthMap which height is equal or less than + // the inputHeight + let foundResolution = _bandwidthMap.find((r) => (r.height <= inputHeight)); if (!foundResolution) - return false; + return undefined; if (upload <= foundResolution.min) return 0; @@ -94,26 +88,23 @@ export default { * @param data new statistics * @param dontUpdateLocalConnectionQuality {boolean} if true - * localConnectionQuality wont be recalculated. + * @param videoType the local video type + * @param isMuted current state of local video, whether it is muted + * @param resolution the current resolution used by local video */ updateLocalStats: - function (data, dontUpdateLocalConnectionQuality, localVideo) { + function (data, dontUpdateLocalConnectionQuality, + videoType, isMuted, resolution) { stats = data; if(!dontUpdateLocalConnectionQuality) { - if (!disableQualityBasedOnBandwidth - && !localVideo.isMuted() - && localVideo.videoType !== 'desktop' - && localVideo.resolution) { - let val = calculateQualityUsingUpload( - localVideo.resolution, - data.bitrate.upload); - if (val) { - localConnectionQuality = val; - } - } else { - var newVal = 100 - stats.packetLoss.total; - localConnectionQuality = - calculateQuality(newVal, localConnectionQuality); - } + let val = this._getNewQualityValue( + stats, + localConnectionQuality, + videoType, + isMuted, + resolution); + if (val !== undefined) + localConnectionQuality = val; } eventEmitter.emit( CQEvents.LOCALSTATS_UPDATED, localConnectionQuality, stats); @@ -132,7 +123,9 @@ export default { /** * Updates remote statistics * @param id the id associated with the statistics - * @param data the statistics + * @param data the statistics received + * @param remoteVideoType the video type of the remote video + * @param isRemoteVideoMuted whether remote video is muted */ updateRemoteStats: function (id, data, remoteVideoType, isRemoteVideoMuted) { @@ -149,28 +142,47 @@ export default { remoteStats[id] = data; - if (disableQualityBasedOnBandwidth - || isRemoteVideoMuted - || remoteVideoType === 'desktop' - || !inputResolution) { - var newVal = 100 - data.packetLoss.total; - var oldVal = remoteConnectionQuality[id]; - remoteConnectionQuality[id] - = calculateQuality(newVal, oldVal || 100); - } else { - let val = calculateQualityUsingUpload( - inputResolution.inputHeight, - data.bitrate.upload); - if (val) { - remoteConnectionQuality[id] = val; - } - } + let val = this._getNewQualityValue( + data, + remoteConnectionQuality[id], + remoteVideoType, + isRemoteVideoMuted, + inputResolution); + if (val !== undefined) + remoteConnectionQuality[id] = val; eventEmitter.emit( CQEvents.REMOTESTATS_UPDATED, id, remoteConnectionQuality[id], remoteStats[id]); }, + /** + * Returns the new quality value based on the input parameters. + * Used to calculate remote and local values. + * @param data the data + * @param lastQualityValue the last value we calculated + * @param videoType need to check whether we are screen sharing + * @param isMuted is video muted + * @param resolution the input resolution used by the camera + * @returns {*} the newly calculated value or undefined if no result + * @private + */ + _getNewQualityValue: + function (data, lastQualityValue, videoType, isMuted, resolution) { + if (disableQualityBasedOnBandwidth + || isMuted + || videoType === 'desktop' + || !resolution) { + return calculateQuality( + 100 - data.packetLoss.total, + lastQualityValue || 100); + } else { + return calculateQualityUsingUpload( + resolution, + data.bitrate.upload); + } + }, + /** * Returns the local statistics. */