From bc76532bd50ae282c0861a9b3c0e01beb277fb25 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 30 Mar 2023 16:57:47 +0100 Subject: [PATCH] Refactor the way group calls hang up (#3234) * Refactor how group call end calls We previously used disposeCall to terminate the call which meant that sometimes a call would never get a hangup event. This changes it so that we always end a call by calling hangup, then do the cleanup when the hangup event arrives, so the cleanup is the same whether we hang up or the other side does. * Some fixes for failing & hanging tests * Add type for the call map --- spec/test-utils/webrtc.ts | 2 ++ spec/unit/webrtc/groupCall.spec.ts | 4 ++++ src/webrtc/groupCall.ts | 27 ++++++++++++++++----------- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/spec/test-utils/webrtc.ts b/spec/test-utils/webrtc.ts index 7bd1d0493..e50a8df45 100644 --- a/spec/test-utils/webrtc.ts +++ b/spec/test-utils/webrtc.ts @@ -511,6 +511,8 @@ export class MockMatrixCall extends TypedEventEmitter(), + isAudioMuted: jest.fn().mockReturnValue(false), + isVideoMuted: jest.fn().mockReturnValue(false), stream: new MockMediaStream("stream"), } as unknown as CallFeed; public remoteUsermediaFeed?: CallFeed; diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 22af26e0c..04f67c862 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -144,6 +144,10 @@ describe("Group Call", function () { } as unknown as RoomMember; }); + afterEach(() => { + groupCall.leave(); + }); + it.each(Object.values(GroupCallState).filter((v) => v !== GroupCallState.LocalCallFeedUninitialized))( "throws when initializing local call feed in %s state", async (state: GroupCallState) => { diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 2e751a50f..c0896c4cd 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -42,6 +42,8 @@ export enum GroupCallTerminationReason { CallEnded = "call_ended", } +export type CallsByUserAndDevice = Map>; + /** * Because event names are just strings, they do need * to be unique over all event types of event emitter. @@ -62,7 +64,7 @@ export enum GroupCallEvent { export type GroupCallEventHandlerMap = { [GroupCallEvent.GroupCallStateChanged]: (newState: GroupCallState, oldState: GroupCallState) => void; [GroupCallEvent.ActiveSpeakerChanged]: (activeSpeaker: CallFeed | undefined) => void; - [GroupCallEvent.CallsChanged]: (calls: Map>) => void; + [GroupCallEvent.CallsChanged]: (calls: CallsByUserAndDevice) => void; [GroupCallEvent.UserMediaFeedsChanged]: (feeds: CallFeed[]) => void; [GroupCallEvent.ScreenshareFeedsChanged]: (feeds: CallFeed[]) => void; [GroupCallEvent.LocalScreenshareStateChanged]: ( @@ -528,12 +530,16 @@ export class GroupCall extends TypedEventEmitter< this.retryCallLoopInterval = undefined; } + if (this.participantsExpirationTimer !== null) { + clearTimeout(this.participantsExpirationTimer); + this.participantsExpirationTimer = null; + } + if (this.state !== GroupCallState.Entered) { return; } - this.forEachCall((call) => this.disposeCall(call, CallErrorCode.UserHangup)); - this.calls.clear(); + this.forEachCall((call) => call.hangup(CallErrorCode.UserHangup, false)); this.activeSpeaker = undefined; clearInterval(this.activeSpeakerLoopInterval); @@ -869,7 +875,8 @@ export class GroupCall extends TypedEventEmitter< `GroupCall ${this.groupCallId} onIncomingCall() incoming call (userId=${opponentUserId}, callId=${newCall.callId})`, ); - if (prevCall) this.disposeCall(prevCall, CallErrorCode.Replaced); + if (prevCall) prevCall.hangup(CallErrorCode.Replaced, false); + this.initCall(newCall); const feeds = this.getLocalFeeds().map((feed) => feed.clone()); @@ -928,7 +935,7 @@ export class GroupCall extends TypedEventEmitter< logger.debug( `GroupCall ${this.groupCallId} placeOutgoingCalls() replacing call (userId=${userId}, deviceId=${deviceId}, callId=${prevCall.callId})`, ); - this.disposeCall(prevCall, CallErrorCode.NewSession); + prevCall.hangup(CallErrorCode.NewSession, false); } const newCall = createNewMatrixCall(this.client, this.room.roomId, { @@ -979,7 +986,7 @@ export class GroupCall extends TypedEventEmitter< ); } - this.disposeCall(newCall, CallErrorCode.SignallingFailed); + newCall.hangup(CallErrorCode.SignallingFailed, false); if (callMap.get(deviceId) === newCall) callMap.delete(deviceId); }); } @@ -1101,10 +1108,6 @@ export class GroupCall extends TypedEventEmitter< return; } - if (call.state !== CallState.Ended) { - call.hangup(hangupReason, false); - } - const usermediaFeed = this.getUserMediaFeed(opponentMemberId, opponentDeviceId); if (usermediaFeed) { @@ -1156,6 +1159,8 @@ export class GroupCall extends TypedEventEmitter< }; private onCallStateChanged = (call: MatrixCall, state: CallState, _oldState: CallState | undefined): void => { + if (state === CallState.Ended) return; + const audioMuted = this.localCallFeed!.isAudioMuted(); if (call.localUsermediaStream && call.isMicrophoneMuted() !== audioMuted) { @@ -1200,7 +1205,7 @@ export class GroupCall extends TypedEventEmitter< this.calls.set(opponentUserId, deviceMap); } - this.disposeCall(prevCall, CallErrorCode.Replaced); + prevCall.hangup(CallErrorCode.Replaced, false); this.initCall(newCall); deviceMap.set(prevCall.getOpponentDeviceId()!, newCall); this.emit(GroupCallEvent.CallsChanged, this.calls);