From 55131be9e66991a6cc9f3d910913607edbcb012e Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty <54324652+jallamsetty1@users.noreply.github.com> Date: Tue, 15 Feb 2022 21:44:03 -0500 Subject: [PATCH] feat(multi-stream-support) Adjust presence and videoType bridge message. (#1887) * feat(multi-stream-support) Adjust presence and videoType message. Adjust the sourceInfo for all the available local tracks. Do not add handlers for videoType changes on the peer since the videoType doesn't change when multiple stream support is enabled. * fix(presence) Send the presence in old format for backwards compat. Send the presence messages in both the old and new format when source name signaling is enabled and sending multiple streams is disabled. * fix(JitsiConference) Send the videoType bridge msg after mute/unmute op is complete. * ref(multi-stream-support) Throw if a track is being replaced with a track of different videoType. This is not supported in the multi sender enabled mode. * Address review comments. --- JitsiConference.js | 122 +++++++++--------- modules/RTC/JitsiLocalTrack.js | 4 + modules/xmpp/SignalingLayerImpl.js | 30 ++++- .../auto/modules/xmpp/SignalingLayerImpl.d.ts | 3 +- 4 files changed, 94 insertions(+), 65 deletions(-) diff --git a/JitsiConference.js b/JitsiConference.js index 684f52c1ea..b1a8db2452 100644 --- a/JitsiConference.js +++ b/JitsiConference.js @@ -778,12 +778,9 @@ JitsiConference.prototype._sendBridgeVideoTypeMessage = function(localtrack) { videoType = BridgeVideoType.DESKTOP_HIGH_FPS; } - if (FeatureFlags.isSourceNameSignalingEnabled()) { - this.rtc.sendSourceVideoType( - getSourceNameForJitsiTrack(this.myUserId(), MediaType.VIDEO, 0), - videoType - ); - } else { + if (FeatureFlags.isSourceNameSignalingEnabled() && localtrack) { + this.rtc.sendSourceVideoType(localtrack.getSourceName(), videoType); + } else if (!FeatureFlags.isSourceNameSignalingEnabled()) { this.rtc.setVideoType(videoType); } }; @@ -1252,6 +1249,13 @@ JitsiConference.prototype.removeTrack = function(track) { * @returns {Promise} resolves when the replacement is finished */ JitsiConference.prototype.replaceTrack = function(oldTrack, newTrack) { + const oldVideoType = oldTrack?.getVideoType(); + const newVideoType = newTrack?.getVideoType(); + + if (FeatureFlags.isMultiStreamSupportEnabled() && oldTrack && newTrack && oldVideoType !== newVideoType) { + throw new Error(`Replacing a track of videoType=${oldVideoType} with a track of videoType=${newVideoType} is` + + ' not supported in this mode.'); + } const oldTrackBelongsToConference = this === oldTrack?.conference; if (oldTrackBelongsToConference && oldTrack.disposed) { @@ -1395,29 +1399,30 @@ JitsiConference.prototype._setupNewTrack = function(newTrack) { * @private */ JitsiConference.prototype._setNewVideoType = function(track) { - if (FeatureFlags.isSourceNameSignalingEnabled() && track) { - // FIXME once legacy signaling using 'sendCommand' is removed, signalingLayer.setTrackVideoType must be adjusted - // to send the presence (not just modify it). - this._signalingLayer.setTrackVideoType(track.getSourceName(), track.videoType); + let videoTypeChanged = false; - // TODO: Optimize to detect whether presence was changed, for now always report changed to send presence - return true; + if (FeatureFlags.isSourceNameSignalingEnabled() && track) { + videoTypeChanged = this._signalingLayer.setTrackVideoType(track.getSourceName(), track.videoType); } - const videoTypeTagName = 'videoType'; + if (!FeatureFlags.isMultiStreamSupportEnabled()) { + const videoTypeTagName = 'videoType'; - // if track is missing we revert to default type Camera, the case where we screenshare and - // we return to be video muted - const trackVideoType = track ? track.videoType : VideoType.CAMERA; + // If track is missing we revert to default type Camera, the case where we screenshare and + // we return to be video muted. + const trackVideoType = track ? track.videoType : VideoType.CAMERA; - // if video type is camera and there is no videoType in presence, we skip adding it, as this is the default one - if (trackVideoType !== VideoType.CAMERA || this.room.getFromPresence(videoTypeTagName)) { - // we will not use this.sendCommand here to avoid sending the presence immediately, as later we may also set - // and the mute status - return this.room.addOrReplaceInPresence(videoTypeTagName, { value: trackVideoType }); + // If video type is camera and there is no videoType in presence, we skip adding it, as this is the default one + if (trackVideoType !== VideoType.CAMERA || this.room.getFromPresence(videoTypeTagName)) { + // We will not use this.sendCommand here to avoid sending the presence immediately, as later we may also + // set the mute status. + const legacyTypeChanged = this.room.addOrReplaceInPresence(videoTypeTagName, { value: trackVideoType }); + + videoTypeChanged = videoTypeChanged || legacyTypeChanged; + } } - return false; + return videoTypeChanged; }; /** @@ -1429,21 +1434,31 @@ JitsiConference.prototype._setNewVideoType = function(track) { * @private */ JitsiConference.prototype._setTrackMuteStatus = function(mediaType, localTrack, isMuted) { - if (FeatureFlags.isSourceNameSignalingEnabled()) { - // TODO When legacy signaling part is removed, remember to adjust signalingLayer.setTrackMuteStatus, so that - // it triggers sending the presence (it only updates it for now, because the legacy code below sends). - this._signalingLayer.setTrackMuteStatus(localTrack?.getSourceName(), isMuted); - } + let presenceChanged = false; - if (!this.room) { - return false; + if (FeatureFlags.isSourceNameSignalingEnabled() && localTrack) { + presenceChanged = this._signalingLayer.setTrackMuteStatus(localTrack.getSourceName(), isMuted); } - if (mediaType === MediaType.AUDIO) { - return this.room.addAudioInfoToPresence(isMuted); + // Add the 'audioMuted' and 'videoMuted' tags when source name signaling is enabled for backward compatibility. + // It won't be used anymore when multiple stream support is enabled. + if (!FeatureFlags.isMultiStreamSupportEnabled()) { + let audioMuteChanged, videoMuteChanged; + + if (!this.room) { + return false; + } + + if (mediaType === MediaType.AUDIO) { + audioMuteChanged = this.room.addAudioInfoToPresence(isMuted); + } else { + videoMuteChanged = this.room.addVideoInfoToPresence(isMuted); + } + + presenceChanged = presenceChanged || audioMuteChanged || videoMuteChanged; } - return this.room.addVideoInfoToPresence(isMuted); + return presenceChanged; }; /** @@ -1470,11 +1485,7 @@ JitsiConference.prototype._addLocalTrackAsUnmute = function(track) { logger.debug('Add local MediaStream as unmute - no P2P Jingle session started yet'); } - return Promise.allSettled(addAsUnmutePromises) - .then(() => { - // Signal the video type to the bridge. - track.isVideoTrack() && this._sendBridgeVideoTypeMessage(track); - }); + return Promise.allSettled(addAsUnmutePromises); }; /** @@ -1498,11 +1509,7 @@ JitsiConference.prototype._removeLocalTrackAsMute = function(track) { logger.debug('Remove local MediaStream - no P2P JingleSession started yet'); } - return Promise.allSettled(removeAsMutePromises) - .then(() => { - // Signal the video type to the bridge. - track.isVideoTrack() && this._sendBridgeVideoTypeMessage(); - }); + return Promise.allSettled(removeAsMutePromises); }; /** @@ -3608,27 +3615,26 @@ JitsiConference.prototype._updateRoomPresence = function(jingleSession, ctx) { ctx.skip = true; } - const localAudioTracks = jingleSession.peerconnection.getLocalTracks(MediaType.AUDIO); - const localVideoTracks = jingleSession.peerconnection.getLocalTracks(MediaType.VIDEO); let presenceChanged = false; + let muteStatusChanged, videoTypeChanged; + const localTracks = this.getLocalTracks(); - if (localAudioTracks && localAudioTracks.length) { - presenceChanged = this._setTrackMuteStatus(MediaType.AUDIO, localAudioTracks[0], localAudioTracks[0].isMuted()); - } else if (this._setTrackMuteStatus(MediaType.AUDIO, undefined, true)) { - presenceChanged = true; + // Set presence for all the available local tracks. + for (const track of localTracks) { + muteStatusChanged = this._setTrackMuteStatus(track.getType(), track, track.isMuted()); + if (track.getType() === MediaType.VIDEO) { + videoTypeChanged = this._setNewVideoType(track); + } + presenceChanged = presenceChanged || muteStatusChanged || videoTypeChanged; } - if (localVideoTracks && localVideoTracks.length) { - const muteStatusChanged = this._setTrackMuteStatus( - MediaType.VIDEO, localVideoTracks[0], localVideoTracks[0].isMuted()); - const videoTypeChanged = this._setNewVideoType(localVideoTracks[0]); + // Set the presence in the legacy format if there are no local tracks and multi stream support is not enabled. + if (!localTracks.length && !FeatureFlags.isMultiStreamSupportEnabled()) { + const audioMuteStatusChanged = this._setTrackMuteStatus(MediaType.AUDIO, undefined, true); + const videoMuteStatusChanged = this._setTrackMuteStatus(MediaType.VIDEO, undefined, true); - presenceChanged = presenceChanged || muteStatusChanged || videoTypeChanged; - } else { - const muteStatusChanged = this._setTrackMuteStatus(MediaType.VIDEO, undefined, true); - const videoTypeChanged = this._setNewVideoType(); // set back to default video type - - presenceChanged = presenceChanged || muteStatusChanged || videoTypeChanged; + videoTypeChanged = this._setNewVideoType(); + presenceChanged = audioMuteStatusChanged || videoMuteStatusChanged || videoTypeChanged; } presenceChanged && this.room.sendPresence(); diff --git a/modules/RTC/JitsiLocalTrack.js b/modules/RTC/JitsiLocalTrack.js index fee79aa811..17884839e6 100644 --- a/modules/RTC/JitsiLocalTrack.js +++ b/modules/RTC/JitsiLocalTrack.js @@ -395,6 +395,7 @@ export default class JitsiLocalTrack extends JitsiTrack { this._unregisterHandlers(); this.stopStream(); this._setStream(null); + resolve(); }, reject); @@ -451,6 +452,9 @@ export default class JitsiLocalTrack extends JitsiTrack { return promise .then(() => { this._sendMuteStatus(muted); + + // Send the videoType message to the bridge. + this.isVideoTrack() && this.conference && this.conference._sendBridgeVideoTypeMessage(this); this.emit(TRACK_MUTE_CHANGED, this); }); } diff --git a/modules/xmpp/SignalingLayerImpl.js b/modules/xmpp/SignalingLayerImpl.js index f0dca8c89a..2a96ee9297 100644 --- a/modules/xmpp/SignalingLayerImpl.js +++ b/modules/xmpp/SignalingLayerImpl.js @@ -72,10 +72,12 @@ export default class SignalingLayerImpl extends SignalingLayer { */ _addLocalSourceInfoToPresence() { if (this.chatRoom) { - this.chatRoom.addOrReplaceInPresence( + return this.chatRoom.addOrReplaceInPresence( SOURCE_INFO_PRESENCE_ELEMENT, { value: JSON.stringify(this._localSourceState) }); } + + return false; } /** @@ -195,7 +197,10 @@ export default class SignalingLayerImpl extends SignalingLayer { emitVideoTypeEvent(from, node.value); } }; - room.addPresenceListener('videoType', this._videoTypeHandler); + + if (!FeatureFlags.isMultiStreamSupportEnabled()) { + room.addPresenceListener('videoType', this._videoTypeHandler); + } this._sourceInfoHandler = (node, mucNick) => { const endpointId = mucNick; @@ -220,11 +225,19 @@ export default class SignalingLayerImpl extends SignalingLayer { } } - const newVideoType = sourceInfoJSON[sourceName].videoType; + // Assume a default videoType of 'camera' for video sources. + const newVideoType = mediaType === MediaType.VIDEO + ? sourceInfoJSON[sourceName].videoType ?? VideoType.CAMERA + : undefined; if (oldSourceState.videoType !== newVideoType) { oldSourceState.videoType = newVideoType; - emitEventsFromHere && emitVideoTypeEvent(endpointId, newVideoType); + + // videoType is not allowed to change on a given JitsiLocalTrack when multi stream support is + // enabled. + emitEventsFromHere + && !FeatureFlags.isMultiStreamSupportEnabled() + && emitVideoTypeEvent(endpointId, newVideoType); } } @@ -387,14 +400,17 @@ export default class SignalingLayerImpl extends SignalingLayer { // FIXME This only adjusts the presence, but doesn't actually send it. Here we temporarily rely on // the legacy signaling part to send the presence. Remember to add "send presence" here when the legacy // signaling is removed. - this._addLocalSourceInfoToPresence(); + return this._addLocalSourceInfoToPresence(); } + + return false; } /** * Sets track's video type. * @param {SourceName} sourceName - the track's source name. * @param {VideoType} videoType - the new video type. + * @returns {boolean} */ setTrackVideoType(sourceName, videoType) { if (!this._localSourceState[sourceName]) { @@ -408,8 +424,10 @@ export default class SignalingLayerImpl extends SignalingLayer { // NOTE this doesn't send the actual presence, because is called from the same place where the legacy video // type is emitted which does the actual sending. A send presence statement needs to be added when // the legacy part is removed. - this._addLocalSourceInfoToPresence(); + return this._addLocalSourceInfoToPresence(); } + + return false; } /** diff --git a/types/auto/modules/xmpp/SignalingLayerImpl.d.ts b/types/auto/modules/xmpp/SignalingLayerImpl.d.ts index 6a9a4f7517..e5c8b30c11 100644 --- a/types/auto/modules/xmpp/SignalingLayerImpl.d.ts +++ b/types/auto/modules/xmpp/SignalingLayerImpl.d.ts @@ -100,11 +100,12 @@ export default class SignalingLayerImpl extends SignalingLayer { * Sets track's video type. * @param {SourceName} sourceName - the track's source name. * @param {VideoType} videoType - the new video type. + * @returns {boolean} */ setTrackVideoType(sourceName: any, videoType: { CAMERA: string; DESKTOP: string; - }): void; + }): boolean; /** * Saves the source name for a track identified by it's ssrc. * @param {number} ssrc the ssrc of the target track.