[STREAM-1178] - Remove support for sending multiple tracks while screensharing. (#975)

Author: zservies

changelog.md

@@ -4,6 +4,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 # [Unreleased](https://github.com/MyPureCloud/genesys-cloud-webrtc-sdk/compare/v11.5.1...HEAD)
+### Changed
+* [STREAM-1178](https://inindca.atlassian.net/browse/STREAM-1178) - REVERT STREAM-825, removed ability to send multiple tracks when screensharing.
 
 # [v11.5.1](https://github.com/MyPureCloud/genesys-cloud-webrtc-sdk/compare/v11.5.0...v11.5.1)
 ### Added

src/sessions/base-session-handler.ts

@@ -477,8 +477,7 @@ export default abstract class BaseSessionHandler {
         }
 
         this.log('debug', 'Adding track to session', { track: t, conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
-        // Use addTrack with the stream to ensure proper track handling for multiple tracks of the same kind
-        promises.push(session.peerConnection.addTrack(t, stream));
+        promises.push(session.peerConnection.addTrack(t));
       });
     } else {
       const errMsg = 'Track based actions are required for this session but the client is not capable';

src/sessions/video-session-handler.ts

@@ -550,13 +550,6 @@ export class VideoSessionHandler extends BaseSessionHandler {
     session.pc.getSenders().forEach(sender => sender.track && sender.track.stop());
   }
 
-  getCameraTrackForSession (session: IExtendedMediaSession): MediaStreamTrack {
-    return session._outboundStream.getVideoTracks().find(track => {
-      // Screen share tracks are in the _screenShareStream, so any video track in _outboundStream should be camera
-      return !session._screenShareStream || !session._screenShareStream.getVideoTracks().find(screenTrack => screenTrack.id === track.id);
-    })
-  }
-
   async setVideoMute (session: IExtendedMediaSession, params: ISessionMuteRequest, skipServerUpdate?: boolean): Promise<void> {
     const replayMuteRequest = !!session.videoMuted === !!params.mute;
 
@@ -569,22 +562,25 @@ export class VideoSessionHandler extends BaseSessionHandler {
 
     const userId = this.sdk._personDetails.id;
 
-    // if we are going to mute, we need to remove the existing camera video track but not any screen share video tracks
+    // if we are going to mute, we need to remove/end the existing camera track
     if (params.mute) {
-      const cameraTrack = this.getCameraTrackForSession(session);
+      // get the first video track
+      const track = session._outboundStream.getVideoTracks().find(t => t);
 
-      if (!cameraTrack) {
+      if (!track) {
         this.log('warn', 'Unable to find outbound camera track', { sessionId: session.id, conversationId: session.conversationId, sessionType: session.sessionType });
       } else {
+
         const sender = this.getSendersByTrackType(session, 'video')
-          .find((sender) => sender.track && sender.track.id === cameraTrack.id);
+          .find((sender) => sender.track && sender.track.id === track.id);
 
         if (sender) {
           await this.removeMediaFromSession(session, sender);
         }
 
-        cameraTrack.stop();
-        session._outboundStream.removeTrack(cameraTrack);
+        track.stop();
+        session._outboundStream.removeTrack(track);
+
       }
 
       if (!skipServerUpdate) {
@@ -593,11 +589,9 @@ export class VideoSessionHandler extends BaseSessionHandler {
 
       // if we are unmuting, we need to get a new camera track and add that to the session
     } else {
-      // Check if we already have a camera track (exclude screen share tracks)
-      const existingCameraTrack = this.getCameraTrackForSession(session);
-
-      if (existingCameraTrack) {
-        this.log('debug', 'Cannot unmute, a camera video track already exists', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
+      // Make sure we don't have any tracks before we decide to spin up another.
+      if (session._outboundStream.getVideoTracks().length > 0) {
+        this.log('debug', 'Cannot unmute, a video track already exists', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
         return;
       }
 
@@ -720,8 +714,11 @@ export class VideoSessionHandler extends BaseSessionHandler {
 
       this.log('info', 'Screen media created', { sessionId: session.id, conversationId: session.conversationId, sessionType: session.sessionType });
 
-      // Add screen share track without replacing existing video track
-      await this.addMediaToSession(session, stream);
+      await this.addReplaceTrackToSession(session, stream.getVideoTracks()[0]);
+
+      if (!session.videoMuted) {
+        await this.setVideoMute(session, { conversationId: session.conversationId, mute: true }, true);
+      }
 
       stream.getTracks().forEach((track: MediaStreamTrack) => {
         track.addEventListener('ended', this.stopScreenShare.bind(this, session));
@@ -750,9 +747,21 @@ export class VideoSessionHandler extends BaseSessionHandler {
     const track = session._screenShareStream.getVideoTracks()[0];
     const sender = session.pc.getSenders().find(sender => sender.track && sender.track.id === track.id);
 
-    if (sender) {
-      // Remove the screen share track without affecting other video tracks
-      await this.removeMediaFromSession(session, sender);
+    if (session._resurrectVideoOnScreenShareEnd) {
+      this.log('info', 'Restarting video track', { conversationId: session.conversationId, sessionId: session.id, sessionType: session.sessionType });
+      try {
+        await this.setVideoMute(session, { conversationId: session.conversationId, mute: false }, true);
+      } catch (err) {
+        /* This is to ensure that if something goes wrong while
+         * fetching the preferred device's media, we still stop screensharing
+         * but return the user to a muted state
+        */
+        await this.setVideoMute(session, { conversationId: session.conversationId, mute: true }, false);
+        track.stop();
+        this.sessionManager.webrtcSessions.notifyScreenShareStop(session);
+      }
+    } else {
+      await sender.replaceTrack(null);
     }
 
     track.stop();

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

@@ -1004,7 +1004,7 @@ describe('setInitialMuteStates', () => {
   beforeEach(() => {
     session = new MockSession() as any;
     audioSender = { track: { kind: 'audio', enabled: true } };
-    videoSender = { track: { kind: 'video', enabled: true, id: 'camera-track-1' } };
+    videoSender = { track: { kind: 'video', enabled: true } };
   });
 
   it('should mute video', async () => {
@@ -1217,7 +1217,7 @@ describe('setVideoMute', () => {
 
     await handler.setVideoMute(session, { conversationId: session.conversationId, mute: false });
 
-    expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a camera video track already exists'), expect.any(Object), undefined);
+    expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a video track already exists'), expect.any(Object), undefined);
   });
 
   it('mute: should mute video track when there is no screen track and remove track from outboundStream', async () => {
@@ -1304,7 +1304,7 @@ describe('setVideoMute', () => {
     const stream = new MockStream(true);
     const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
     jest.spyOn(mockSessionManager, 'getAllActiveSessions').mockReturnValue([{ id: session.id } as IExtendedMediaSession ]);
-    jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
+    jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
 
     session._outboundStream = {
       addTrack: jest.fn(),
@@ -1325,7 +1325,7 @@ describe('setVideoMute', () => {
     const stream = new MockStream(true);
     const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
     jest.spyOn(mockSessionManager, 'getAllActiveSessions').mockReturnValue([{ id: session.id } as IExtendedMediaSession ]);
-    jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
+    jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
 
     session._outboundStream = {
       addTrack: jest.fn(),
@@ -1345,7 +1345,7 @@ describe('setVideoMute', () => {
     const unmuteDeviceId = 'device-id';
     const stream = new MockStream(true);
     const spy = jest.spyOn(mockSdk.media, 'startMedia').mockResolvedValue(stream as any);
-    jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
+    jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
 
     session._outboundStream = {
       addTrack: jest.fn(),
@@ -1359,51 +1359,6 @@ describe('setVideoMute', () => {
     expect(session.unmute).not.toHaveBeenCalled();
     expect(stream.getVideoTracks()[0].stop).toHaveBeenCalled();
   });
-
-  it('mute: should find camera track when screen share exists', async () => {
-    const screenTrack = new MockTrack('video');
-    screenTrack.id = 'screen-track-id';
-    const cameraTrack = new MockTrack('video');
-    cameraTrack.id = 'camera-track-id';
-
-    session._screenShareStream = {
-      getVideoTracks: jest.fn().mockReturnValue([screenTrack])
-    } as any;
-
-    session._outboundStream = {
-      removeTrack: jest.fn(),
-      getVideoTracks: jest.fn().mockReturnValue([cameraTrack])
-    } as any;
-
-    jest.spyOn(handler, 'getSendersByTrackType').mockReturnValue([
-      { track: cameraTrack }
-    ] as any);
-
-    await handler.setVideoMute(session, { conversationId: session.conversationId, mute: true });
-
-    expect(cameraTrack.stop).toHaveBeenCalled();
-    expect(session.mute).toHaveBeenCalledWith(userId, 'video');
-  });
-
-  it('unmute: should check for existing camera track when screen share exists', async () => {
-    const screenTrack = new MockTrack('video');
-    screenTrack.id = 'screen-track-id';
-    const cameraTrack = new MockTrack('video');
-    cameraTrack.id = 'camera-track-id';
-
-    session._screenShareStream = {
-      getVideoTracks: jest.fn().mockReturnValue([screenTrack])
-    } as any;
-
-    session._outboundStream = {
-      addTrack: jest.fn(),
-      getVideoTracks: jest.fn().mockReturnValue([cameraTrack])
-    } as any;
-
-    await handler.setVideoMute(session, { conversationId: session.conversationId, mute: false });
-
-    expect(mockSdk.logger.debug).toHaveBeenCalledWith(expect.stringContaining('Cannot unmute, a camera video track already exists'), expect.any(Object), undefined);
-  });
 });
 
 describe('setAudioMute', () => {
@@ -1526,29 +1481,33 @@ describe('getSendersByTrackType', () => {
 
 describe('startScreenShare', () => {
   let displayMediaSpy: jest.SpyInstance<Promise<MediaStream>>;
-  let addMediaToSessionSpy: jest.SpyInstance<Promise<void>>;
+  let videoMuteSpy: jest.SpyInstance<Promise<void>>;
+  let addReplaceTrackToSession: jest.SpyInstance<Promise<any>>;
   let session: VideoMediaSession;
 
   beforeEach(() => {
     displayMediaSpy = jest.spyOn(mockSdk.media, 'startDisplayMedia').mockResolvedValue(new MockStream({ video: true }) as any);
-    addMediaToSessionSpy = jest.spyOn(handler, 'addMediaToSession').mockResolvedValue();
+    videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockResolvedValue();
+    addReplaceTrackToSession = jest.spyOn(handler, 'addReplaceTrackToSession').mockResolvedValue();
     session = new MockSession() as any;
   });
 
-  it('should start media and add screen share track without replacing existing video', async () => {
+  it('should start media and mute video if it is not already muted', async () => {
     await handler.startScreenShare(session);
 
     expect(displayMediaSpy).toHaveBeenCalled();
-    expect(addMediaToSessionSpy).toHaveBeenCalled();
+    expect(videoMuteSpy).toHaveBeenCalled();
+    expect(addReplaceTrackToSession).toHaveBeenCalled();
     expect(mockSessionManager.webrtcSessions.notifyScreenShareStart).toHaveBeenCalled();
   });
 
-  it('should add screen share track regardless of video mute state', async () => {
+  it('should not mute video if already muted', async () => {
     session.videoMuted = true;
     await handler.startScreenShare(session);
 
     expect(displayMediaSpy).toHaveBeenCalled();
-    expect(addMediaToSessionSpy).toHaveBeenCalled();
+    expect(videoMuteSpy).not.toHaveBeenCalled();
+    expect(addReplaceTrackToSession).toHaveBeenCalled();
     expect(mockSessionManager.webrtcSessions.notifyScreenShareStart).toHaveBeenCalled();
   });
 
@@ -1573,47 +1532,64 @@ describe('startScreenShare', () => {
 });
 
 describe('stopScreenShare', () => {
+  let videoMuteSpy;
   let removeMediaSpy;
   let session;
 
   beforeEach(() => {
+    videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockResolvedValue();
     removeMediaSpy = jest.spyOn(handler, 'removeMediaFromSession').mockResolvedValue();
     session = new MockSession();
   });
 
-  it('should do nothing if there is no active screen share', async () => {
+  it('should do nothing if there is no screenshare stream', async () => {
     session._screenShareStream = null;
 
     await handler.stopScreenShare(session);
 
+    expect(videoMuteSpy).not.toHaveBeenCalled();
     expect(removeMediaSpy).not.toHaveBeenCalled();
     expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).not.toHaveBeenCalled();
   });
 
-  it('should remove screen share track and stop it', async () => {
+  it('should stop screen share tracks and unmute video if resurrectVideoOnScreenShareEnd', async () => {
+    session._resurrectVideoOnScreenShareEnd = true;
     session._screenShareStream = new MockStream({ video: true });
-    const screenTrack = session._screenShareStream._tracks[0];
-
-    jest.spyOn(session.pc, 'getSenders').mockReturnValue([{ track: screenTrack }]);
 
     await handler.stopScreenShare(session);
 
-    expect(removeMediaSpy).toHaveBeenCalled();
-    expect(screenTrack.stop).toHaveBeenCalled();
+    expect(videoMuteSpy).toHaveBeenCalled();
+    expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
     expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
   });
 
-  it('should stop screen share track even if no sender found', async () => {
+  it('should not unmute video if not resurrect', async () => {
+    session.resurrectVideoOnScreenShareEnd = false;
     session._screenShareStream = new MockStream({ video: true });
-    const screenTrack = session._screenShareStream._tracks[0];
 
-    jest.spyOn(session.pc, 'getSenders').mockReturnValue([]);
+    const replaceSpy = jest.fn();
+    jest.spyOn(session.pc, 'getSenders').mockReturnValue([{ track: session._screenShareStream._tracks[0], replaceTrack: replaceSpy }]);
 
     await handler.stopScreenShare(session);
 
-    expect(removeMediaSpy).not.toHaveBeenCalled();
-    expect(screenTrack.stop).toHaveBeenCalled();
+    expect(videoMuteSpy).not.toHaveBeenCalled();
+    expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
+    expect(replaceSpy).toHaveBeenCalled();
+    expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
+  });
+
+  it('should toggle off of screen share AND keep video unmuted if something goes wrong when fetching device media', async () => {
+    videoMuteSpy = jest.spyOn(handler, 'setVideoMute').mockRejectedValueOnce({ message: 'Could not start video source' }).mockResolvedValueOnce();
+    session._resurrectVideoOnScreenShareEnd = true;
+    session._screenShareStream = new MockStream({ video: true });
+
+    await handler.stopScreenShare(session);
+
+    expect(videoMuteSpy).toHaveBeenNthCalledWith(1, session, { conversationId: session.conversationId, mute: false }, true);
+    expect(videoMuteSpy).toHaveBeenNthCalledWith(2, session, { conversationId: session.conversationId, mute: true }, false);
+    expect(session._screenShareStream._tracks[0].stop).toHaveBeenCalled();
     expect(mockSessionManager.webrtcSessions.notifyScreenShareStop).toHaveBeenCalled();
+    jest.resetAllMocks();
   });
 });