From d600504d858c39ba8d36a962576ef129963471f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Wed, 2 Aug 2017 17:00:51 +0200 Subject: [PATCH] [RN] Refactor video muting Simplify the code by using a bitfied instead of a couple of boolean flags. This allows us to mute the video from multiple places and only make the unmute effective once they have all unmuted. Alas, this cannot be applied to the web without a massive refactor, because it uses the track muted state as the source of truth instead of the media state. --- react/features/base/conference/actionTypes.js | 14 ----- react/features/base/conference/actions.js | 60 +++++-------------- react/features/base/conference/middleware.js | 46 ++++++++++++-- react/features/base/conference/reducer.js | 23 ------- react/features/base/media/actions.js | 30 ++++++---- react/features/base/media/constants.js | 20 ++++++- react/features/base/media/middleware.js | 5 +- react/features/base/media/reducer.js | 2 +- react/features/base/tracks/actions.js | 5 ++ react/features/base/tracks/middleware.js | 8 ++- .../features/mobile/audio-mode/middleware.js | 14 ++++- .../features/mobile/background/actionTypes.js | 14 ----- react/features/mobile/background/actions.js | 34 +---------- react/features/mobile/background/reducer.js | 7 --- .../features/mobile/full-screen/middleware.js | 7 ++- react/features/mobile/proximity/middleware.js | 7 ++- 16 files changed, 134 insertions(+), 162 deletions(-) diff --git a/react/features/base/conference/actionTypes.js b/react/features/base/conference/actionTypes.js index ca7a0fe8f..5e95c0090 100644 --- a/react/features/base/conference/actionTypes.js +++ b/react/features/base/conference/actionTypes.js @@ -75,20 +75,6 @@ export const LOCK_STATE_CHANGED = Symbol('LOCK_STATE_CHANGED'); */ export const SET_AUDIO_ONLY = Symbol('SET_AUDIO_ONLY'); -/** - * The type of (redux) action which signals that video will be muted because the - * audio-only mode was enabled/disabled. - * - * { - * type: _SET_AUDIO_ONLY_VIDEO_MUTED, - * muted: boolean - * } - * - * @protected - */ -export const _SET_AUDIO_ONLY_VIDEO_MUTED - = Symbol('_SET_AUDIO_ONLY_VIDEO_MUTED'); - /** * The type of (redux) action to set whether or not the displayed large video is * in high-definition. diff --git a/react/features/base/conference/actions.js b/react/features/base/conference/actions.js index 12633ca9d..3b0282f37 100644 --- a/react/features/base/conference/actions.js +++ b/react/features/base/conference/actions.js @@ -1,5 +1,5 @@ import { JitsiConferenceEvents } from '../lib-jitsi-meet'; -import { setVideoMuted } from '../media'; +import { setAudioMuted, setVideoMuted } from '../media'; import { dominantSpeakerChanged, getLocalParticipant, @@ -19,7 +19,6 @@ import { CONFERENCE_WILL_LEAVE, LOCK_STATE_CHANGED, SET_AUDIO_ONLY, - _SET_AUDIO_ONLY_VIDEO_MUTED, SET_LARGE_VIDEO_HD_STATUS, SET_LASTN, SET_PASSWORD, @@ -61,6 +60,19 @@ function _addConferenceListeners(conference, dispatch) { JitsiConferenceEvents.LOCK_STATE_CHANGED, (...args) => dispatch(lockStateChanged(conference, ...args))); + // Dispatches into features/base/media follow: + + // FIXME: This is needed because when Jicofo tells us to start muted + // lib-jitsi-meet does the actual muting. Perhaps this should be refactored + // so applications are hinted to start muted, but lib-jitsi-meet doesn't + // take action. + conference.on( + JitsiConferenceEvents.STARTED_MUTED, + () => { + dispatch(setAudioMuted(Boolean(conference.startAudioMuted))); + dispatch(setVideoMuted(Boolean(conference.startVideoMuted))); + }); + // Dispatches into features/base/tracks follow: conference.on( @@ -296,58 +308,18 @@ export function lockStateChanged(conference, locked) { * * @param {boolean} audioOnly - True if the conference should be audio only; * false, otherwise. - * @private * @returns {{ * type: SET_AUDIO_ONLY, * audioOnly: boolean * }} */ -function _setAudioOnly(audioOnly) { +export function setAudioOnly(audioOnly) { return { type: SET_AUDIO_ONLY, audioOnly }; } -/** - * Signals that the app should mute video because it's now in audio-only mode, - * or unmute it because it no longer is. If video was already muted, nothing - * will happen; otherwise, it will be muted. When audio-only mode is disabled, - * the previous state will be restored. - * - * @param {boolean} muted - True if video should be muted; false, otherwise. - * @protected - * @returns {Function} - */ -export function _setAudioOnlyVideoMuted(muted: boolean) { - return (dispatch, getState) => { - if (muted) { - const { video } = getState()['features/base/media']; - - if (video.muted) { - // Video is already muted, do nothing. - return; - } - } else { - const { audioOnlyVideoMuted } - = getState()['features/base/conference']; - - if (!audioOnlyVideoMuted) { - // We didn't mute video, do nothing. - return; - } - } - - // Remember that local video was muted due to the audio-only mode - // vs user's choice. - dispatch({ - type: _SET_AUDIO_ONLY_VIDEO_MUTED, - muted - }); - dispatch(setVideoMuted(muted)); - }; -} - /** * Action to set whether or not the currently displayed large video is in * high-definition. @@ -488,6 +460,6 @@ export function toggleAudioOnly() { return (dispatch: Dispatch<*>, getState: Function) => { const { audioOnly } = getState()['features/base/conference']; - return dispatch(_setAudioOnly(!audioOnly)); + return dispatch(setAudioOnly(!audioOnly)); }; } diff --git a/react/features/base/conference/middleware.js b/react/features/base/conference/middleware.js index fbec4c46a..5dc30a72b 100644 --- a/react/features/base/conference/middleware.js +++ b/react/features/base/conference/middleware.js @@ -2,6 +2,7 @@ import UIEvents from '../../../../service/UI/UIEvents'; import { CONNECTION_ESTABLISHED } from '../connection'; +import { setVideoMuted, VIDEO_MUTISM_AUTHORITY } from '../media'; import { getLocalParticipant, getParticipantById, @@ -12,10 +13,16 @@ import { TRACK_ADDED, TRACK_REMOVED } from '../tracks'; import { createConference, - _setAudioOnlyVideoMuted, + setAudioOnly, setLastN } from './actions'; -import { CONFERENCE_JOINED, SET_AUDIO_ONLY, SET_LASTN } from './actionTypes'; +import { + CONFERENCE_FAILED, + CONFERENCE_JOINED, + CONFERENCE_LEFT, + SET_AUDIO_ONLY, + SET_LASTN +} from './actionTypes'; import { _addLocalTracksToConference, _handleParticipantError, @@ -36,6 +43,10 @@ MiddlewareRegistry.register(store => next => action => { case CONFERENCE_JOINED: return _conferenceJoined(store, next, action); + case CONFERENCE_FAILED: + case CONFERENCE_LEFT: + return _conferenceFailedOrLeft(store, next, action); + case PIN_PARTICIPANT: return _pinParticipant(store, next, action); @@ -79,6 +90,29 @@ function _connectionEstablished(store, next, action) { return result; } +/** + * Does extra sync up on properties that may need to be updated, after + * the conference failed or was left. + * + * @param {Store} store - The Redux store in which the specified action is being + * dispatched. + * @param {Dispatch} next - The Redux dispatch function to dispatch the + * specified action to the specified store. + * @param {Action} action - The Redux action {@link CONFERENCE_FAILED} or + * {@link CONFERENCE_LEFT} which is being dispatched in the specified store. + * @private + * @returns {Object} The new state that is the result of the reduction of the + * specified action. + */ +function _conferenceFailedOrLeft(store, next, action) { + const result = next(action); + const { audioOnly } = store.getState()['features/base/conference']; + + audioOnly && store.dispatch(setAudioOnly(false)); + + return result; +} + /** * Does extra sync up on properties that may need to be updated, after * the conference was joined. @@ -171,17 +205,17 @@ function _pinParticipant(store, next, action) { * @returns {Object} The new state that is the result of the reduction of the * specified action. */ -function _setAudioOnly(store, next, action) { +function _setAudioOnly({ dispatch }, next, action) { const result = next(action); const { audioOnly } = action; // Set lastN to 0 in case audio-only is desired; leave it as undefined, // otherwise, and the default lastN value will be chosen automatically. - store.dispatch(setLastN(audioOnly ? 0 : undefined)); + dispatch(setLastN(audioOnly ? 0 : undefined)); - // Mute local video - store.dispatch(_setAudioOnlyVideoMuted(audioOnly)); + // Mute the local video. + dispatch(setVideoMuted(audioOnly, VIDEO_MUTISM_AUTHORITY.AUDIO_ONLY)); if (typeof APP !== 'undefined') { // TODO This should be a temporary solution that lasts only until diff --git a/react/features/base/conference/reducer.js b/react/features/base/conference/reducer.js index 297b92b71..5773302dc 100644 --- a/react/features/base/conference/reducer.js +++ b/react/features/base/conference/reducer.js @@ -11,7 +11,6 @@ import { CONFERENCE_WILL_LEAVE, LOCK_STATE_CHANGED, SET_AUDIO_ONLY, - _SET_AUDIO_ONLY_VIDEO_MUTED, SET_LARGE_VIDEO_HD_STATUS, SET_PASSWORD, SET_ROOM @@ -45,9 +44,6 @@ ReducerRegistry.register('features/base/conference', (state = {}, action) => { case SET_AUDIO_ONLY: return _setAudioOnly(state, action); - case _SET_AUDIO_ONLY_VIDEO_MUTED: - return _setAudioOnlyVideoMuted(state, action); - case SET_LARGE_VIDEO_HD_STATUS: return _setLargeVideoHDStatus(state, action); @@ -82,8 +78,6 @@ function _conferenceFailed(state, { conference, error }) { : undefined; return assign(state, { - audioOnly: undefined, - audioOnlyVideoMuted: undefined, conference: undefined, joining: undefined, leaving: undefined, @@ -161,8 +155,6 @@ function _conferenceLeft(state, { conference }) { } return assign(state, { - audioOnly: undefined, - audioOnlyVideoMuted: undefined, conference: undefined, joining: undefined, leaving: undefined, @@ -250,21 +242,6 @@ function _setAudioOnly(state, action) { return set(state, 'audioOnly', action.audioOnly); } -/** - * Reduces a specific Redux action _SET_AUDIO_ONLY_VIDEO_MUTED of the feature - * base/conference. - * - * @param {Object} state - The Redux state of the feature base/conference. - * @param {Action} action - The Redux action SET_AUDIO_ONLY_VIDEO_MUTED to - * reduce. - * @private - * @returns {Object} The new state of the feature base/conference after the - * reduction of the specified action. - */ -function _setAudioOnlyVideoMuted(state, action) { - return set(state, 'audioOnlyVideoMuted', action.muted); -} - /** * Reduces a specific Redux action SET_LARGE_VIDEO_HD_STATUS of the feature * base/conference. diff --git a/react/features/base/media/actions.js b/react/features/base/media/actions.js index d26602941..c7a82cf27 100644 --- a/react/features/base/media/actions.js +++ b/react/features/base/media/actions.js @@ -10,7 +10,7 @@ import { SET_VIDEO_MUTED, TOGGLE_CAMERA_FACING_MODE } from './actionTypes'; -import { CAMERA_FACING_MODE } from './constants'; +import { CAMERA_FACING_MODE, VIDEO_MUTISM_AUTHORITY } from './constants'; /** * Action to adjust the availability of the local audio. @@ -84,15 +84,23 @@ export function setVideoAvailable(available: boolean) { * * @param {boolean} muted - True if the local video is to be muted or false if * the local video is to be unmuted. - * @returns {{ - * type: SET_VIDEO_MUTED, - * muted: boolean - * }} + * @param {number} authority - The {@link VIDEO_MUTISM_AUTHORITY} which is + * muting/unmuting the local video. + * @returns {Function} */ -export function setVideoMuted(muted: boolean) { - return { - type: SET_VIDEO_MUTED, - muted +export function setVideoMuted( + muted: boolean, + authority: number = VIDEO_MUTISM_AUTHORITY.USER) { + return (dispatch: Dispatch<*>, getState: Function) => { + const oldValue = getState()['features/base/media'].video.muted; + + // eslint-disable-next-line no-bitwise + const newValue = muted ? oldValue | authority : oldValue & ~authority; + + return dispatch({ + type: SET_VIDEO_MUTED, + muted: newValue + }); }; } @@ -135,6 +143,8 @@ export function toggleVideoMuted() { return (dispatch: Dispatch<*>, getState: Function) => { const muted = getState()['features/base/media'].video.muted; - return dispatch(setVideoMuted(!muted)); + // XXX The following directly invokes the action creator in order to + // silence Flow. + return setVideoMuted(!muted)(dispatch, getState); }; } diff --git a/react/features/base/media/constants.js b/react/features/base/media/constants.js index 35d34b8d8..7ffa7298c 100644 --- a/react/features/base/media/constants.js +++ b/react/features/base/media/constants.js @@ -9,7 +9,7 @@ export const CAMERA_FACING_MODE = { }; /** - * The set of media types for a track. + * The set of media types. * * @enum {string} */ @@ -18,8 +18,24 @@ export const MEDIA_TYPE = { VIDEO: 'video' }; +/* eslint-disable no-bitwise */ + /** - * The set of video types for a video track. + * The types of authorities which may mute/unmute the local video. + * + * @enum {number} + */ +export const VIDEO_MUTISM_AUTHORITY = { + AUDIO_ONLY: 1 << 0, + BACKGROUND: 1 << 1, + USER: 1 << 2 +}; + +/* eslint-enable no-bitwise */ + +/** + * The types of video tracks. + * * @enum {string} */ export const VIDEO_TYPE = { diff --git a/react/features/base/media/middleware.js b/react/features/base/media/middleware.js index 4d7b907b2..f2b5ff809 100644 --- a/react/features/base/media/middleware.js +++ b/react/features/base/media/middleware.js @@ -80,7 +80,8 @@ function _setRoom({ dispatch, getState }, next, action) { (audio.muted !== audioMuted) && dispatch(setAudioMuted(audioMuted)); (video.facingMode !== CAMERA_FACING_MODE.USER) && dispatch(setCameraFacingMode(CAMERA_FACING_MODE.USER)); - (video.muted !== videoMuted) && dispatch(setVideoMuted(videoMuted)); + (Boolean(video.muted) !== videoMuted) + && dispatch(setVideoMuted(videoMuted)); return next(action); } @@ -95,7 +96,7 @@ function _setRoom({ dispatch, getState }, next, action) { */ function _syncTrackMutedState({ dispatch, getState }, track) { const state = getState()['features/base/media']; - const muted = state[track.mediaType].muted; + const muted = Boolean(state[track.mediaType].muted); // XXX If muted state of track when it was added is different from our media // muted state, we need to mute track and explicitly modify 'muted' property diff --git a/react/features/base/media/reducer.js b/react/features/base/media/reducer.js index 9bbd77eab..b3dc17563 100644 --- a/react/features/base/media/reducer.js +++ b/react/features/base/media/reducer.js @@ -73,7 +73,7 @@ function _audio(state = AUDIO_INITIAL_MEDIA_STATE, action) { const VIDEO_INITIAL_MEDIA_STATE = { available: true, facingMode: CAMERA_FACING_MODE.USER, - muted: false + muted: 0 }; /** diff --git a/react/features/base/tracks/actions.js b/react/features/base/tracks/actions.js index c24c71179..259aa7003 100644 --- a/react/features/base/tracks/actions.js +++ b/react/features/base/tracks/actions.js @@ -358,12 +358,17 @@ function _getLocalTracksToChange(currentTracks, newTracks) { */ export function setTrackMuted(track, muted) { return dispatch => { + muted = Boolean(muted); // eslint-disable-line no-param-reassign + if (track.isMuted() === muted) { return Promise.resolve(); } const f = muted ? 'mute' : 'unmute'; + // FIXME: This operation disregards the authority. It is not a problem + // (on mobile) at the moment, but it will be once we start not creating + // tracks early. Refactor this then. return track[f]().catch(error => { console.error(`set track ${f} failed`, error); diff --git a/react/features/base/tracks/middleware.js b/react/features/base/tracks/middleware.js index 0f4660657..c63ac141c 100644 --- a/react/features/base/tracks/middleware.js +++ b/react/features/base/tracks/middleware.js @@ -123,9 +123,15 @@ MiddlewareRegistry.register(store => next => action => { } else { APP.UI.setAudioMuted(participantID, isMuted); } + + // XXX This function synchronizes track states with media states. + // This is not required in React, because media is the source of + // truth, synchronization should always happen in the media -> track + // direction. The old web, however, does the opposite, hence the + // need for this. + return _trackUpdated(store, next, action); } - return _trackUpdated(store, next, action); } return next(action); diff --git a/react/features/mobile/audio-mode/middleware.js b/react/features/mobile/audio-mode/middleware.js index ec9b59cf0..781b577ed 100644 --- a/react/features/mobile/audio-mode/middleware.js +++ b/react/features/mobile/audio-mode/middleware.js @@ -39,12 +39,20 @@ MiddlewareRegistry.register(store => next => action => { break; } - case SET_AUDIO_ONLY: - mode - = action.audioOnly + case SET_AUDIO_ONLY: { + const { conference } = store.getState()['features/base/conference']; + + if (conference) { + mode + = action.audioOnly ? AudioMode.AUDIO_CALL : AudioMode.VIDEO_CALL; + } else { + mode = null; + } + break; + } default: mode = null; diff --git a/react/features/mobile/background/actionTypes.js b/react/features/mobile/background/actionTypes.js index 3f8fe6d88..fc9f36c7b 100644 --- a/react/features/mobile/background/actionTypes.js +++ b/react/features/mobile/background/actionTypes.js @@ -10,20 +10,6 @@ */ export const _SET_APP_STATE_LISTENER = Symbol('_SET_APP_STATE_LISTENER'); -/** - * The type of redux action which signals that video will be muted because the - * app is going to the background. - * - * { - * type: _SET_BACKGROUND_VIDEO_MUTED, - * muted: boolean - * } - * - * @protected - */ -export const _SET_BACKGROUND_VIDEO_MUTED - = Symbol('_SET_BACKGROUND_VIDEO_MUTED'); - /** * The type of redux action which signals that the app state has changed (in * terms of execution mode). The app state can be one of 'active', 'inactive', diff --git a/react/features/mobile/background/actions.js b/react/features/mobile/background/actions.js index f724dba27..198a06b5d 100644 --- a/react/features/mobile/background/actions.js +++ b/react/features/mobile/background/actions.js @@ -1,9 +1,8 @@ import { setLastN } from '../../base/conference'; -import { setVideoMuted } from '../../base/media'; +import { setVideoMuted, VIDEO_MUTISM_AUTHORITY } from '../../base/media'; import { _SET_APP_STATE_LISTENER, - _SET_BACKGROUND_VIDEO_MUTED, APP_STATE_CHANGED } from './actionTypes'; @@ -42,35 +41,8 @@ export function _setBackgroundVideoMuted(muted: boolean) { // for last N will be chosen automatically. const { audioOnly } = getState()['features/base/conference']; - if (audioOnly) { - return; - } - - dispatch(setLastN(muted ? 0 : undefined)); - - if (muted) { - const { video } = getState()['features/base/media']; - - if (video.muted) { - // Video is already muted, do nothing. - return; - } - } else { - const { videoMuted } = getState()['features/background']; - - if (!videoMuted) { - // We didn't mute video, do nothing. - return; - } - } - - // Remember that local video was muted due to the app going to the - // background vs user's choice. - dispatch({ - type: _SET_BACKGROUND_VIDEO_MUTED, - muted - }); - dispatch(setVideoMuted(muted)); + !audioOnly && dispatch(setLastN(muted ? 0 : undefined)); + dispatch(setVideoMuted(muted, VIDEO_MUTISM_AUTHORITY.BACKGROUND)); }; } diff --git a/react/features/mobile/background/reducer.js b/react/features/mobile/background/reducer.js index 9ad8191da..21c618ac7 100644 --- a/react/features/mobile/background/reducer.js +++ b/react/features/mobile/background/reducer.js @@ -2,7 +2,6 @@ import { ReducerRegistry } from '../../base/redux'; import { _SET_APP_STATE_LISTENER, - _SET_BACKGROUND_VIDEO_MUTED, APP_STATE_CHANGED } from './actionTypes'; @@ -14,12 +13,6 @@ ReducerRegistry.register('features/background', (state = {}, action) => { appStateListener: action.listener }; - case _SET_BACKGROUND_VIDEO_MUTED: - return { - ...state, - videoMuted: action.muted - }; - case APP_STATE_CHANGED: return { ...state, diff --git a/react/features/mobile/full-screen/middleware.js b/react/features/mobile/full-screen/middleware.js index e6b7e4a37..b7be61bbc 100644 --- a/react/features/mobile/full-screen/middleware.js +++ b/react/features/mobile/full-screen/middleware.js @@ -61,10 +61,13 @@ MiddlewareRegistry.register(store => next => action => { break; } - case SET_AUDIO_ONLY: - fullScreen = !action.audioOnly; + case SET_AUDIO_ONLY: { + const { conference } = store.getState()['features/base/conference']; + + fullScreen = conference ? !action.audioOnly : false; break; } + } if (fullScreen !== null) { _setFullScreen(fullScreen) diff --git a/react/features/mobile/proximity/middleware.js b/react/features/mobile/proximity/middleware.js index 9cd20f08e..f2da529ba 100644 --- a/react/features/mobile/proximity/middleware.js +++ b/react/features/mobile/proximity/middleware.js @@ -31,10 +31,13 @@ MiddlewareRegistry.register(store => next => action => { _setProximityEnabled(false); break; - case SET_AUDIO_ONLY: - _setProximityEnabled(action.audioOnly); + case SET_AUDIO_ONLY: { + const { conference } = store.getState()['features/base/conference']; + + conference && _setProximityEnabled(action.audioOnly); break; } + } return next(action); });