From 87b11551801517dcd5d0129469573d93d98569ee Mon Sep 17 00:00:00 2001 From: Hristo Terezov Date: Mon, 14 Dec 2020 15:38:29 -0600 Subject: [PATCH] fix(video):Always show avatar if video is inactive --- modules/UI/videolayout/LargeVideoManager.js | 14 +---- modules/UI/videolayout/RemoteVideo.js | 28 ++++----- modules/UI/videolayout/SmallVideo.js | 4 +- modules/UI/videolayout/VideoContainer.js | 21 +------ modules/UI/videolayout/VideoLayout.js | 11 +--- react/features/base/participants/actions.js | 18 +++--- react/features/base/participants/functions.js | 41 +------------ .../features/base/participants/middleware.js | 57 +------------------ react/features/base/participants/reducer.js | 1 - 9 files changed, 30 insertions(+), 165 deletions(-) diff --git a/modules/UI/videolayout/LargeVideoManager.js b/modules/UI/videolayout/LargeVideoManager.js index 4d78e7a032..191bac6099 100644 --- a/modules/UI/videolayout/LargeVideoManager.js +++ b/modules/UI/videolayout/LargeVideoManager.js @@ -218,21 +218,11 @@ export default class LargeVideoManager { // change the avatar url on large this.updateAvatar(); - // If the user's connection is disrupted then the avatar will be - // displayed in case we have no video image cached. That is if - // there was a user switch (image is lost on stream detach) or if - // the video was not rendered, before the connection has failed. - const wasUsersImageCached - = !isUserSwitch && container.wasVideoRendered; const isVideoMuted = !stream || stream.isMuted(); const participant = getParticipantById(APP.store.getState(), id); const connectionStatus = participant?.connectionStatus; - const isVideoRenderable - = !isVideoMuted - && (APP.conference.isLocalId(id) - || connectionStatus - === JitsiParticipantConnectionStatus.ACTIVE - || wasUsersImageCached); + const isVideoRenderable = !isVideoMuted + && (APP.conference.isLocalId(id) || connectionStatus === JitsiParticipantConnectionStatus.ACTIVE); const showAvatar = isVideoContainer diff --git a/modules/UI/videolayout/RemoteVideo.js b/modules/UI/videolayout/RemoteVideo.js index 5e20316cf4..fc35004318 100644 --- a/modules/UI/videolayout/RemoteVideo.js +++ b/modules/UI/videolayout/RemoteVideo.js @@ -331,23 +331,20 @@ export default class RemoteVideo extends SmallVideo { } /** - * The remote video is considered "playable" once the can play event has been received. It will be allowed to - * display video also in {@link JitsiParticipantConnectionStatus.INTERRUPTED} if the video has received the canplay - * event and was not muted while not in ACTIVE state. This basically means that there is stalled video image cached - * that could be displayed. It's used to show "grey video image" in user's thumbnail when there are connectivity - * issues. + * The remote video is considered "playable" once the can play event has been received. * * @inheritdoc * @override */ isVideoPlayable() { const participant = getParticipantById(APP.store.getState(), this.id); - const { connectionStatus, mutedWhileDisconnected } = participant || {}; + const { connectionStatus } = participant || {}; - return super.isVideoPlayable() - && this._canPlayEventReceived - && (connectionStatus === JitsiParticipantConnectionStatus.ACTIVE - || (connectionStatus === JitsiParticipantConnectionStatus.INTERRUPTED && !mutedWhileDisconnected)); + return ( + super.isVideoPlayable() + && this._canPlayEventReceived + && connectionStatus === JitsiParticipantConnectionStatus.ACTIVE + ); } /** @@ -373,6 +370,8 @@ export default class RemoteVideo extends SmallVideo { * @param {*} stream */ waitForPlayback(streamElement, stream) { + $(streamElement).hide(); + const webRtcStream = stream.getOriginalStream(); const isVideo = stream.isVideoTrack(); @@ -382,7 +381,12 @@ export default class RemoteVideo extends SmallVideo { const listener = () => { this._canPlayEventReceived = true; - this.VideoLayout.remoteVideoActive(streamElement, this.id); + + logger.info(`${this.id} video is now active`, streamElement); + if (streamElement) { + $(streamElement).show(); + } + streamElement.removeEventListener('canplay', listener); // Refresh to show the video @@ -422,8 +426,6 @@ export default class RemoteVideo extends SmallVideo { // Put new stream element always in front streamElement = UIUtils.prependChild(this.container, streamElement); - $(streamElement).hide(); - this.waitForPlayback(streamElement, stream); stream.attach(streamElement); diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index d05a30c1d8..1271a59130 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -433,7 +433,7 @@ export default class SmallVideo { */ computeDisplayModeInput() { let isScreenSharing = false; - let connectionStatus, mutedWhileDisconnected; + let connectionStatus; const state = APP.store.getState(); const participant = getParticipantById(state, this.id); @@ -443,7 +443,6 @@ export default class SmallVideo { isScreenSharing = typeof track !== 'undefined' && track.videoType === 'desktop'; connectionStatus = participant.connectionStatus; - mutedWhileDisconnected = participant.mutedWhileDisconnected; } return { @@ -454,7 +453,6 @@ export default class SmallVideo { isVideoPlayable: this.isVideoPlayable(), hasVideo: Boolean(this.selectVideoElement().length), connectionStatus, - mutedWhileDisconnected, canPlayEventReceived: this._canPlayEventReceived, videoStream: Boolean(this.videoStream), isScreenSharing, diff --git a/modules/UI/videolayout/VideoContainer.js b/modules/UI/videolayout/VideoContainer.js index 2b553ee462..ced00a33cb 100644 --- a/modules/UI/videolayout/VideoContainer.js +++ b/modules/UI/videolayout/VideoContainer.js @@ -233,14 +233,6 @@ export class VideoContainer extends LargeContainer { this.$remotePresenceMessage = $('#remotePresenceMessage'); - /** - * Indicates whether or not the video stream attached to the video - * element has started(which means that there is any image rendered - * even if the video is stalled). - * @type {boolean} - */ - this.wasVideoRendered = false; - this.$wrapper = $('#largeVideoWrapper'); /** @@ -249,17 +241,12 @@ export class VideoContainer extends LargeContainer { * video anyway. */ this.$wrapperParent = this.$wrapper.parent(); - this.avatarHeight = $('#dominantSpeakerAvatarContainer').height(); - - const onPlayingCallback = function(event) { + this.$video[0].onplaying = function(event) { if (typeof resizeContainer === 'function') { resizeContainer(event); } - this.wasVideoRendered = true; - }.bind(this); - - this.$video[0].onplaying = onPlayingCallback; + }; /** * A Set of functions to invoke when the video element resizes. @@ -491,10 +478,6 @@ export class VideoContainer extends LargeContainer { return; } - // The stream has changed, so the image will be lost on detach - this.wasVideoRendered = false; - - // detach old stream if (this.stream) { this.stream.detach(this.$video[0]); diff --git a/modules/UI/videolayout/VideoLayout.js b/modules/UI/videolayout/VideoLayout.js index a43afef74f..0a97375f65 100644 --- a/modules/UI/videolayout/VideoLayout.js +++ b/modules/UI/videolayout/VideoLayout.js @@ -1,4 +1,4 @@ -/* global APP, $ */ +/* global APP */ import Logger from 'jitsi-meet-logger'; @@ -314,15 +314,6 @@ const VideoLayout = { remoteVideo.updateView(); }, - // FIXME: what does this do??? - remoteVideoActive(videoElement, resourceJid) { - logger.info(`${resourceJid} video is now active`, videoElement); - if (videoElement) { - $(videoElement).show(); - } - this._updateLargeVideoIfDisplayed(resourceJid, true); - }, - /** * On video muted event. */ diff --git a/react/features/base/participants/actions.js b/react/features/base/participants/actions.js index 6a766171b7..c5b2dd32c9 100644 --- a/react/features/base/participants/actions.js +++ b/react/features/base/participants/actions.js @@ -19,8 +19,7 @@ import { import { getLocalParticipant, getNormalizedDisplayName, - getParticipantDisplayName, - figureOutMutedWhileDisconnectedStatus + getParticipantDisplayName } from './functions'; /** @@ -217,15 +216,12 @@ export function muteRemoteParticipant(id) { * }} */ export function participantConnectionStatusChanged(id, connectionStatus) { - return (dispatch, getState) => { - dispatch({ - type: PARTICIPANT_UPDATED, - participant: { - connectionStatus, - id, - mutedWhileDisconnected: figureOutMutedWhileDisconnectedStatus(getState(), id, connectionStatus) - } - }); + return { + type: PARTICIPANT_UPDATED, + participant: { + connectionStatus, + id + } }; } diff --git a/react/features/base/participants/functions.js b/react/features/base/participants/functions.js index 73211dc59a..28bd8aef1a 100644 --- a/react/features/base/participants/functions.js +++ b/react/features/base/participants/functions.js @@ -6,7 +6,7 @@ import type { Store } from 'redux'; import { JitsiParticipantConnectionStatus } from '../lib-jitsi-meet'; import { MEDIA_TYPE, shouldRenderVideoTrack } from '../media'; import { toState } from '../redux'; -import { getTrackByMediaTypeAndParticipant, isRemoteTrackMuted } from '../tracks'; +import { getTrackByMediaTypeAndParticipant } from '../tracks'; import { createDeferred } from '../util'; import { @@ -369,45 +369,6 @@ export function shouldRenderParticipantVideo(stateful: Object | Function, id: st return participantIsInLargeVideoWithScreen; } -/** - * Figures out the value of mutedWhileDisconnected status by taking into - * account remote participant's network connectivity and video muted status. - * The flag is set to true if remote participant's video gets muted - * during his media connection disruption. This is to prevent black video - * being render on the thumbnail, because even though once the video has - * been played the image usually remains on the video element it seems that - * after longer period of the video element being hidden this image can be - * lost. - * - * @param {Object|Function} stateful - Object or function that can be resolved - * to the Redux state. - * @param {string} participantID - The ID of the participant. - * @param {string} [connectionStatus] - A connection status to be used. - * @returns {boolean} - The mutedWhileDisconnected value. - */ -export function figureOutMutedWhileDisconnectedStatus( - stateful: Function | Object, participantID: string, connectionStatus: ?string) { - const state = toState(stateful); - const participant = getParticipantById(state, participantID); - - if (!participant || participant.local) { - return undefined; - } - - const isActive = (connectionStatus || participant.connectionStatus) === JitsiParticipantConnectionStatus.ACTIVE; - const isVideoMuted = isRemoteTrackMuted(state['features/base/tracks'], MEDIA_TYPE.VIDEO, participantID); - let mutedWhileDisconnected = participant.mutedWhileDisconnected || false; - - if (!isActive && isVideoMuted) { - mutedWhileDisconnected = true; - } else if (isActive && !isVideoMuted) { - mutedWhileDisconnected = false; - } - - return mutedWhileDisconnected; -} - - /** * Resolves the first loadable avatar URL for a participant. * diff --git a/react/features/base/participants/middleware.js b/react/features/base/participants/middleware.js index 5f4a6f1f3c..ec762e7a89 100644 --- a/react/features/base/participants/middleware.js +++ b/react/features/base/participants/middleware.js @@ -12,7 +12,6 @@ import { import { JitsiConferenceEvents } from '../lib-jitsi-meet'; import { MiddlewareRegistry, StateListenerRegistry } from '../redux'; import { playSound, registerSound, unregisterSound } from '../sounds'; -import { getTrackByJitsiTrack, TRACK_ADDED, TRACK_REMOVED, TRACK_UPDATED } from '../tracks'; import { DOMINANT_SPEAKER_CHANGED, @@ -42,8 +41,7 @@ import { getLocalParticipant, getParticipantById, getParticipantCount, - getParticipantDisplayName, - figureOutMutedWhileDisconnectedStatus + getParticipantDisplayName } from './functions'; import { PARTICIPANT_JOINED_FILE, PARTICIPANT_LEFT_FILE } from './sounds'; @@ -137,10 +135,6 @@ MiddlewareRegistry.register(store => next => action => { case PARTICIPANT_UPDATED: return _participantJoinedOrUpdated(store, next, action); - case TRACK_ADDED: - case TRACK_REMOVED: - case TRACK_UPDATED: - return _trackChanged(store, next, action); } return next(action); @@ -460,55 +454,6 @@ function _registerSounds({ dispatch }) { dispatch(registerSound(PARTICIPANT_LEFT_SOUND_ID, PARTICIPANT_LEFT_FILE)); } -/** - * Notifies the feature base/participants that the action there has been a change in the tracks of the participants. - * - * @param {Store} store - The redux store in which the specified {@code action} is being dispatched. - * @param {Dispatch} next - The redux {@code dispatch} function to dispatch the specified {@code action} in the - * specified {@code store}. - * @param {Action} action - The redux action {@code PARTICIPANT_JOINED} or {@code PARTICIPANT_UPDATED} which is being - * dispatched in the specified {@code store}. - * @private - * @returns {Object} The value returned by {@code next(action)}. - */ -function _trackChanged({ dispatch, getState }, next, action) { - const { jitsiTrack } = action.track; - let track; - - if (action.type === TRACK_REMOVED) { - track = getTrackByJitsiTrack(getState()['features/base/tracks'], jitsiTrack); - } - - const result = next(action); - - if (action.type !== TRACK_REMOVED) { - track = getTrackByJitsiTrack(getState()['features/base/tracks'], jitsiTrack); - } - - if (typeof track === 'undefined' || track.local) { - return result; - } - - const { participantId } = track; - const state = getState(); - const participant = getParticipantById(state, participantId); - - if (!participant) { - return result; - } - - const mutedWhileDisconnected = figureOutMutedWhileDisconnectedStatus(state, participantId); - - if (participant.mutedWhileDisconnected !== mutedWhileDisconnected) { - dispatch(participantUpdated({ - id: participantId, - mutedWhileDisconnected - })); - } - - return result; -} - /** * Unregisters sounds related with the participants feature. * diff --git a/react/features/base/participants/reducer.js b/react/features/base/participants/reducer.js index eb39349fdd..48e224da99 100644 --- a/react/features/base/participants/reducer.js +++ b/react/features/base/participants/reducer.js @@ -221,7 +221,6 @@ function _participantJoined({ participant }) { isJigasi, loadableAvatarUrl, local: local || false, - mutedWhileDisconnected: local ? undefined : false, name, pinned: pinned || false, presence,