Просмотр исходного кода

fix(tracks): enqueue track replacement

The process for doing a replaceLocalTrack is async. Is it
possible to trigger replaceLocalTrack multiple times before
each call is finished. This leads to situations where
replaceLocalTrack is called multiple times with oldTrack being
null and a new track. In this scenario, each new track will be
added, causing UI issues such as the local participant's
large video not displaying for remote participants.

The action replaceLocalTrack is used when unmuting audio or
video, when creating new tracks on device switch, and when
toggling screensharing. These actions can collide with each
other. One way to fix this would be to queue replaceLocalTrack.
master
Leonard Kim 7 лет назад
Родитель
Сommit
3927f29ba8
3 измененных файлов: 127 добавлений и 20 удалений
  1. 53
    20
      conference.js
  2. 63
    0
      modules/util/TaskQueue.js
  3. 11
    0
      modules/util/helpers.js

+ 53
- 20
conference.js Просмотреть файл

@@ -11,6 +11,7 @@ import * as RemoteControlEvents
11 11
     from './service/remotecontrol/RemoteControlEvents';
12 12
 import UIEvents from './service/UI/UIEvents';
13 13
 import UIUtil from './modules/UI/util/UIUtil';
14
+import { createTaskQueue } from './modules/util/helpers';
14 15
 import * as JitsiMeetConferenceEvents from './ConferenceEvents';
15 16
 
16 17
 import {
@@ -274,6 +275,27 @@ function redirectToStaticPage(pathname) {
274 275
     windowLocation.pathname = newPathname;
275 276
 }
276 277
 
278
+/**
279
+ * A queue for the async replaceLocalTrack action so that multiple audio
280
+ * replacements cannot happen simultaneously. This solves the issue where
281
+ * replaceLocalTrack is called multiple times with an oldTrack of null, causing
282
+ * multiple local tracks of the same type to be used.
283
+ *
284
+ * @private
285
+ * @type {Object}
286
+ */
287
+const _replaceLocalAudioTrackQueue = createTaskQueue();
288
+
289
+/**
290
+ * A task queue for replacement local video tracks. This separate queue exists
291
+ * so video replacement is not blocked by audio replacement tasks in the queue
292
+ * {@link _replaceLocalAudioTrackQueue}.
293
+ *
294
+ * @private
295
+ * @type {Object}
296
+ */
297
+const _replaceLocalVideoTrackQueue = createTaskQueue();
298
+
277 299
 /**
278 300
  *
279 301
  */
@@ -856,9 +878,6 @@ export default {
856 878
             return;
857 879
         }
858 880
 
859
-        // FIXME it is possible to queue this task twice, but it's not causing
860
-        // any issues. Specifically this can happen when the previous
861
-        // get user media call is blocked on "ask user for permissions" dialog.
862 881
         if (!this.localVideo && !mute) {
863 882
             const maybeShowErrorDialog = error => {
864 883
                 showUI && APP.UI.showCameraErrorNotification(error);
@@ -1261,16 +1280,23 @@ export default {
1261 1280
      * @returns {Promise}
1262 1281
      */
1263 1282
     useVideoStream(newStream) {
1264
-        return APP.store.dispatch(
1265
-            replaceLocalTrack(this.localVideo, newStream, room))
1266
-            .then(() => {
1267
-                this.localVideo = newStream;
1268
-                this._setSharingScreen(newStream);
1269
-                if (newStream) {
1270
-                    APP.UI.addLocalStream(newStream);
1271
-                }
1272
-                this.setVideoMuteStatus(this.isLocalVideoMuted());
1283
+        return new Promise((resolve, reject) => {
1284
+            _replaceLocalVideoTrackQueue.enqueue(onFinish => {
1285
+                APP.store.dispatch(
1286
+                replaceLocalTrack(this.localVideo, newStream, room))
1287
+                    .then(() => {
1288
+                        this.localVideo = newStream;
1289
+                        this._setSharingScreen(newStream);
1290
+                        if (newStream) {
1291
+                            APP.UI.addLocalStream(newStream);
1292
+                        }
1293
+                        this.setVideoMuteStatus(this.isLocalVideoMuted());
1294
+                    })
1295
+                    .then(resolve)
1296
+                    .catch(reject)
1297
+                    .then(onFinish);
1273 1298
             });
1299
+        });
1274 1300
     },
1275 1301
 
1276 1302
     /**
@@ -1300,15 +1326,22 @@ export default {
1300 1326
      * @returns {Promise}
1301 1327
      */
1302 1328
     useAudioStream(newStream) {
1303
-        return APP.store.dispatch(
1304
-            replaceLocalTrack(this.localAudio, newStream, room))
1305
-            .then(() => {
1306
-                this.localAudio = newStream;
1307
-                if (newStream) {
1308
-                    APP.UI.addLocalStream(newStream);
1309
-                }
1310
-                this.setAudioMuteStatus(this.isLocalAudioMuted());
1329
+        return new Promise((resolve, reject) => {
1330
+            _replaceLocalAudioTrackQueue.enqueue(onFinish => {
1331
+                APP.store.dispatch(
1332
+                replaceLocalTrack(this.localAudio, newStream, room))
1333
+                    .then(() => {
1334
+                        this.localAudio = newStream;
1335
+                        if (newStream) {
1336
+                            APP.UI.addLocalStream(newStream);
1337
+                        }
1338
+                        this.setAudioMuteStatus(this.isLocalAudioMuted());
1339
+                    })
1340
+                    .then(resolve)
1341
+                    .catch(reject)
1342
+                    .then(onFinish);
1311 1343
             });
1344
+        });
1312 1345
     },
1313 1346
 
1314 1347
     /**

+ 63
- 0
modules/util/TaskQueue.js Просмотреть файл

@@ -0,0 +1,63 @@
1
+const logger = require('jitsi-meet-logger').getLogger(__filename);
2
+
3
+/**
4
+ * Manages a queue of functions where the current function in progress will
5
+ * automatically execute the next queued function.
6
+ */
7
+export class TaskQueue {
8
+    /**
9
+     * Creates a new instance of {@link TaskQueue} and sets initial instance
10
+     * variable values.
11
+     */
12
+    constructor() {
13
+        this._queue = [];
14
+        this._currentTask = null;
15
+
16
+        this._onTaskComplete = this._onTaskComplete.bind(this);
17
+    }
18
+
19
+    /**
20
+     * Adds a new function to the queue. It will be immediately invoked if no
21
+     * other functions are queued.
22
+     *
23
+     * @param {Function} taskFunction - The function to be queued for execution.
24
+     * @private
25
+     * @returns {void}
26
+     */
27
+    enqueue(taskFunction) {
28
+        this._queue.push(taskFunction);
29
+        this._executeNext();
30
+    }
31
+
32
+    /**
33
+     * If no queued task is currently executing, invokes the first task in the
34
+     * queue if any.
35
+     *
36
+     * @private
37
+     * @returns {void}
38
+     */
39
+    _executeNext() {
40
+        if (this._currentTask) {
41
+            logger.warn('Task queued while a task is in progress.');
42
+
43
+            return;
44
+        }
45
+
46
+        this._currentTask = this._queue.shift() || null;
47
+
48
+        if (this._currentTask) {
49
+            this._currentTask(this._onTaskComplete);
50
+        }
51
+    }
52
+
53
+    /**
54
+     * Prepares to invoke the next function in the queue.
55
+     *
56
+     * @private
57
+     * @returns {void}
58
+     */
59
+    _onTaskComplete() {
60
+        this._currentTask = null;
61
+        this._executeNext();
62
+    }
63
+}

+ 11
- 0
modules/util/helpers.js Просмотреть файл

@@ -1,3 +1,5 @@
1
+import { TaskQueue } from './TaskQueue';
2
+
1 3
 /**
2 4
  * Create deferred object.
3 5
  *
@@ -13,3 +15,12 @@ export function createDeferred() {
13 15
 
14 16
     return deferred;
15 17
 }
18
+
19
+/**
20
+ * Returns an instance of {@link TaskQueue}.
21
+ *
22
+ * @returns {Object}
23
+ */
24
+export function createTaskQueue() {
25
+    return new TaskQueue();
26
+}

Загрузка…
Отмена
Сохранить