Skip to content

Commit

Permalink
fix(codec) Debounce the call that calc codec intersection set. (#2622)
Browse files Browse the repository at this point in the history
* 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
jallamsetty1 authored Jan 27, 2025

Verified

This commit was signed with the committer’s verified signature.
pendo324 Justin
1 parent ca734af commit dfc23df
Showing 6 changed files with 145 additions and 40 deletions.
6 changes: 3 additions & 3 deletions modules/RTC/TPCUtils.js
Original file line number Diff line number Diff line change
@@ -512,10 +512,10 @@ export class TPCUtils {
const rtpSender = this.pc.findSenderForTrack(localVideoTrack.getTrack());

if (this.pc.usesCodecSelectionAPI() && rtpSender) {
const { codecs } = rtpSender.getParameters();
const { encodings } = rtpSender.getParameters();

if (codecs?.length) {
return codecs[0].mimeType.split('/')[1].toLowerCase();
if (encodings[0].codec) {
return encodings[0].codec.mimeType.split('/')[1].toLowerCase();
}
}

26 changes: 18 additions & 8 deletions modules/RTC/TraceablePeerConnection.js
Original file line number Diff line number Diff line change
@@ -1436,8 +1436,18 @@ TraceablePeerConnection.prototype.setVideoCodecs = function(codecList, screensha
this.codecSettings.screenshareCodec = screenshareCodec;
}

if (this.usesCodecSelectionAPI()) {
this.configureVideoSenderEncodings();
if (!this.usesCodecSelectionAPI()) {
return;
}

for (const track of this.getLocalVideoTracks()) {
const currentCodec = this.tpcUtils.getConfiguredVideoCodec(track);

if (screenshareCodec && track.getVideoType() === VideoType.DESKTOP && screenshareCodec !== currentCodec) {
this.configureVideoSenderEncodings(track, screenshareCodec);
} else if (currentCodec !== codecList[0]) {
this.configureVideoSenderEncodings(track);
}
}
};

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

if (localVideoTrack) {
return this.setSenderVideoConstraints(
this._senderMaxHeights.get(localVideoTrack.getSourceName()),
localVideoTrack,
preferredCodec);
const height = this._senderMaxHeights.get(localVideoTrack.getSourceName()) ?? VIDEO_QUALITY_LEVELS[0].height;

return this.setSenderVideoConstraints(height, localVideoTrack, preferredCodec);
}
const promises = [];

113 changes: 93 additions & 20 deletions modules/qualitycontrol/CodecSelection.spec.js
Original file line number Diff line number Diff line change
@@ -50,43 +50,57 @@ describe('Codec Selection', () => {
};

qualityController = new QualityController(conference, options);
jasmine.clock().install();
spyOn(jingleSession, 'setVideoCodecs');
});

it('and remote endpoints use the new codec selection logic', () => {
afterEach(() => {
jasmine.clock().uninstall();
});

it('and remote endpoints use the new codec selection logic', async () => {
// Add a second user joining the call.
participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'vp9', 'vp8' ]);

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');

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

expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');

// Make p2 leave the call
// Make p2 leave the call.
conference.removeParticipant(participant2);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');
});

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

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');

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

expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], 'vp9');

// Make p1 leave the call
conference.removeParticipant(participant1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);

await nextTick(1000);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'vp8' ], 'vp9');
});
});

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

qualityController = new QualityController(conference, options);
spyOn(jingleSession, 'setVideoCodecs');
jasmine.clock().install();
});

it('and remote endpoints use the new codec selection logic', () => {
afterEach(() => {
jasmine.clock().uninstall();
});

it('and remote endpoints use the new codec selection logic', async () => {
// Add a second user joining the call.
participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'vp9', 'vp8', 'h264' ]);

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

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

expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);

// Make p2 leave the call
conference.removeParticipant(participant2);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
});

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

await nextTick(1200);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);
});

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

await nextTick(1000);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);

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

expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8' ], undefined);

// Make p1 leave the call
conference.removeParticipant(participant1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(3);

jasmine.clock().tick(1000);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(2);
});
});

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

participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);

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

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

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

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

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

participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);

await nextTick(1000);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp8', 'vp9', 'av1' ], undefined);

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

participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'av1', 'vp9', 'vp8' ]);

await nextTick(1000);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], undefined);

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

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

await nextTick(1000);

// Expect the local endpoint to continue sending VP9.
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'vp9', 'av1', 'vp8' ], undefined);

@@ -328,4 +364,41 @@ describe('Codec Selection', () => {
expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(0);
});
});

describe('When multiple joins and leaves happen in a quick burst', () => {
beforeEach(() => {
options = {
jvb: {
preferenceOrder: [ 'AV1', 'VP9', 'VP8' ],
screenshareCodec: 'VP9'
},
p2p: {}
};
jasmine.clock().install();
qualityController = new QualityController(conference, options);
spyOn(jingleSession, 'setVideoCodecs');
});

afterEach(() => {
jasmine.clock().uninstall();
});

it('should call setVideoCodecs only once within the same tick', async () => {
participant1 = new MockParticipant('remote-1');
conference.addParticipant(participant1, [ 'vp9', 'vp8' ]);

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

// Make p1 and p2 leave the call.
conference.removeParticipant(participant2);
conference.removeParticipant(participant1);

await nextTick(1000);

expect(jingleSession.setVideoCodecs).toHaveBeenCalledTimes(1);
expect(jingleSession.setVideoCodecs).toHaveBeenCalledWith([ 'av1', 'vp9', 'vp8' ], 'vp9');
});
});
});
4 changes: 2 additions & 2 deletions modules/qualitycontrol/QualityController.spec.js
Original file line number Diff line number Diff line change
@@ -33,7 +33,7 @@ describe('QualityController', () => {
p2p: {}
};
localTrack = new MockLocalTrack('1', 720, 'camera');
qualityController = new QualityController(conference, options, true);
qualityController = new QualityController(conference, options);
sourceStats = {
avgEncodeTime: 12,
codec: 'VP8',
@@ -165,7 +165,7 @@ describe('QualityController', () => {
p2p: {}
};
localTrack = new MockLocalTrack('1', 720, 'camera');
qualityController = new QualityController(conference, options, true);
qualityController = new QualityController(conference, options);
sourceStats = {
avgEncodeTime: 12,
codec: 'VP8',
33 changes: 27 additions & 6 deletions modules/qualitycontrol/QualityController.ts
Original file line number Diff line number Diff line change
@@ -145,12 +145,13 @@ export class QualityController {
this._conference.on(
JitsiConferenceEvents.CONFERENCE_VISITOR_CODECS_CHANGED,
(codecList: CodecMimeType[]) => this._codecController.updateVisitorCodecs(codecList));
this._conference.on(
JitsiConferenceEvents.USER_JOINED,
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession));
this._conference.on(
JitsiConferenceEvents.USER_LEFT,
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession));

// Debounce the calls to codec selection when there is a burst of joins and leaves.
const debouncedSelectCodec = this._debounce(
() => this._codecController.selectPreferredCodec(this._conference.jvbJingleSession),
1000);
this._conference.on(JitsiConferenceEvents.USER_JOINED, debouncedSelectCodec.bind(this));
this._conference.on(JitsiConferenceEvents.USER_LEFT, debouncedSelectCodec.bind(this));
this._conference.rtc.on(
RTCEvents.SENDER_VIDEO_CONSTRAINTS_CHANGED,
(videoConstraints: IVideoConstraints) => this._sendVideoController.onSenderConstraintsReceived(videoConstraints));
@@ -159,6 +160,26 @@ export class QualityController {
(tpc: TraceablePeerConnection, stats: Map<number, IOutboundRtpStats>) => this._processOutboundRtpStats(tpc, stats));
}

/**
* Creates a debounced function that delays the execution of the provided function until after the specified delay
* has elapsed. Unlike typical debounce implementations, the timer does not reset when the function is called again
* within the delay period.
*
* @param {Function} func - The function to be debounced.
* @param {number} delay - The delay in milliseconds.
* @returns {Function} - The debounced function.
*/
_debounce(func: Function, delay: number) {
return function (...args) {
if (!this._timer) {
this._timer = setTimeout(() => {
this._timer = null;
func.apply(this, args);
}, delay);
}
};
}

/**
* Adjusts the lastN value so that fewer remote video sources are received from the bridge in an attempt to improve
* encode resolution of the outbound video streams based on cpuLimited parameter passed. If cpuLimited is false,
3 changes: 2 additions & 1 deletion modules/xmpp/JingleSessionPC.js
Original file line number Diff line number Diff line change
@@ -2286,7 +2286,6 @@ export default class JingleSessionPC extends JingleSession {
*/
setVideoCodecs(codecList, screenshareCodec) {
if (this._assertNotEnded()) {
logger.info(`${this} setVideoCodecs: codecList=${codecList}, screenshareCodec=${screenshareCodec}`);
this.peerconnection.setVideoCodecs(codecList, screenshareCodec);

// 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 {
videoType: VideoType.CAMERA
});

logger.info(`${this} setVideoCodecs: codecList=${codecList}, screenshareCodec=${screenshareCodec}`);

// Initiate a renegotiate for the codec setting to take effect.
const workFunction = finishedCallback => {
this._renegotiate()

0 comments on commit dfc23df

Please sign in to comment.