From 7f21c591aeac5cfc545b4d29ac6cfa834d383241 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 9 Oct 2020 18:45:45 +0100 Subject: [PATCH 1/3] Fixes for call state machine * Set 'connecting' state before sending answer, otherwise it can race with ICE connecting * Ignore completed ice connection state: connected is what we care about * Null-check remotestream when stopping media * Comments --- src/webrtc/call.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index 25c2b3fb8..441034cf4 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -645,8 +645,8 @@ export class MatrixCall extends EventEmitter { }; private sendAnswer() { + this.setState(CallState.Connecting); this.sendEvent('m.call.answer', this.answerContent).then(() => { - this.setState(CallState.Connecting); // If this isn't the first time we've tried to send the answer, // we may have candidates queued up, so send them now. this.sendCandidateQueue(); @@ -896,12 +896,11 @@ export class MatrixCall extends EventEmitter { return; // because ICE can still complete as we're ending the call } logger.debug( - "Ice connection state changed to: " + this.peerConn.iceConnectionState, + "ICE connection state changed to: " + this.peerConn.iceConnectionState, ); // ideally we'd consider the call to be connected when we get media but // chrome doesn't implement any of the 'onstarted' events yet - if (this.peerConn.iceConnectionState == 'completed' || - this.peerConn.iceConnectionState == 'connected') { + if (this.peerConn.iceConnectionState == 'connected') { this.setState(CallState.Connected); } else if (this.peerConn.iceConnectionState == 'failed') { this.hangup(CallErrorCode.IceFailed, false); @@ -1099,8 +1098,10 @@ export class MatrixCall extends EventEmitter { } } - for (const track of this.remoteStream.getTracks()) { - track.stop(); + if (this.remoteStream) { + for (const track of this.remoteStream.getTracks()) { + track.stop(); + } } } @@ -1174,6 +1175,7 @@ export class MatrixCall extends EventEmitter { iceServers: this.turnServers, }); + // 'connectionstatechange' would be better, but firefox doesn't implement that. pc.addEventListener('iceconnectionstatechange', this.onIceConnectionStateChanged); pc.addEventListener('signalingstatechange', this.onSignallingStateChanged); pc.addEventListener('icecandidate', this.gotLocalIceCandidate); From 5c4b7a321322f2774872960a5a85369bd6e8531d Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 9 Oct 2020 18:51:35 +0100 Subject: [PATCH 2/3] Add user_hangup error code but special case it for now & don't send it: it needs voip v1 --- src/webrtc/call.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index 441034cf4..22d4114b7 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -92,6 +92,9 @@ enum MediaQueueId { } export enum CallErrorCode { + /** The user chose to end the call */ + UserHangup = 'user_hangup', + /** An error code when the local client failed to create an offer. */ LocalOfferFailed = 'local_offer_failed', /** @@ -531,8 +534,10 @@ export class MatrixCall extends EventEmitter { const content = { version: 0, call_id: this.callId, - reason: reason, }; + // Continue to send no reason for user hangups temporarily, until + // clients understand the user_hangup reason (voip v1) + if (reason !== CallErrorCode.UserHangup) content['reason'] = reason; this.sendEvent('m.call.hangup', content); } From b28bad651e214239bd4f53fd336c6b8f97b0e612 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 12 Oct 2020 10:15:58 +0100 Subject: [PATCH 3/3] Make events names an enum --- src/webrtc/call.ts | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/webrtc/call.ts b/src/webrtc/call.ts index 22d4114b7..0a8d3b512 100644 --- a/src/webrtc/call.ts +++ b/src/webrtc/call.ts @@ -85,6 +85,13 @@ export enum CallParty { Remote = 'remote', } +export enum CallEvent { + Hangup = 'hangup', + State = 'state', + Error = 'error', + Replaced = 'replaced', +} + enum MediaQueueId { RemoteVideo = 'remote_video', RemoteAudio = 'remote_audio', @@ -300,7 +307,7 @@ export class MatrixCall extends EventEmitter { const audioConstraints = getUserMediaVideoContraints(CallType.Voice); this.placeCallWithConstraints(audioConstraints); } catch (err) { - this.emit("error", + this.emit(CallEvent.Error, new CallError( CallErrorCode.NoUserMedia, "Failed to get screen-sharing stream: ", err, @@ -446,7 +453,7 @@ export class MatrixCall extends EventEmitter { if (this.peerConn.signalingState != 'closed') { this.peerConn.close(); } - this.emit("hangup"); + this.emit(CallEvent.Hangup); } }, this.msg.lifetime - event.getLocalAge()); } @@ -517,7 +524,7 @@ export class MatrixCall extends EventEmitter { newCall.remoteVideoElement = this.remoteVideoElement; newCall.remoteAudioElement = this.remoteAudioElement; this.successor = newCall; - this.emit("replaced", newCall); + this.emit(CallEvent.Replaced, newCall); this.hangup(CallErrorCode.Replaced, true); } @@ -666,7 +673,7 @@ export class MatrixCall extends EventEmitter { code = CallErrorCode.UnknownDevices; message = "Unknown devices present in the room"; } - this.emit("error", new CallError(code, message, error)); + this.emit(CallEvent.Error, new CallError(code, message, error)); throw error; }); } @@ -862,7 +869,7 @@ export class MatrixCall extends EventEmitter { this.client.cancelPendingEvent(error.event); this.terminate(CallParty.Local, code, false); - this.emit("error", new CallError(code, message, error)); + this.emit(CallEvent.Error, new CallError(code, message, error)); } }; @@ -871,7 +878,7 @@ export class MatrixCall extends EventEmitter { this.terminate(CallParty.Local, CallErrorCode.LocalOfferFailed, false); this.emit( - "error", + CallEvent.Error, new CallError( CallErrorCode.LocalOfferFailed, "Failed to get local offer!", err, @@ -887,7 +894,7 @@ export class MatrixCall extends EventEmitter { this.terminate(CallParty.Local, CallErrorCode.NoUserMedia, false); this.emit( - "error", + CallEvent.Error, new CallError( CallErrorCode.NoUserMedia, "Couldn't start capturing media! Is your microphone set up and " + @@ -1015,7 +1022,7 @@ export class MatrixCall extends EventEmitter { setState(state: CallState) { const oldState = this.state; this.state = state; - this.emit("state", state, oldState); + this.emit(CallEvent.State, state, oldState); } /** @@ -1086,7 +1093,7 @@ export class MatrixCall extends EventEmitter { this.peerConn.close(); } if (shouldEmit) { - this.emit("hangup", self); + this.emit(CallEvent.Hangup, self); } }