From cfa0711909e227e55b44814837de720ee5b8c285 Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Tue, 1 Mar 2022 17:39:09 -0600 Subject: [PATCH] Put "is speaking?" threshold in remote config; lower default --- ts/RemoteConfig.ts | 1 + ts/calling/constants.ts | 2 +- ts/calling/getAudioLevelForSpeaking.ts | 20 ++++++++++++++++++++ ts/services/calling.ts | 5 +++++ ts/state/ducks/calling.ts | 14 +++++++++----- ts/test-electron/state/ducks/calling_test.ts | 3 +++ 6 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 ts/calling/getAudioLevelForSpeaking.ts diff --git a/ts/RemoteConfig.ts b/ts/RemoteConfig.ts index 1834a7d69d..cf7feefafc 100644 --- a/ts/RemoteConfig.ts +++ b/ts/RemoteConfig.ts @@ -8,6 +8,7 @@ import * as log from './logging/log'; export type ConfigKeyType = | 'desktop.announcementGroup' + | 'desktop.calling.audioLevelForSpeaking' | 'desktop.clientExpiration' | 'desktop.groupCallOutboundRing' | 'desktop.internalUser' diff --git a/ts/calling/constants.ts b/ts/calling/constants.ts index c3c3f9eb97..8de9f53ca4 100644 --- a/ts/calling/constants.ts +++ b/ts/calling/constants.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-only export const AUDIO_LEVEL_INTERVAL_MS = 250; -export const AUDIO_LEVEL_FOR_SPEAKING = 0.25; +export const DEFAULT_AUDIO_LEVEL_FOR_SPEAKING = 0.15; export const REQUESTED_VIDEO_WIDTH = 640; export const REQUESTED_VIDEO_HEIGHT = 480; diff --git a/ts/calling/getAudioLevelForSpeaking.ts b/ts/calling/getAudioLevelForSpeaking.ts new file mode 100644 index 0000000000..543f0707e8 --- /dev/null +++ b/ts/calling/getAudioLevelForSpeaking.ts @@ -0,0 +1,20 @@ +// Copyright 2022 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import type * as RemoteConfig from '../RemoteConfig'; +import { DEFAULT_AUDIO_LEVEL_FOR_SPEAKING } from './constants'; + +export function getAudioLevelForSpeaking( + getValueFromRemoteConfig: typeof RemoteConfig.getValue +): number { + const configValue = getValueFromRemoteConfig( + 'desktop.calling.audioLevelForSpeaking' + ); + if (typeof configValue !== 'string') { + return DEFAULT_AUDIO_LEVEL_FOR_SPEAKING; + } + + const result = parseFloat(configValue); + const isResultValid = result > 0 && result <= 1; + return isResultValid ? result : DEFAULT_AUDIO_LEVEL_FOR_SPEAKING; +} diff --git a/ts/services/calling.ts b/ts/services/calling.ts index ae5f9cf624..e93f89216f 100644 --- a/ts/services/calling.ts +++ b/ts/services/calling.ts @@ -37,6 +37,7 @@ import { } from 'ringrtc'; import { uniqBy, noop } from 'lodash'; +import * as RemoteConfig from '../RemoteConfig'; import type { ActionsType as UxActionsType, GroupCallParticipantInfoType, @@ -62,6 +63,7 @@ import { getAudioDeviceModule, parseAudioDeviceModule, } from '../calling/audioDeviceModule'; +import { getAudioLevelForSpeaking } from '../calling/getAudioLevelForSpeaking'; import { findBestMatchingAudioDeviceIndex, findBestMatchingCameraId, @@ -682,6 +684,9 @@ export class CallingClass { const localAudioLevel = groupCall.getLocalDeviceState().audioLevel; this.uxActions?.groupCallAudioLevelsChange({ + audioLevelForSpeaking: getAudioLevelForSpeaking( + RemoteConfig.getValue + ), conversationId, localAudioLevel, remoteDeviceStates, diff --git a/ts/state/ducks/calling.ts b/ts/state/ducks/calling.ts index c19b246c1b..62a571a410 100644 --- a/ts/state/ducks/calling.ts +++ b/ts/state/ducks/calling.ts @@ -15,7 +15,6 @@ import { getPlatform } from '../selectors/user'; import { isConversationTooBigToRing } from '../../conversations/isConversationTooBigToRing'; import { missingCaseError } from '../../util/missingCaseError'; import { calling } from '../../services/calling'; -import { AUDIO_LEVEL_FOR_SPEAKING } from '../../calling/constants'; import type { StateType as RootStateType } from '../reducer'; import type { ChangeIODevicePayloadType, @@ -464,6 +463,7 @@ type DeclineCallActionType = { }; type GroupCallAudioLevelsChangeActionPayloadType = Readonly<{ + audioLevelForSpeaking: number; conversationId: string; localAudioLevel: number; remoteDeviceStates: ReadonlyArray<{ audioLevel: number; demuxId: number }>; @@ -1697,8 +1697,12 @@ export function reducer( } if (action.type === GROUP_CALL_AUDIO_LEVELS_CHANGE) { - const { conversationId, localAudioLevel, remoteDeviceStates } = - action.payload; + const { + audioLevelForSpeaking, + conversationId, + localAudioLevel, + remoteDeviceStates, + } = action.payload; const { activeCallState } = state; const existingCall = getGroupCall(conversationId, state); @@ -1709,14 +1713,14 @@ export function reducer( return state; } - const amISpeaking = localAudioLevel > AUDIO_LEVEL_FOR_SPEAKING; + const amISpeaking = localAudioLevel > audioLevelForSpeaking; const speakingDemuxIds = new Set(); remoteDeviceStates.forEach(({ audioLevel, demuxId }) => { // We expect `audioLevel` to be a number but have this check just in case. if ( typeof audioLevel === 'number' && - audioLevel > AUDIO_LEVEL_FOR_SPEAKING + audioLevel > audioLevelForSpeaking ) { speakingDemuxIds.add(demuxId); } diff --git a/ts/test-electron/state/ducks/calling_test.ts b/ts/test-electron/state/ducks/calling_test.ts index 0bb3491e32..497c5551f0 100644 --- a/ts/test-electron/state/ducks/calling_test.ts +++ b/ts/test-electron/state/ducks/calling_test.ts @@ -765,6 +765,7 @@ describe('calling duck', () => { it("does nothing if there's no relevant call", () => { const action = groupCallAudioLevelsChange({ + audioLevelForSpeaking: 0.25, conversationId: 'garbage', localAudioLevel: 1, remoteDeviceStates, @@ -788,6 +789,7 @@ describe('calling duck', () => { }, }; const action = groupCallAudioLevelsChange({ + audioLevelForSpeaking: 0.25, conversationId: 'fake-group-call-conversation-id', localAudioLevel: 0.1, remoteDeviceStates, @@ -800,6 +802,7 @@ describe('calling duck', () => { it('updates the set of speaking participants, including yourself', () => { const action = groupCallAudioLevelsChange({ + audioLevelForSpeaking: 0.25, conversationId: 'fake-group-call-conversation-id', localAudioLevel: 0.8, remoteDeviceStates,