Skip to content

Commit b6a591d

Browse files
author
Aidan Zimmermann
committed
MR-825: add ability to send multiple tracks when screensharing
1 parent 8e49394 commit b6a591d

5 files changed

Lines changed: 107 additions & 78 deletions

File tree

‎.DS_Store‎

6 KB
Binary file not shown.

‎genesys-cloud-webrtc-sdk.iml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module type="WEB_MODULE" version="4">
3+
<component name="NewModuleRootManager" inherit-compiler-output="true">
4+
<exclude-output />
5+
<content url="file://$MODULE_DIR$" />
6+
<orderEntry type="inheritedJdk" />
7+
<orderEntry type="sourceFolder" forTests="false" />
8+
</component>
9+
</module>

‎src/sessions/base-session-handler.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,8 @@ export default abstract class BaseSessionHandler {
466466
}
467467

468468
this.log('debug', 'Adding track to session', { track: t, conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
469-
promises.push(session.peerConnection.addTrack(t));
469+
// Use addTrack with the stream to ensure proper track handling for multiple tracks of the same kind
470+
promises.push(session.peerConnection.addTrack(t, stream));
470471
});
471472
} else {
472473
const errMsg = 'Track based actions are required for this session but the client is not capable';
@@ -503,14 +504,18 @@ export default abstract class BaseSessionHandler {
503504
await this.waitForSessionConnected(session);
504505
}
505506

506-
// find a transceiver with the same kind of track
507+
// find a transceiver with the same kind of track that doesn't already have a track
508+
// or find the first available transceiver for replacement
507509
const transceiver = session.peerConnection.getTransceivers().find(t => {
510+
return (t.receiver.track?.kind === track.kind || t.sender.track?.kind === track.kind) &&
511+
(!t.sender.track || t.sender.track.readyState === 'ended');
512+
}) || session.peerConnection.getTransceivers().find(t => {
508513
return t.receiver.track?.kind === track.kind || t.sender.track?.kind === track.kind;
509514
});
510515

511516
let sender: RTCRtpSender;
512517

513-
if (transceiver) {
518+
if (transceiver && transceiver.sender.track) {
514519
await transceiver.sender.replaceTrack(track);
515520
sender = transceiver.sender;
516521
} else {

‎src/sessions/video-session-handler.ts‎

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,7 @@ export class VideoSessionHandler extends BaseSessionHandler {
529529
return;
530530
}
531531

532+
// Ensure we have at least one video transceiver for camera
532533
const videoTransceiver = session.pc.getTransceivers().find(transceiver => transceiver.receiver.track && transceiver.receiver.track.kind === 'video');
533534
if (!videoTransceiver) {
534535
session.pc.addTransceiver('video', { direction: 'sendrecv' });
@@ -562,25 +563,26 @@ export class VideoSessionHandler extends BaseSessionHandler {
562563

563564
const userId = this.sdk._personDetails.id;
564565

565-
// if we are going to mute, we need to remove/end the existing camera track
566+
// if we are going to mute, we need to remove/end the existing camera track (but not screen share)
566567
if (params.mute) {
567-
// get the first video track
568-
const track = session._outboundStream.getVideoTracks().find(t => t);
568+
// get camera tracks from outbound stream (exclude screen share tracks)
569+
const cameraTrack = session._outboundStream.getVideoTracks().find(track => {
570+
// Screen share tracks are in the _screenShareStream, so any video track in _outboundStream should be camera
571+
return !session._screenShareStream || !session._screenShareStream.getVideoTracks().find(screenTrack => screenTrack.id === track.id);
572+
});
569573

570-
if (!track) {
574+
if (!cameraTrack) {
571575
this.log('warn', 'Unable to find outbound camera track', { sessionId: session.id, conversationId: session.conversationId, sessionType: session.sessionType });
572576
} else {
573-
574577
const sender = this.getSendersByTrackType(session, 'video')
575-
.find((sender) => sender.track && sender.track.id === track.id);
578+
.find((sender) => sender.track && sender.track.id === cameraTrack.id);
576579

577580
if (sender) {
578581
await this.removeMediaFromSession(session, sender);
579582
}
580583

581-
track.stop();
582-
session._outboundStream.removeTrack(track);
583-
584+
cameraTrack.stop();
585+
session._outboundStream.removeTrack(cameraTrack);
584586
}
585587

586588
if (!skipServerUpdate) {
@@ -589,9 +591,13 @@ export class VideoSessionHandler extends BaseSessionHandler {
589591

590592
// if we are unmuting, we need to get a new camera track and add that to the session
591593
} else {
592-
// Make sure we don't have any tracks before we decide to spin up another.
593-
if (session._outboundStream.getVideoTracks().length > 0) {
594-
this.log('debug', 'Cannot unmute, a video track already exists', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
594+
// Check if we already have a camera track (exclude screen share tracks)
595+
const existingCameraTrack = session._outboundStream.getVideoTracks().find(track => {
596+
return !session._screenShareStream || !session._screenShareStream.getVideoTracks().find(screenTrack => screenTrack.id === track.id);
597+
});
598+
599+
if (existingCameraTrack) {
600+
this.log('debug', 'Cannot unmute, a camera video track already exists', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
595601
return;
596602
}
597603

@@ -714,11 +720,8 @@ export class VideoSessionHandler extends BaseSessionHandler {
714720

715721
this.log('info', 'Screen media created', { sessionId: session.id, conversationId: session.conversationId, sessionType: session.sessionType });
716722

717-
await this.addReplaceTrackToSession(session, stream.getVideoTracks()[0]);
718-
719-
if (!session.videoMuted) {
720-
await this.setVideoMute(session, { conversationId: session.conversationId, mute: true }, true);
721-
}
723+
// Add screen share track without replacing existing video track
724+
await this.addMediaToSession(session, stream);
722725

723726
stream.getTracks().forEach((track: MediaStreamTrack) => {
724727
track.addEventListener('ended', this.stopScreenShare.bind(this, session));
@@ -747,21 +750,9 @@ export class VideoSessionHandler extends BaseSessionHandler {
747750
const track = session._screenShareStream.getVideoTracks()[0];
748751
const sender = session.pc.getSenders().find(sender => sender.track && sender.track.id === track.id);
749752

750-
if (session._resurrectVideoOnScreenShareEnd) {
751-
this.log('info', 'Restarting video track', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
752-
try {
753-
await this.setVideoMute(session, { conversationId: session.conversationId, mute: false }, true);
754-
} catch (err) {
755-
/* This is to ensure that if something goes wrong while
756-
* fetching the preferred device's media, we still stop screensharing
757-
* but return the user to a muted state
758-
*/
759-
await this.setVideoMute(session, { conversationId: session.conversationId, mute: true }, false);
760-
track.stop();
761-
this.sessionManager.webrtcSessions.notifyScreenShareStop(session);
762-
}
763-
} else {
764-
await sender.replaceTrack(null);
753+
if (sender) {
754+
// Remove the screen share track without affecting other video tracks
755+
await this.removeMediaFromSession(session, sender);
765756
}
766757

767758
track.stop();

‎test/unit/sessions/video-session-handler.test.ts‎

Lines changed: 67 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,7 @@ describe('setInitialMuteStates', () => {
10061006
beforeEach(() => {
10071007
session = new MockSession() as any;
10081008
audioSender = { track: { kind: 'audio', enabled: true } };
1009-
videoSender = { track: { kind: 'video', enabled: true } };
1009+
videoSender = { track: { kind: 'video', enabled: true, id: 'camera-track-1' } };
10101010
});
10111011

10121012
it('should mute video', async () => {
@@ -1219,7 +1219,7 @@ describe('setVideoMute', () => {
12191219

12201220
await handler.setVideoMute(session, { conversationId: session.conversationId, mute: false });
12211221

1222-
expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a video track already exists'), expect.any(Object), undefined);
1222+
expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a camera video track already exists'), expect.any(Object), undefined);
12231223
});
12241224

12251225
it('mute: should mute video track when there is no screen track and remove track from outboundStream', async () => {
@@ -1306,7 +1306,7 @@ describe('setVideoMute', () => {
13061306
const stream = new MockStream(true);
13071307
const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
13081308
jest.spyOn(mockSessionManager, 'getAllActiveSessions').mockReturnValue([{ id: session.id } as IExtendedMediaSession ]);
1309-
jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
1309+
jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
13101310

13111311
session._outboundStream = {
13121312
addTrack: jest.fn(),
@@ -1327,7 +1327,7 @@ describe('setVideoMute', () => {
13271327
const stream = new MockStream(true);
13281328
const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
13291329
jest.spyOn(mockSessionManager, 'getAllActiveSessions').mockReturnValue([{ id: session.id } as IExtendedMediaSession ]);
1330-
jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
1330+
jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
13311331

13321332
session._outboundStream = {
13331333
addTrack: jest.fn(),
@@ -1347,7 +1347,7 @@ describe('setVideoMute', () => {
13471347
const unmuteDeviceId = 'device-id';
13481348
const stream = new MockStream(true);
13491349
const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
1350-
jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
1350+
jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
13511351

13521352
session._outboundStream = {
13531353
addTrack: jest.fn(),
@@ -1361,6 +1361,51 @@ describe('setVideoMute', () => {
13611361
expect(session.unmute).not.toHaveBeenCalled();
13621362
expect(stream.getVideoTracks()[0].stop).toHaveBeenCalled();
13631363
});
1364+
1365+
it('mute: should find camera track when screen share exists', async () => {
1366+
const screenTrack = new MockTrack('video');
1367+
screenTrack.id = 'screen-track-id';
1368+
const cameraTrack = new MockTrack('video');
1369+
cameraTrack.id = 'camera-track-id';
1370+
1371+
session._screenShareStream = {
1372+
getVideoTracks: jest.fn().mockReturnValue([screenTrack])
1373+
} as any;
1374+
1375+
session._outboundStream = {
1376+
removeTrack: jest.fn(),
1377+
getVideoTracks: jest.fn().mockReturnValue([cameraTrack])
1378+
} as any;
1379+
1380+
jest.spyOn(handler, 'getSendersByTrackType').mockReturnValue([
1381+
{ track: cameraTrack }
1382+
] as any);
1383+
1384+
await handler.setVideoMute(session, { conversationId: session.conversationId, mute: true });
1385+
1386+
expect(cameraTrack.stop).toHaveBeenCalled();
1387+
expect(session.mute).toHaveBeenCalledWith(userId, 'video');
1388+
});
1389+
1390+
it('unmute: should check for existing camera track when screen share exists', async () => {
1391+
const screenTrack = new MockTrack('video');
1392+
screenTrack.id = 'screen-track-id';
1393+
const cameraTrack = new MockTrack('video');
1394+
cameraTrack.id = 'camera-track-id';
1395+
1396+
session._screenShareStream = {
1397+
getVideoTracks: jest.fn().mockReturnValue([screenTrack])
1398+
} as any;
1399+
1400+
session._outboundStream = {
1401+
addTrack: jest.fn(),
1402+
getVideoTracks: jest.fn().mockReturnValue([cameraTrack])
1403+
} as any;
1404+
1405+
await handler.setVideoMute(session, { conversationId: session.conversationId, mute: false });
1406+
1407+
expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a camera video track already exists'), expect.any(Object), undefined);
1408+
});
13641409
});
13651410

13661411
describe('setAudioMute', () => {
@@ -1483,33 +1528,29 @@ describe('getSendersByTrackType', () => {
14831528

14841529
describe('startScreenShare', () => {
14851530
let displayMediaSpy: jest.SpyInstance<Promise<MediaStream>>;
1486-
let videoMuteSpy: jest.SpyInstance<Promise<void>>;
1487-
let addReplaceTrackToSession: jest.SpyInstance<Promise<any>>;
1531+
let addMediaToSessionSpy: jest.SpyInstance<Promise<void>>;
14881532
let session: VideoMediaSession;
14891533

14901534
beforeEach(() => {
14911535
displayMediaSpy = jest.spyOn(mockSdk.media, 'startDisplayMedia').mockResolvedValue(new MockStream({ video: true }) as any);
1492-
videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockResolvedValue();
1493-
addReplaceTrackToSession = jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
1536+
addMediaToSessionSpy = jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
14941537
session = new MockSession() as any;
14951538
});
14961539

1497-
it('should start media and mute video if it is not already muted', async () => {
1540+
it('should start media and add screen share track without replacing existing video', async () => {
14981541
await handler.startScreenShare(session);
14991542

15001543
expect(displayMediaSpy).toHaveBeenCalled();
1501-
expect(videoMuteSpy).toHaveBeenCalled();
1502-
expect(addReplaceTrackToSession).toHaveBeenCalled();
1544+
expect(addMediaToSessionSpy).toHaveBeenCalled();
15031545
expect(mockSessionManager.webrtcSessions.notifyScreenShareStart).toHaveBeenCalled();
15041546
});
15051547

1506-
it('should not mute video if already muted', async () => {
1548+
it('should add screen share track regardless of video mute state', async () => {
15071549
session.videoMuted = true;
15081550
await handler.startScreenShare(session);
15091551

15101552
expect(displayMediaSpy).toHaveBeenCalled();
1511-
expect(videoMuteSpy).not.toHaveBeenCalled();
1512-
expect(addReplaceTrackToSession).toHaveBeenCalled();
1553+
expect(addMediaToSessionSpy).toHaveBeenCalled();
15131554
expect(mockSessionManager.webrtcSessions.notifyScreenShareStart).toHaveBeenCalled();
15141555
});
15151556

@@ -1534,12 +1575,10 @@ describe('startScreenShare', () => {
15341575
});
15351576

15361577
describe('stopScreenShare', () => {
1537-
let videoMuteSpy;
15381578
let removeMediaSpy;
15391579
let session;
15401580

15411581
beforeEach(() => {
1542-
videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockResolvedValue();
15431582
removeMediaSpy = jest.spyOn(handler, 'removeMediaFromSession').mockResolvedValue();
15441583
session = new MockSession();
15451584
});
@@ -1549,49 +1588,34 @@ describe('stopScreenShare', () => {
15491588

15501589
await handler.stopScreenShare(session);
15511590

1552-
expect(videoMuteSpy).not.toHaveBeenCalled();
15531591
expect(removeMediaSpy).not.toHaveBeenCalled();
15541592
expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).not.toHaveBeenCalled();
15551593
});
15561594

1557-
it('should stop screen share tracks and unmute video if resurrectVideoOnScreenShareEnd', async () => {
1558-
session._resurrectVideoOnScreenShareEnd = true;
1595+
it('should remove screen share track and stop it', async () => {
15591596
session._screenShareStream = new MockStream({ video: true });
1597+
const screenTrack = session._screenShareStream._tracks[0];
15601598

1561-
await handler.stopScreenShare(session);
1562-
1563-
expect(videoMuteSpy).toHaveBeenCalled();
1564-
expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
1565-
expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
1566-
});
1567-
1568-
it('should not unmute video if not resurrect', async () => {
1569-
session.resurrectVideoOnScreenShareEnd = false;
1570-
session._screenShareStream = new MockStream({ video: true });
1571-
1572-
const replaceSpy = jest.fn();
1573-
jest.spyOn(session.pc, 'getSenders').mockReturnValue([{ track: session._screenShareStream._tracks[0], replaceTrack: replaceSpy }]);
1599+
jest.spyOn(session.pc, 'getSenders').mockReturnValue([{ track: screenTrack }]);
15741600

15751601
await handler.stopScreenShare(session);
15761602

1577-
expect(videoMuteSpy).not.toHaveBeenCalled();
1578-
expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
1579-
expect(replaceSpy).toHaveBeenCalled();
1603+
expect(removeMediaSpy).toHaveBeenCalled();
1604+
expect(screenTrack.stop).toHaveBeenCalled();
15801605
expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
15811606
});
15821607

1583-
it('should toggle off of screen share AND keep video unmuted if something goes wrong when fetching device media', async () => {
1584-
videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockRejectedValueOnce({ message: 'Could not start video source' }).mockResolvedValueOnce();
1585-
session._resurrectVideoOnScreenShareEnd = true;
1608+
it('should stop screen share track even if no sender found', async () => {
15861609
session._screenShareStream = new MockStream({ video: true });
1610+
const screenTrack = session._screenShareStream._tracks[0];
1611+
1612+
jest.spyOn(session.pc, 'getSenders').mockReturnValue([]);
15871613

15881614
await handler.stopScreenShare(session);
15891615

1590-
expect(videoMuteSpy).toHaveBeenNthCalledWith(1, session, { conversationId: session.conversationId, mute: false }, true);
1591-
expect(videoMuteSpy).toHaveBeenNthCalledWith(2, session, { conversationId: session.conversationId, mute: true }, false);
1592-
expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
1616+
expect(removeMediaSpy).not.toHaveBeenCalled();
1617+
expect(screenTrack.stop).toHaveBeenCalled();
15931618
expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
1594-
jest.resetAllMocks();
15951619
});
15961620
});
15971621

0 commit comments

Comments
 (0)
⚡