From 2a1b41e585af92af42d7cd1e374711ee618caee3 Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty Date: Tue, 19 Mar 2024 17:18:39 -0400 Subject: [PATCH] fix(TPC): reconfigure the encodings to suspend/resume media after jvb<->p2p switch. Also fixes the issue with video freezing after video mute and unmute when the user joins the call with startSilent=true. --- JitsiConference.js | 5 -- modules/RTC/TPCUtils.js | 19 ++-- modules/RTC/TPCUtils.spec.js | 116 +++++++++++++++++++++++++ modules/RTC/TraceablePeerConnection.js | 46 ++++++---- modules/xmpp/JingleSessionPC.js | 19 ++-- 5 files changed, 169 insertions(+), 36 deletions(-) diff --git a/JitsiConference.js b/JitsiConference.js index 17faaa9ac6..4793aa043e 100644 --- a/JitsiConference.js +++ b/JitsiConference.js @@ -1388,11 +1388,6 @@ JitsiConference.prototype._setupNewTrack = function(newTrack) { this.rtc.addLocalTrack(newTrack); newTrack.setConference(this); - // Suspend media on the inactive media session since it gets automatically enabled for a newly added source. - if (this.isP2PActive()) { - this._suspendMediaTransferForJvbConnection(); - } - // Add event handlers. newTrack.muteHandler = this._fireMuteChangeEvent.bind(this, newTrack); newTrack.addEventListener(JitsiTrackEvents.TRACK_MUTE_CHANGED, newTrack.muteHandler); diff --git a/modules/RTC/TPCUtils.js b/modules/RTC/TPCUtils.js index 796406f95c..4cfd4f8012 100644 --- a/modules/RTC/TPCUtils.js +++ b/modules/RTC/TPCUtils.js @@ -409,20 +409,23 @@ export class TPCUtils { const encodingsState = videoStreamEncodings .map(encoding => height / encoding.scaleResolutionDownBy) .map((frameHeight, idx) => { + let activeState = false; + + // When video is suspended on the media session. + if (!this.pc.videoTransferActive) { + return activeState; + } + // Single video stream. if (!this.pc.isSpatialScalabilityOn() || this._isRunningInFullSvcMode(codec)) { const { active } = this._calculateActiveEncodingParams(localVideoTrack, codec, newHeight); - return idx === 0 ? active : false; + return idx === 0 ? active : activeState; } - // Multiple video streams. - let active = false; - if (newHeight > 0) { if (localVideoTrack.getVideoType() === VideoType.CAMERA) { - - active = frameHeight <= newHeight + activeState = frameHeight <= newHeight // Keep the LD stream enabled even when the LD stream's resolution is higher than of the // requested resolution. This can happen when camera is captured at high resolutions like 4k @@ -433,12 +436,12 @@ export class TPCUtils { } else { // For screenshare, keep the HD layer enabled always and the lower layers only for high fps // screensharing. - active = videoStreamEncodings[idx].scaleResolutionDownBy === SIM_LAYERS[2].scaleFactor + activeState = videoStreamEncodings[idx].scaleResolutionDownBy === SIM_LAYERS[2].scaleFactor || !this._isScreenshareBitrateCapped(localVideoTrack); } } - return active; + return activeState; }); return encodingsState; diff --git a/modules/RTC/TPCUtils.spec.js b/modules/RTC/TPCUtils.spec.js index b22ea07df8..f0c435383e 100644 --- a/modules/RTC/TPCUtils.spec.js +++ b/modules/RTC/TPCUtils.spec.js @@ -224,6 +224,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -311,6 +312,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -441,6 +443,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = true; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -487,6 +490,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc._capScreenshareBitrate = true; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -528,6 +532,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = false; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -573,6 +578,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -655,6 +661,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -692,6 +699,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -780,6 +788,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = true; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -826,6 +835,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = false; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -871,6 +881,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -979,6 +990,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1087,6 +1099,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1154,6 +1167,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1191,6 +1205,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1285,6 +1300,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = true; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1333,6 +1349,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = false; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1380,6 +1397,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1447,6 +1465,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1484,6 +1503,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1595,6 +1615,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = true; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1641,6 +1662,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc._capScreenshareBitrate = false; pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1686,6 +1708,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1753,6 +1776,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1821,6 +1845,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; pc._capScreenshareBitrate = false; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -1884,6 +1909,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2009,6 +2035,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2128,6 +2155,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2239,6 +2267,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2351,6 +2380,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; pc._capScreenshareBitrate = true; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2419,6 +2449,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; pc._capScreenshareBitrate = false; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2486,6 +2517,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, false /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2571,6 +2603,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2657,6 +2690,7 @@ describe('TPCUtils', () => { beforeEach(() => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; + pc.videoTransferActive = true; tpcUtils = new TPCUtils(pc); }); @@ -2744,6 +2778,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; pc._capScreenshareBitrate = true; + pc.videoTransferActive = true; const utils = new TPCUtils(pc); it('and requested desktop resolution is 2160', () => { @@ -2861,6 +2896,7 @@ describe('TPCUtils', () => { pc = new MockPeerConnection('1', true, true /* simulcast */); pc.options = { videoQuality }; pc._capScreenshareBitrate = true; + pc.videoTransferActive = true; const utils = new TPCUtils(pc); it('and requested desktop resolution is 2160', () => { @@ -2960,5 +2996,85 @@ describe('TPCUtils', () => { expect(activeState[2]).toBe(false); }); }); + + describe('for VP9 camera tracks and the jvb connection is suspended', () => { + const track = new MockJitsiLocalTrack(720, 'video', 'camera'); + const codec = CodecMimeType.VP9; + + beforeEach(() => { + pc = new MockPeerConnection('1', true, true /* simulcast */); + pc.options = {}; + pc.videoTransferActive = false; + tpcUtils = new TPCUtils(pc); + }); + + afterEach(() => { + pc = null; + tpcUtils = null; + }); + + it('and requested resolution is 720', () => { + height = 720; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(false); + expect(activeState[1]).toBe(false); + expect(activeState[2]).toBe(false); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(1200000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L3T3_KEY); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor[0]).toBe(SIM_LAYERS[2].scaleFactor); + }); + + it('and requested resolution is 360', () => { + height = 360; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(false); + expect(activeState[1]).toBe(false); + expect(activeState[2]).toBe(false); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(300000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L2T3_KEY); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor[0]).toBe(SIM_LAYERS[1].scaleFactor); + }); + + it('and requested resolution is 180', () => { + height = 180; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(false); + expect(activeState[1]).toBe(false); + expect(activeState[2]).toBe(false); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(100000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L1T3); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor[0]).toBe(SIM_LAYERS[0].scaleFactor); + }); + + it('and requested resolution is 0', () => { + height = 0; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(false); + expect(activeState[1]).toBe(false); + expect(activeState[2]).toBe(false); + }); + }); }); }); diff --git a/modules/RTC/TraceablePeerConnection.js b/modules/RTC/TraceablePeerConnection.js index cda4c5a416..017be95835 100644 --- a/modules/RTC/TraceablePeerConnection.js +++ b/modules/RTC/TraceablePeerConnection.js @@ -1755,7 +1755,7 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) { return Promise.resolve(); } - logger.debug(`${this} TPC.replaceTrack old=${oldTrack}, new=${newTrack}`); + logger.info(`${this} TPC.replaceTrack old=${oldTrack}, new=${newTrack}`); return this.tpcUtils.replaceTrack(oldTrack, newTrack) .then(transceiver => { @@ -1803,11 +1803,12 @@ TraceablePeerConnection.prototype.replaceTrack = function(oldTrack, newTrack) { = newTrack || browser.isFirefox() ? MediaDirection.SENDRECV : MediaDirection.RECVONLY; } - // Avoid configuring the encodings on Chromium/Safari until simulcast is configured - // for the newly added track using SDP munging which happens during the renegotiation. - const configureEncodingsPromise = browser.usesSdpMungingForSimulcast() || !newTrack - ? Promise.resolve() - : this.tpcUtils.setEncodings(newTrack); + // Avoid configuring the video encodings on Chromium/Safari until simulcast is configured + // for the newly added video track using SDP munging which happens during the renegotiation. + const configureEncodingsPromise + = !newTrack || (newTrack.getType() === MediaType.VIDEO && browser.usesSdpMungingForSimulcast()) + ? Promise.resolve() + : this.tpcUtils.setEncodings(newTrack); return configureEncodingsPromise.then(() => this.isP2P); }); @@ -2065,19 +2066,31 @@ TraceablePeerConnection.prototype._setMaxBitrates = function(description, isLoca }; /** - * Configures the stream encodings depending on the video type and the bitrates configured. + * Configures the stream encodings for the audio tracks that are added to the peerconnection. * - * @param {JitsiLocalTrack} - The local track for which the sender encodings have to configured. + * @param {JitsiLocalTrack} localAudioTrack - The local audio track. * @returns {Promise} promise that will be resolved when the operation is successful and rejected otherwise. */ -TraceablePeerConnection.prototype.configureSenderVideoEncodings = function(localVideoTrack = null) { - // If media is suspended on the jvb peerconnection, make sure that media stays disabled. The default 'active' state - // for the encodings after the source is added to the peerconnection is 'true', so it needs to be explicitly - // disabled after the source is added. - if (!this.isP2P && !(this.videoTransferActive && this.audioTransferActive)) { - return this.tpcUtils.setMediaTransferActive(false); +TraceablePeerConnection.prototype.configureAudioSenderEncodings = function(localAudioTrack = null) { + if (localAudioTrack) { + return this.tpcUtils.setEncodings(localAudioTrack); } + const promises = []; + + for (const track of this.getLocalTracks(MediaType.AUDIO)) { + promises.push(this.tpcUtils.setEncodings(track)); + } + + return Promise.allSettled(promises); +}; +/** + * Configures the stream encodings depending on the video type and the bitrates configured. + * + * @param {JitsiLocalTrack} - The local track for which the sender encodings have to configured. + * @returns {Promise} promise that will be resolved when the operation is successful and rejected otherwise. + */ +TraceablePeerConnection.prototype.configureVideoSenderEncodings = function(localVideoTrack = null) { if (localVideoTrack) { return this.setSenderVideoConstraints( this._senderMaxHeights.get(localVideoTrack.getSourceName()), @@ -2196,9 +2209,8 @@ TraceablePeerConnection.prototype.setSenderVideoConstraints = function(frameHeig } const sourceName = localVideoTrack.getSourceName(); - // Ignore sender constraints if the media on the peerconnection is suspended (jvb conn when p2p is currently active) - // or if the video track is muted. - if ((!this.isP2P && !this.videoTransferActive) || localVideoTrack.isMuted()) { + // Ignore sender constraints if the video track is muted. + if (localVideoTrack.isMuted()) { this._senderMaxHeights.set(sourceName, frameHeight); return Promise.resolve(); diff --git a/modules/xmpp/JingleSessionPC.js b/modules/xmpp/JingleSessionPC.js index 03f62fd88e..9d9386a0a8 100644 --- a/modules/xmpp/JingleSessionPC.js +++ b/modules/xmpp/JingleSessionPC.js @@ -647,6 +647,7 @@ export default class JingleSessionPC extends JingleSession { const workFunction = finishedCallback => { this._renegotiate() + .then(() => this.peerconnection.configureAudioSenderEncodings()) .then(() => finishedCallback(), error => finishedCallback(error)); }; @@ -1191,7 +1192,7 @@ export default class JingleSessionPC extends JingleSession { // Initiate a renegotiate for the codec setting to take effect. const workFunction = finishedCallback => { this._renegotiate() - .then(() => this.peerconnection.configureSenderVideoEncodings()) + .then(() => this.peerconnection.configureVideoSenderEncodings()) .then( () => { logger.debug(`${this} setVideoCodecs task is done`); @@ -2195,17 +2196,23 @@ export default class JingleSessionPC extends JingleSession { * @returns {Promise} */ setMediaTransferActive(active) { + const changed = this.peerconnection.audioTransferActive !== active + || this.peerconnection.videoTransferActive !== active; + + if (!changed) { + return Promise.resolve(); + } + return this.peerconnection.tpcUtils.setMediaTransferActive(active) .then(() => { this.peerconnection.audioTransferActive = active; this.peerconnection.videoTransferActive = active; - // Reconfigure the video tracks so that only the correct encodings are active. + // Reconfigure the audio and video tracks so that only the correct encodings are active. const promises = []; - for (const track of this.rtc.getLocalVideoTracks()) { - promises.push(this.peerconnection.configureSenderVideoEncodings(track)); - } + promises.push(this.peerconnection.configureVideoSenderEncodings()); + promises.push(this.peerconnection.configureAudioSenderEncodings()); return Promise.allSettled(promises); }); @@ -2358,7 +2365,7 @@ export default class JingleSessionPC extends JingleSession { // Configure the video encodings after the track is unmuted. If the user joins the call muted and // unmutes it the first time, all the parameters need to be configured. if (track.isVideoTrack()) { - return this.peerconnection.configureSenderVideoEncodings(track); + return this.peerconnection.configureVideoSenderEncodings(track); } }); }