Bläddra i källkod

fix(local tracks): do not fire "track stopped" during mute

Flag 'dontFireRemoveEvent' was unreliable, because if
the JitsiLocalTrack is muted/unmuted rapidly it's state can be false
at the time when ended event is triggered. The event handlers must be
unregistered to handle this gracefully.
master
paweldomas 8 år sedan
förälder
incheckning
9137912d75
2 ändrade filer med 70 tillägg och 21 borttagningar
  1. 3
    6
      modules/RTC/JitsiLocalTrack.js
  2. 67
    15
      modules/RTC/JitsiTrack.js

+ 3
- 6
modules/RTC/JitsiLocalTrack.js Visa fil

@@ -56,10 +56,7 @@ export default class JitsiLocalTrack extends JitsiTrack {
56 56
             stream,
57 57
             track,
58 58
             () => {
59
-                if (!this.dontFireRemoveEvent) {
60
-                    this.emit(JitsiTrackEvents.LOCAL_TRACK_STOPPED);
61
-                }
62
-                this.dontFireRemoveEvent = false;
59
+                this.emit(JitsiTrackEvents.LOCAL_TRACK_STOPPED);
63 60
             } /* inactiveHandler */,
64 61
             mediaType,
65 62
             videoType);
@@ -69,7 +66,6 @@ export default class JitsiLocalTrack extends JitsiTrack {
69 66
          * @type {number}
70 67
          */
71 68
         this.rtcId = rtcId;
72
-        this.dontFireRemoveEvent = false;
73 69
         this.resolution = resolution;
74 70
         this.sourceId = sourceId;
75 71
         this.sourceType = sourceType;
@@ -342,7 +338,6 @@ export default class JitsiLocalTrack extends JitsiTrack {
342 338
         let promise = Promise.resolve();
343 339
 
344 340
         this.inMuteOrUnmuteProgress = true;
345
-        this.dontFireRemoveEvent = false;
346 341
 
347 342
         // A function that will print info about muted status transition
348 343
         const logMuteInfo = () => logger.info(`Mute ${this}: ${mute}`);
@@ -361,6 +356,8 @@ export default class JitsiLocalTrack extends JitsiTrack {
361 356
                 this._removeStreamFromConferenceAsMute(() => {
362 357
                     // FIXME: Maybe here we should set the SRC for the
363 358
                     // containers to something
359
+                    // We don't want any events to be fired on this stream
360
+                    this._unregisterHandlers();
364 361
                     this._stopMediaStream();
365 362
                     this._setStream(null);
366 363
                     resolve();

+ 67
- 15
modules/RTC/JitsiTrack.js Visa fil

@@ -95,7 +95,7 @@ export default class JitsiTrack extends EventEmitter {
95 95
         this.type = trackMediaType;
96 96
         this.track = track;
97 97
         this.videoType = videoType;
98
-        this.handlers = {};
98
+        this.handlers = new Map();
99 99
 
100 100
         /**
101 101
          * Indicates whether this JitsiTrack has been disposed. If true, this
@@ -105,11 +105,30 @@ export default class JitsiTrack extends EventEmitter {
105 105
          * @type {boolean}
106 106
          */
107 107
         this.disposed = false;
108
-        this._setHandler('inactive', streamInactiveHandler);
108
+
109
+        /**
110
+         * The inactive handler which will be triggered when the underlying
111
+         * media stream ends.
112
+         * @type {Function}
113
+         */
114
+        this._streamInactiveHandler = streamInactiveHandler;
115
+        this._bindInactiveHandler(streamInactiveHandler);
109 116
     }
110 117
 
111 118
     /* eslint-enable max-params */
112 119
 
120
+    /**
121
+     * Binds the inactive handler.
122
+     * @param {Function} streamInactiveHandler
123
+     * @private
124
+     */
125
+    _bindInactiveHandler(streamInactiveHandler) {
126
+        if (RTCBrowserType.isFirefox()) {
127
+            implementOnEndedHandling(this);
128
+        }
129
+        addMediaStreamInactiveHandler(this.stream, streamInactiveHandler);
130
+    }
131
+
113 132
     /**
114 133
      * Sets handler to the WebRTC MediaStream or MediaStreamTrack object
115 134
      * depending on the passed type.
@@ -117,20 +136,45 @@ export default class JitsiTrack extends EventEmitter {
117 136
      * @param {Function} handler the handler.
118 137
      */
119 138
     _setHandler(type, handler) {
120
-        this.handlers[type] = handler;
139
+        if (!trackHandler2Prop.hasOwnProperty(type)) {
140
+            logger.error(`Invalid handler type ${type}`);
141
+
142
+            return;
143
+        }
144
+        if (handler) {
145
+            this.handlers.set(type, handler);
146
+        } else {
147
+            this.handlers.delete(type);
148
+        }
149
+
150
+        if (this.stream) {
151
+            // FIXME why only video tacks ?
152
+            for (const track of this.stream.getVideoTracks()) {
153
+                track[trackHandler2Prop[type]] = handler;
154
+            }
155
+        }
156
+    }
157
+
158
+    /**
159
+     * Unregisters all event handlers bound to the underlying media stream/track
160
+     * @private
161
+     */
162
+    _unregisterHandlers() {
121 163
         if (!this.stream) {
164
+            logger.warn(
165
+                `${this}: unable to unregister handlers - no stream object`);
166
+
122 167
             return;
123 168
         }
124 169
 
125
-        if (type === 'inactive') {
126
-            if (RTCBrowserType.isFirefox()) {
127
-                implementOnEndedHandling(this);
170
+        for (const type of this.handlers.keys()) {
171
+            // FIXME why only video tracks ?
172
+            for (const videoTrack of this.stream.getVideoTracks()) {
173
+                videoTrack[trackHandler2Prop[type]] = undefined;
128 174
             }
129
-            addMediaStreamInactiveHandler(this.stream, handler);
130
-        } else if (trackHandler2Prop.hasOwnProperty(type)) {
131
-            this.stream.getVideoTracks().forEach(track => {
132
-                track[trackHandler2Prop[type]] = handler;
133
-            }, this);
175
+        }
176
+        if (this._streamInactiveHandler) {
177
+            addMediaStreamInactiveHandler(this.stream, undefined);
134 178
         }
135 179
     }
136 180
 
@@ -140,11 +184,19 @@ export default class JitsiTrack extends EventEmitter {
140 184
      * @param {MediaStream} stream the new stream.
141 185
      */
142 186
     _setStream(stream) {
187
+        if (this.stream === stream) {
188
+            logger.warn(`Attempt to set the same stream twice on ${this}`);
189
+
190
+            return;
191
+        }
192
+
143 193
         this.stream = stream;
144
-        Object.keys(this.handlers).forEach(type => {
145
-            typeof this.handlers[type] === 'function'
146
-                && this._setHandler(type, this.handlers[type]);
147
-        }, this);
194
+        for (const type of this.handlers.keys()) {
195
+            this._setHandler(type, this.handlers.get(type));
196
+        }
197
+        if (this._streamInactiveHandler && this.stream) {
198
+            this._bindInactiveHandler(this._streamInactiveHandler);
199
+        }
148 200
     }
149 201
 
150 202
     /**

Laddar…
Avbryt
Spara