Browse Source

Fix startAudioOnly and startWithVideoMuted collision on start from URL

Zoltan Bettenbuk suggested the following:

         const state = getState();

          if (desiredTypes.length === 0) {
 -            const { audio, video } = state['features/base/media'];
 -
 -            audio.muted || desiredTypes.push(MEDIA_TYPE.AUDIO);
 -            video.muted || desiredTypes.push(MEDIA_TYPE.VIDEO);
 +            const startAudioOnly = getPropertyValue(state, 'startAudioOnly');
 +            const startWithAudioMuted
 +                = getPropertyValue(state, 'startWithAudioMuted');
 +            const startWithVideoMuted
 +                = getPropertyValue(state, 'startWithVideoMuted');
 +
 +            if (!startWithAudioMuted) {
 +                desiredTypes.push(MEDIA_TYPE.AUDIO);
 +            }
 +            if (!startAudioOnly && !startWithVideoMuted) {
 +                desiredTypes.push(MEDIA_TYPE.VIDEO);
 +            }
          }

          const availableTypes

The final commit is really a different implementation of the same idea
but takes into account that the state of base/media already contains the
intent of the URL and notices the delay in the realization of the
background app state.

Additionally, unbreaks one more case where setAudioOnly is incorrectly
dispatched on CONFERENCE_LEFT or CONFERENCE_FAILED and, consequently,
overrides the intent of the URL.
j8
zbettenbuk 7 years ago
parent
commit
008645568c
2 changed files with 29 additions and 51 deletions
  1. 14
    49
      react/features/base/conference/middleware.js
  2. 15
    2
      react/features/base/tracks/actions.js

+ 14
- 49
react/features/base/conference/middleware.js View File

@@ -22,14 +22,11 @@ import { TRACK_ADDED, TRACK_REMOVED } from '../tracks';
22 22
 import {
23 23
     conferenceLeft,
24 24
     createConference,
25
-    setAudioOnly,
26 25
     setLastN,
27 26
     toggleAudioOnly
28 27
 } from './actions';
29 28
 import {
30
-    CONFERENCE_FAILED,
31 29
     CONFERENCE_JOINED,
32
-    CONFERENCE_LEFT,
33 30
     DATA_CHANNEL_OPENED,
34 31
     SET_AUDIO_ONLY,
35 32
     SET_LASTN,
@@ -58,10 +55,6 @@ MiddlewareRegistry.register(store => next => action => {
58 55
     case CONNECTION_ESTABLISHED:
59 56
         return _connectionEstablished(store, next, action);
60 57
 
61
-    case CONFERENCE_FAILED:
62
-    case CONFERENCE_LEFT:
63
-        return _conferenceFailedOrLeft(store, next, action);
64
-
65 58
     case CONFERENCE_JOINED:
66 59
         return _conferenceJoined(store, next, action);
67 60
 
@@ -116,43 +109,6 @@ function _connectionEstablished({ dispatch }, next, action) {
116 109
     return result;
117 110
 }
118 111
 
119
-/**
120
- * Does extra sync up on properties that may need to be updated after the
121
- * conference failed or was left.
122
- *
123
- * @param {Store} store - The redux store in which the specified action is being
124
- * dispatched.
125
- * @param {Dispatch} next - The redux dispatch function to dispatch the
126
- * specified action to the specified store.
127
- * @param {Action} action - The redux action {@link CONFERENCE_FAILED} or
128
- * {@link CONFERENCE_LEFT} which is being dispatched in the specified store.
129
- * @private
130
- * @returns {Object} The value returned by {@code next(action)}.
131
- */
132
-function _conferenceFailedOrLeft({ dispatch, getState }, next, action) {
133
-    const result = next(action);
134
-
135
-    const state = getState();
136
-    const { audioOnly } = state['features/base/conference'];
137
-    const { startAudioOnly } = state['features/base/profile'];
138
-
139
-    // FIXME: Consider implementing a standalone audio-only feature that handles
140
-    // all these state changes.
141
-    if (audioOnly) {
142
-        if (!startAudioOnly) {
143
-            sendAnalytics(createAudioOnlyChangedEvent(false));
144
-            logger.log('Audio only disabled');
145
-            dispatch(setAudioOnly(false));
146
-        }
147
-    } else if (startAudioOnly) {
148
-        sendAnalytics(createAudioOnlyChangedEvent(true));
149
-        logger.log('Audio only enabled');
150
-        dispatch(setAudioOnly(true));
151
-    }
152
-
153
-    return result;
154
-}
155
-
156 112
 /**
157 113
  * Does extra sync up on properties that may need to be updated after the
158 114
  * conference was joined.
@@ -262,25 +218,34 @@ function _pinParticipant({ getState }, next, action) {
262 218
  * @returns {Object} The value returned by {@code next(action)}.
263 219
  */
264 220
 function _setAudioOnly({ dispatch, getState }, next, action) {
221
+    const { audioOnly: oldValue } = getState()['features/base/conference'];
265 222
     const result = next(action);
266
-
267
-    const { audioOnly } = getState()['features/base/conference'];
223
+    const { audioOnly: newValue } = getState()['features/base/conference'];
224
+
225
+    // Send analytics. We could've done it in the action creator setAudioOnly.
226
+    // I don't know why it has to happen as early as possible but the analytics
227
+    // were originally sent before the SET_AUDIO_ONLY action was even dispatched
228
+    // in the redux store so I'm now sending the analytics as early as possible.
229
+    if (oldValue !== newValue) {
230
+        sendAnalytics(createAudioOnlyChangedEvent(newValue));
231
+        logger.log(`Audio-only ${newValue ? 'enabled' : 'disabled'}`);
232
+    }
268 233
 
269 234
     // Set lastN to 0 in case audio-only is desired; leave it as undefined,
270 235
     // otherwise, and the default lastN value will be chosen automatically.
271
-    dispatch(setLastN(audioOnly ? 0 : undefined));
236
+    dispatch(setLastN(newValue ? 0 : undefined));
272 237
 
273 238
     // Mute/unmute the local video.
274 239
     dispatch(
275 240
         setVideoMuted(
276
-            audioOnly,
241
+            newValue,
277 242
             VIDEO_MUTISM_AUTHORITY.AUDIO_ONLY,
278 243
             /* ensureTrack */ true));
279 244
 
280 245
     if (typeof APP !== 'undefined') {
281 246
         // TODO This should be a temporary solution that lasts only until
282 247
         // video tracks and all ui is moved into react/redux on the web.
283
-        APP.UI.emitEvent(UIEvents.TOGGLE_AUDIO_ONLY, audioOnly);
248
+        APP.UI.emitEvent(UIEvents.TOGGLE_AUDIO_ONLY, newValue);
284 249
     }
285 250
 
286 251
     return result;

+ 15
- 2
react/features/base/tracks/actions.js View File

@@ -7,7 +7,8 @@ import {
7 7
     CAMERA_FACING_MODE,
8 8
     MEDIA_TYPE,
9 9
     setAudioMuted,
10
-    setVideoMuted
10
+    setVideoMuted,
11
+    VIDEO_MUTISM_AUTHORITY
11 12
 } from '../media';
12 13
 import { getLocalParticipant } from '../participants';
13 14
 
@@ -41,7 +42,19 @@ export function createDesiredLocalTracks(...desiredTypes) {
41 42
             const { audio, video } = state['features/base/media'];
42 43
 
43 44
             audio.muted || desiredTypes.push(MEDIA_TYPE.AUDIO);
44
-            video.muted || desiredTypes.push(MEDIA_TYPE.VIDEO);
45
+
46
+            // XXX When the app is coming into the foreground from the
47
+            // background in order to handle a URL, it may realize the new
48
+            // background state soon after it has tried to create the local
49
+            // tracks requested by the URL. Ignore
50
+            // VIDEO_MUTISM_AUTHORITY.BACKGROUND and create the local video
51
+            // track if no other VIDEO_MUTISM_AUTHORITY has muted it. The local
52
+            // video track will be muted until the app realizes the new
53
+            // background state.
54
+
55
+            // eslint-disable-next-line no-bitwise
56
+            (video.muted & ~VIDEO_MUTISM_AUTHORITY.BACKGROUND)
57
+                || desiredTypes.push(MEDIA_TYPE.VIDEO);
45 58
         }
46 59
 
47 60
         const availableTypes

Loading…
Cancel
Save