From 704e14f008697c0993782b50092208fad35cfa33 Mon Sep 17 00:00:00 2001 From: yanas Date: Tue, 21 Mar 2017 12:14:13 -0500 Subject: [PATCH] Handle last n in the client (#1389) * Handle last n in the client * fix(LargeVideoManager.js): Fixes check for low bandwidth. Needs more work * fix(LargeVideoManager.js): Fixes the Shared Video test. * fix(LargeVideoManager): Fix shared video view and remove last n checks. * fix(LargeVideoManager): Fixes jsdoc comment * fix(RemoteVideo): Fix connection status check * fix(LargeVideoManager,RemoteVideo): Syntax errors --- conference.js | 16 +- lang/main.json | 4 +- modules/UI/videolayout/ConnectionIndicator.js | 22 +-- modules/UI/videolayout/LargeVideoManager.js | 22 ++- modules/UI/videolayout/RemoteVideo.js | 71 ++------ modules/UI/videolayout/SmallVideo.js | 4 +- modules/UI/videolayout/VideoLayout.js | 171 ++---------------- 7 files changed, 74 insertions(+), 236 deletions(-) diff --git a/conference.js b/conference.js index 8ec3bb43c8..dd1c64890b 100644 --- a/conference.js +++ b/conference.js @@ -1283,8 +1283,9 @@ export default { */ room.on( ConferenceEvents.LAST_N_ENDPOINTS_CHANGED, (ids, enteringIds) => { - APP.UI.handleLastNEndpoints(ids, enteringIds); + APP.UI.handleLastNEndpoints(ids, enteringIds); }); + room.on( ConferenceEvents.PARTICIPANT_CONN_STATUS_CHANGED, (id, isActive) => { @@ -1940,5 +1941,18 @@ export default { */ removeListener (eventName, listener) { eventEmitter.removeListener(eventName, listener); + }, + + /** + * Checks if the participant given by participantId is currently in the + * last N set if there's one supported. + * + * @param participantId the identifier of the participant + * @returns {boolean} {true} if the participant given by the participantId + * is currently in the last N set or if there's no last N set at this point + * and {false} otherwise + */ + isInLastN (participantId) { + return room.isInLastN(participantId); } }; diff --git a/lang/main.json b/lang/main.json index bd5f7879a5..a24ff39eac 100644 --- a/lang/main.json +++ b/lang/main.json @@ -382,7 +382,9 @@ "FETCH_SESSION_ID": "Obtaining session-id...", "GOT_SESSION_ID": "Obtaining session-id... Done", "GET_SESSION_ID_ERROR": "Get session-id error: __code__", - "USER_CONNECTION_INTERRUPTED": "__displayName__ is having connectivity issues..." + "USER_CONNECTION_INTERRUPTED": "__displayName__ is having connectivity issues...", + "LOW_BANDWIDTH": "Video for __displayName__ has been turned off to save bandwidth" + }, "recording": { diff --git a/modules/UI/videolayout/ConnectionIndicator.js b/modules/UI/videolayout/ConnectionIndicator.js index 0d234e4a51..0366258b37 100644 --- a/modules/UI/videolayout/ConnectionIndicator.js +++ b/modules/UI/videolayout/ConnectionIndicator.js @@ -348,17 +348,17 @@ ConnectionIndicator.prototype.remove = function() { * the user is having connectivity issues. */ ConnectionIndicator.prototype.updateConnectionStatusIndicator -= function (isActive) { - this.isConnectionActive = isActive; - if (this.isConnectionActive) { - $(this.interruptedIndicator).hide(); - $(this.emptyIcon).show(); - $(this.fullIcon).show(); - } else { - $(this.interruptedIndicator).show(); - $(this.emptyIcon).hide(); - $(this.fullIcon).hide(); - } + = function (isActive) { + this.isConnectionActive = isActive; + if (this.isConnectionActive) { + $(this.interruptedIndicator).hide(); + $(this.emptyIcon).show(); + $(this.fullIcon).show(); + } else { + $(this.interruptedIndicator).show(); + $(this.emptyIcon).hide(); + $(this.fullIcon).hide(); + } }; /** diff --git a/modules/UI/videolayout/LargeVideoManager.js b/modules/UI/videolayout/LargeVideoManager.js index 865300be00..29a74ea8ab 100644 --- a/modules/UI/videolayout/LargeVideoManager.js +++ b/modules/UI/videolayout/LargeVideoManager.js @@ -127,7 +127,9 @@ export default class LargeVideoManager { // the video was not rendered, before the connection has failed. const isHavingConnectivityIssues = APP.conference.isParticipantConnectionActive(id) === false; - if (isHavingConnectivityIssues + + if (videoType === VIDEO_CONTAINER_TYPE + && isHavingConnectivityIssues && (isUserSwitch || !container.wasVideoRendered)) { showAvatar = true; } @@ -155,10 +157,15 @@ export default class LargeVideoManager { // Make sure no notification about remote failure is shown as // its UI conflicts with the one for local connection interrupted. + const isConnected = APP.conference.isConnectionInterrupted() + || !isHavingConnectivityIssues; + this.updateParticipantConnStatusIndication( id, - APP.conference.isConnectionInterrupted() - || !isHavingConnectivityIssues); + isConnected, + (isHavingConnectivityIssues) + ? "connection.USER_CONNECTION_INTERRUPTED" + : "connection.LOW_BANDWIDTH"); // resolve updateLargeVideo promise after everything is done promise.then(resolve); @@ -180,10 +187,11 @@ export default class LargeVideoManager { * @param {string} id the id of remote participant(MUC nickname) * @param {boolean} isConnected true if the connection is active or false * when the user is having connectivity issues. + * @param {string} messageKey the i18n key of the message * * @private */ - updateParticipantConnStatusIndication (id, isConnected) { + updateParticipantConnStatusIndication (id, isConnected, messageKey) { // Apply grey filter on the large video this.videoContainer.showRemoteConnectionProblemIndicator(!isConnected); @@ -196,7 +204,7 @@ export default class LargeVideoManager { let displayName = APP.conference.getParticipantDisplayName(id); this._setRemoteConnectionMessage( - "connection.USER_CONNECTION_INTERRUPTED", + messageKey, { displayName: displayName }); // Show it now only if the VideoContainer is on top @@ -332,7 +340,7 @@ export default class LargeVideoManager { */ showRemoteConnectionMessage (show) { if (typeof show !== 'boolean') { - show = APP.conference.isParticipantConnectionActive(this.id); + show = !APP.conference.isParticipantConnectionActive(this.id); } if (show) { @@ -458,7 +466,7 @@ export default class LargeVideoManager { // "avatar" and "video connection" can not be displayed both // at the same time, but the latter is of higher priority and it // will hide the avatar one if will be displayed. - this.showRemoteConnectionMessage(/* fet the current state */); + this.showRemoteConnectionMessage(/* fetch the current state */); this.showLocalConnectionMessage(/* fetch the current state */); } }); diff --git a/modules/UI/videolayout/RemoteVideo.js b/modules/UI/videolayout/RemoteVideo.js index 44f14b98a1..db69a74041 100644 --- a/modules/UI/videolayout/RemoteVideo.js +++ b/modules/UI/videolayout/RemoteVideo.js @@ -460,12 +460,12 @@ RemoteVideo.prototype.setMutedView = function(isMuted) { * @private */ RemoteVideo.prototype._figureOutMutedWhileDisconnected -= function(isDisconnected) { - if (isDisconnected && this.isVideoMuted) { - this.mutedWhileDisconnected = true; - } else if (!isDisconnected && !this.isVideoMuted) { - this.mutedWhileDisconnected = false; - } + = function(isDisconnected) { + if (isDisconnected && this.isVideoMuted) { + this.mutedWhileDisconnected = true; + } else if (!isDisconnected && !this.isVideoMuted) { + this.mutedWhileDisconnected = false; + } }; /** @@ -554,8 +554,7 @@ RemoteVideo.prototype.isVideoPlayable = function () { */ RemoteVideo.prototype.updateView = function () { - this.updateConnectionStatusIndicator( - null /* will obtain the status from 'conference' */); + this.updateConnectionStatusIndicator(); // This must be called after 'updateConnectionStatusIndicator' because it // affects the display mode by modifying 'mutedWhileDisconnected' flag @@ -564,19 +563,13 @@ RemoteVideo.prototype.updateView = function () { /** * Updates the UI to reflect user's connectivity status. - * @param isActive {boolean|null} 'true' if user's connection is active or - * 'false' when the use is having some connectivity issues and a warning - * should be displayed. When 'null' is passed then the current value will be - * obtained from the conference instance. */ -RemoteVideo.prototype.updateConnectionStatusIndicator = function (isActive) { - // Check for initial value if 'isActive' is not defined - if (typeof isActive !== "boolean") { - isActive = this.isConnectionActive(); - if (isActive === null) { - // Cancel processing at this point - no update - return; - } +RemoteVideo.prototype.updateConnectionStatusIndicator = function () { + const isActive = this.isConnectionActive(); + + if (isActive === null) { + // Cancel processing at this point - no update + return; } logger.debug(this.id + " thumbnail is connection active ? " + isActive); @@ -700,44 +693,6 @@ RemoteVideo.prototype.addRemoteStreamElement = function (stream) { } }; -/** - * Show/hide peer container for the given id. - */ -RemoteVideo.prototype.showPeerContainer = function (state) { - if (!this.container) - return; - - var isHide = state === 'hide'; - var resizeThumbnails = false; - - if (!isHide) { - if (!$(this.container).is(':visible')) { - resizeThumbnails = true; - $(this.container).show(); - } - // Call updateView, so that we'll figure out if avatar - // should be displayed based on video muted status and whether or not - // it's in the lastN set - this.updateView(); - } - else if ($(this.container).is(':visible') && isHide) - { - resizeThumbnails = true; - $(this.container).hide(); - if(this.connectionIndicator) - this.connectionIndicator.hide(); - } - - if (resizeThumbnails) { - this.VideoLayout.resizeThumbnails(); - } - - // We want to be able to pin a participant from the contact list, even - // if he's not in the lastN set! - // ContactList.setClickable(id, !isHide); - -}; - RemoteVideo.prototype.updateResolution = function (resolution) { if (this.connectionIndicator) { this.connectionIndicator.updateResolution(resolution); diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index 5e9ffbe120..aa3d12423d 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -1,4 +1,4 @@ -/* global $, JitsiMeetJS, interfaceConfig */ +/* global $, APP, JitsiMeetJS, interfaceConfig */ const logger = require("jitsi-meet-logger").getLogger(__filename); import Avatar from "../avatar/Avatar"; @@ -446,7 +446,7 @@ SmallVideo.prototype.isCurrentlyOnLargeVideo = function () { SmallVideo.prototype.isVideoPlayable = function() { return this.videoStream // Is there anything to display ? && !this.isVideoMuted && !this.videoStream.isMuted() // Muted ? - && (this.isLocal || this.VideoLayout.isInLastN(this.id)); + && (this.isLocal || APP.conference.isInLastN(this.id)); }; /** diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index 822b7954c5..3ef4eaf460 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -1,4 +1,4 @@ -/* global config, APP, $, interfaceConfig */ +/* global APP, $, interfaceConfig */ const logger = require("jitsi-meet-logger").getLogger(__filename); import FilmStrip from "./FilmStrip"; @@ -14,10 +14,6 @@ var remoteVideos = {}; var localVideoThumbnail = null; var currentDominantSpeaker = null; -var localLastNCount = config.channelLastN; -var localLastNSet = []; -var lastNEndpointsCache = []; -var lastNPickupId = null; var eventEmitter = null; @@ -60,8 +56,6 @@ function onContactClicked (id) { // let the bridge adjust its lastN set for myjid and store // the pinned user in the lastNPickupId variable to be // picked up later by the lastN changed event handler. - - lastNPickupId = id; eventEmitter.emit(UIEvents.PINNED_ENDPOINT, remoteVideo, true); } } @@ -114,8 +108,6 @@ var VideoLayout = { // the local video thumb maybe one pixel this.resizeThumbnails(false, true); - this.lastNCount = config.channelLastN; - this.registerListeners(); }, @@ -162,14 +154,6 @@ var VideoLayout = { largeVideo.updateLargeVideoAudioLevel(lvl); }, - isInLastN (resource) { - return this.lastNCount < 0 || // lastN is disabled - // lastNEndpoints cache not built yet - (this.lastNCount > 0 && !lastNEndpointsCache.length) || - (lastNEndpointsCache && - lastNEndpointsCache.indexOf(resource) !== -1); - }, - changeLocalAudio (stream) { let localAudio = document.getElementById('localAudio'); localAudio = stream.attach(localAudio); @@ -457,13 +441,8 @@ var VideoLayout = { remoteVideo.setVideoType(VIDEO_CONTAINER_TYPE); } - // In case this is not currently in the last n we don't show it. - if (localLastNCount && localLastNCount > 0 && - FilmStrip.getThumbs().remoteThumbs.length >= localLastNCount + 2) { - remoteVideo.showPeerContainer('hide'); - } else { - VideoLayout.resizeThumbnails(false, true); - } + VideoLayout.resizeThumbnails(false, true); + // Initialize the view remoteVideo.updateView(); }, @@ -713,142 +692,22 @@ var VideoLayout = { * endpoints */ onLastNEndpointsChanged (lastNEndpoints, endpointsEnteringLastN) { - if (this.lastNCount !== lastNEndpoints.length) - this.lastNCount = lastNEndpoints.length; - - lastNEndpointsCache = lastNEndpoints; - - // Say A, B, C, D, E, and F are in a conference and LastN = 3. - // - // If LastN drops to, say, 2, because of adaptivity, then E should see - // thumbnails for A, B and C. A and B are in E's server side LastN set, - // so E sees them. C is only in E's local LastN set. - // - // If F starts talking and LastN = 3, then E should see thumbnails for - // F, A, B. B gets "ejected" from E's server side LastN set, but it - // enters E's local LastN ejecting C. - - // Increase the local LastN set size, if necessary. - if (this.lastNCount > localLastNCount) { - localLastNCount = this.lastNCount; - } - - // Update the local LastN set preserving the order in which the - // endpoints appeared in the LastN/local LastN set. - var nextLocalLastNSet = lastNEndpoints.slice(0); - for (var i = 0; i < localLastNSet.length; i++) { - if (nextLocalLastNSet.length >= localLastNCount) { - break; - } - - var resourceJid = localLastNSet[i]; - if (nextLocalLastNSet.indexOf(resourceJid) === -1) { - nextLocalLastNSet.push(resourceJid); - } - } - - localLastNSet = nextLocalLastNSet; - var updateLargeVideo = false; - - // Handle LastN/local LastN changes. - FilmStrip.getThumbs().remoteThumbs.each(( index, element ) => { - var resourceJid = getPeerContainerResourceId(element); - var smallVideo = remoteVideos[resourceJid]; - - // We do not want to process any logic for our own(local) video - // because the local participant is never in the lastN set. - // The code of this function might detect that the local participant - // has been dropped out of the lastN set and will update the large - // video - // Detected from avatar tests, where lastN event override - // local video pinning - if(APP.conference.isLocalId(resourceJid)) - return; - - var isReceived = true; - if (resourceJid && - lastNEndpoints.indexOf(resourceJid) < 0 && - localLastNSet.indexOf(resourceJid) < 0) { - logger.log("Remove from last N", resourceJid); - if (smallVideo) - smallVideo.showPeerContainer('hide'); - else if (!APP.conference.isLocalId(resourceJid)) - logger.error("No remote video for: " + resourceJid); - isReceived = false; - } else if (resourceJid && - //TOFIX: smallVideo may be undefined - smallVideo.isVisible() && - lastNEndpoints.indexOf(resourceJid) < 0 && - localLastNSet.indexOf(resourceJid) >= 0) { - - // TOFIX: if we're here we already know that the smallVideo - // exists. Look at the previous FIX above. - if (smallVideo) - smallVideo.showPeerContainer('avatar'); - else if (!APP.conference.isLocalId(resourceJid)) - logger.error("No remote video for: " + resourceJid); - isReceived = false; - } - - if (!isReceived) { - // resourceJid has dropped out of the server side lastN set, so - // it is no longer being received. If resourceJid was being - // displayed in the large video we have to switch to another - // user. - if (!updateLargeVideo && - this.isCurrentlyOnLarge(resourceJid)) { - updateLargeVideo = true; - } - } - }); - - if (!endpointsEnteringLastN || endpointsEnteringLastN.length < 0) - endpointsEnteringLastN = lastNEndpoints; - - if (endpointsEnteringLastN && endpointsEnteringLastN.length > 0) { - endpointsEnteringLastN.forEach(function (resourceJid) { - - var remoteVideo = remoteVideos[resourceJid]; - if (remoteVideo) - remoteVideo.showPeerContainer('show'); - if (!remoteVideo.isVisible()) { - logger.log("Add to last N", resourceJid); - - remoteVideo.addRemoteStreamElement(remoteVideo.videoStream); - - if (lastNPickupId == resourceJid) { - // Clean up the lastN pickup id. - lastNPickupId = null; - - VideoLayout.handleVideoThumbClicked(resourceJid); - - updateLargeVideo = false; + Object.keys(remoteVideos).forEach( + id => { + if (lastNEndpoints.length > 0 + && lastNEndpoints.indexOf(id) < 0 + || endpointsEnteringLastN.length > 0 + && endpointsEnteringLastN.indexOf(id) > 0) { + + let remoteVideo = (id) ? remoteVideos[id] : null; + if (remoteVideo) { + remoteVideo.updateView(); + if (remoteVideo.isCurrentlyOnLargeVideo()) + this.updateLargeVideo(id); } - remoteVideo.waitForPlayback( - remoteVideo.selectVideoElement()[0], - remoteVideo.videoStream); } }); - } - - // The endpoint that was being shown in the large video has dropped out - // of the lastN set and there was no lastN pickup jid. We need to update - // the large video now. - - if (updateLargeVideo) { - var resource; - // Find out which endpoint to show in the large video. - for (i = 0; i < lastNEndpoints.length; i++) { - resource = lastNEndpoints[i]; - if (!resource || APP.conference.isLocalId(resource)) - continue; - - // videoSrcToSsrc needs to be update for this call to succeed. - this.updateLargeVideo(resource); - break; - } - } }, /**