From 173f7a50612f09e69ae441cacf9cd9ee1e9c9d82 Mon Sep 17 00:00:00 2001 From: Gregoire Parant Date: Tue, 20 Oct 2020 18:02:44 +0200 Subject: [PATCH] Fix webrct finish event delay --- front/src/WebRtc/MediaManager.ts | 3 +-- front/src/WebRtc/SimplePeer.ts | 31 +++++++++++++++++-------- front/src/WebRtc/VideoPeer.ts | 39 +++++++++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/front/src/WebRtc/MediaManager.ts b/front/src/WebRtc/MediaManager.ts index eb65b555..0cbfe123 100644 --- a/front/src/WebRtc/MediaManager.ts +++ b/front/src/WebRtc/MediaManager.ts @@ -455,8 +455,7 @@ export class MediaManager { addStreamRemoteVideo(userId: string, stream : MediaStream){ const remoteVideo = this.remoteVideo.get(userId); if (remoteVideo === undefined) { - console.error('Unable to find video for ', userId); - return; + throw `Unable to find video for ${userId}`; } remoteVideo.srcObject = stream; } diff --git a/front/src/WebRtc/SimplePeer.ts b/front/src/WebRtc/SimplePeer.ts index 6f38bc27..f1a66bcc 100644 --- a/front/src/WebRtc/SimplePeer.ts +++ b/front/src/WebRtc/SimplePeer.ts @@ -126,10 +126,19 @@ export class SimplePeer { * create peer connection to bind users */ private createPeerConnection(user : UserSimplePeerInterface) : VideoPeer | null{ - if( - this.PeerConnectionArray.has(user.userId) - ){ - console.log('Peer connection already exists to user '+user.userId) + const peerConnection = this.PeerConnectionArray.get(user.userId) + if(peerConnection){ + if(peerConnection.destroyed){ + peerConnection.toClose = true; + peerConnection.destroy(); + let peerConnexionDeleted = this.PeerConnectionArray.delete(user.userId); + if(!peerConnexionDeleted){ + throw 'Error to delete peer connection'; + } + this.createPeerConnection(user); + }else { + peerConnection.toClose = false; + } return null; } @@ -150,6 +159,7 @@ export class SimplePeer { mediaManager.addActiveVideo("" + user.userId, reportCallback, name); const peer = new VideoPeer(user.userId, user.initiator ? user.initiator : false, this.Connection); + peer.toClose = false; // When a connection is established to a video stream, and if a screen sharing is taking place, // the user sharing screen should also initiate a connection to the remote user! peer.on('connect', () => { @@ -200,16 +210,17 @@ export class SimplePeer { //mediaManager.removeActiveVideo(userId); const peer = this.PeerConnectionArray.get(userId); if (peer === undefined) { - console.warn("Tried to close connection for user "+userId+" but could not find user") + console.warn("closeConnection => Tried to close connection for user "+userId+" but could not find user"); return; } + //create temp perr to close + peer.toClose = true; peer.destroy(); // FIXME: I don't understand why "Closing connection with" message is displayed TWICE before "Nb users in peerConnectionArray" // I do understand the method closeConnection is called twice, but I don't understand how they manage to run in parallel. - //console.log('Closing connection with '+userId); - this.PeerConnectionArray.delete(userId); + this.closeScreenSharingConnection(userId); - //console.log('Nb users in peerConnectionArray '+this.PeerConnectionArray.size); + for (const peerConnectionListener of this.peerConnectionListeners) { peerConnectionListener.onDisconnect(userId); } @@ -228,7 +239,7 @@ export class SimplePeer { mediaManager.removeActiveScreenSharingVideo("" + userId); const peer = this.PeerScreenSharingConnectionArray.get(userId); if (peer === undefined) { - console.warn("Tried to close connection for user "+userId+" but could not find user") + console.warn("closeScreenSharingConnection => Tried to close connection for user "+userId+" but could not find user") return; } // FIXME: I don't understand why "Closing connection with" message is displayed TWICE before "Nb users in peerConnectionArray" @@ -294,7 +305,7 @@ export class SimplePeer { } } catch (e) { console.error(`receiveWebrtcSignal => ${data.userId}`, e); - //force delete and reconnect peer connexion + //force delete and recreate peer connexion this.PeerScreenSharingConnectionArray.delete(data.userId); this.receiveWebrtcScreenSharingSignal(data); } diff --git a/front/src/WebRtc/VideoPeer.ts b/front/src/WebRtc/VideoPeer.ts index 9331bea7..aa8a5d17 100644 --- a/front/src/WebRtc/VideoPeer.ts +++ b/front/src/WebRtc/VideoPeer.ts @@ -9,7 +9,10 @@ const Peer: SimplePeerNamespace.SimplePeer = require('simple-peer'); * A peer connection used to transmit video / audio signals between 2 peers. */ export class VideoPeer extends Peer { - constructor(private userId: number, initiator: boolean, private connection: RoomConnection) { + public toClose: boolean = false; + public _connected: boolean = false; + + constructor(public userId: number, initiator: boolean, private connection: RoomConnection) { super({ initiator: initiator ? initiator : false, reconnectTimer: 10000, @@ -57,6 +60,8 @@ export class VideoPeer extends Peer { });*/ this.on('close', () => { + this._connected = false; + this.toClose = true; this.destroy(); }); @@ -67,6 +72,7 @@ export class VideoPeer extends Peer { }); this.on('connect', () => { + this._connected = true; mediaManager.isConnected("" + this.userId); console.info(`connect => ${this.userId}`); }); @@ -88,6 +94,10 @@ export class VideoPeer extends Peer { } }); + this.once('finish', () => { + this._onFinish(); + }); + this.pushVideoToRemoteUser(); } @@ -108,7 +118,15 @@ export class VideoPeer extends Peer { mediaManager.disabledVideoByUserId(this.userId); mediaManager.disabledMicrophoneByUserId(this.userId); } else { - mediaManager.addStreamRemoteVideo("" + this.userId, stream); + try { + mediaManager.addStreamRemoteVideo("" + this.userId, stream); + }catch (err){ + console.error(err); + //Force add streem video + setTimeout(() => { + this.stream(stream); + }, 500); + } } } @@ -117,16 +135,31 @@ export class VideoPeer extends Peer { */ public destroy(error?: Error): void { try { + this._connected = false + if(!this.toClose){ + return; + } mediaManager.removeActiveVideo("" + this.userId); // FIXME: I don't understand why "Closing connection with" message is displayed TWICE before "Nb users in peerConnectionArray" // I do understand the method closeConnection is called twice, but I don't understand how they manage to run in parallel. - //console.log('Closing connection with '+userId); super.destroy(error); } catch (err) { console.error("VideoPeer::destroy", err) } } + _onFinish () { + if (this.destroyed) return + const destroySoon = () => { + this.destroy(); + } + if (this._connected) { + destroySoon(); + } else { + this.once('connect', destroySoon); + } + } + private pushVideoToRemoteUser() { try { const localStream: MediaStream | null = mediaManager.localStream;