From 226ce4a7842071bfc64bfdde284be5800b1270e1 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Thu, 16 Feb 2023 19:14:16 +0100 Subject: [PATCH 01/13] groupCall: add configuration param to allow no audio and no camera --- src/client.ts | 16 +++++++++++++++- src/webrtc/groupCall.ts | 26 ++++++++++++++++++++++++-- src/webrtc/groupCallEventHandler.ts | 1 + 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/client.ts b/src/client.ts index 5ea515fe9e6..0fef1f02284 100644 --- a/src/client.ts +++ b/src/client.ts @@ -371,6 +371,13 @@ export interface ICreateClientOpts { * Defaults to a built-in English handler with basic pluralisation. */ roomNameGenerator?: (roomId: string, state: RoomNameState) => string | null; + + /** + * If true, participant can join group call without video and audio this has to be allowed. By default, a local + * media stream is needed to establish a group call. + * Default: false. + */ + isVoipWithNoMediaAllowed?: boolean; } export interface IMatrixClientCreateOpts extends ICreateClientOpts { @@ -1169,6 +1176,7 @@ export class MatrixClient extends TypedEventEmitter { + groupCall.initCallWithoutVideoAndAudio = this.isVoipWithNoMediaAllowed; + return groupCall; + }); } /** diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 4e9971f10ad..6a0def8163c 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -209,6 +209,7 @@ export class GroupCall extends TypedEventEmitter< private initWithAudioMuted = false; private initWithVideoMuted = false; private initCallFeedPromise?: Promise; + private allowCallWithOutVideoAndAudio = false; public constructor( private client: MatrixClient, @@ -374,8 +375,14 @@ export class GroupCall extends TypedEventEmitter< try { stream = await this.client.getMediaHandler().getUserMediaStream(true, this.type === GroupCallType.Video); } catch (error) { - this.state = GroupCallState.LocalCallFeedUninitialized; - throw error; + // If is allowed to join a call without a media stream, then we add an empty stream. + // @FIXME: Attention this could break with Mute and Unmute logic! + if (this.allowCallWithOutVideoAndAudio) { + stream = new MediaStream(); + } else { + this.state = GroupCallState.LocalCallFeedUninitialized; + throw error; + } } // The call could've been disposed while we were waiting, and could @@ -1461,4 +1468,19 @@ export class GroupCall extends TypedEventEmitter< ); } }; + + /** + * allowCallWithOutVideoAndAudio should be a GroupCall property. However, I make it as runtime configurable + * property because this additional behavior has a big impact on other features and is a big API change. + * Joining without a camera and video should only be explicitly allowed, because there can be unexpected behavior + * when later querying the media access rights from browser that the client cannot intercept. Join without came + * and audio should be a property like PTT. + */ + public get initCallWithoutVideoAndAudio(): boolean { + return this.allowCallWithOutVideoAndAudio; + } + + public set initCallWithoutVideoAndAudio(allowCallWithOutVideoAndAudio: boolean) { + this.allowCallWithOutVideoAndAudio = allowCallWithOutVideoAndAudio; + } } diff --git a/src/webrtc/groupCallEventHandler.ts b/src/webrtc/groupCallEventHandler.ts index 2efed52933c..e82cddfd2a4 100644 --- a/src/webrtc/groupCallEventHandler.ts +++ b/src/webrtc/groupCallEventHandler.ts @@ -187,6 +187,7 @@ export class GroupCallEventHandler { content?.dataChannelsEnabled, dataChannelOptions, ); + groupCall.initCallWithoutVideoAndAudio = this.client.isVoipWithNoMediaAllowed; this.groupCalls.set(room.roomId, groupCall); this.client.emit(GroupCallEventHandlerEvent.Incoming, groupCall); From 9904a35ab6ab1a693557dd9285a1db45830fa6ab Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Thu, 16 Feb 2023 19:26:50 +0100 Subject: [PATCH 02/13] groupCall: enable datachannel to do no media group calls --- src/client.ts | 4 +++- src/webrtc/call.ts | 11 ++++++++++- src/webrtc/groupCall.ts | 10 ++++++++-- src/webrtc/groupCallEventHandler.ts | 4 +++- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/client.ts b/src/client.ts index 0fef1f02284..869b0293af7 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1889,6 +1889,8 @@ export class MatrixClient extends TypedEventEmitter; + // Used to allow connection without Video and Audio. To establish a webrtc connection without media a Data channel is + // needed At the moment this property is true if we allow MatrixClient with isVoipWithNoMediaAllowed = true + private readonly isOnlyDataChannelAllowed: boolean; /** * Construct a new Matrix Call. @@ -420,6 +423,8 @@ export class MatrixCall extends TypedEventEmitter { + // Because we need a Local Call Feed to establish a call connection, we avoid muting video in case of empty + // video track. In this way we go sure if a client implements muting we don't raise an error. + if (this.allowCallWithOutVideoAndAudio && this.localCallFeed?.stream.getVideoTracks().length === 0) { + return false; + } // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through diff --git a/src/webrtc/groupCallEventHandler.ts b/src/webrtc/groupCallEventHandler.ts index e82cddfd2a4..369b367ff61 100644 --- a/src/webrtc/groupCallEventHandler.ts +++ b/src/webrtc/groupCallEventHandler.ts @@ -184,7 +184,9 @@ export class GroupCallEventHandler { isPtt, callIntent, groupCallId, - content?.dataChannelsEnabled, + // Because without Media section a WebRTC connection is not possible, so need a RTCDataChannel to set up a + // no media WebRTC connection anyway. + content?.dataChannelsEnabled || this.client.isVoipWithNoMediaAllowed, dataChannelOptions, ); groupCall.initCallWithoutVideoAndAudio = this.client.isVoipWithNoMediaAllowed; From cce668507284da8690c89b14db8f07798ed4f7b9 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 17 Feb 2023 15:39:18 +0100 Subject: [PATCH 03/13] groupCall: changed call no media property as object property --- src/client.ts | 8 ++------ src/webrtc/groupCall.ts | 21 +++------------------ src/webrtc/groupCallEventHandler.ts | 2 +- 3 files changed, 6 insertions(+), 25 deletions(-) diff --git a/src/client.ts b/src/client.ts index 869b0293af7..55a8f05627b 100644 --- a/src/client.ts +++ b/src/client.ts @@ -1897,15 +1897,11 @@ export class MatrixClient extends TypedEventEmitter { - groupCall.initCallWithoutVideoAndAudio = this.isVoipWithNoMediaAllowed; - return groupCall; - }); + ).create(); } /** diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 8bed0bd13c9..0b6ef170aa1 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -209,7 +209,6 @@ export class GroupCall extends TypedEventEmitter< private initWithAudioMuted = false; private initWithVideoMuted = false; private initCallFeedPromise?: Promise; - private allowCallWithOutVideoAndAudio = false; public constructor( private client: MatrixClient, @@ -217,6 +216,7 @@ export class GroupCall extends TypedEventEmitter< public type: GroupCallType, public isPtt: boolean, public intent: GroupCallIntent, + public readonly initCallWithoutVideoAndAudio: boolean, groupCallId?: string, private dataChannelsEnabled?: boolean, private dataChannelOptions?: IGroupCallDataChannelOptions, @@ -378,7 +378,7 @@ export class GroupCall extends TypedEventEmitter< // If is allowed to join a call without a media stream, then we // don't throw an error here. But we need an empty Local Feed to establish // a connection later. - if (this.allowCallWithOutVideoAndAudio) { + if (this.initCallWithoutVideoAndAudio) { stream = new MediaStream(); } else { this.state = GroupCallState.LocalCallFeedUninitialized; @@ -619,7 +619,7 @@ export class GroupCall extends TypedEventEmitter< public async setLocalVideoMuted(muted: boolean): Promise { // Because we need a Local Call Feed to establish a call connection, we avoid muting video in case of empty // video track. In this way we go sure if a client implements muting we don't raise an error. - if (this.allowCallWithOutVideoAndAudio && this.localCallFeed?.stream.getVideoTracks().length === 0) { + if (this.initCallWithoutVideoAndAudio && this.localCallFeed?.stream.getVideoTracks().length === 0) { return false; } // hasAudioDevice can block indefinitely if the window has lost focus, @@ -1474,19 +1474,4 @@ export class GroupCall extends TypedEventEmitter< ); } }; - - /** - * allowCallWithOutVideoAndAudio should be a GroupCall property. However, I make it as runtime configurable - * property because this additional behavior has a big impact on other features and is a big API change. - * Joining without a camera and video should only be explicitly allowed, because there can be unexpected behavior - * when later querying the media access rights from browser that the client cannot intercept. Join without came - * and audio should be a property like PTT. - */ - public get initCallWithoutVideoAndAudio(): boolean { - return this.allowCallWithOutVideoAndAudio; - } - - public set initCallWithoutVideoAndAudio(allowCallWithOutVideoAndAudio: boolean) { - this.allowCallWithOutVideoAndAudio = allowCallWithOutVideoAndAudio; - } } diff --git a/src/webrtc/groupCallEventHandler.ts b/src/webrtc/groupCallEventHandler.ts index 369b367ff61..7392a75adab 100644 --- a/src/webrtc/groupCallEventHandler.ts +++ b/src/webrtc/groupCallEventHandler.ts @@ -183,13 +183,13 @@ export class GroupCallEventHandler { callType, isPtt, callIntent, + this.client.isVoipWithNoMediaAllowed, groupCallId, // Because without Media section a WebRTC connection is not possible, so need a RTCDataChannel to set up a // no media WebRTC connection anyway. content?.dataChannelsEnabled || this.client.isVoipWithNoMediaAllowed, dataChannelOptions, ); - groupCall.initCallWithoutVideoAndAudio = this.client.isVoipWithNoMediaAllowed; this.groupCalls.set(room.roomId, groupCall); this.client.emit(GroupCallEventHandlerEvent.Incoming, groupCall); From c57aadb320164f1eea8f728c99fc54260745604b Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 17 Feb 2023 15:45:49 +0100 Subject: [PATCH 04/13] groupCall: fix existing unit tests --- spec/unit/webrtc/groupCall.spec.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 4b68100113e..8f79563788f 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -109,7 +109,7 @@ const mockGetStateEvents = (type: EventType, userId?: string): MatrixEvent[] | M const ONE_HOUR = 1000 * 60 * 60; const createAndEnterGroupCall = async (cli: MatrixClient, room: Room): Promise => { - const groupCall = new GroupCall(cli, room, GroupCallType.Video, false, GroupCallIntent.Prompt, FAKE_CONF_ID); + const groupCall = new GroupCall(cli, room, GroupCallType.Video, false, GroupCallIntent.Prompt, false, FAKE_CONF_ID); await groupCall.create(); await groupCall.enter(); @@ -135,7 +135,7 @@ describe("Group Call", function () { mockClient = typedMockClient as unknown as MatrixClient; room = new Room(FAKE_ROOM_ID, mockClient, FAKE_USER_ID_1); - groupCall = new GroupCall(mockClient, room, GroupCallType.Video, false, GroupCallIntent.Prompt); + groupCall = new GroupCall(mockClient, room, GroupCallType.Video, false, GroupCallIntent.Prompt, false); room.currentState.members[FAKE_USER_ID_1] = { userId: FAKE_USER_ID_1, membership: "join", @@ -484,7 +484,7 @@ describe("Group Call", function () { describe("PTT calls", () => { beforeEach(async () => { // replace groupcall with a PTT one - groupCall = new GroupCall(mockClient, room, GroupCallType.Video, true, GroupCallIntent.Prompt); + groupCall = new GroupCall(mockClient, room, GroupCallType.Video, true, GroupCallIntent.Prompt, false); await groupCall.create(); @@ -647,6 +647,7 @@ describe("Group Call", function () { GroupCallType.Video, false, GroupCallIntent.Prompt, + false, FAKE_CONF_ID, ); @@ -656,6 +657,7 @@ describe("Group Call", function () { GroupCallType.Video, false, GroupCallIntent.Prompt, + false, FAKE_CONF_ID, ); }); @@ -1465,6 +1467,7 @@ describe("Group Call", function () { GroupCallType.Video, false, GroupCallIntent.Prompt, + false, FAKE_CONF_ID, ); await groupCall.create(); From 53ec8e0bde8c028a7de22841cb46644e9d3ade7b Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Wed, 22 Feb 2023 17:24:33 +0100 Subject: [PATCH 05/13] groupCall: remove not needed flag --- src/webrtc/groupCall.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 0b6ef170aa1..5dfaa6214f2 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -619,7 +619,7 @@ export class GroupCall extends TypedEventEmitter< public async setLocalVideoMuted(muted: boolean): Promise { // Because we need a Local Call Feed to establish a call connection, we avoid muting video in case of empty // video track. In this way we go sure if a client implements muting we don't raise an error. - if (this.initCallWithoutVideoAndAudio && this.localCallFeed?.stream.getVideoTracks().length === 0) { + if (this.localCallFeed?.stream.getVideoTracks().length === 0) { return false; } // hasAudioDevice can block indefinitely if the window has lost focus, From 1cad2d747792e341c40123bc41fa83ae9fc4b3f9 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 24 Feb 2023 09:59:02 +0100 Subject: [PATCH 06/13] groupCall: rename property to allow no media calls --- .gitignore | 1 + src/webrtc/groupCall.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 6bc9edb40a3..98d71fcd60a 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ out .vscode .vscode/ +.idea/ diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 5dfaa6214f2..5f7de1e81e6 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -216,7 +216,7 @@ export class GroupCall extends TypedEventEmitter< public type: GroupCallType, public isPtt: boolean, public intent: GroupCallIntent, - public readonly initCallWithoutVideoAndAudio: boolean, + public readonly allowCallWithoutVideoAndAudio: boolean, groupCallId?: string, private dataChannelsEnabled?: boolean, private dataChannelOptions?: IGroupCallDataChannelOptions, @@ -378,7 +378,7 @@ export class GroupCall extends TypedEventEmitter< // If is allowed to join a call without a media stream, then we // don't throw an error here. But we need an empty Local Feed to establish // a connection later. - if (this.initCallWithoutVideoAndAudio) { + if (this.allowCallWithoutVideoAndAudio) { stream = new MediaStream(); } else { this.state = GroupCallState.LocalCallFeedUninitialized; From 9880a2e264eee0c43e681242d790e2acb97cb1ea Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 24 Feb 2023 14:44:50 +0100 Subject: [PATCH 07/13] groupCall: mute unmute even without device --- src/webrtc/groupCall.ts | 57 +++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 8 deletions(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 5f7de1e81e6..d83121bf620 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -553,7 +553,17 @@ export class GroupCall extends TypedEventEmitter< // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - if (!muted && !(await this.client.getMediaHandler().hasAudioDevice())) { + const hasAudioDevice = await this.client + .getMediaHandler() + .hasAudioDevice() + .catch((_) => { + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, + ); + return false; + }); + + if (!muted && !hasAudioDevice) { return false; } @@ -592,6 +602,22 @@ export class GroupCall extends TypedEventEmitter< logger.log( `GroupCall ${this.groupCallId} setMicrophoneMuted() (streamId=${this.localCallFeed.stream.id}, muted=${muted})`, ); + + // We needed this here to avoid an error in case user join a call without a device. + if (!muted) { + const stream = await this.client + .getMediaHandler() + .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()) + .catch((_) => null); + if (stream === null) { + // if case permission denied to get a stream stop this here + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, + ); + return false; + } + } + this.localCallFeed.setAudioVideoMuted(muted, null); // I don't believe its actually necessary to enable these tracks: they // are the one on the GroupCall's own CallFeed and are cloned before being @@ -617,15 +643,20 @@ export class GroupCall extends TypedEventEmitter< * @returns Whether muting/unmuting was successful */ public async setLocalVideoMuted(muted: boolean): Promise { - // Because we need a Local Call Feed to establish a call connection, we avoid muting video in case of empty - // video track. In this way we go sure if a client implements muting we don't raise an error. - if (this.localCallFeed?.stream.getVideoTracks().length === 0) { - return false; - } // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - if (!muted && !(await this.client.getMediaHandler().hasVideoDevice())) { + const hasVideoDevice = await this.client + .getMediaHandler() + .hasVideoDevice() + .catch((_) => { + logger.log( + `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, + ); + return false; + }); + + if (!muted && !hasVideoDevice) { return false; } @@ -634,7 +665,17 @@ export class GroupCall extends TypedEventEmitter< `GroupCall ${this.groupCallId} setLocalVideoMuted() (stream=${this.localCallFeed.stream.id}, muted=${muted})`, ); - const stream = await this.client.getMediaHandler().getUserMediaStream(true, !muted); + const stream = await this.client + .getMediaHandler() + .getUserMediaStream(true, !muted) + .catch((_) => null); + if (stream === null) { + // if case permission denied to get a stream stop this here + logger.log( + `GroupCall ${this.groupCallId} setLocalVideoMuted() no device or permission to receive local stream, muted=${muted}`, + ); + return false; + } await this.updateLocalUsermediaStream(stream); this.localCallFeed.setAudioVideoMuted(null, muted); setTracksEnabled(this.localCallFeed.stream.getVideoTracks(), !muted); From 0d95be9e060ac2e4bc61db95c96656bd7f7afc05 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 24 Feb 2023 15:50:19 +0100 Subject: [PATCH 08/13] groupCall: switch to promise callbacks --- src/webrtc/groupCall.ts | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index d83121bf620..936c59d41ae 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -556,12 +556,15 @@ export class GroupCall extends TypedEventEmitter< const hasAudioDevice = await this.client .getMediaHandler() .hasAudioDevice() - .catch((_) => { - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, - ); - return false; - }); + .then( + (v) => v, + (_) => { + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, + ); + return false; + }, + ); if (!muted && !hasAudioDevice) { return false; @@ -608,7 +611,10 @@ export class GroupCall extends TypedEventEmitter< const stream = await this.client .getMediaHandler() .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()) - .catch((_) => null); + .then( + (s) => s, + (_) => null, + ); if (stream === null) { // if case permission denied to get a stream stop this here logger.log( @@ -649,12 +655,15 @@ export class GroupCall extends TypedEventEmitter< const hasVideoDevice = await this.client .getMediaHandler() .hasVideoDevice() - .catch((_) => { - logger.log( - `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, - ); - return false; - }); + .then( + (value) => value, + (_) => { + logger.log( + `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, + ); + return false; + }, + ); if (!muted && !hasVideoDevice) { return false; @@ -668,7 +677,10 @@ export class GroupCall extends TypedEventEmitter< const stream = await this.client .getMediaHandler() .getUserMediaStream(true, !muted) - .catch((_) => null); + .then( + (s) => s, + (_) => null, + ); if (stream === null) { // if case permission denied to get a stream stop this here logger.log( From aeaecdc7efc0c075a93fabaf3b7289c1bd16e650 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 24 Feb 2023 16:18:59 +0100 Subject: [PATCH 09/13] groupCall: switch to try catch --- src/webrtc/groupCall.ts | 92 +++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 53 deletions(-) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 936c59d41ae..c593bcfa0ca 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -553,23 +553,16 @@ export class GroupCall extends TypedEventEmitter< // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - const hasAudioDevice = await this.client - .getMediaHandler() - .hasAudioDevice() - .then( - (v) => v, - (_) => { - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, - ); - return false; - }, + try { + if (!muted && !(await this.client.getMediaHandler().hasAudioDevice())) { + return false; + } + } catch (e) { + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, ); - - if (!muted && !hasAudioDevice) { return false; } - const sendUpdatesBefore = !muted && this.isPtt; // set a timer for the maximum transmit time on PTT calls @@ -607,21 +600,25 @@ export class GroupCall extends TypedEventEmitter< ); // We needed this here to avoid an error in case user join a call without a device. - if (!muted) { - const stream = await this.client - .getMediaHandler() - .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()) - .then( - (s) => s, - (_) => null, - ); - if (stream === null) { - // if case permission denied to get a stream stop this here - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, - ); - return false; + // I can not use .then .catch functions because linter :-( + try { + if (!muted) { + const stream = await this.client + .getMediaHandler() + .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()); + if (stream === null) { + // if case permission denied to get a stream stop this here + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`, + ); + return false; + } } + } catch (e) { + logger.log( + `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, + ); + return false; } this.localCallFeed.setAudioVideoMuted(muted, null); @@ -652,20 +649,14 @@ export class GroupCall extends TypedEventEmitter< // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - const hasVideoDevice = await this.client - .getMediaHandler() - .hasVideoDevice() - .then( - (value) => value, - (_) => { - logger.log( - `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, - ); - return false; - }, + try { + if (!muted && !(await this.client.getMediaHandler().hasVideoDevice())) { + return false; + } + } catch (e) { + logger.log( + `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, ); - - if (!muted && !hasVideoDevice) { return false; } @@ -674,23 +665,18 @@ export class GroupCall extends TypedEventEmitter< `GroupCall ${this.groupCallId} setLocalVideoMuted() (stream=${this.localCallFeed.stream.id}, muted=${muted})`, ); - const stream = await this.client - .getMediaHandler() - .getUserMediaStream(true, !muted) - .then( - (s) => s, - (_) => null, - ); - if (stream === null) { - // if case permission denied to get a stream stop this here + try { + const stream = await this.client.getMediaHandler().getUserMediaStream(true, !muted); + await this.updateLocalUsermediaStream(stream); + this.localCallFeed.setAudioVideoMuted(null, muted); + setTracksEnabled(this.localCallFeed.stream.getVideoTracks(), !muted); + } catch (_) { + // No permission to video device logger.log( `GroupCall ${this.groupCallId} setLocalVideoMuted() no device or permission to receive local stream, muted=${muted}`, ); return false; } - await this.updateLocalUsermediaStream(stream); - this.localCallFeed.setAudioVideoMuted(null, muted); - setTracksEnabled(this.localCallFeed.stream.getVideoTracks(), !muted); } else { logger.log(`GroupCall ${this.groupCallId} setLocalVideoMuted() no stream muted (muted=${muted})`); this.initWithVideoMuted = muted; From c74900732dc4fce80a49393df96da129fbf86435 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Fri, 24 Feb 2023 18:11:39 +0100 Subject: [PATCH 10/13] test: filter dummy code from coverage --- src/webrtc/groupCall.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index c593bcfa0ca..fb656ee91d9 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -558,6 +558,7 @@ export class GroupCall extends TypedEventEmitter< return false; } } catch (e) { + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, ); @@ -608,6 +609,7 @@ export class GroupCall extends TypedEventEmitter< .getUserMediaStream(true, !this.localCallFeed.isVideoMuted()); if (stream === null) { // if case permission denied to get a stream stop this here + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`, ); @@ -615,6 +617,7 @@ export class GroupCall extends TypedEventEmitter< } } } catch (e) { + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`, ); @@ -654,6 +657,7 @@ export class GroupCall extends TypedEventEmitter< return false; } } catch (e) { + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, ); @@ -661,6 +665,7 @@ export class GroupCall extends TypedEventEmitter< } if (this.localCallFeed) { + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setLocalVideoMuted() (stream=${this.localCallFeed.stream.id}, muted=${muted})`, ); @@ -672,6 +677,7 @@ export class GroupCall extends TypedEventEmitter< setTracksEnabled(this.localCallFeed.stream.getVideoTracks(), !muted); } catch (_) { // No permission to video device + /* istanbul ignore next */ logger.log( `GroupCall ${this.groupCallId} setLocalVideoMuted() no device or permission to receive local stream, muted=${muted}`, ); From c6294e046ebec1c5c479d1ceda09a1f6343cafb9 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Mon, 27 Feb 2023 09:45:26 +0100 Subject: [PATCH 11/13] test: extend media mute tests --- spec/unit/webrtc/groupCall.spec.ts | 32 ++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index 8f79563788f..a78c568ad58 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -884,11 +884,43 @@ describe("Group Call", function () { expect(await groupCall.setMicrophoneMuted(false)).toBe(false); }); + it("returns false when no permission for audio device", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + jest.spyOn(mockClient.getMediaHandler(), "hasAudioDevice").mockRejectedValueOnce( + new Error("No Permission"), + ); + expect(await groupCall.setMicrophoneMuted(false)).toBe(false); + }); + + it("returns false when no permission for audio stream", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce( + new Error("No Permission"), + ); + expect(await groupCall.setMicrophoneMuted(false)).toBe(false); + }); + it("returns false when unmuting video with no video device", async () => { const groupCall = await createAndEnterGroupCall(mockClient, room); jest.spyOn(mockClient.getMediaHandler(), "hasVideoDevice").mockResolvedValue(false); expect(await groupCall.setLocalVideoMuted(false)).toBe(false); }); + + it("returns false when no permission for video device", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + jest.spyOn(mockClient.getMediaHandler(), "hasVideoDevice").mockRejectedValueOnce( + new Error("No Permission"), + ); + expect(await groupCall.setLocalVideoMuted(false)).toBe(false); + }); + + it("returns false when no permission for video stream", async () => { + const groupCall = await createAndEnterGroupCall(mockClient, room); + jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce( + new Error("No Permission"), + ); + expect(await groupCall.setLocalVideoMuted(false)).toBe(false); + }); }); describe("remote muting", () => { From 913a231eec6b8a87efe23a1ff499e154f422e63b Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Thu, 2 Mar 2023 09:43:12 +0100 Subject: [PATCH 12/13] groupCall: move permission check to device handler --- spec/unit/webrtc/groupCall.spec.ts | 16 ---------------- spec/unit/webrtc/mediaHandler.spec.ts | 10 ++++++++++ src/webrtc/groupCall.ts | 21 +++------------------ src/webrtc/mediaHandler.ts | 18 ++++++++++++++---- 4 files changed, 27 insertions(+), 38 deletions(-) diff --git a/spec/unit/webrtc/groupCall.spec.ts b/spec/unit/webrtc/groupCall.spec.ts index a78c568ad58..02e3a18b495 100644 --- a/spec/unit/webrtc/groupCall.spec.ts +++ b/spec/unit/webrtc/groupCall.spec.ts @@ -884,14 +884,6 @@ describe("Group Call", function () { expect(await groupCall.setMicrophoneMuted(false)).toBe(false); }); - it("returns false when no permission for audio device", async () => { - const groupCall = await createAndEnterGroupCall(mockClient, room); - jest.spyOn(mockClient.getMediaHandler(), "hasAudioDevice").mockRejectedValueOnce( - new Error("No Permission"), - ); - expect(await groupCall.setMicrophoneMuted(false)).toBe(false); - }); - it("returns false when no permission for audio stream", async () => { const groupCall = await createAndEnterGroupCall(mockClient, room); jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce( @@ -906,14 +898,6 @@ describe("Group Call", function () { expect(await groupCall.setLocalVideoMuted(false)).toBe(false); }); - it("returns false when no permission for video device", async () => { - const groupCall = await createAndEnterGroupCall(mockClient, room); - jest.spyOn(mockClient.getMediaHandler(), "hasVideoDevice").mockRejectedValueOnce( - new Error("No Permission"), - ); - expect(await groupCall.setLocalVideoMuted(false)).toBe(false); - }); - it("returns false when no permission for video stream", async () => { const groupCall = await createAndEnterGroupCall(mockClient, room); jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce( diff --git a/spec/unit/webrtc/mediaHandler.spec.ts b/spec/unit/webrtc/mediaHandler.spec.ts index 1b3a815b00a..f50baec1f06 100644 --- a/spec/unit/webrtc/mediaHandler.spec.ts +++ b/spec/unit/webrtc/mediaHandler.spec.ts @@ -242,6 +242,11 @@ describe("Media Handler", function () { ); expect(await mediaHandler.hasAudioDevice()).toEqual(false); }); + + it("returns false if the system not permitting access audio inputs", async () => { + mockMediaDevices.enumerateDevices.mockRejectedValueOnce(new Error("No Permission")); + expect(await mediaHandler.hasAudioDevice()).toEqual(false); + }); }); describe("hasVideoDevice", () => { @@ -255,6 +260,11 @@ describe("Media Handler", function () { ); expect(await mediaHandler.hasVideoDevice()).toEqual(false); }); + + it("returns false if the system not permitting access video inputs", async () => { + mockMediaDevices.enumerateDevices.mockRejectedValueOnce(new Error("No Permission")); + expect(await mediaHandler.hasVideoDevice()).toEqual(false); + }); }); describe("getUserMediaStream", () => { diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index fb656ee91d9..f6ab54fa2e1 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -553,17 +553,10 @@ export class GroupCall extends TypedEventEmitter< // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - try { - if (!muted && !(await this.client.getMediaHandler().hasAudioDevice())) { - return false; - } - } catch (e) { - /* istanbul ignore next */ - logger.log( - `GroupCall ${this.groupCallId} setMicrophoneMuted() no audio device or permission for audio device, muted=${muted}`, - ); + if (!muted && !(await this.client.getMediaHandler().hasAudioDevice())) { return false; } + const sendUpdatesBefore = !muted && this.isPtt; // set a timer for the maximum transmit time on PTT calls @@ -652,15 +645,7 @@ export class GroupCall extends TypedEventEmitter< // hasAudioDevice can block indefinitely if the window has lost focus, // and it doesn't make much sense to keep a device from being muted, so // we always allow muted = true changes to go through - try { - if (!muted && !(await this.client.getMediaHandler().hasVideoDevice())) { - return false; - } - } catch (e) { - /* istanbul ignore next */ - logger.log( - `GroupCall ${this.groupCallId} setLocalVideoMuted() no video device or permission for video device, muted=${muted}`, - ); + if (!muted && !(await this.client.getMediaHandler().hasVideoDevice())) { return false; } diff --git a/src/webrtc/mediaHandler.ts b/src/webrtc/mediaHandler.ts index b472b7a4b3d..b6e7d2372d6 100644 --- a/src/webrtc/mediaHandler.ts +++ b/src/webrtc/mediaHandler.ts @@ -185,13 +185,23 @@ export class MediaHandler extends TypedEventEmitter< } public async hasAudioDevice(): Promise { - const devices = await navigator.mediaDevices.enumerateDevices(); - return devices.filter((device) => device.kind === "audioinput").length > 0; + try { + const devices = await navigator.mediaDevices.enumerateDevices(); + return devices.filter((device) => device.kind === "audioinput").length > 0; + } catch (err) { + logger.log(`MediaHandler hasAudioDevice() calling navigator.mediaDevices.enumerateDevices with error`); + return false; + } } public async hasVideoDevice(): Promise { - const devices = await navigator.mediaDevices.enumerateDevices(); - return devices.filter((device) => device.kind === "videoinput").length > 0; + try { + const devices = await navigator.mediaDevices.enumerateDevices(); + return devices.filter((device) => device.kind === "videoinput").length > 0; + } catch (err) { + logger.log(`MediaHandler hasVideoDevice() calling navigator.mediaDevices.enumerateDevices with error`); + return false; + } } /** From f7f00843a17d97ec9d70f7cb492254c4bdf6b5f8 Mon Sep 17 00:00:00 2001 From: Enrico Schwendig Date: Thu, 2 Mar 2023 10:42:23 +0100 Subject: [PATCH 13/13] mediaHandler: add error in log statement --- src/webrtc/mediaHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webrtc/mediaHandler.ts b/src/webrtc/mediaHandler.ts index b6e7d2372d6..7f65835d18f 100644 --- a/src/webrtc/mediaHandler.ts +++ b/src/webrtc/mediaHandler.ts @@ -189,7 +189,7 @@ export class MediaHandler extends TypedEventEmitter< const devices = await navigator.mediaDevices.enumerateDevices(); return devices.filter((device) => device.kind === "audioinput").length > 0; } catch (err) { - logger.log(`MediaHandler hasAudioDevice() calling navigator.mediaDevices.enumerateDevices with error`); + logger.log(`MediaHandler hasAudioDevice() calling navigator.mediaDevices.enumerateDevices with error`, err); return false; } } @@ -199,7 +199,7 @@ export class MediaHandler extends TypedEventEmitter< const devices = await navigator.mediaDevices.enumerateDevices(); return devices.filter((device) => device.kind === "videoinput").length > 0; } catch (err) { - logger.log(`MediaHandler hasVideoDevice() calling navigator.mediaDevices.enumerateDevices with error`); + logger.log(`MediaHandler hasVideoDevice() calling navigator.mediaDevices.enumerateDevices with error`, err); return false; } }