diff --git a/JitsiConference.js b/JitsiConference.js index 17faaa9ac6..131f0fd4c1 100644 --- a/JitsiConference.js +++ b/JitsiConference.js @@ -1075,6 +1075,8 @@ JitsiConference.prototype.getTranscriptionStatus = function() { * another video track in the conference. */ JitsiConference.prototype.addTrack = function(track) { + logger.info(`addTrack: ${track}`); + if (!track) { throw new Error('addTrack - a track is required'); } @@ -1099,24 +1101,23 @@ JitsiConference.prototype.addTrack = function(track) { this.getLocalTracks(mediaType)?.length); track.setSourceName(sourceName); - const addTrackPromises = []; - this.p2pJingleSession && addTrackPromises.push(this.p2pJingleSession.addTracks([ track ])); - this.jvbJingleSession && addTrackPromises.push(this.jvbJingleSession.addTracks([ track ])); + const addTracksToJingleSessions = this._addTracksToSessions([ track ]); - return Promise.all(addTrackPromises) - .then(() => { - this._setupNewTrack(track); - this._sendBridgeVideoTypeMessage(track); - this._updateRoomPresence(this.getActiveMediaSession()); + return addTracksToJingleSessions.then(() => { + this._setupNewTrack(track); + this._sendBridgeVideoTypeMessage(track); + this._updateRoomPresence(this.getActiveMediaSession()); - if (this.isMutedByFocus || this.isVideoMutedByFocus) { - this._fireMuteChangeEvent(track); - } - }); + if (this.isMutedByFocus || this.isVideoMutedByFocus) { + this._fireMuteChangeEvent(track); + } + + logger.info(`addTrack: ${track} - done`); + }); } - return Promise.reject(new Error(`Cannot add second ${mediaType} track to the conference`)); + return Promise.reject(new Error(`Cannot add second "${track.getVideoType()}" video track`)); } return this.replaceTrack(null, track) @@ -1127,6 +1128,45 @@ JitsiConference.prototype.addTrack = function(track) { if (track.getVideoType() === VideoType.DESKTOP) { this._updateRoomPresence(this.getActiveMediaSession()); } + + logger.info(`addTrack: ${track} - done`); + }); +}; + +/** + * Adds tracks to the media sessions. + * + * @param {JitsiLocalTrack[]} tracks - the tracks to add. + * @returns {Promise} + * @private + */ +JitsiConference.prototype._addTracksToSessions = function(tracks) { + const addTrackPromises = []; + const p2pAddTrack = (this.p2pJingleSession && addTrackPromises.push(this.p2pJingleSession.addTracks(tracks))) + || logger.info(`addTracks tracks=${tracks} - no P2P session`); + const jvbAddTrack = (this.jvbJingleSession && addTrackPromises.push(this.jvbJingleSession.addTracks(tracks))) + || logger.info(`addTracks tracks=${tracks} - no JVB session`); + + return Promise.all(addTrackPromises) + .then(() => { + // It can happen that a new session is started while JingleSessionPC operation is in progress. + // Queue up the track operation on the new session to make sure it catches up with the correct + // tracks state. + // Example: + // 1. JitsiConference.replaceTrack(camera1, camera2); + // 2. At the time there's only JVB session, it's replaceTrack will take 100ms. + // 3. 50 ms later - a P2P session is started, it will use camera1 as "local tracks" to start the session, + // because the replacement has not finished and this.rtc.localTracks still contains camera1. + // 4. Another 50ms later, JVB session finishes the replaceTrack and here we see that there's a new P2P + // session. Schedule a catch-up replaceTrack operation and make it part of the original operation chain. + const catchUpPromises = []; + + !p2pAddTrack && this.p2pJingleSession && catchUpPromises.push(this.p2pJingleSession.addTracks(tracks)) + && logger.info(`addTracks tracks=${tracks} catch up operation scheduled for P2P session`); + !jvbAddTrack && this.jvbJingleSession && catchUpPromises.push(this.jvbJingleSession.addTracks(tracks)) + && logger.info(`addTracks tracks=${tracks} catch up operation scheduled for JVB session`); + + return Promise.all(catchUpPromises); }); }; @@ -1261,6 +1301,8 @@ JitsiConference.prototype.removeTrack = function(track) { * @returns {Promise} resolves when the replacement is finished */ JitsiConference.prototype.replaceTrack = function(oldTrack, newTrack) { + logger.info(`replaceTrack old=${oldTrack} new=${newTrack}`); + const oldVideoType = oldTrack?.getVideoType(); const mediaType = oldTrack?.getType() || newTrack?.getType(); const newVideoType = newTrack?.getVideoType(); @@ -1311,10 +1353,12 @@ JitsiConference.prototype.replaceTrack = function(oldTrack, newTrack) { this._fireMuteChangeEvent(newTrack); } + logger.info(`replaceTrack old=${oldTrack} new=${newTrack} - done`); + return Promise.resolve(); }) .catch(error => { - logger.error(`replaceTrack failed: ${error?.stack}`); + logger.error(`replaceTrack old=${oldTrack} new=${newTrack} - failed: ${error?.message} ${error?.stack}`); return Promise.reject(error); }); @@ -1333,21 +1377,38 @@ JitsiConference.prototype.replaceTrack = function(oldTrack, newTrack) { * @private */ JitsiConference.prototype._doReplaceTrack = function(oldTrack, newTrack) { - const replaceTrackPromises = []; + const replaceTracks = []; + const jvbReplaceTrack + = (this.jvbJingleSession && replaceTracks.push(this.jvbJingleSession.replaceTrack(oldTrack, newTrack))) + || logger.info('_doReplaceTrack - no JVB JingleSession'); - if (this.jvbJingleSession) { - replaceTrackPromises.push(this.jvbJingleSession.replaceTrack(oldTrack, newTrack)); - } else { - logger.info('_doReplaceTrack - no JVB JingleSession'); - } + const p2pReplaceTrack + = (this.p2pJingleSession && replaceTracks.push(this.p2pJingleSession.replaceTrack(oldTrack, newTrack))) + || logger.info('_doReplaceTrack - no P2P JingleSession'); - if (this.p2pJingleSession) { - replaceTrackPromises.push(this.p2pJingleSession.replaceTrack(oldTrack, newTrack)); - } else { - logger.info('_doReplaceTrack - no P2P JingleSession'); - } - - return Promise.all(replaceTrackPromises); + return Promise.all(replaceTracks) + .then(() => { + // It can happen that a new session is started while JingleSessionPC operation is in progress. + // Queue up the track operation on the new session to make sure it catches up with the correct + // tracks state. + // Example: + // 1. JitsiConference.replaceTrack(camera1, camera2); + // 2. At the time there's only JVB session, it's replaceTrack will take 100ms. + // 3. 50 ms later - a P2P session is started, it will use camera1 as "local tracks" to start the session, + // because the replacement has not finished and this.rtc.localTracks still contains camera1. + // 4. Another 50ms later, JVB session finishes the replaceTrack and here we see that there's a new P2P + // session. Schedule a catch-up replaceTrack operation and make it part of the original operation chain. + const catchUpPromises = []; + + !p2pReplaceTrack && this.p2pJingleSession + && catchUpPromises.push(this.p2pJingleSession.replaceTrack(oldTrack, newTrack)) + && logger.info('_doReplaceTrack - scheduled catch-up replaceTrack for the P2P session'); + !jvbReplaceTrack && this.jvbJingleSession + && catchUpPromises.push(this.jvbJingleSession.replaceTrack(oldTrack, newTrack)) + && logger.info('_doReplaceTrack - scheduled catch-up replaceTrack for the JVB session'); + + return Promise.all(catchUpPromises); + }); }; /** diff --git a/JitsiConference.spec.ts b/JitsiConference.spec.ts new file mode 100644 index 0000000000..687c0c2a4d --- /dev/null +++ b/JitsiConference.spec.ts @@ -0,0 +1,330 @@ +import FeatureFlags from './modules/flags/FeatureFlags'; +import { nextTick } from './modules/util/TestUtils'; + +import JitsiConference from './JitsiConference'; +import { MockJitsiLocalTrack } from './modules/RTC/MockClasses'; +import Listenable from './modules/util/Listenable'; +import { MockChatRoom } from './modules/xmpp/MockClasses'; +import { MediaType } from './service/RTC/MediaType'; +import { VideoType } from './service/RTC/VideoType'; +import browser from './modules/browser'; + +class MockXmpp extends Listenable { + connection: { + options: { hosts: {}; videoQuality: {} }; + xmpp: MockXmpp; + jingle: { newP2PJingleSession: () => undefined } + }; + constructor() { + super(); + } + + isRoomCreated() { + return false; + } + + createRoom() { + return new MockChatRoom(); + } + + newP2PJingleSession() { + return undefined; + } +} + +class MockOffer { + find() { + return { + attr: () => { + // This is not written in a generic way and is intended for this condition to return true (to make p2p work): + // https://github.com/jitsi/lib-jitsi-meet/blob/a519f18b9ae33f34968b60be3ab123c81288f602/JitsiConference.js#L2092 + return '0'; + } + } + } +} + +function createMockConfig() { + return { + hosts: { + }, + videoQuality: { + } + }; +} + +function createMockConnection() { + const xmpp = new MockXmpp(); + const connection = { + xmpp, + options: createMockConfig(), + jingle: { + newP2PJingleSession: () => undefined, + } + }; + xmpp.connection = connection; + + return connection; +} + +class MockJingleSessionPC { + private isP2P: boolean; + private peerconnection: { + getLocalTracks: () => MockJitsiLocalTrack[], + getStats: () => Promise<[]>, + getAudioLevels: () => [], + + }; + private _delayMs: number; + constructor({ isP2P }: { isP2P: boolean }) { + this.isP2P = isP2P; + this._delayMs = 0; + this.peerconnection = { + getLocalTracks: () => [], + getStats: () => Promise.resolve([]), + getAudioLevels: () => [], + } + } + + isReady() { + return true; + } + + initialize() { } + + acceptOffer(offer, success, failure, localTracks) { } + + invite(localTracks: MockJitsiLocalTrack[]) { } + + _executeWithDelay(workload: () => void) { + return this._delayMs > 0 ? setTimeout(workload, this._delayMs) : workload(); + } + + async replaceTrack(oldTrack: MockJitsiLocalTrack, newTrack: MockJitsiLocalTrack) { + return new Promise(resolve => this._executeWithDelay(resolve)); + } + + async addTracks(tracks: MockJitsiLocalTrack[]) { + return new Promise(resolve => this._executeWithDelay(resolve)); + } + + addDelayToTrackOperations(delayMs) { + this._delayMs = delayMs; + } + + close() { + + } +} + +function startJvbSession(conference) { + const jvbSession = new MockJingleSessionPC({ isP2P: false }); + + conference.onIncomingCall(jvbSession); + + return jvbSession; +} + +function startP2PSession(conference) { + const p2pJingleSession = new MockJingleSessionPC({ isP2P: true }); + + // Need to inject the mock class here: + spyOn(conference.xmpp.connection.jingle, 'newP2PJingleSession').and.returnValue(p2pJingleSession); + + // This executes the code-path that sends an invite to the 'usr@server.com/conn1' JID + conference._startP2PSession('user@server.com/conn1'); + + return p2pJingleSession; +} + +describe('JitsiConference', () => { + let conference; + + beforeEach(() => { + conference = new JitsiConference({ + name: 'test-conf-1', + connection: createMockConnection(), + config: createMockConfig() + }); + jasmine.clock().install(); + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + describe('addTrack', () => { + it('should throw an error when a falsy value is passed a the track argument', async () => { + try { + conference.addTrack(undefined); + fail('addTrack should throw'); + } catch (error) { + expect(error.message).toBe('addTrack - a track is required'); + } + }); + it('should throw if 2nd video track of the same video kind is added', async () => { + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + const cameraTrack2 = new MockJitsiLocalTrack(720, MediaType.VIDEO, VideoType.CAMERA); + const screenTrack1 = new MockJitsiLocalTrack(1080, MediaType.VIDEO, VideoType.DESKTOP); + + await conference.addTrack(cameraTrack1); + await conference.addTrack(screenTrack1); + await conference.addTrack(cameraTrack2) + .then( + () => fail('did not throw'), + error => expect(error.message).toBe('Cannot add second "camera" video track') + ); + }); + it('should be a NOOP if the track is in the conference already', async () => { + const jvbSession = startJvbSession(conference); + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + + // FIXME JingleSessionPC.replaceTrack is used to add primary video track instead of addTrack + // const addTracksSpy = spyOn(jvbSession, 'addTracks'); + const replaceTracksSpy = spyOn(jvbSession, 'replaceTrack'); + + await conference.addTrack(cameraTrack1); + await conference.addTrack(cameraTrack1); + await conference.addTrack(cameraTrack1); + + expect(replaceTracksSpy) + .withContext('add track on the JingleSession should have been called once with the camera track') + .toHaveBeenCalledOnceWith(null, cameraTrack1); + }); + }); + describe('JVB JingleSession should pickup the local tracks', () => { + it('when created while track operation is in-progress on the P2P session', async () => { + const p2pJingleSession = startP2PSession(conference); + + p2pJingleSession.addDelayToTrackOperations(1000); + + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + + conference.addTrack(cameraTrack1); + + await nextTick(500); + + const jvbSession = new MockJingleSessionPC({ isP2P: false }); + // FIXME adjust to addTrack once JitsiConference will start using addTrack instead of replace for the primary tracks + const replaceTrackSpy = spyOn(jvbSession, 'replaceTrack'); + + conference.onIncomingCall(jvbSession); + + await nextTick(1000); + expect(replaceTrackSpy) + .withContext('replaceTrack should have been called with a track at the 1500ms mark') + .toHaveBeenCalledOnceWith(null, cameraTrack1); + }); + }) + describe('Peer-to-peer JingleSession should pickup the local tracks', () => { + it('when offer created while track operation is in-progress on the JVB session', async () => { + const jvbSession = startJvbSession(conference); + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + + // It will take 1000ms to add local track to the conference + jvbSession.addDelayToTrackOperations(1000); + conference.addTrack(cameraTrack1); + + // After 500ms start the P2P session + await nextTick(500); + const p2pJingleSession = new MockJingleSessionPC({ isP2P: true }); + // FIXME adjust to addTrack once JitsiConference will start using addTrack instead of replace for the primary tracks + const replaceTrackSpy = spyOn(p2pJingleSession, 'replaceTrack'); + spyOn(conference.xmpp.connection.jingle, 'newP2PJingleSession').and.returnValue(p2pJingleSession); + conference._startP2PSession('jid'); + + await nextTick(1000); + expect(replaceTrackSpy) + .withContext('invite should have been called with the camera track by the 1500ms mark') + .toHaveBeenCalledOnceWith(null, cameraTrack1); + }); + it('when offer accepted while track operation is in-progress on the JVB session', async () => { + const jvbSession = startJvbSession(conference); + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + + // It will take 1000ms to add local track to the conference + jvbSession.addDelayToTrackOperations(1000); + conference.addTrack(cameraTrack1); + + // After 500ms start the P2P session. Note that addTrack is still in progress and the conference needs to + // wait with calling 'accept offer', so that the track is included correctly (there's no flow that would + // pick it up later). + await nextTick(500); + // Different mocks to satisfy the P2P flow starting at conference.onIncomingCall: + spyOn(conference, 'isP2PEnabled').and.returnValue(true); + spyOn(conference, '_shouldBeInP2PMode').and.returnValue(true); + const p2pSession = new MockJingleSessionPC({ isP2P: true }); + // FIXME adjust to addTrack once JitsiConference will start using addTrack instead of replace for the primary tracks + const replaceTrackSpy = spyOn(p2pSession, 'replaceTrack'); + const mockOffer = new MockOffer(); + conference.onIncomingCall(p2pSession, mockOffer); + + await nextTick(1000); + expect(replaceTrackSpy) + .withContext('replaceTrack should have been called with a track on the P2P session by the 1500ms mark') + .toHaveBeenCalledOnceWith(null, cameraTrack1); + }); + }) + describe('replaceTrack', () => { + it('replaces tracks correctly when waiting for past promises to resolve', async () => { + const cameraTrack1 = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + const cameraTrack2 = new MockJitsiLocalTrack(361, MediaType.VIDEO, VideoType.CAMERA); + const cameraTrack3 = new MockJitsiLocalTrack(362, MediaType.VIDEO, VideoType.CAMERA); + const cameraTrack4 = new MockJitsiLocalTrack(363, MediaType.VIDEO, VideoType.CAMERA); + + const jvbSession = startJvbSession(conference); + + const addTracksSpy = spyOn(jvbSession, 'addTracks'); + const replaceTrackSpy = spyOn(jvbSession, 'replaceTrack'); + + await conference.addTrack(cameraTrack1); + await conference.replaceTrack(cameraTrack1, cameraTrack2); + await conference.replaceTrack(cameraTrack2, cameraTrack3); + await conference.replaceTrack(cameraTrack3, cameraTrack4); + expect(conference.getLocalVideoTracks()[0]) + .withContext('track 3 should have been replaced with track 4') + .toBe(cameraTrack4); + + // FIXME replaceTrack is used on JingleSessionPC instead of the addTrack method + // expect(addTracksSpy) + // .toHaveBeenCalledOnceWith([cameraTrack1]); + // FIXME +1 accounts for replace track used to add the first track of same video type + expect(replaceTrackSpy) + .toHaveBeenCalledTimes(3 + 1); + + // Verify that the tracks were actually replaced in the expected sequence on the JingleSession level: + + // FIXME addTrack - called as replaceTrack(null, cameraTrack1) + expect(replaceTrackSpy.calls.all()[0].args[0]).toEqual(null); + expect(replaceTrackSpy.calls.all()[0].args[1]).toEqual(cameraTrack1); + + // replaceTrack(cameraTrack1, cameraTrack2) + expect(replaceTrackSpy.calls.all()[1].args[0]).toEqual(cameraTrack1); + expect(replaceTrackSpy.calls.all()[1].args[1]).toEqual(cameraTrack2); + + // replaceTrack(cameraTrack2, cameraTrack3) + expect(replaceTrackSpy.calls.all()[2].args[0]).toEqual(cameraTrack2); + expect(replaceTrackSpy.calls.all()[2].args[1]).toEqual(cameraTrack3); + + // replaceTrack(cameraTrack3, cameraTrack4) + expect(replaceTrackSpy.calls.all()[3].args[0]).toEqual(cameraTrack3); + expect(replaceTrackSpy.calls.all()[3].args[1]).toEqual(cameraTrack4); + }); + it('should not allow to replace tracks of different video types', async () => { + const cameraTrack = new MockJitsiLocalTrack(360, MediaType.VIDEO, VideoType.CAMERA); + const screenTrack = new MockJitsiLocalTrack(1080, MediaType.VIDEO, VideoType.DESKTOP); + + await conference.addTrack(cameraTrack); + + try { + conference.replaceTrack(cameraTrack, screenTrack); + fail('Should throw an error'); + } catch(error) { + expect(error.message) + .toBe( + 'Replacing a track of videoType=camera with a track of videoType=desktop' + + ' is not supported in this mode.'); + } + }); + }) +}); diff --git a/modules/RTC/MockClasses.js b/modules/RTC/MockClasses.js index b69ef1c9e4..4cd72c9c11 100644 --- a/modules/RTC/MockClasses.js +++ b/modules/RTC/MockClasses.js @@ -1,7 +1,11 @@ import transform from 'sdp-transform'; +import { MediaType } from '../../service/RTC/MediaType'; +import Listenable from '../util/Listenable'; + /* eslint-disable no-empty-function */ /* eslint-disable max-len */ +/* eslint-disable require-jsdoc */ /** * MockRTCPeerConnection that return the local description sdp. @@ -307,18 +311,47 @@ export class MockTrack { } } +let trackId = 1; + /** * MockJitsiLocalTrack */ -export class MockJitsiLocalTrack { +export class MockJitsiLocalTrack extends Listenable { /** * A constructor */ constructor(height, mediaType, videoType) { + super(); this.resolution = height; this.track = new MockTrack(height); this.type = mediaType; this.videoType = videoType; + this._id = trackId; + trackId += 1; // The track id is useful to distinguish between instances (see toString) + } + + setSourceName(sourceName) { + this.sourceName = sourceName; + } + + getSourceName() { + return this.sourceName; + } + + setConference(conference) { + this.conference = conference; + } + + isAudioTrack() { + return this.getType() === MediaType.AUDIO; + } + + isVideoTrack() { + return this.getType() === MediaType.VIDEO; + } + + isMuted() { + return false; } /** @@ -360,4 +393,14 @@ export class MockJitsiLocalTrack { getVideoType() { return this.videoType; } + + _sendMuteStatus() { } + + toString() { + return `JitsiLocalTrack[id=${this._id},sourceName=${this.sourceName},type=${this.type}]`; + } } + +/* eslint-enable no-empty-function */ +/* eslint-enable max-len */ +/* eslint-enable require-jsdoc */ diff --git a/modules/xmpp/MockClasses.js b/modules/xmpp/MockClasses.js index 701bdaf49a..dfae5741d0 100644 --- a/modules/xmpp/MockClasses.js +++ b/modules/xmpp/MockClasses.js @@ -3,16 +3,45 @@ import { Strophe } from 'strophe.js'; import Listenable from '../util/Listenable'; /* eslint-disable no-empty-function */ +/* eslint-disable require-jsdoc */ /** * Mock {@link ChatRoom}. */ export class MockChatRoom extends Listenable { + constructor() { + super(); + this.connectionTimes = {}; + } + /** * {@link ChatRoom.addPresenceListener}. */ addPresenceListener() { } + + removePresenceListener() { + + } + + addOrReplaceInPresence() { + } + + setParticipantPropertyListener() { + + } + + isFocus() { + return true; // Used to verify incoming JVB jingle session + } + + getMeetingId() { + return undefined; // Doesn't matter in the tests we have so far. + } + + leave() { + + } } /** @@ -80,3 +109,4 @@ export class MockStropheConnection extends Listenable { } } /* eslint-enable no-empty-function */ +/* eslint-enable require-jsdoc */