Skip to content

Commit dfc23df

Browse files
jallamsetty1jallamsetty1
authored
fix(codec) Debounce the call that calc codec intersection set. (#2622)
* fix(codec) Debounce the call that calc codec intersection set. Calculate codec intersection set only once per second even when there is a burst of joins/leaves. Also, check for current codec before chaining a codec change operation when codec selection API is used. * squash: Address review comments
1 parent ca734af commit dfc23df

File tree

6 files changed

+145
-40
lines changed

6 files changed

+145
-40
lines changed

‎modules/RTC/TPCUtils.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -512,10 +512,10 @@ export class TPCUtils {
512512
const rtpSender = this.pc.findSenderForTrack(localVideoTrack.getTrack());
513513

514514
if (this.pc.usesCodecSelectionAPI() && rtpSender) {
515-
const { codecs } = rtpSender.getParameters();
515+
const { encodings } = rtpSender.getParameters();
516516

517-
if (codecs?.length) {
518-
return codecs[0].mimeType.split('/')[1].toLowerCase();
517+
if (encodings[0].codec) {
518+
return encodings[0].codec.mimeType.split('/')[1].toLowerCase();
519519
}
520520
}
521521

‎modules/RTC/TraceablePeerConnection.js‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,8 +1436,18 @@ TraceablePeerConnection.prototype.setVideoCodecs = function(codecList, screensha
14361436
this.codecSettings.screenshareCodec = screenshareCodec;
14371437
}
14381438

1439-
if (this.usesCodecSelectionAPI()) {
1440-
this.configureVideoSenderEncodings();
1439+
if (!this.usesCodecSelectionAPI()) {
1440+
return;
1441+
}
1442+
1443+
for (const track of this.getLocalVideoTracks()) {
1444+
const currentCodec = this.tpcUtils.getConfiguredVideoCodec(track);
1445+
1446+
if (screenshareCodec && track.getVideoType() === VideoType.DESKTOP && screenshareCodec !== currentCodec) {
1447+
this.configureVideoSenderEncodings(track, screenshareCodec);
1448+
} else if (currentCodec !== codecList[0]) {
1449+
this.configureVideoSenderEncodings(track);
1450+
}
14411451
}
14421452
};
14431453

@@ -1904,16 +1914,16 @@ TraceablePeerConnection.prototype._enableSenderEncodings = async function(sender
19041914
* that is currently selected.
19051915
*
19061916
* @param {JitsiLocalTrack} - The local track for which the sender encodings have to configured.
1917+
* @param {CodecMimeType} - The preferred codec for the video track.
19071918
* @returns {Promise} promise that will be resolved when the operation is successful and rejected otherwise.
19081919
*/
1909-
TraceablePeerConnection.prototype.configureVideoSenderEncodings = function(localVideoTrack = null) {
1910-
const preferredCodec = this.codecSettings.codecList[0];
1920+
TraceablePeerConnection.prototype.configureVideoSenderEncodings = function(localVideoTrack, codec) {
1921+
const preferredCodec = codec ?? this.codecSettings.codecList[0];
19111922

19121923
if (localVideoTrack) {
1913-
return this.setSenderVideoConstraints(
1914-
this._senderMaxHeights.get(localVideoTrack.getSourceName()),
1915-
localVideoTrack,
1916-
preferredCodec);
1924+
const height = this._senderMaxHeights.get(localVideoTrack.getSourceName()) ?? VIDEO_QUALITY_LEVELS[0].height;
1925+
1926+
return this.setSenderVideoConstraints(height, localVideoTrack, preferredCodec);
19171927
}
19181928
const promises = [];
19191929

‎modules/qualitycontrol/CodecSelection.spec.js‎

Lines changed: 93 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,43 +50,57 @@ describe('Codec Selection', () => {
5050
};
5151

5252
qualityController = new QualityController(conference, options);
53+
jasmine.clock().install();
5354
spyOn(jingleSession, 'setVideoCodecs');
5455
});
5556

56-
it('and remote endpoints use the new codec selection logic', () => {
57+
afterEach(() => {
58+
jasmine.clock().uninstall();
59+
});
60+
61+
it('and remote endpoints use the new codec selection logic', async () => {
5762
// Add a second user joining the call.
5863
participant1 = new MockParticipant('remote-1');
5964
conference.addParticipant(participant1, [ 'vp9', 'vp8' ]);
6065

66+
await nextTick(1000);
67+
6168
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
69+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');
6270

6371
// Add a third user joining the call with a subset of codecs.
6472
participant2 = new MockParticipant('remote-2');
6573
conference.addParticipant(participant2, [ 'vp8' ]);
6674

67-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');
68-
69-
// Make p2 leave the call
75+
// Make p2 leave the call.
7076
conference.removeParticipant(participant2);
71-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);
77+
78+
await nextTick(1000);
79+
80+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
81+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');
7282
});
7383

74-
it('and remote endpoints use the old codec selection logic (RN)', () => {
84+
it('and remote endpoints use the old codec selection logic (RN)', async () => {
7585
// Add a second user joining the call.
7686
participant1 = new MockParticipant('remote-1');
7787
conference.addParticipant(participant1, null, 'vp8');
7888

89+
await nextTick(1000);
90+
91+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
7992
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');
8093

8194
// Add a third user (newer) to the call.
8295
participant2 = new MockParticipant('remote-2');
8396
conference.addParticipant(participant2, [ 'vp9', 'vp8' ]);
8497

85-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');
86-
8798
// Make p1 leave the call
8899
conference.removeParticipant(participant1);
89-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);
100+
101+
await nextTick(1000);
102+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
103+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');
90104
});
91105
});
92106

@@ -102,50 +116,61 @@ describe('Codec Selection', () => {
102116

103117
qualityController = new QualityController(conference, options);
104118
spyOn(jingleSession, 'setVideoCodecs');
119+
jasmine.clock().install();
105120
});
106121

107-
it('and remote endpoints use the new codec selection logic', () => {
122+
afterEach(() => {
123+
jasmine.clock().uninstall();
124+
});
125+
126+
it('and remote endpoints use the new codec selection logic', async () => {
108127
// Add a second user joining the call.
109128
participant1 = new MockParticipant('remote-1');
110129
conference.addParticipant(participant1, [ 'vp9', 'vp8', 'h264' ]);
111130

131+
await nextTick(1000);
112132
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
133+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], undefined);
113134

114135
// Add a third user joining the call with a subset of codecs.
115136
participant2 = new MockParticipant('remote-2');
116137
conference.addParticipant(participant2, [ 'vp8' ]);
117138

118-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);
119-
120139
// Make p2 leave the call
121140
conference.removeParticipant(participant2);
122-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);
141+
142+
await nextTick(1000);
143+
144+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
123145
});
124146

125-
it('and remote endpoint prefers a codec that is locally disabled', () => {
147+
it('and remote endpoint prefers a codec that is locally disabled', async () => {
126148
// Add a second user joining the call the prefers H.264 and VP8.
127149
participant1 = new MockParticipant('remote-1');
128150
conference.addParticipant(participant1, [ 'h264', 'vp8' ]);
129151

152+
await nextTick(1200);
130153
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);
131154
});
132155

133-
it('and remote endpoints use the old codec selection logic (RN)', () => {
156+
it('and remote endpoints use the old codec selection logic (RN)', async () => {
134157
// Add a second user joining the call.
135158
participant1 = new MockParticipant('remote-1');
136159
conference.addParticipant(participant1, null, 'vp8');
137160

161+
await nextTick(1000);
162+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
138163
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);
139164

140165
// Add a third user (newer) to the call.
141166
participant2 = new MockParticipant('remote-2');
142167
conference.addParticipant(participant2, [ 'vp9', 'vp8', 'h264' ]);
143168

144-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);
145-
146169
// Make p1 leave the call
147170
conference.removeParticipant(participant1);
148-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);
171+
172+
jasmine.clock().tick(1000);
173+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
149174
});
150175
});
151176

@@ -171,11 +196,14 @@ describe('Codec Selection', () => {
171196

172197
participant1 = new MockParticipant('remote-1');
173198
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);
199+
200+
await nextTick(1000);
201+
174202
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);
175203

176204
participant2 = new MockParticipant('remote-2');
177205
conference.addParticipant(participant2, [ 'av1', 'vp9', 'vp8' ]);
178-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);
206+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
179207

180208
qualityController.codecController.changeCodecPreferenceOrder(localTrack, 'av1');
181209

@@ -187,6 +215,7 @@ describe('Codec Selection', () => {
187215

188216
// Expect the local endpoint to continue sending VP9.
189217
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'av1', 'vp8' ], undefined);
218+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);
190219
});
191220

192221
it('and does not change codec if the current codec is already the lowest complexity codec', async () => {
@@ -196,6 +225,8 @@ describe('Codec Selection', () => {
196225

197226
participant1 = new MockParticipant('remote-1');
198227
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);
228+
229+
await nextTick(1000);
199230
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8', 'vp9', 'av1' ], undefined);
200231

201232
participant2 = new MockParticipant('remote-2');
@@ -239,11 +270,14 @@ describe('Codec Selection', () => {
239270

240271
participant1 = new MockParticipant('remote-1');
241272
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);
273+
274+
await nextTick(1000);
275+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
242276
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);
243277

244278
participant2 = new MockParticipant('remote-2');
245279
conference.addParticipant(participant2, [ 'av1', 'vp9', 'vp8' ]);
246-
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);
280+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
247281

248282
const sourceStats = {
249283
avgEncodeTime: 12,
@@ -269,6 +303,8 @@ describe('Codec Selection', () => {
269303
participant3 = new MockParticipant('remote-3');
270304
conference.addParticipant(participant3, [ 'av1', 'vp9', 'vp8' ]);
271305

306+
await nextTick(1000);
307+
272308
// Expect the local endpoint to continue sending VP9.
273309
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'av1', 'vp8' ], undefined);
274310

@@ -328,4 +364,41 @@ describe('Codec Selection', () => {
328364
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(0);
329365
});
330366
});
367+
368+
describe('When multiple joins and leaves happen in a quick burst', () => {
369+
beforeEach(() => {
370+
options = {
371+
jvb: {
372+
preferenceOrder: [ 'AV1', 'VP9', 'VP8' ],
373+
screenshareCodec: 'VP9'
374+
},
375+
p2p: {}
376+
};
377+
jasmine.clock().install();
378+
qualityController = new QualityController(conference, options);
379+
spyOn(jingleSession, 'setVideoCodecs');
380+
});
381+
382+
afterEach(() => {
383+
jasmine.clock().uninstall();
384+
});
385+
386+
it('should call setVideoCodecs only once within the same tick', async () => {
387+
participant1 = new MockParticipant('remote-1');
388+
conference.addParticipant(participant1, [ 'vp9', 'vp8' ]);
389+
390+
// Add a third user joining the call with a subset of codecs.
391+
participant2 = new MockParticipant('remote-2');
392+
conference.addParticipant(participant2, [ 'vp8' ]);
393+
394+
// Make p1 and p2 leave the call.
395+
conference.removeParticipant(participant2);
396+
conference.removeParticipant(participant1);
397+
398+
await nextTick(1000);
399+
400+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
401+
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], 'vp9');
402+
});
403+
});
331404
});

‎modules/qualitycontrol/QualityController.spec.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('QualityController', () => {
3333
p2p: {}
3434
};
3535
localTrack = new MockLocalTrack('1', 720, 'camera');
36-
qualityController = new QualityController(conference, options, true);
36+
qualityController = new QualityController(conference, options);
3737
sourceStats = {
3838
avgEncodeTime: 12,
3939
codec: 'VP8',
@@ -165,7 +165,7 @@ describe('QualityController', () => {
165165
p2p: {}
166166
};
167167
localTrack = new MockLocalTrack('1', 720, 'camera');
168-
qualityController = new QualityController(conference, options, true);
168+
qualityController = new QualityController(conference, options);
169169
sourceStats = {
170170
avgEncodeTime: 12,
171171
codec: 'VP8',

‎modules/qualitycontrol/QualityController.ts‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,13 @@ export class QualityController {
145145
this._conference.on(
146146
JitsiConferenceEvents.CONFERENCE_VISITOR_CODECS_CHANGED,
147147
(codecList: CodecMimeType[]) => this._codecController.updateVisitorCodecs(codecList));
148-
this._conference.on(
149-
JitsiConferenceEvents.USER_JOINED,
150-
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession));
151-
this._conference.on(
152-
JitsiConferenceEvents.USER_LEFT,
153-
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession));
148+
149+
// Debounce the calls to codec selection when there is a burst of joins and leaves.
150+
const debouncedSelectCodec = this._debounce(
151+
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession),
152+
1000);
153+
this._conference.on(JitsiConferenceEvents.USER_JOINED, debouncedSelectCodec.bind(this));
154+
this._conference.on(JitsiConferenceEvents.USER_LEFT, debouncedSelectCodec.bind(this));
154155
this._conference.rtc.on(
155156
RTCEvents.SENDER_VIDEO_CONSTRAINTS_CHANGED,
156157
(videoConstraints: IVideoConstraints) => this._sendVideoController.onSenderConstraintsReceived(videoConstraints));
@@ -159,6 +160,26 @@ export class QualityController {
159160
(tpc: TraceablePeerConnection, stats: Map<number, IOutboundRtpStats>) => this._processOutboundRtpStats(tpc, stats));
160161
}
161162

163+
/**
164+
* Creates a debounced function that delays the execution of the provided function until after the specified delay
165+
* has elapsed. Unlike typical debounce implementations, the timer does not reset when the function is called again
166+
* within the delay period.
167+
*
168+
* @param {Function} func - The function to be debounced.
169+
* @param {number} delay - The delay in milliseconds.
170+
* @returns {Function} - The debounced function.
171+
*/
172+
_debounce(func: Function, delay: number) {
173+
return function (...args) {
174+
if (!this._timer) {
175+
this._timer = setTimeout(() => {
176+
this._timer = null;
177+
func.apply(this, args);
178+
}, delay);
179+
}
180+
};
181+
}
182+
162183
/**
163184
* Adjusts the lastN value so that fewer remote video sources are received from the bridge in an attempt to improve
164185
* encode resolution of the outbound video streams based on cpuLimited parameter passed. If cpuLimited is false,

‎modules/xmpp/JingleSessionPC.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2286,7 +2286,6 @@ export default class JingleSessionPC extends JingleSession {
22862286
*/
22872287
setVideoCodecs(codecList, screenshareCodec) {
22882288
if (this._assertNotEnded()) {
2289-
logger.info(`${this} setVideoCodecs: codecList=${codecList}, screenshareCodec=${screenshareCodec}`);
22902289
this.peerconnection.setVideoCodecs(codecList, screenshareCodec);
22912290

22922291
// Browser throws an error when H.264 is set on the encodings. Therefore, munge the SDP when H.264 needs to
@@ -2310,6 +2309,8 @@ export default class JingleSessionPC extends JingleSession {
23102309
videoType: VideoType.CAMERA
23112310
});
23122311

2312+
logger.info(`${this} setVideoCodecs: codecList=${codecList}, screenshareCodec=${screenshareCodec}`);
2313+
23132314
// Initiate a renegotiate for the codec setting to take effect.
23142315
const workFunction = finishedCallback => {
23152316
this._renegotiate()

0 commit comments

Comments
 (0)