Put "is speaking?" threshold in remote config; lower default

signalapp · Mar 1, 2022 · cfa0711 · cfa0711
1 parent 2b0c98f
commit cfa0711
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<number>();
     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
@@ -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,