From 91f422d0c34283fe42add59564ed2172adcc2c09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 21 Aug 2020 22:53:17 +0200 Subject: [PATCH] Fixing stop of stream in bi-directional screen sharing. --- front/src/WebRtc/MediaManager.ts | 54 ++++++++++++++++++++------- front/src/WebRtc/ScreenSharingPeer.ts | 33 +++++++++++++--- front/src/WebRtc/SimplePeer.ts | 45 ++++++++++++++-------- 3 files changed, 98 insertions(+), 34 deletions(-) diff --git a/front/src/WebRtc/MediaManager.ts b/front/src/WebRtc/MediaManager.ts index 63814ee4..314fea14 100644 --- a/front/src/WebRtc/MediaManager.ts +++ b/front/src/WebRtc/MediaManager.ts @@ -1,5 +1,5 @@ -import * as SimplePeerNamespace from "simple-peer"; import {DivImportance, layoutManager} from "./LayoutManager"; +import {HtmlUtils} from "./HtmlUtils"; const videoConstraint: boolean|MediaTrackConstraints = { width: { ideal: 1280 }, @@ -8,7 +8,8 @@ const videoConstraint: boolean|MediaTrackConstraints = { }; type UpdatedLocalStreamCallback = (media: MediaStream) => void; -type UpdatedScreenSharingCallback = (media: MediaStream) => void; +type StartScreenSharingCallback = (media: MediaStream) => void; +type StopScreenSharingCallback = (media: MediaStream) => void; // TODO: Split MediaManager in 2 classes: MediaManagerUI (in charge of HTML) and MediaManager (singleton in charge of the camera only) // TODO: verify that microphone event listeners are not triggered plenty of time NOW (since MediaManager is created many times!!!!) @@ -29,7 +30,8 @@ export class MediaManager { video: videoConstraint }; updatedLocalStreamCallBacks : Set = new Set(); - updatedScreenSharingCallBacks : Set = new Set(); + startScreenSharingCallBacks : Set = new Set(); + stopScreenSharingCallBacks : Set = new Set(); constructor() { @@ -87,9 +89,14 @@ export class MediaManager { this.updatedLocalStreamCallBacks.add(callback); } - public onUpdateScreenSharing(callback: UpdatedScreenSharingCallback): void { + public onStartScreenSharing(callback: StartScreenSharingCallback): void { - this.updatedScreenSharingCallBacks.add(callback); + this.startScreenSharingCallBacks.add(callback); + } + + public onStopScreenSharing(callback: StopScreenSharingCallback): void { + + this.stopScreenSharingCallBacks.add(callback); } removeUpdateLocalStreamEventListener(callback: UpdatedLocalStreamCallback): void { @@ -102,8 +109,14 @@ export class MediaManager { } } - private triggerUpdatedScreenSharingCallbacks(stream: MediaStream): void { - for (const callback of this.updatedScreenSharingCallBacks) { + private triggerStartedScreenSharingCallbacks(stream: MediaStream): void { + for (const callback of this.startScreenSharingCallBacks) { + callback(stream); + } + } + + private triggerStoppedScreenSharingCallbacks(stream: MediaStream): void { + for (const callback of this.stopScreenSharingCallBacks) { callback(stream); } } @@ -164,20 +177,26 @@ export class MediaManager { this.monitorClose.style.display = "none"; this.monitor.style.display = "block"; this.getScreenMedia().then((stream) => { - this.triggerUpdatedScreenSharingCallbacks(stream); + this.triggerStartedScreenSharingCallbacks(stream); }); } private disableScreenSharing() { this.monitorClose.style.display = "block"; this.monitor.style.display = "none"; + this.removeActiveScreenSharingVideo('me'); this.localScreenCapture?.getTracks().forEach((track: MediaStreamTrack) => { track.stop(); }); - this.localScreenCapture = null; + if (this.localScreenCapture === null) { + console.warn('Weird: trying to remove a screen sharing that is not enabled'); + return; + } + const localScreenCapture = this.localScreenCapture; this.getCamera().then((stream) => { - this.triggerUpdatedScreenSharingCallbacks(stream); + this.triggerStoppedScreenSharingCallbacks(localScreenCapture); }); + this.localScreenCapture = null; } //get screen @@ -194,6 +213,9 @@ export class MediaManager { }; } + this.addScreenSharingActiveVideo('me', DivImportance.Normal); + HtmlUtils.getElementByIdOrFail('screen-sharing-me').srcObject = stream; + return stream; }) .catch((err: unknown) => { @@ -306,8 +328,8 @@ export class MediaManager { * * @param userId */ - addScreenSharingActiveVideo(userId : string){ - this.webrtcInAudio.play(); + addScreenSharingActiveVideo(userId : string, divImportance: DivImportance = DivImportance.Important){ + //this.webrtcInAudio.play(); userId = `screen-sharing-${userId}`; const html = ` @@ -316,7 +338,7 @@ export class MediaManager { `; - layoutManager.add(DivImportance.Important, userId, html); + layoutManager.add(divImportance, userId, html); this.remoteVideo.set(userId, this.getElementByIdOrFail(userId)); } @@ -389,6 +411,12 @@ export class MediaManager { remoteVideo.srcObject = stream; } addStreamRemoteScreenSharing(userId : string, stream : MediaStream){ + // In the case of screen sharing (going both ways), we may need to create the HTML element if it does not exist yet + const remoteVideo = this.remoteVideo.get(`screen-sharing-${userId}`); + if (remoteVideo === undefined) { + this.addScreenSharingActiveVideo(userId); + } + this.addStreamRemoteVideo(`screen-sharing-${userId}`, stream); } diff --git a/front/src/WebRtc/ScreenSharingPeer.ts b/front/src/WebRtc/ScreenSharingPeer.ts index 3ce3c409..35f43201 100644 --- a/front/src/WebRtc/ScreenSharingPeer.ts +++ b/front/src/WebRtc/ScreenSharingPeer.ts @@ -8,6 +8,11 @@ const Peer: SimplePeerNamespace.SimplePeer = require('simple-peer'); * A peer connection used to transmit video / audio signals between 2 peers. */ export class ScreenSharingPeer extends Peer { + /** + * Whether this connection is currently receiving a video stream from a remote user. + */ + private isReceivingStream:boolean = false; + constructor(private userId: string, initiator: boolean, private connection: Connection) { super({ initiator: initiator ? initiator : false, @@ -35,13 +40,20 @@ export class ScreenSharingPeer extends Peer { this.stream(stream); }); - /*this.on('track', (track: MediaStreamTrack, stream: MediaStream) => { - });*/ - this.on('close', () => { this.destroy(); }); + this.on('data', (chunk: Buffer) => { + // We unfortunately need to rely on an event to let the other party know a stream has stopped. + // It seems there is no native way to detect that. + const message = JSON.parse(chunk.toString('utf8')); + if (message.streamEnded !== true) { + console.error('Unexpected message on screen sharing peer connection'); + } + mediaManager.removeActiveScreenSharingVideo(this.userId); + }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any this.on('error', (err: any) => { console.error(`screen sharing error => ${this.userId} => ${err.code}`, err); @@ -74,11 +86,17 @@ export class ScreenSharingPeer extends Peer { console.log(`stream => ${this.userId} => `, stream); if(!stream){ mediaManager.removeActiveScreenSharingVideo(this.userId); + this.isReceivingStream = false; } else { mediaManager.addStreamRemoteScreenSharing(this.userId, stream); + this.isReceivingStream = true; } } + public isReceivingScreenSharingStream(): boolean { + return this.isReceivingStream; + } + public destroy(error?: Error): void { try { mediaManager.removeActiveScreenSharingVideo(this.userId); @@ -98,9 +116,12 @@ export class ScreenSharingPeer extends Peer { return; } - for (const track of localScreenCapture.getTracks()) { - this.addTrack(track, localScreenCapture); - } + this.addStream(localScreenCapture); return; } + + public stopPushingScreenSharingToRemoteUser(stream: MediaStream) { + this.removeStream(stream); + this.write(new Buffer(JSON.stringify({streamEnded: true}))); + } } diff --git a/front/src/WebRtc/SimplePeer.ts b/front/src/WebRtc/SimplePeer.ts index 02573273..3acd65c5 100644 --- a/front/src/WebRtc/SimplePeer.ts +++ b/front/src/WebRtc/SimplePeer.ts @@ -34,6 +34,7 @@ export class SimplePeer { private PeerConnectionArray: Map = new Map(); private readonly sendLocalVideoStreamCallback: (media: MediaStream) => void; private readonly sendLocalScreenSharingStreamCallback: (media: MediaStream) => void; + private readonly stopLocalScreenSharingStreamCallback: (media: MediaStream) => void; private readonly peerConnectionListeners: Array = new Array(); constructor(Connection: Connection, WebRtcRoomId: string = "test-webrtc") { @@ -42,8 +43,10 @@ export class SimplePeer { // We need to go through this weird bound function pointer in order to be able to "free" this reference later. this.sendLocalVideoStreamCallback = this.sendLocalVideoStream.bind(this); this.sendLocalScreenSharingStreamCallback = this.sendLocalScreenSharingStream.bind(this); + this.stopLocalScreenSharingStreamCallback = this.stopLocalScreenSharingStream.bind(this); mediaManager.onUpdateLocalStream(this.sendLocalVideoStreamCallback); - mediaManager.onUpdateScreenSharing(this.sendLocalScreenSharingStreamCallback); + mediaManager.onStartScreenSharing(this.sendLocalScreenSharingStreamCallback); + mediaManager.onStopScreenSharing(this.stopLocalScreenSharingStreamCallback); this.initialise(); } @@ -332,14 +335,22 @@ export class SimplePeer { * Triggered locally when clicking on the screen sharing button */ public sendLocalScreenSharingStream() { - if (mediaManager.localScreenCapture) { - for (const user of this.Users) { - this.sendLocalScreenSharingStreamToUser(user.userId); - } - } else { - for (const user of this.Users) { - this.stopLocalScreenSharingStreamToUser(user.userId); - } + if (!mediaManager.localScreenCapture) { + console.error('Could not find localScreenCapture to share') + return; + } + + for (const user of this.Users) { + this.sendLocalScreenSharingStreamToUser(user.userId); + } + } + + /** + * Triggered locally when clicking on the screen sharing button + */ + public stopLocalScreenSharingStream(stream: MediaStream) { + for (const user of this.Users) { + this.stopLocalScreenSharingStreamToUser(user.userId, stream); } } @@ -360,17 +371,21 @@ export class SimplePeer { } } - private stopLocalScreenSharingStreamToUser(userId: string): void { + private stopLocalScreenSharingStreamToUser(userId: string, stream: MediaStream): void { const PeerConnectionScreenSharing = this.PeerScreenSharingConnectionArray.get(userId); if (!PeerConnectionScreenSharing) { throw new Error('Weird, screen sharing connection to user ' + userId + 'not found') } console.log("updatedScreenSharing => destroy", PeerConnectionScreenSharing); - // FIXME: maybe we don't want to destroy the connexion if it is used in the other way around! - // FIXME: maybe we don't want to destroy the connexion if it is used in the other way around! - // FIXME: maybe we don't want to destroy the connexion if it is used in the other way around! - PeerConnectionScreenSharing.destroy(); - this.PeerScreenSharingConnectionArray.delete(userId); + + // Stop sending stream and close peer connection if peer is not sending stream too + PeerConnectionScreenSharing.stopPushingScreenSharingToRemoteUser(stream); + + if (!PeerConnectionScreenSharing.isReceivingScreenSharingStream()) { + PeerConnectionScreenSharing.destroy(); + + this.PeerScreenSharingConnectionArray.delete(userId); + } } }