From b29647bd739f5cb8ec21fe104300a1bbed8c4d0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 25 May 2021 06:48:21 +0200 Subject: Replace anonymous with arrow function so "this" is properly defined MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/localmedia.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/utils/webrtc/simplewebrtc/localmedia.js b/src/utils/webrtc/simplewebrtc/localmedia.js index 890961dd4..ad7e84903 100644 --- a/src/utils/webrtc/simplewebrtc/localmedia.js +++ b/src/utils/webrtc/simplewebrtc/localmedia.js @@ -634,8 +634,8 @@ LocalMedia.prototype.resume = function() { LocalMedia.prototype._setAudioEnabled = function(bool) { this._audioEnabled = bool - this.localStreams.forEach(function(stream) { - stream.getAudioTracks().forEach(function(track) { + this.localStreams.forEach(stream => { + stream.getAudioTracks().forEach(track => { track.enabled = !!bool }) }) @@ -643,8 +643,8 @@ LocalMedia.prototype._setAudioEnabled = function(bool) { LocalMedia.prototype._setVideoEnabled = function(bool) { this._videoEnabled = bool - this.localStreams.forEach(function(stream) { - stream.getVideoTracks().forEach(function(track) { + this.localStreams.forEach(stream => { + stream.getVideoTracks().forEach(track => { track.enabled = !!bool }) }) -- cgit v1.2.3 From 51b24cc9b5d5241c1b97615c2f2b9f0374d82754 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 18 Jun 2021 20:27:51 +0200 Subject: Do not replace tracks when a connection has not been established yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a connection has not started yet tracks can not be replaced, as Firefox will get "stuck" and not replace the tracks even if tried later again once connected. To address that now the tracks are replaced only when the connection is not in the "new" state (even if the connection is in the "checking" state replacing the track seems to work fine). If a track is tried to be replaced in the "new" state the action is enqueued and executed once the connection is started. Besides that as the replace track actions are now enqueued this also prevents replacing a track before the previous replacement has finished. This should not happen, but if it did it could have caused issues due to the track to be replaced not having been set yet. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/peer.js | 74 ++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js index dbf6a2380..df22c5ff6 100644 --- a/src/utils/webrtc/simplewebrtc/peer.js +++ b/src/utils/webrtc/simplewebrtc/peer.js @@ -31,6 +31,7 @@ function Peer(options) { this.receiveMedia = options.receiveMedia || this.parent.config.receiveMedia this.channels = {} this.pendingDCMessages = [] // key (datachannel label) -> value (array[pending messages]) + this._pendingReplaceTracksQueue = [] this.sid = options.sid || Date.now().toString() this.pc = new RTCPeerConnection(this.parent.config.peerConnectionConfig) this.pc.addEventListener('icecandidate', this.onIceCandidate.bind(this)) @@ -47,6 +48,10 @@ function Peer(options) { this.pc.addEventListener('negotiationneeded', this.emit.bind(this, 'negotiationNeeded')) this.pc.addEventListener('iceconnectionstatechange', this.emit.bind(this, 'iceConnectionStateChange')) this.pc.addEventListener('iceconnectionstatechange', function() { + if (self.pc.iceConnectionState !== 'new') { + self._processPendingReplaceTracks() + } + switch (self.pc.iceConnectionState) { case 'failed': // currently, in chrome only the initiator goes to failed @@ -375,8 +380,67 @@ Peer.prototype.end = function() { } Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { + this._pendingReplaceTracksQueue.push({ newTrack, oldTrack, stream }) + + if (this._pendingReplaceTracksQueue.length === 1) { + this._processPendingReplaceTracks() + } +} + +/** + * Process pending replace track actions. + * + * All the pending replace track actions are executed from the oldest to the + * newest, waiting until the previous action was executed before executing the + * next one. + * + * The process may be stopped if the connection is lost, or if a track needs to + * be added rather than replaced, which requires a renegotiation. In both cases + * the process will start again once the connection is restablished. + */ +Peer.prototype._processPendingReplaceTracks = function() { + while (this._pendingReplaceTracksQueue.length > 0) { + if (this.pc.iceConnectionState === 'new') { + // Do not replace the tracks when the connection has not started + // yet, as Firefox can get "stuck" and not replace the tracks even + // if tried later again once connected. + return + } + + const pending = this._pendingReplaceTracksQueue.shift() + + try { + await this._replaceTrack(pending.newTrack, pending.oldTrack, pending.stream) + } catch (exception) { + // If the track is added instead of replaced a renegotiation will be + // needed, so stop replacing tracks. + return + } + } +} + +/** + * Replaces the old track with the new track in the appropriate sender. + * + * If a new track is provided but no sender was found the new track is added + * instead of replaced (which will require a renegotiation). + * + * The method returns a promise which is fulfilled once the track was replaced + * in the appropriate sender, or immediately if no sender was found and no track + * was added. If a track had to be added the promise is rejected instead. + * + * @param {MediaStreamTrack|null} newTrack the new track to set. + * @param {MediaStreamTrack|null} oldTrack the old track to be replaced. + * @param {MediaStream} stream the stream that the new track belongs to. + * @returns {Promise} + */ +Peer.prototype._replaceTrack = async function(newTrack, oldTrack, stream) { let senderFound = false + // The track should be replaced in just one sender, but an array of promises + // is used to be on the safe side. + const replaceTrackPromises = [] + this.pc.getSenders().forEach(sender => { if (sender.track !== oldTrack) { return @@ -409,13 +473,17 @@ Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { senderFound = true - sender.replaceTrack(newTrack).catch(error => { + const replaceTrackPromise = sender.replaceTrack(newTrack) + + replaceTrackPromise.catch(error => { if (error.name === 'InvalidModificationError') { console.debug('Track could not be replaced, negotiation needed') } else { console.error('Track could not be replaced: ', error, oldTrack, newTrack) } }) + + replaceTrackPromises.push(replaceTrackPromise) }) // If the call started when the audio or video device was not active there @@ -423,7 +491,11 @@ Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { // instead of replaced. if (!senderFound && newTrack) { this.pc.addTrack(newTrack, stream) + + return Promise.reject(new Error('Track added instead of replaced')) } + + return Promise.allSettled(replaceTrackPromises) } Peer.prototype.handleRemoteStreamAdded = function(event) { -- cgit v1.2.3 From 661d5fd1e897983700f9aa2fb69c3a6673112a80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 21 Jun 2021 04:34:00 +0200 Subject: Do not process pending replace tracks again if they are being processed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If "processPendingReplaceTracks" was called while a processing was on-going (for example, on quick connection state changes) this would cause another processing in parallel, which could have unexpected results. To prevent that now there is at most a single processing on-going at any time. The method also returns a promise now, which will make possible to enqueue further actions after the processing has finished even if the processing was actually started in a different call. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/peer.js | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js index df22c5ff6..4f81eac75 100644 --- a/src/utils/webrtc/simplewebrtc/peer.js +++ b/src/utils/webrtc/simplewebrtc/peer.js @@ -32,6 +32,7 @@ function Peer(options) { this.channels = {} this.pendingDCMessages = [] // key (datachannel label) -> value (array[pending messages]) this._pendingReplaceTracksQueue = [] + this._processPendingReplaceTracksPromise = null this.sid = options.sid || Date.now().toString() this.pc = new RTCPeerConnection(this.parent.config.peerConnectionConfig) this.pc.addEventListener('icecandidate', this.onIceCandidate.bind(this)) @@ -382,9 +383,7 @@ Peer.prototype.end = function() { Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { this._pendingReplaceTracksQueue.push({ newTrack, oldTrack, stream }) - if (this._pendingReplaceTracksQueue.length === 1) { - this._processPendingReplaceTracks() - } + this._processPendingReplaceTracks() } /** @@ -397,14 +396,36 @@ Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { * The process may be stopped if the connection is lost, or if a track needs to * be added rather than replaced, which requires a renegotiation. In both cases * the process will start again once the connection is restablished. + * + * @returns {Promise} a Promise fulfilled when the processing ends; if it was + * completed the resolved value is true, and if it was stopped before + * finishing the resolved value is false. */ Peer.prototype._processPendingReplaceTracks = function() { + if (this._processPendingReplaceTracksPromise) { + return this._processPendingReplaceTracksPromise + } + + this._processPendingReplaceTracksPromise = this._processPendingReplaceTracksAsync() + + // For compatibility with older browsers "finally" should not be used on + // Promises. + this._processPendingReplaceTracksPromise.then(() => { + this._processPendingReplaceTracksPromise = null + }).catch(() => { + this._processPendingReplaceTracksPromise = null + }) + + return this._processPendingReplaceTracksPromise +} + +Peer.prototype._processPendingReplaceTracksAsync = async function() { while (this._pendingReplaceTracksQueue.length > 0) { if (this.pc.iceConnectionState === 'new') { // Do not replace the tracks when the connection has not started // yet, as Firefox can get "stuck" and not replace the tracks even // if tried later again once connected. - return + return false } const pending = this._pendingReplaceTracksQueue.shift() @@ -414,9 +435,11 @@ Peer.prototype._processPendingReplaceTracks = function() { } catch (exception) { // If the track is added instead of replaced a renegotiation will be // needed, so stop replacing tracks. - return + return false } } + + return true } /** -- cgit v1.2.3 From abe5633824b06ddfd97869dab6cb12ebbc8407ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 25 May 2021 06:50:33 +0200 Subject: Stop sent streams when media is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to stop the sent streams the track can be replaced in the sender by a null track. To differentiate between a null track due to no device being selected or the original track being stopped an additional attribute, "trackDisabled", is attached now to each sender. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/localmedia.js | 8 ++++ src/utils/webrtc/simplewebrtc/peer.js | 59 ++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/utils/webrtc/simplewebrtc/localmedia.js b/src/utils/webrtc/simplewebrtc/localmedia.js index ad7e84903..a7a008923 100644 --- a/src/utils/webrtc/simplewebrtc/localmedia.js +++ b/src/utils/webrtc/simplewebrtc/localmedia.js @@ -637,6 +637,10 @@ LocalMedia.prototype._setAudioEnabled = function(bool) { this.localStreams.forEach(stream => { stream.getAudioTracks().forEach(track => { track.enabled = !!bool + + // MediaStreamTrack does not emit an event when the enabled property + // changes, so it needs to be explicitly notified. + this.emit('localTrackEnabledChanged', track, stream) }) }) } @@ -646,6 +650,10 @@ LocalMedia.prototype._setVideoEnabled = function(bool) { this.localStreams.forEach(stream => { stream.getVideoTracks().forEach(track => { track.enabled = !!bool + + // MediaStreamTrack does not emit an event when the enabled property + // changes, so it needs to be explicitly notified. + this.emit('localTrackEnabledChanged', track, stream) }) }) } diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js index 4f81eac75..44feb991d 100644 --- a/src/utils/webrtc/simplewebrtc/peer.js +++ b/src/utils/webrtc/simplewebrtc/peer.js @@ -88,6 +88,9 @@ function Peer(options) { // TODO What would happen if the track is replaced while the peer is // still negotiating the offer and answer? this.parent.on('localTrackReplaced', this.handleLocalTrackReplacedBound) + + this.handleLocalTrackEnabledChangedBound = this.handleLocalTrackEnabledChanged.bind(this) + this.parent.on('localTrackEnabledChanged', this.handleLocalTrackEnabledChangedBound) } } @@ -378,6 +381,7 @@ Peer.prototype.end = function() { this.pc.close() this.handleStreamRemoved() this.parent.off('localTrackReplaced', this.handleLocalTrackReplacedBound) + this.parent.off('localTrackEnabledChanged', this.handleLocalTrackEnabledChangedBound) } Peer.prototype.handleLocalTrackReplaced = function(newTrack, oldTrack, stream) { @@ -445,6 +449,12 @@ Peer.prototype._processPendingReplaceTracksAsync = async function() { /** * Replaces the old track with the new track in the appropriate sender. * + * If the new track is disabled the old track will be replaced by a null track + * instead, which stops the sent data. The old and new tracks can be the same + * track, which can be used to start or stop sending the track data depending on + * whether the track is enabled or disabled (at the time of being passed to this + * method). + * * If a new track is provided but no sender was found the new track is added * instead of replaced (which will require a renegotiation). * @@ -465,16 +475,29 @@ Peer.prototype._replaceTrack = async function(newTrack, oldTrack, stream) { const replaceTrackPromises = [] this.pc.getSenders().forEach(sender => { - if (sender.track !== oldTrack) { + if (sender.track !== oldTrack && sender.trackDisabled !== oldTrack) { + return + } + + if ((sender.track || sender.trackDisabled) && !oldTrack) { return } if (!sender.track && !newTrack) { + // The old track was disabled and thus already stopped, so it does + // not need to be replaced, but the null track needs to be set as + // the disabled track. + if (sender.trackDisabled === oldTrack) { + sender.trackDisabled = newTrack + } + return } if (!sender.kind && sender.track) { sender.kind = sender.track.kind + } else if (!sender.kind && sender.trackDisabled) { + sender.kind = sender.trackDisabled.kind } else if (!sender.kind) { this.pc.getTransceivers().forEach(transceiver => { if (transceiver.sender === sender) { @@ -496,9 +519,32 @@ Peer.prototype._replaceTrack = async function(newTrack, oldTrack, stream) { senderFound = true + // Save reference to trackDisabled to be able to restore it if the track + // can not be replaced. + const oldTrackDisabled = sender.trackDisabled + + if (newTrack && !newTrack.enabled) { + sender.trackDisabled = newTrack + } else { + sender.trackDisabled = null + } + + if (!sender.track && !newTrack.enabled) { + // Nothing to replace now, it will be done once the track is + // enabled. + return + } + + if (sender.track && newTrack && !newTrack.enabled) { + // Replace with a null track to stop the sender. + newTrack = null + } + const replaceTrackPromise = sender.replaceTrack(newTrack) replaceTrackPromise.catch(error => { + sender.trackDisabled = oldTrackDisabled + if (error.name === 'InvalidModificationError') { console.debug('Track could not be replaced, negotiation needed') } else { @@ -521,6 +567,17 @@ Peer.prototype._replaceTrack = async function(newTrack, oldTrack, stream) { return Promise.allSettled(replaceTrackPromises) } +Peer.prototype.handleLocalTrackEnabledChanged = function(track, stream) { + const sender = this.pc.getSenders().find(sender => sender.track === track) + const stoppedSender = this.pc.getSenders().find(sender => sender.trackDisabled === track) + + if (track.enabled && stoppedSender) { + this.handleLocalTrackReplacedBound(track, track, stream) + } else if (!track.enabled && sender) { + this.handleLocalTrackReplacedBound(track, track, stream) + } +} + Peer.prototype.handleRemoteStreamAdded = function(event) { const self = this if (this.stream) { -- cgit v1.2.3 From 087432048effdbd457c71fac484acdf31725cf45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 25 May 2021 06:53:25 +0200 Subject: Stop sent streams for initially disabled media MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Besides reacting to disable state changes the sent streams are also stopped now when tracks are initially disabled and no event is emitted. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/peer.js | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js index 44feb991d..c30b76192 100644 --- a/src/utils/webrtc/simplewebrtc/peer.js +++ b/src/utils/webrtc/simplewebrtc/peer.js @@ -33,6 +33,7 @@ function Peer(options) { this.pendingDCMessages = [] // key (datachannel label) -> value (array[pending messages]) this._pendingReplaceTracksQueue = [] this._processPendingReplaceTracksPromise = null + this._initialStreamSetup = false this.sid = options.sid || Date.now().toString() this.pc = new RTCPeerConnection(this.parent.config.peerConnectionConfig) this.pc.addEventListener('icecandidate', this.onIceCandidate.bind(this)) @@ -50,7 +51,25 @@ function Peer(options) { this.pc.addEventListener('iceconnectionstatechange', this.emit.bind(this, 'iceConnectionStateChange')) this.pc.addEventListener('iceconnectionstatechange', function() { if (self.pc.iceConnectionState !== 'new') { - self._processPendingReplaceTracks() + self._processPendingReplaceTracks().then(finished => { + if (finished === false || self._initialStreamSetup) { + return + } + + // Ensure that initially disabled tracks are stopped after + // establishing a connection. + self.pc.getSenders().forEach(sender => { + if (sender.track) { + // The stream is not known, but it is only used when the + // track is added, so it can be ignored here. + self.handleLocalTrackEnabledChanged(sender.track, null) + } + }) + + self._initialStreamSetup = true + }) + } else { + self._initialStreamSetup = false } switch (self.pc.iceConnectionState) { -- cgit v1.2.3 From a9ca74c3a726a2b1c1f09714b952960c2b05db17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 21 Jun 2021 21:09:06 +0200 Subject: Do not trigger processing of pending replace track actions on receivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Receiver only peers do not handle local track changes, so there is no need to trigger the processing of pending replace track actions in that case, as there will be none. Signed-off-by: Daniel Calviño Sánchez --- src/utils/webrtc/simplewebrtc/peer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/webrtc/simplewebrtc/peer.js b/src/utils/webrtc/simplewebrtc/peer.js index c30b76192..893bdeb77 100644 --- a/src/utils/webrtc/simplewebrtc/peer.js +++ b/src/utils/webrtc/simplewebrtc/peer.js @@ -50,7 +50,7 @@ function Peer(options) { this.pc.addEventListener('negotiationneeded', this.emit.bind(this, 'negotiationNeeded')) this.pc.addEventListener('iceconnectionstatechange', this.emit.bind(this, 'iceConnectionStateChange')) this.pc.addEventListener('iceconnectionstatechange', function() { - if (self.pc.iceConnectionState !== 'new') { + if (!options.receiverOnly && self.pc.iceConnectionState !== 'new') { self._processPendingReplaceTracks().then(finished => { if (finished === false || self._initialStreamSetup) { return -- cgit v1.2.3