[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.
This commit is contained in:
Saúl Ibarra Corretgé 2017-08-02 17:00:51 +02:00 committed by Lyubo Marinov
parent bd4766648a
commit d600504d85
16 changed files with 134 additions and 162 deletions

View File

@ -75,20 +75,6 @@ export const LOCK_STATE_CHANGED = Symbol('LOCK_STATE_CHANGED');
*/ */
export const SET_AUDIO_ONLY = Symbol('SET_AUDIO_ONLY'); 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 * The type of (redux) action to set whether or not the displayed large video is
* in high-definition. * in high-definition.

View File

@ -1,5 +1,5 @@
import { JitsiConferenceEvents } from '../lib-jitsi-meet'; import { JitsiConferenceEvents } from '../lib-jitsi-meet';
import { setVideoMuted } from '../media'; import { setAudioMuted, setVideoMuted } from '../media';
import { import {
dominantSpeakerChanged, dominantSpeakerChanged,
getLocalParticipant, getLocalParticipant,
@ -19,7 +19,6 @@ import {
CONFERENCE_WILL_LEAVE, CONFERENCE_WILL_LEAVE,
LOCK_STATE_CHANGED, LOCK_STATE_CHANGED,
SET_AUDIO_ONLY, SET_AUDIO_ONLY,
_SET_AUDIO_ONLY_VIDEO_MUTED,
SET_LARGE_VIDEO_HD_STATUS, SET_LARGE_VIDEO_HD_STATUS,
SET_LASTN, SET_LASTN,
SET_PASSWORD, SET_PASSWORD,
@ -61,6 +60,19 @@ function _addConferenceListeners(conference, dispatch) {
JitsiConferenceEvents.LOCK_STATE_CHANGED, JitsiConferenceEvents.LOCK_STATE_CHANGED,
(...args) => dispatch(lockStateChanged(conference, ...args))); (...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: // Dispatches into features/base/tracks follow:
conference.on( conference.on(
@ -296,58 +308,18 @@ export function lockStateChanged(conference, locked) {
* *
* @param {boolean} audioOnly - True if the conference should be audio only; * @param {boolean} audioOnly - True if the conference should be audio only;
* false, otherwise. * false, otherwise.
* @private
* @returns {{ * @returns {{
* type: SET_AUDIO_ONLY, * type: SET_AUDIO_ONLY,
* audioOnly: boolean * audioOnly: boolean
* }} * }}
*/ */
function _setAudioOnly(audioOnly) { export function setAudioOnly(audioOnly) {
return { return {
type: SET_AUDIO_ONLY, type: SET_AUDIO_ONLY,
audioOnly 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 * Action to set whether or not the currently displayed large video is in
* high-definition. * high-definition.
@ -488,6 +460,6 @@ export function toggleAudioOnly() {
return (dispatch: Dispatch<*>, getState: Function) => { return (dispatch: Dispatch<*>, getState: Function) => {
const { audioOnly } = getState()['features/base/conference']; const { audioOnly } = getState()['features/base/conference'];
return dispatch(_setAudioOnly(!audioOnly)); return dispatch(setAudioOnly(!audioOnly));
}; };
} }

View File

@ -2,6 +2,7 @@
import UIEvents from '../../../../service/UI/UIEvents'; import UIEvents from '../../../../service/UI/UIEvents';
import { CONNECTION_ESTABLISHED } from '../connection'; import { CONNECTION_ESTABLISHED } from '../connection';
import { setVideoMuted, VIDEO_MUTISM_AUTHORITY } from '../media';
import { import {
getLocalParticipant, getLocalParticipant,
getParticipantById, getParticipantById,
@ -12,10 +13,16 @@ import { TRACK_ADDED, TRACK_REMOVED } from '../tracks';
import { import {
createConference, createConference,
_setAudioOnlyVideoMuted, setAudioOnly,
setLastN setLastN
} from './actions'; } 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 { import {
_addLocalTracksToConference, _addLocalTracksToConference,
_handleParticipantError, _handleParticipantError,
@ -36,6 +43,10 @@ MiddlewareRegistry.register(store => next => action => {
case CONFERENCE_JOINED: case CONFERENCE_JOINED:
return _conferenceJoined(store, next, action); return _conferenceJoined(store, next, action);
case CONFERENCE_FAILED:
case CONFERENCE_LEFT:
return _conferenceFailedOrLeft(store, next, action);
case PIN_PARTICIPANT: case PIN_PARTICIPANT:
return _pinParticipant(store, next, action); return _pinParticipant(store, next, action);
@ -79,6 +90,29 @@ function _connectionEstablished(store, next, action) {
return result; 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 * Does extra sync up on properties that may need to be updated, after
* the conference was joined. * 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 * @returns {Object} The new state that is the result of the reduction of the
* specified action. * specified action.
*/ */
function _setAudioOnly(store, next, action) { function _setAudioOnly({ dispatch }, next, action) {
const result = next(action); const result = next(action);
const { audioOnly } = action; const { audioOnly } = action;
// Set lastN to 0 in case audio-only is desired; leave it as undefined, // Set lastN to 0 in case audio-only is desired; leave it as undefined,
// otherwise, and the default lastN value will be chosen automatically. // otherwise, and the default lastN value will be chosen automatically.
store.dispatch(setLastN(audioOnly ? 0 : undefined)); dispatch(setLastN(audioOnly ? 0 : undefined));
// Mute local video // Mute the local video.
store.dispatch(_setAudioOnlyVideoMuted(audioOnly)); dispatch(setVideoMuted(audioOnly, VIDEO_MUTISM_AUTHORITY.AUDIO_ONLY));
if (typeof APP !== 'undefined') { if (typeof APP !== 'undefined') {
// TODO This should be a temporary solution that lasts only until // TODO This should be a temporary solution that lasts only until

View File

@ -11,7 +11,6 @@ import {
CONFERENCE_WILL_LEAVE, CONFERENCE_WILL_LEAVE,
LOCK_STATE_CHANGED, LOCK_STATE_CHANGED,
SET_AUDIO_ONLY, SET_AUDIO_ONLY,
_SET_AUDIO_ONLY_VIDEO_MUTED,
SET_LARGE_VIDEO_HD_STATUS, SET_LARGE_VIDEO_HD_STATUS,
SET_PASSWORD, SET_PASSWORD,
SET_ROOM SET_ROOM
@ -45,9 +44,6 @@ ReducerRegistry.register('features/base/conference', (state = {}, action) => {
case SET_AUDIO_ONLY: case SET_AUDIO_ONLY:
return _setAudioOnly(state, action); return _setAudioOnly(state, action);
case _SET_AUDIO_ONLY_VIDEO_MUTED:
return _setAudioOnlyVideoMuted(state, action);
case SET_LARGE_VIDEO_HD_STATUS: case SET_LARGE_VIDEO_HD_STATUS:
return _setLargeVideoHDStatus(state, action); return _setLargeVideoHDStatus(state, action);
@ -82,8 +78,6 @@ function _conferenceFailed(state, { conference, error }) {
: undefined; : undefined;
return assign(state, { return assign(state, {
audioOnly: undefined,
audioOnlyVideoMuted: undefined,
conference: undefined, conference: undefined,
joining: undefined, joining: undefined,
leaving: undefined, leaving: undefined,
@ -161,8 +155,6 @@ function _conferenceLeft(state, { conference }) {
} }
return assign(state, { return assign(state, {
audioOnly: undefined,
audioOnlyVideoMuted: undefined,
conference: undefined, conference: undefined,
joining: undefined, joining: undefined,
leaving: undefined, leaving: undefined,
@ -250,21 +242,6 @@ function _setAudioOnly(state, action) {
return set(state, 'audioOnly', action.audioOnly); 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 * Reduces a specific Redux action SET_LARGE_VIDEO_HD_STATUS of the feature
* base/conference. * base/conference.

View File

@ -10,7 +10,7 @@ import {
SET_VIDEO_MUTED, SET_VIDEO_MUTED,
TOGGLE_CAMERA_FACING_MODE TOGGLE_CAMERA_FACING_MODE
} from './actionTypes'; } 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. * 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 * @param {boolean} muted - True if the local video is to be muted or false if
* the local video is to be unmuted. * the local video is to be unmuted.
* @returns {{ * @param {number} authority - The {@link VIDEO_MUTISM_AUTHORITY} which is
* type: SET_VIDEO_MUTED, * muting/unmuting the local video.
* muted: boolean * @returns {Function}
* }}
*/ */
export function setVideoMuted(muted: boolean) { export function setVideoMuted(
return { muted: boolean,
type: SET_VIDEO_MUTED, authority: number = VIDEO_MUTISM_AUTHORITY.USER) {
muted 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) => { return (dispatch: Dispatch<*>, getState: Function) => {
const muted = getState()['features/base/media'].video.muted; 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);
}; };
} }

View File

@ -9,7 +9,7 @@ export const CAMERA_FACING_MODE = {
}; };
/** /**
* The set of media types for a track. * The set of media types.
* *
* @enum {string} * @enum {string}
*/ */
@ -18,8 +18,24 @@ export const MEDIA_TYPE = {
VIDEO: 'video' 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} * @enum {string}
*/ */
export const VIDEO_TYPE = { export const VIDEO_TYPE = {

View File

@ -80,7 +80,8 @@ function _setRoom({ dispatch, getState }, next, action) {
(audio.muted !== audioMuted) && dispatch(setAudioMuted(audioMuted)); (audio.muted !== audioMuted) && dispatch(setAudioMuted(audioMuted));
(video.facingMode !== CAMERA_FACING_MODE.USER) (video.facingMode !== CAMERA_FACING_MODE.USER)
&& dispatch(setCameraFacingMode(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); return next(action);
} }
@ -95,7 +96,7 @@ function _setRoom({ dispatch, getState }, next, action) {
*/ */
function _syncTrackMutedState({ dispatch, getState }, track) { function _syncTrackMutedState({ dispatch, getState }, track) {
const state = getState()['features/base/media']; 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 // 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 // muted state, we need to mute track and explicitly modify 'muted' property

View File

@ -73,7 +73,7 @@ function _audio(state = AUDIO_INITIAL_MEDIA_STATE, action) {
const VIDEO_INITIAL_MEDIA_STATE = { const VIDEO_INITIAL_MEDIA_STATE = {
available: true, available: true,
facingMode: CAMERA_FACING_MODE.USER, facingMode: CAMERA_FACING_MODE.USER,
muted: false muted: 0
}; };
/** /**

View File

@ -358,12 +358,17 @@ function _getLocalTracksToChange(currentTracks, newTracks) {
*/ */
export function setTrackMuted(track, muted) { export function setTrackMuted(track, muted) {
return dispatch => { return dispatch => {
muted = Boolean(muted); // eslint-disable-line no-param-reassign
if (track.isMuted() === muted) { if (track.isMuted() === muted) {
return Promise.resolve(); return Promise.resolve();
} }
const f = muted ? 'mute' : 'unmute'; 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 => { return track[f]().catch(error => {
console.error(`set track ${f} failed`, error); console.error(`set track ${f} failed`, error);

View File

@ -123,9 +123,15 @@ MiddlewareRegistry.register(store => next => action => {
} else { } else {
APP.UI.setAudioMuted(participantID, isMuted); 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); return next(action);

View File

@ -39,12 +39,20 @@ MiddlewareRegistry.register(store => next => action => {
break; break;
} }
case SET_AUDIO_ONLY: case SET_AUDIO_ONLY: {
mode const { conference } = store.getState()['features/base/conference'];
= action.audioOnly
if (conference) {
mode
= action.audioOnly
? AudioMode.AUDIO_CALL ? AudioMode.AUDIO_CALL
: AudioMode.VIDEO_CALL; : AudioMode.VIDEO_CALL;
} else {
mode = null;
}
break; break;
}
default: default:
mode = null; mode = null;

View File

@ -10,20 +10,6 @@
*/ */
export const _SET_APP_STATE_LISTENER = Symbol('_SET_APP_STATE_LISTENER'); 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 * 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', * terms of execution mode). The app state can be one of 'active', 'inactive',

View File

@ -1,9 +1,8 @@
import { setLastN } from '../../base/conference'; import { setLastN } from '../../base/conference';
import { setVideoMuted } from '../../base/media'; import { setVideoMuted, VIDEO_MUTISM_AUTHORITY } from '../../base/media';
import { import {
_SET_APP_STATE_LISTENER, _SET_APP_STATE_LISTENER,
_SET_BACKGROUND_VIDEO_MUTED,
APP_STATE_CHANGED APP_STATE_CHANGED
} from './actionTypes'; } from './actionTypes';
@ -42,35 +41,8 @@ export function _setBackgroundVideoMuted(muted: boolean) {
// for last N will be chosen automatically. // for last N will be chosen automatically.
const { audioOnly } = getState()['features/base/conference']; const { audioOnly } = getState()['features/base/conference'];
if (audioOnly) { !audioOnly && dispatch(setLastN(muted ? 0 : undefined));
return; dispatch(setVideoMuted(muted, VIDEO_MUTISM_AUTHORITY.BACKGROUND));
}
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));
}; };
} }

View File

@ -2,7 +2,6 @@ import { ReducerRegistry } from '../../base/redux';
import { import {
_SET_APP_STATE_LISTENER, _SET_APP_STATE_LISTENER,
_SET_BACKGROUND_VIDEO_MUTED,
APP_STATE_CHANGED APP_STATE_CHANGED
} from './actionTypes'; } from './actionTypes';
@ -14,12 +13,6 @@ ReducerRegistry.register('features/background', (state = {}, action) => {
appStateListener: action.listener appStateListener: action.listener
}; };
case _SET_BACKGROUND_VIDEO_MUTED:
return {
...state,
videoMuted: action.muted
};
case APP_STATE_CHANGED: case APP_STATE_CHANGED:
return { return {
...state, ...state,

View File

@ -61,10 +61,13 @@ MiddlewareRegistry.register(store => next => action => {
break; break;
} }
case SET_AUDIO_ONLY: case SET_AUDIO_ONLY: {
fullScreen = !action.audioOnly; const { conference } = store.getState()['features/base/conference'];
fullScreen = conference ? !action.audioOnly : false;
break; break;
} }
}
if (fullScreen !== null) { if (fullScreen !== null) {
_setFullScreen(fullScreen) _setFullScreen(fullScreen)

View File

@ -31,10 +31,13 @@ MiddlewareRegistry.register(store => next => action => {
_setProximityEnabled(false); _setProximityEnabled(false);
break; break;
case SET_AUDIO_ONLY: case SET_AUDIO_ONLY: {
_setProximityEnabled(action.audioOnly); const { conference } = store.getState()['features/base/conference'];
conference && _setProximityEnabled(action.audioOnly);
break; break;
} }
}
return next(action); return next(action);
}); });