From ad6e1af0c2b495a0975b88624dab713d7f8dc06b Mon Sep 17 00:00:00 2001 From: Jaya Allamsetty Date: Sat, 7 Oct 2023 14:59:15 -0400 Subject: [PATCH] Make codec name in settings case insensitive. Also read the deprecated max bitrates correctly, add unit tests to test it. --- modules/RTC/TPCUtils.js | 33 +++--- modules/RTC/TPCUtils.spec.js | 193 +++++++++++++++++++++++++------- modules/xmpp/JingleSessionPC.js | 23 +++- 3 files changed, 187 insertions(+), 62 deletions(-) diff --git a/modules/RTC/TPCUtils.js b/modules/RTC/TPCUtils.js index d8471b8653..f41df56e02 100644 --- a/modules/RTC/TPCUtils.js +++ b/modules/RTC/TPCUtils.js @@ -43,13 +43,16 @@ export class TPCUtils { if (videoQualitySettings) { for (const codec of VIDEO_CODECS) { - const bitrateSettings = videoQualitySettings[codec]?.maxBitratesVideo - ?? videoQualitySettings[codec.toUpperCase()]?.maxBitratesVideo - ?? (videoQualitySettings.maxBitratesVideo - && (videoQualitySettings.maxBitratesVideo[codec] - || videoQualitySettings.maxBitratesVideo[codec.toUpperCase()])); - const scalalabilityModeSettings = videoQualitySettings[codec] - ?? videoQualitySettings[codec.toUpperCase()]; + const codecConfig = videoQualitySettings[codec]; + + if (!codecConfig) { + continue; // eslint-disable-line no-continue + } + const bitrateSettings = codecConfig?.maxBitratesVideo + + // Read the deprecated settings for max bitrates. + ?? (videoQualitySettings.maxbitratesvideo + && videoQualitySettings.maxbitratesvideo[codec.toUpperCase()]); if (bitrateSettings) { [ 'low', 'standard', 'high', 'ssHigh' ].forEach(value => { @@ -61,14 +64,14 @@ export class TPCUtils { // TODO - Check if AV1 DD extension headers are negotiated and add that check here for AV1 and H.264. const scalabilityModeEnabled = this.codecSettings[codec].scalabilityModeEnabled - && (typeof scalalabilityModeSettings?.scalabilityModeEnabled === 'undefined' - || scalalabilityModeSettings.scalabilityModeEnabled); + && (typeof codecConfig.scalabilityModeEnabled === 'undefined' + || codecConfig.scalabilityModeEnabled); if (scalabilityModeEnabled) { - scalalabilityModeSettings?.useSimulcast - && (this.codecSettings[codec].useSimulcast = scalalabilityModeSettings.useSimulcast); - scalalabilityModeSettings?.useL3T3Mode - && (this.codecSettings[codec].useL3T3Mode = scalalabilityModeSettings.useL3T3Mode); + codecConfig.useSimulcast + && (this.codecSettings[codec].useSimulcast = codecConfig.useSimulcast); + codecConfig.useL3T3Mode + && (this.codecSettings[codec].useL3T3Mode = codecConfig.useL3T3Mode); } else { this.codecSettings[codec].scalabilityModeEnabled = false; } @@ -88,7 +91,7 @@ export class TPCUtils { _calculateActiveEncodingParams(localVideoTrack, codec, newHeight) { const codecBitrates = this.codecSettings[codec].maxBitratesVideo; const height = localVideoTrack.getHeight(); - const desktopShareBitrate = this.pc.options?.videoQuality?.desktopBitrate || DESKTOP_SHARE_RATE; + const desktopShareBitrate = this.pc.options?.videoQuality?.desktopbitrate || DESKTOP_SHARE_RATE; const isScreenshare = localVideoTrack.getVideoType() === VideoType.DESKTOP; let scalabilityMode = this.codecSettings[codec].useL3T3Mode ? VideoEncoderScalabilityMode.L3T3 : VideoEncoderScalabilityMode.L3T3_KEY; @@ -505,7 +508,7 @@ export class TPCUtils { */ calculateEncodingsBitrates(localVideoTrack, codec, newHeight) { const videoType = localVideoTrack.getVideoType(); - const desktopShareBitrate = this.pc.options?.videoQuality?.desktopBitrate || DESKTOP_SHARE_RATE; + const desktopShareBitrate = this.pc.options?.videoQuality?.desktopbitrate || DESKTOP_SHARE_RATE; const encodingsBitrates = this._getVideoStreamEncodings(localVideoTrack.getVideoType(), codec) .map((encoding, idx) => { let bitrate = encoding.maxBitrate; diff --git a/modules/RTC/TPCUtils.spec.js b/modules/RTC/TPCUtils.spec.js index 45119025f4..4059416744 100644 --- a/modules/RTC/TPCUtils.spec.js +++ b/modules/RTC/TPCUtils.spec.js @@ -1377,17 +1377,16 @@ describe('TPCUtils', () => { const codec = CodecMimeType.AV1; // Configure AV1 to run in simulcast mode. - const av1Settings = { - maxBitratesVideo: { - low: 300000, - standard: 600000, - high: 2000000, - ssHigh: 2500000 - }, - useSimulcast: true - }; const videoQuality = { - av1: av1Settings + av1: { + maxBitratesVideo: { + low: 300000, + standard: 600000, + high: 2000000, + ssHigh: 2500000 + }, + useSimulcast: true + } }; beforeEach(() => { @@ -1495,17 +1494,16 @@ describe('TPCUtils', () => { const codec = CodecMimeType.VP9; // Configure VP9 to run in simulcast mode. - const vp9Settings = { - maxBitratesVideo: { - low: 300000, - standard: 600000, - high: 2000000, - ssHigh: 2500000 - }, - useSimulcast: true - }; const videoQuality = { - vp9: vp9Settings + vp9: { + maxBitratesVideo: { + low: 300000, + standard: 600000, + high: 2000000, + ssHigh: 2500000 + }, + useSimulcast: true + } }; beforeEach(() => { @@ -1613,11 +1611,10 @@ describe('TPCUtils', () => { const codec = CodecMimeType.VP9; // Configure VP9 to run in K-SVC mode. - const vp9Settings = { - scalabilityModeEnabled: false - }; const videoQuality = { - vp9: vp9Settings + vp9: { + scalabilityModeEnabled: false + } }; beforeEach(() => { @@ -1717,11 +1714,10 @@ describe('TPCUtils', () => { const codec = CodecMimeType.VP9; // Configure VP9 to run in K-SVC mode. - const vp9Settings = { - scalabilityModeEnabled: false - }; const videoQuality = { - vp9: vp9Settings + vp9: { + scalabilityModeEnabled: false + } }; beforeEach(() => { @@ -1821,11 +1817,10 @@ describe('TPCUtils', () => { const codec = CodecMimeType.VP9; // Configure VP9 to run in K-SVC mode. - const vp9Settings = { - scalabilityModeEnabled: false - }; const videoQuality = { - vp9: vp9Settings + vp9: { + scalabilityModeEnabled: false + } }; beforeEach(() => { @@ -1886,11 +1881,10 @@ describe('TPCUtils', () => { const codec = CodecMimeType.VP9; // Configure VP9 to run in K-SVC mode. - const vp9Settings = { - scalabilityModeEnabled: false - }; const videoQuality = { - vp9: vp9Settings + vp9: { + scalabilityModeEnabled: false + } }; beforeEach(() => { @@ -1946,16 +1940,15 @@ describe('TPCUtils', () => { }); }); - describe('for H.264 camera tracks', () => { + describe('for H.264 camera tracks, scalability mode is disabled', () => { const track = new MockJitsiLocalTrack(720, 'video', 'camera'); const codec = CodecMimeType.H264; // Configure VP9 to run in simulcast mode. - const h264Settings = { - scalabilityModeEnabled: false - }; const videoQuality = { - h264: h264Settings + h264: { + scalabilityModeEnabled: false + } }; beforeEach(() => { @@ -2027,5 +2020,123 @@ describe('TPCUtils', () => { expect(scalabilityModes).toBe(undefined); }); }); + + describe('for VP9 camera tracks when deprecated settings are used for overriding bitrates', () => { + const track = new MockJitsiLocalTrack(720, 'video', 'camera'); + const codec = CodecMimeType.VP9; + + // Configure VP9 bitrates using the deprecated settings. + const videoQuality = { + vp9: { + useSimulcast: true + }, + maxbitratesvideo: { + VP9: { + low: 300000, + standard: 600000, + high: 2000000 + } + } + }; + + beforeEach(() => { + pc = new MockPeerConnection('1', true, true /* simulcast */); + pc.options = { videoQuality }; + 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(true); + expect(activeState[1]).toBe(true); + expect(activeState[2]).toBe(true); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(300000); + expect(bitrates[1]).toBe(600000); + expect(bitrates[2]).toBe(2000000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[1]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[2]).toBe(VideoEncoderScalabilityMode.L1T3); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor).toBe(undefined); + }); + + it('and requested resolution is 360', () => { + height = 360; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(true); + expect(activeState[1]).toBe(true); + expect(activeState[2]).toBe(false); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(300000); + expect(bitrates[1]).toBe(600000); + expect(bitrates[2]).toBe(2000000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[1]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[2]).toBe(VideoEncoderScalabilityMode.L1T3); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor).toBe(undefined); + }); + + it('and requested resolution is 180', () => { + height = 180; + + activeState = tpcUtils.calculateEncodingsActiveState(track, codec, height); + expect(activeState[0]).toBe(true); + expect(activeState[1]).toBe(false); + expect(activeState[2]).toBe(false); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(300000); + expect(bitrates[1]).toBe(600000); + expect(bitrates[2]).toBe(2000000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[1]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[2]).toBe(VideoEncoderScalabilityMode.L1T3); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor).toBe(undefined); + }); + + 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); + + bitrates = tpcUtils.calculateEncodingsBitrates(track, codec, height); + expect(bitrates[0]).toBe(300000); + expect(bitrates[1]).toBe(600000); + expect(bitrates[2]).toBe(2000000); + + scalabilityModes = tpcUtils.calculateEncodingsScalabilityMode(track, codec, height); + expect(scalabilityModes[0]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[1]).toBe(VideoEncoderScalabilityMode.L1T3); + expect(scalabilityModes[2]).toBe(VideoEncoderScalabilityMode.L1T3); + + scaleFactor = tpcUtils.calculateEncodingsScaleFactor(track, codec, height); + expect(scaleFactor).toBe(undefined); + }); + }); }); }); diff --git a/modules/xmpp/JingleSessionPC.js b/modules/xmpp/JingleSessionPC.js index 7287572e37..895950620c 100644 --- a/modules/xmpp/JingleSessionPC.js +++ b/modules/xmpp/JingleSessionPC.js @@ -402,20 +402,31 @@ export default class JingleSessionPC extends JingleSession { pcOptions.capScreenshareBitrate = false; pcOptions.codecSettings = options.codecSettings; pcOptions.enableInsertableStreams = options.enableInsertableStreams; - pcOptions.videoQuality = options.videoQuality; + let h264SimulcastEnabled = browser.supportsScalabilityModeAPI(); + + if (options.videoQuality) { + const settings = Object.entries(options.videoQuality) + .map(entry => { + entry[0] = entry[0].toLowerCase(); + + return entry; + }); + + pcOptions.videoQuality = Object.fromEntries(settings); + const h264Settings = pcOptions.videoQuality[CodecMimeType.H264]; + + h264SimulcastEnabled = h264SimulcastEnabled + && (typeof h264Settings === 'undefined' || h264Settings.scalabilityModeEnabled); + } pcOptions.forceTurnRelay = options.forceTurnRelay; pcOptions.audioQuality = options.audioQuality; pcOptions.usesUnifiedPlan = this.usesUnifiedPlan = browser.supportsUnifiedPlan(); const preferredJvbCodec = options.codecSettings?.codecList[0]; - const explicityDisabled = options.videoQuality - && options.videoQuality[CodecMimeType.H264] - && options.videoQuality[CodecMimeType.H264].scalabilityModeEnabled !== 'undefined' - && !options.videoQuality[CodecMimeType.H264].scalabilityModeEnabled; pcOptions.disableSimulcast = this.isP2P ? true : options.disableSimulcast - ?? (preferredJvbCodec?.toLowerCase() === CodecMimeType.H264 && explicityDisabled); + ?? (preferredJvbCodec?.toLowerCase() === CodecMimeType.H264 && !h264SimulcastEnabled); if (!this.isP2P) { // Do not send lower spatial layers for low fps screenshare and enable them only for high fps screenshare.