From 91197bc69f062cc492afebb6f37a5cb3beac5519 Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty <54324652+jallamsetty1@users.noreply.github.com> Date: Mon, 10 May 2021 16:06:19 -0400 Subject: [PATCH] fix(quality-control): Send the new receiver constraints on state changes. The client now listens for changes to lastN, selectedEndpoints and maxReceiverVideoQuality in redux to trigger sending bridge message in the new format. This fixes an issue where the stage view <-> tile view changes prompt two receiver constraints messages to be sent, first with the maxHeight update and then with the selected endpoints update. --- react/features/base/lastn/middleware.js | 34 +--- react/features/large-video/actions.any.js | 29 +--- react/features/video-layout/actionTypes.js | 6 + react/features/video-layout/actions.js | 18 ++ react/features/video-layout/reducer.js | 8 + react/features/video-quality/middleware.js | 182 +------------------- react/features/video-quality/subscriber.js | 190 +++++++++++++++++++++ 7 files changed, 239 insertions(+), 228 deletions(-) create mode 100644 react/features/video-quality/subscriber.js diff --git a/react/features/base/lastn/middleware.js b/react/features/base/lastn/middleware.js index 0e0d9459d6..630ac1c3b2 100644 --- a/react/features/base/lastn/middleware.js +++ b/react/features/base/lastn/middleware.js @@ -18,13 +18,12 @@ import { import { MiddlewareRegistry } from '../redux'; import { isLocalVideoTrackDesktop } from '../tracks/functions'; -import { SET_LAST_N } from './actionTypes'; +import { setLastN } from './actions'; import { limitLastN } from './functions'; import logger from './logger'; declare var APP: Object; - MiddlewareRegistry.register(store => next => action => { const result = next(action); @@ -41,12 +40,6 @@ MiddlewareRegistry.register(store => next => action => { case SET_TILE_VIEW: _updateLastN(store); break; - case SET_LAST_N: { - const { lastN } = action; - - _updateLastN(store, lastN); - break; - } } return result; @@ -56,11 +49,10 @@ MiddlewareRegistry.register(store => next => action => { * Updates the last N value in the conference based on the current state of the redux store. * * @param {Store} store - The redux store. - * @param {number} value - The last-n value to be set. * @private * @returns {void} */ -function _updateLastN({ getState }, value = null) { +function _updateLastN({ dispatch, getState }) { const state = getState(); const { conference } = state['features/base/conference']; const { enabled: audioOnly } = state['features/base/audio-only']; @@ -77,12 +69,11 @@ function _updateLastN({ getState }, value = null) { } // Select the lastN value based on the following preference order. - // 1. The value passed to the setLastN action that is dispatched. - // 2. The last-n value in redux. - // 3. The last-n value from 'startLastN' if it is specified in config.js - // 4. The last-n value from 'channelLastN' if specified in config.js. - // 5. -1 as the default value. - let lastNSelected = value || lastN || (config.startLastN ?? (config.channelLastN ?? -1)); + // 1. The last-n value in redux. + // 2. The last-n value from 'startLastN' if it is specified in config.js + // 3. The last-n value from 'channelLastN' if specified in config.js. + // 4. -1 as the default value. + let lastNSelected = lastN || (config.startLastN ?? (config.channelLastN ?? -1)); // Apply last N limit based on the # of participants and config settings. const limitedLastN = limitLastN(participantCount, lastNLimits); @@ -111,15 +102,6 @@ function _updateLastN({ getState }, value = null) { lastNSelected = 1; } - if (conference.getLastN() === lastNSelected) { - return; - } - logger.info(`Setting last N to: ${lastNSelected}`); - - try { - conference.setLastN(lastNSelected); - } catch (err) { - logger.error(`Failed to set lastN: ${err}`); - } + dispatch(setLastN(lastNSelected)); } diff --git a/react/features/large-video/actions.any.js b/react/features/large-video/actions.any.js index e41f916333..e3f0d26378 100644 --- a/react/features/large-video/actions.any.js +++ b/react/features/large-video/actions.any.js @@ -2,15 +2,9 @@ import type { Dispatch } from 'redux'; -import { - createSelectParticipantFailedEvent, - sendAnalytics -} from '../analytics'; -import { _handleParticipantError } from '../base/conference'; import { MEDIA_TYPE } from '../base/media'; import { getParticipants } from '../base/participants'; -import { reportError } from '../base/util'; -import { shouldDisplayTileView } from '../video-layout'; +import { selectEndpoints, shouldDisplayTileView } from '../video-layout'; import { SELECT_LARGE_VIDEO_PARTICIPANT, @@ -25,24 +19,11 @@ import { export function selectParticipant() { return (dispatch: Dispatch, getState: Function) => { const state = getState(); - const { conference } = state['features/base/conference']; - - if (conference) { - const ids = shouldDisplayTileView(state) - ? getParticipants(state).map(participant => participant.id) - : [ state['features/large-video'].participantId ]; - - try { - conference.selectParticipants(ids); - } catch (err) { - _handleParticipantError(err); + const ids = shouldDisplayTileView(state) + ? getParticipants(state).map(participant => participant.id) + : [ state['features/large-video'].participantId ]; - sendAnalytics(createSelectParticipantFailedEvent(err)); - - reportError( - err, `Failed to select participants ${ids.toString()}`); - } - } + dispatch(selectEndpoints(ids)); }; } diff --git a/react/features/video-layout/actionTypes.js b/react/features/video-layout/actionTypes.js index 0b19fa84b3..400297efe5 100644 --- a/react/features/video-layout/actionTypes.js +++ b/react/features/video-layout/actionTypes.js @@ -10,6 +10,12 @@ export const SCREEN_SHARE_REMOTE_PARTICIPANTS_UPDATED = 'SCREEN_SHARE_REMOTE_PARTICIPANTS_UPDATED'; +/** + * The type of the action which sets the list of the endpoints to be selected for video forwarding + * from the bridge. + */ +export const SELECT_ENDPOINTS = 'SELECT_ENDPOINTS'; + /** * The type of the action which enables or disables the feature for showing * video thumbnails in a two-axis tile view. diff --git a/react/features/video-layout/actions.js b/react/features/video-layout/actions.js index 0494589f10..d54e69ae48 100644 --- a/react/features/video-layout/actions.js +++ b/react/features/video-layout/actions.js @@ -4,10 +4,28 @@ import type { Dispatch } from 'redux'; import { SCREEN_SHARE_REMOTE_PARTICIPANTS_UPDATED, + SELECT_ENDPOINTS, SET_TILE_VIEW } from './actionTypes'; import { shouldDisplayTileView } from './functions'; +/** + * Creates a (redux) action which signals that a new set of remote endpoints need to be selected. + * + * @param {Array} participantIds - The remote participants that are currently selected + * for video forwarding from the bridge. + * @returns {{ + * type: SELECT_ENDPOINTS, + * particpantsIds: Array + * }} + */ +export function selectEndpoints(participantIds: Array) { + return { + type: SELECT_ENDPOINTS, + participantIds + }; +} + /** * Creates a (redux) action which signals that the list of known remote participants * with screen shares has changed. diff --git a/react/features/video-layout/reducer.js b/react/features/video-layout/reducer.js index db4e744279..f5bfe7c8cd 100644 --- a/react/features/video-layout/reducer.js +++ b/react/features/video-layout/reducer.js @@ -4,6 +4,7 @@ import { ReducerRegistry } from '../base/redux'; import { SCREEN_SHARE_REMOTE_PARTICIPANTS_UPDATED, + SELECT_ENDPOINTS, SET_TILE_VIEW } from './actionTypes'; @@ -34,6 +35,13 @@ ReducerRegistry.register(STORE_NAME, (state = DEFAULT_STATE, action) => { }; } + case SELECT_ENDPOINTS: { + return { + ...state, + selectedEndpoints: action.participantIds + }; + } + case SET_TILE_VIEW: return { ...state, diff --git a/react/features/video-quality/middleware.js b/react/features/video-quality/middleware.js index dfa7c70557..60ae11e19b 100644 --- a/react/features/video-quality/middleware.js +++ b/react/features/video-quality/middleware.js @@ -1,21 +1,13 @@ // @flow -import { - CONFERENCE_JOINED, - DATA_CHANNEL_OPENED -} from '../base/conference'; +import { CONFERENCE_JOINED } from '../base/conference'; import { SET_CONFIG } from '../base/config'; -import { getParticipantCount } from '../base/participants'; -import { MiddlewareRegistry, StateListenerRegistry } from '../base/redux'; -import { shouldDisplayTileView } from '../video-layout'; +import { MiddlewareRegistry } from '../base/redux'; -import { setPreferredVideoQuality, setMaxReceiverVideoQuality } from './actions'; -import { VIDEO_QUALITY_LEVELS } from './constants'; -import { getReceiverVideoQualityLevel } from './functions'; +import { setPreferredVideoQuality } from './actions'; import logger from './logger'; -import { getMinHeightForQualityLvlMap } from './selector'; -declare var APP: Object; +import './subscriber'; /** * Implements the middleware of the feature video-quality. @@ -24,10 +16,6 @@ declare var APP: Object; * @returns {Function} */ MiddlewareRegistry.register(({ dispatch, getState }) => next => action => { - if (action.type === DATA_CHANNEL_OPENED) { - return _syncReceiveVideoQuality(getState, next, action); - } - const result = next(action); switch (action.type) { @@ -57,165 +45,3 @@ MiddlewareRegistry.register(({ dispatch, getState }) => next => action => { return result; }); - -/** - * Implements a state listener in order to calculate max receiver video quality. - */ -StateListenerRegistry.register( - /* selector */ state => { - const { reducedUI } = state['features/base/responsive-ui']; - const _shouldDisplayTileView = shouldDisplayTileView(state); - const thumbnailSize = state['features/filmstrip']?.tileViewDimensions?.thumbnailSize; - const participantCount = getParticipantCount(state); - - return { - displayTileView: _shouldDisplayTileView, - participantCount, - reducedUI, - thumbnailHeight: thumbnailSize?.height - }; - }, - /* listener */ ({ displayTileView, participantCount, reducedUI, thumbnailHeight }, { dispatch, getState }) => { - const state = getState(); - const { maxReceiverVideoQuality } = state['features/video-quality']; - const { maxFullResolutionParticipants = 2 } = state['features/base/config']; - - let newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.ULTRA; - - if (reducedUI) { - newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.LOW; - } else if (displayTileView && !Number.isNaN(thumbnailHeight)) { - newMaxRecvVideoQuality = getReceiverVideoQualityLevel(thumbnailHeight, getMinHeightForQualityLvlMap(state)); - - // Override HD level calculated for the thumbnail height when # of participants threshold is exceeded - if (maxReceiverVideoQuality !== newMaxRecvVideoQuality && maxFullResolutionParticipants !== -1) { - const override - = participantCount > maxFullResolutionParticipants - && newMaxRecvVideoQuality > VIDEO_QUALITY_LEVELS.STANDARD; - - logger.info(`Video quality level for thumbnail height: ${thumbnailHeight}, ` - + `is: ${newMaxRecvVideoQuality}, ` - + `override: ${String(override)}, ` - + `max full res N: ${maxFullResolutionParticipants}`); - - if (override) { - newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.STANDARD; - } - } - } - - if (maxReceiverVideoQuality !== newMaxRecvVideoQuality) { - dispatch(setMaxReceiverVideoQuality(newMaxRecvVideoQuality)); - } - }, { - deepEquals: true - }); - -/** - * Helper function for updating the preferred receiver video constraint, based - * on the user preference and the internal maximum. - * - * @param {JitsiConference} conference - The JitsiConference instance for the - * current call. - * @param {number} preferred - The user preferred max frame height. - * @param {number} max - The maximum frame height the application should - * receive. - * @returns {void} - */ -function _setReceiverVideoConstraint(conference, preferred, max) { - if (conference) { - const value = Math.min(preferred, max); - - conference.setReceiverVideoConstraint(value); - logger.info(`setReceiverVideoConstraint: ${value}`); - } -} - -/** - * Helper function for updating the preferred sender video constraint, based - * on the user preference. - * - * @param {JitsiConference} conference - The JitsiConference instance for the - * current call. - * @param {number} preferred - The user preferred max frame height. - * @returns {void} - */ -function _setSenderVideoConstraint(conference, preferred) { - if (conference) { - conference.setSenderVideoConstraint(preferred) - .catch(err => { - logger.error(`Changing sender resolution to ${preferred} failed - ${err} `); - }); - } -} - -/** - * Sets the maximum receive video quality. - * - * @param {Function} getState - The redux function which returns the current redux state. - * @param {Dispatch} next - The redux {@code dispatch} function to dispatch the - * specified {@code action} to the specified {@code store}. - * @param {Action} action - The redux action {@code DATA_CHANNEL_STATUS_CHANGED} - * which is being dispatched in the specified {@code store}. - * @private - * @returns {Object} The value returned by {@code next(action)}. - */ -function _syncReceiveVideoQuality(getState, next, action) { - const state = getState(); - const { - conference - } = state['features/base/conference']; - const { - maxReceiverVideoQuality, - preferredVideoQuality - } = state['features/video-quality']; - - _setReceiverVideoConstraint( - conference, - preferredVideoQuality, - maxReceiverVideoQuality); - - return next(action); -} - - -/** - * Registers a change handler for state['features/base/conference'] to update - * the preferred video quality levels based on user preferred and internal - * settings. - */ -StateListenerRegistry.register( - /* selector */ state => { - const { conference } = state['features/base/conference']; - const { - maxReceiverVideoQuality, - preferredVideoQuality - } = state['features/video-quality']; - - return { - conference, - maxReceiverVideoQuality, - preferredVideoQuality - }; - }, - /* listener */ (currentState, store, previousState = {}) => { - const { - conference, - maxReceiverVideoQuality, - preferredVideoQuality - } = currentState; - const changedConference = conference !== previousState.conference; - const changedPreferredVideoQuality = preferredVideoQuality !== previousState.preferredVideoQuality; - const changedMaxVideoQuality = maxReceiverVideoQuality !== previousState.maxReceiverVideoQuality; - - if (changedConference || changedPreferredVideoQuality || changedMaxVideoQuality) { - _setReceiverVideoConstraint(conference, preferredVideoQuality, maxReceiverVideoQuality); - } - if (changedConference || changedPreferredVideoQuality) { - _setSenderVideoConstraint(conference, preferredVideoQuality); - } - - if (typeof APP !== 'undefined' && changedPreferredVideoQuality) { - APP.API.notifyVideoQualityChanged(preferredVideoQuality); - } - }); diff --git a/react/features/video-quality/subscriber.js b/react/features/video-quality/subscriber.js new file mode 100644 index 0000000000..854147dc81 --- /dev/null +++ b/react/features/video-quality/subscriber.js @@ -0,0 +1,190 @@ +// @flow + +import debounce from 'lodash/debounce'; + +import { _handleParticipantError } from '../base/conference'; +import { getParticipantCount } from '../base/participants'; +import { StateListenerRegistry } from '../base/redux'; +import { reportError } from '../base/util'; +import { shouldDisplayTileView } from '../video-layout'; + +import { setMaxReceiverVideoQuality } from './actions'; +import { VIDEO_QUALITY_LEVELS } from './constants'; +import { getReceiverVideoQualityLevel } from './functions'; +import logger from './logger'; +import { getMinHeightForQualityLvlMap } from './selector'; + +declare var APP: Object; + +/** + * StateListenerRegistry provides a reliable way of detecting changes to selected + * endpoints state and dispatching additional actions. The listener is debounced + * so that the client doesn't end up sending too many bridge messages when the user is + * scrolling through the thumbnails prompting updates to the selected endpoints. + */ +StateListenerRegistry.register( + /* selector */ state => state['features/video-layout'].selectedEndpoints, + /* listener */ debounce((selectedEndpoints, store) => { + _updateReceiverVideoConstraints(store); + }, 1000)); + +/** + * StateListenerRegistry provides a reliable way of detecting changes to + * lastn state and dispatching additional actions. + */ +StateListenerRegistry.register( + /* selector */ state => state['features/base/lastn'].lastN, + /* listener */ (lastN, store) => { + _updateReceiverVideoConstraints(store); + }); + +/** + * StateListenerRegistry provides a reliable way of detecting changes to + * maxReceiverVideoQuality and preferredVideoQuality state and dispatching additional actions. + */ +StateListenerRegistry.register( + /* selector */ state => { + const { + maxReceiverVideoQuality, + preferredVideoQuality + } = state['features/video-quality']; + + return { + maxReceiverVideoQuality, + preferredVideoQuality + }; + }, + /* listener */ (currentState, store, previousState = {}) => { + const { maxReceiverVideoQuality, preferredVideoQuality } = currentState; + const changedPreferredVideoQuality = preferredVideoQuality !== previousState.preferredVideoQuality; + const changedReceiverVideoQuality = maxReceiverVideoQuality !== previousState.maxReceiverVideoQuality; + + if (changedPreferredVideoQuality) { + _setSenderVideoConstraint(preferredVideoQuality, store); + typeof APP !== 'undefined' && APP.API.notifyVideoQualityChanged(preferredVideoQuality); + } + changedReceiverVideoQuality && _updateReceiverVideoConstraints(store); + }); + +/** + * Implements a state listener in order to calculate max receiver video quality. + */ +StateListenerRegistry.register( + /* selector */ state => { + const { reducedUI } = state['features/base/responsive-ui']; + const _shouldDisplayTileView = shouldDisplayTileView(state); + const thumbnailSize = state['features/filmstrip']?.tileViewDimensions?.thumbnailSize; + const participantCount = getParticipantCount(state); + + return { + displayTileView: _shouldDisplayTileView, + participantCount, + reducedUI, + thumbnailHeight: thumbnailSize?.height + }; + }, + /* listener */ ({ displayTileView, participantCount, reducedUI, thumbnailHeight }, { dispatch, getState }) => { + const state = getState(); + const { maxReceiverVideoQuality } = state['features/video-quality']; + const { maxFullResolutionParticipants = 2 } = state['features/base/config']; + + let newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.ULTRA; + + if (reducedUI) { + newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.LOW; + } else if (displayTileView && !Number.isNaN(thumbnailHeight)) { + newMaxRecvVideoQuality = getReceiverVideoQualityLevel(thumbnailHeight, getMinHeightForQualityLvlMap(state)); + + // Override HD level calculated for the thumbnail height when # of participants threshold is exceeded + if (maxReceiverVideoQuality !== newMaxRecvVideoQuality && maxFullResolutionParticipants !== -1) { + const override + = participantCount > maxFullResolutionParticipants + && newMaxRecvVideoQuality > VIDEO_QUALITY_LEVELS.STANDARD; + + logger.info(`Video quality level for thumbnail height: ${thumbnailHeight}, ` + + `is: ${newMaxRecvVideoQuality}, ` + + `override: ${String(override)}, ` + + `max full res N: ${maxFullResolutionParticipants}`); + + if (override) { + newMaxRecvVideoQuality = VIDEO_QUALITY_LEVELS.STANDARD; + } + } + } + + if (maxReceiverVideoQuality !== newMaxRecvVideoQuality) { + dispatch(setMaxReceiverVideoQuality(newMaxRecvVideoQuality)); + } + }, { + deepEquals: true + }); + +/** + * Helper function for updating the preferred sender video constraint, based on the user preference. + * + * @param {number} preferred - The user preferred max frame height. + * @returns {void} + */ +function _setSenderVideoConstraint(preferred, { getState }) { + const state = getState(); + const { conference } = state['features/base/conference']; + + if (!conference) { + return; + } + + logger.info(`Setting sender resolution to ${preferred}`); + conference.setSenderVideoConstraint(preferred) + .catch(error => { + _handleParticipantError(error); + reportError(error, `Changing sender resolution to ${preferred} failed.`); + }); +} + +/** + * Private helper to calculate the receiver video constraints and set them on the bridge channel. + * + * @param {*} store - The redux store. + * @returns {void} + */ +function _updateReceiverVideoConstraints({ getState }) { + const state = getState(); + const { conference } = state['features/base/conference']; + + if (!conference) { + return; + } + const { lastN } = state['features/base/lastn']; + const { maxReceiverVideoQuality, preferredVideoQuality } = state['features/video-quality']; + const { selectedEndpoints } = state['features/video-layout']; + const maxFrameHeight = Math.min(maxReceiverVideoQuality, preferredVideoQuality); + const receiverConstraints = { + constraints: {}, + defaultConstraints: { 'maxHeight': VIDEO_QUALITY_LEVELS.LOW }, + lastN, + onStageEndpoints: [], + selectedEndpoints: [] + }; + + if (!selectedEndpoints?.length) { + return; + } + + // Stage view. + if (selectedEndpoints?.length === 1) { + receiverConstraints.constraints[selectedEndpoints[0]] = { 'maxHeight': maxFrameHeight }; + receiverConstraints.onStageEndpoints = selectedEndpoints; + + // Tile view. + } else { + receiverConstraints.defaultConstraints = { 'maxHeight': maxFrameHeight }; + } + + logger.info(`Setting receiver video constraints to ${JSON.stringify(receiverConstraints)}`); + try { + conference.setReceiverConstraints(receiverConstraints); + } catch (error) { + _handleParticipantError(error); + reportError(error, `Failed to set receiver video constraints ${JSON.stringify(receiverConstraints)}`); + } +}