Proper use of getPropertyValue in base/media

react/features/base/media/middleware.js

@@ -1,4 +1,4 @@
-/* @flow */
+// @flow
 
 import {
     createStartAudioOnlyEvent,
@@ -6,7 +6,7 @@ import {
     createSyncTrackStateEvent,
     sendAnalytics
 } from '../../analytics';
-import { SET_ROOM, setAudioOnly } from '../conference';
+import { isRoomValid, SET_ROOM, setAudioOnly } from '../conference';
 import JitsiMeetJS from '../lib-jitsi-meet';
 import { getPropertyValue } from '../profile';
 import { MiddlewareRegistry } from '../redux';
@@ -14,6 +14,10 @@ import { setTrackMuted, TRACK_ADDED } from '../tracks';
 
 import { setAudioMuted, setCameraFacingMode, setVideoMuted } from './actions';
 import { CAMERA_FACING_MODE } from './constants';
+import {
+    _AUDIO_INITIAL_MEDIA_STATE,
+    _VIDEO_INITIAL_MEDIA_STATE
+} from './reducer';
 
 const logger = require('jitsi-meet-logger').getLogger(__filename);
 
@@ -30,8 +34,9 @@ MiddlewareRegistry.register(store => next => action => {
 
     case TRACK_ADDED: {
         const result = next(action);
+        const { track } = action;
 
-        action.track.local && _syncTrackMutedState(store, action.track);
+        track.local && _syncTrackMutedState(store, track);
 
         return result;
     }
@@ -55,25 +60,51 @@ MiddlewareRegistry.register(store => next => action => {
  * specified {@code action}.
  */
 function _setRoom({ dispatch, getState }, next, action) {
-    const { room } = action;
-
-    // Read the config.
+    // Figure out the desires/intents i.e. the state of base/media. There are
+    // multiple desires/intents ordered by precedence such as server-side
+    // config, config overrides in the user-supplied URL, user's own app
+    // settings, etc.
 
     const state = getState();
-    const hasRoom = Boolean(room);
-
-    const audioMuted = Boolean(getPropertyValue(state, 'startWithAudioMuted', {
-        urlParams: hasRoom
-    }));
-    const videoMuted = Boolean(getPropertyValue(state, 'startWithVideoMuted', {
-        urlParams: hasRoom
-    }));
-
-    sendAnalytics(createStartMutedConfigurationEvent(
-        'local', audioMuted, videoMuted));
-
-    logger.log(`Start muted: ${audioMuted ? 'audio, ' : ''}${
-        videoMuted ? 'video' : ''}`);
+    const { room } = action;
+    const roomIsValid = isRoomValid(room);
+
+    // XXX The configurations/preferences/settings startWithAudioMuted,
+    // startWithVideoMuted, and startAudioOnly were introduced for
+    // conferences/meetings. So it makes sense for these to not be considered
+    // outside of conferences/meetings (e.g. WelcomePage). Later on, though, we
+    // introduced a "Video <-> Voice" toggle on the WelcomePage which utilizes
+    // startAudioOnly outside of conferences/meetings so that particular
+    // configuration/preference/setting employs slightly exclusive logic.
+    const mutedSources = {
+        // We have startWithAudioMuted and startWithVideoMuted here:
+        config: true,
+        profile: true,
+
+        // XXX We've already overwritten base/config with urlParams. However,
+        // profile is more important than the server-side config. Consequently,
+        // we need to read from urlParams anyway:
+        urlParams: true,
+
+        // We don't have startWithAudioMuted and startWithVideoMuted here:
+        jwt: false
+    };
+    const audioMuted
+        = roomIsValid
+            ? Boolean(
+                getPropertyValue(state, 'startWithAudioMuted', mutedSources))
+            : _AUDIO_INITIAL_MEDIA_STATE.muted;
+    const videoMuted
+        = roomIsValid
+            ? Boolean(
+                getPropertyValue(state, 'startWithVideoMuted', mutedSources))
+            : _VIDEO_INITIAL_MEDIA_STATE.muted;
+
+    sendAnalytics(
+        createStartMutedConfigurationEvent('local', audioMuted, videoMuted));
+    logger.log(
+        `Start muted: ${audioMuted ? 'audio, ' : ''}${
+            videoMuted ? 'video' : ''}`);
 
     // Unconditionally express the desires/expectations/intents of the app and
     // the user i.e. the state of base/media. Eventually, practice/reality i.e.
@@ -82,29 +113,57 @@ function _setRoom({ dispatch, getState }, next, action) {
     dispatch(setCameraFacingMode(CAMERA_FACING_MODE.USER));
     dispatch(setVideoMuted(videoMuted));
 
-    // config.startAudioOnly
+    // startAudioOnly
     //
     // FIXME Technically, the audio-only feature is owned by base/conference,
     // not base/media so the following should be in base/conference.
     // Practically, I presume it was easier to write the source code here
-    // because it looks like config.startWithAudioMuted and
-    // config.startWithVideoMuted.
-    if (room) {
-        let audioOnly;
-
-        if (JitsiMeetJS.mediaDevices.supportsVideo()) {
-            audioOnly = Boolean(getPropertyValue(state, 'startAudioOnly'));
-        } else {
-            // Always default to being audio only if the current environment
-            // does not support sending or receiving video.
-            audioOnly = true;
-        }
-
-        sendAnalytics(createStartAudioOnlyEvent(audioOnly));
-        logger.log(`Start audio only set to ${audioOnly.toString()}`);
-        dispatch(setAudioOnly(audioOnly));
+    // because it looks like startWithAudioMuted and startWithVideoMuted.
+    //
+    // XXX After the introduction of the "Video <-> Voice" toggle on the
+    // WelcomePage, startAudioOnly is utilized even outside of
+    // conferences/meetings.
+    let audioOnly;
+
+    if (JitsiMeetJS.mediaDevices.supportsVideo()) {
+        audioOnly
+            = Boolean(
+                getPropertyValue(
+                    state,
+                    'startAudioOnly',
+                    /* sources */ {
+                        // FIXME Practically, base/config is (really) correct
+                        // only if roomIsValid. At the time of this writing,
+                        // base/config is overwritten by URL params which leaves
+                        // base/config incorrect on the WelcomePage after
+                        // leaving a conference which explicitly overwrites
+                        // base/config with URL params.
+                        config: roomIsValid,
+
+                        // XXX We've already overwritten base/config with
+                        // urlParams if roomIsValid. However, profile is more
+                        // important than the server-side config. Consequently,
+                        // we need to read from urlParams anyway. We also
+                        // probably want to read from urlParams when
+                        // !roomIsValid.
+                        urlParams: true,
+
+                        // The following don't have complications around whether
+                        // they are defined or not:
+                        jwt: false,
+                        profile: true
+                    }));
+    } else {
+        // Default to audio-only if the (execution) environment does not
+        // support (sending and/or receiving) video.
+        audioOnly = true;
     }
 
+    sendAnalytics(createStartAudioOnlyEvent(audioOnly));
+    logger.log(`Start audio only set to ${audioOnly.toString()}`);
+
+    dispatch(setAudioOnly(audioOnly));
+
     return next(action);
 }
 
@@ -127,8 +186,10 @@ function _syncTrackMutedState({ getState }, track) {
     // fired before track gets to state.
     if (track.muted !== muted) {
         sendAnalytics(createSyncTrackStateEvent(track.mediaType, muted));
-        logger.log(`Sync ${track.mediaType} track muted state to ${
-            muted ? 'muted' : 'unmuted'}`);
+        logger.log(
+            `Sync ${track.mediaType} track muted state to ${
+                muted ? 'muted' : 'unmuted'}`);
+
         track.muted = muted;
         setTrackMuted(track.jitsiTrack, muted);
     }

react/features/base/media/reducer.js

@@ -22,12 +22,16 @@ import { CAMERA_FACING_MODE } from './constants';
  * @property {boolean} muted=false - Audio muted state.
  */
 
+// FIXME Technically, _AUDIO_INITIAL_MEDIA_STATE is a constant internal to the
+// feature base/media and used in multiple files so it should be in
+// constants.js. Practically though, AudioMediaState would then be used in
+// multiple files as well so I don't know where and how to move it.
 /**
  * Initial state for local audio.
  *
  * @type {AudioMediaState}
  */
-const AUDIO_INITIAL_MEDIA_STATE = {
+export const _AUDIO_INITIAL_MEDIA_STATE = {
     available: true,
     muted: false
 };
@@ -41,7 +45,7 @@ const AUDIO_INITIAL_MEDIA_STATE = {
  * @private
  * @returns {AudioMediaState}
  */
-function _audio(state = AUDIO_INITIAL_MEDIA_STATE, action) {
+function _audio(state = _AUDIO_INITIAL_MEDIA_STATE, action) {
     switch (action.type) {
     case SET_AUDIO_AVAILABLE:
         return {
@@ -68,12 +72,16 @@ function _audio(state = AUDIO_INITIAL_MEDIA_STATE, action) {
  * @property {boolean} muted=false - Video muted state.
  */
 
+// FIXME Technically, _VIDEO_INITIAL_MEDIA_STATE is a constant internal to the
+// feature base/media and used in multiple files so it should be in
+// constants.js. Practically though, VideoMediaState would then be used in
+// multiple files as well so I don't know where and how to move it.
 /**
  * Initial state for video.
  *
  * @type {VideoMediaState}
  */
-const VIDEO_INITIAL_MEDIA_STATE = {
+export const _VIDEO_INITIAL_MEDIA_STATE = {
     available: true,
     facingMode: CAMERA_FACING_MODE.USER,
     muted: 0,
@@ -94,7 +102,7 @@ const VIDEO_INITIAL_MEDIA_STATE = {
  * @private
  * @returns {VideoMediaState}
  */
-function _video(state = VIDEO_INITIAL_MEDIA_STATE, action) {
+function _video(state = _VIDEO_INITIAL_MEDIA_STATE, action) {
     switch (action.type) {
     case CONFERENCE_FAILED:
     case CONFERENCE_LEFT:
@@ -168,7 +176,7 @@ ReducerRegistry.register('features/base/media', combineReducers({
 function _clearAllVideoTransforms(state) {
     return {
         ...state,
-        transforms: VIDEO_INITIAL_MEDIA_STATE.transforms
+        transforms: _VIDEO_INITIAL_MEDIA_STATE.transforms
     };
 }
 

react/features/base/profile/middleware.js

@@ -7,7 +7,9 @@ import { MiddlewareRegistry, toState } from '../redux';
 import { PROFILE_UPDATED } from './actionTypes';
 
 /**
- * A middleWare to update the local participant when the profile is updated.
+ * The middleware of the feature base/profile. Distributes changes to the state
+ * of base/profile to the states of other features computed from the state of
+ * base/profile.
  *
  * @param {Store} store - The redux store.
  * @returns {Function}
@@ -18,21 +20,21 @@ MiddlewareRegistry.register(store => next => action => {
     switch (action.type) {
     case PROFILE_UPDATED:
         _updateLocalParticipant(store);
-        _maybeUpdateStartAudioOnly(store, action);
+        _maybeSetAudioOnly(store, action);
     }
 
     return result;
 });
 
 /**
- * Updates startAudioOnly flag if it's updated in the profile.
+ * Updates {@code startAudioOnly} flag if it's updated in the profile.
  *
- * @private
  * @param {Store} store - The redux store.
  * @param {Object} action - The redux action.
+ * @private
  * @returns {void}
  */
-function _maybeUpdateStartAudioOnly(
+function _maybeSetAudioOnly(
         { dispatch },
         { profile: { startAudioOnly } }) {
     if (typeof startAudioOnly === 'boolean') {
@@ -44,6 +46,7 @@ function _maybeUpdateStartAudioOnly(
  * Updates the local participant according to profile changes.
  *
  * @param {Store} store - The redux store.
+ * @private
  * @returns {void}
  */
 function _updateLocalParticipant(store) {