From 41b4cce6ec98516c2182c9ae38b19d95156a05db Mon Sep 17 00:00:00 2001 From: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com> Date: Fri, 25 Feb 2022 09:24:05 -0600 Subject: [PATCH] Show local speaking indicator for group calls --- stylesheets/_modules.scss | 9 +----- ts/calling/constants.ts | 5 ++- ts/components/CallManager.stories.tsx | 1 + ts/components/CallScreen.stories.tsx | 2 ++ ts/components/CallScreen.tsx | 13 ++++---- ts/components/CallingAudioIndicator.tsx | 32 ++++++++++++++----- ts/components/CallingPip.stories.tsx | 1 + ts/components/GroupCallRemoteParticipant.tsx | 2 +- ts/services/calling.ts | 7 ++-- ts/state/ducks/calling.ts | 26 +++++++++++++-- ts/state/smart/CallManager.tsx | 1 + ts/test-electron/state/ducks/calling_test.ts | 14 +++++++- .../state/selectors/calling_test.ts | 3 +- ts/types/Calling.ts | 3 +- 14 files changed, 87 insertions(+), 32 deletions(-) diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index 1603cede04..e1059b5c4e 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -4291,17 +4291,10 @@ button.module-image__border-overlay:focus { } } - &--audio-muted::before { - @include color-svg( - '../images/icons/v2/mic-off-solid-28.svg', - $color-white - ); + .CallingAudioIndicator { bottom: 6px; - content: ''; - height: 14px; position: absolute; right: 6px; - width: 14px; z-index: $z-index-base; } } diff --git a/ts/calling/constants.ts b/ts/calling/constants.ts index 1c7b1a6ba0..c3c3f9eb97 100644 --- a/ts/calling/constants.ts +++ b/ts/calling/constants.ts @@ -1,6 +1,9 @@ -// Copyright 2020-2021 Signal Messenger, LLC +// Copyright 2020-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +export const AUDIO_LEVEL_INTERVAL_MS = 250; +export const AUDIO_LEVEL_FOR_SPEAKING = 0.25; + export const REQUESTED_VIDEO_WIDTH = 640; export const REQUESTED_VIDEO_HEIGHT = 480; export const REQUESTED_VIDEO_FRAMERATE = 30; diff --git a/ts/components/CallManager.stories.tsx b/ts/components/CallManager.stories.tsx index 2428bfad0b..8d97fcd5c2 100644 --- a/ts/components/CallManager.stories.tsx +++ b/ts/components/CallManager.stories.tsx @@ -50,6 +50,7 @@ const getCommonActiveCallData = () => ({ joinedAt: Date.now(), hasLocalAudio: boolean('hasLocalAudio', true), hasLocalVideo: boolean('hasLocalVideo', false), + amISpeaking: boolean('amISpeaking', false), isInSpeakerView: boolean('isInSpeakerView', false), outgoingRing: boolean('outgoingRing', true), pip: boolean('pip', false), diff --git a/ts/components/CallScreen.stories.tsx b/ts/components/CallScreen.stories.tsx index 6b17704b91..2e23541d25 100644 --- a/ts/components/CallScreen.stories.tsx +++ b/ts/components/CallScreen.stories.tsx @@ -44,6 +44,7 @@ const conversation = getDefaultConversation({ type OverridePropsBase = { hasLocalAudio?: boolean; hasLocalVideo?: boolean; + amISpeaking?: boolean; isInSpeakerView?: boolean; }; @@ -120,6 +121,7 @@ const createActiveCallProp = ( 'hasLocalVideo', overrideProps.hasLocalVideo || false ), + amISpeaking: boolean('amISpeaking', overrideProps.amISpeaking || false), isInSpeakerView: boolean( 'isInSpeakerView', overrideProps.isInSpeakerView || false diff --git a/ts/components/CallScreen.tsx b/ts/components/CallScreen.tsx index d078e2d2f2..1238a4ad16 100644 --- a/ts/components/CallScreen.tsx +++ b/ts/components/CallScreen.tsx @@ -38,6 +38,7 @@ import { NeedsScreenRecordingPermissionsModal } from './NeedsScreenRecordingPerm import { missingCaseError } from '../util/missingCaseError'; import * as KeyboardLayout from '../services/keyboardLayout'; import { useActivateSpeakerViewOnPresenting } from '../hooks/useActivateSpeakerViewOnPresenting'; +import { CallingAudioIndicator } from './CallingAudioIndicator'; export type PropsType = { activeCall: ActiveCallType; @@ -125,6 +126,7 @@ export const CallScreen: React.FC = ({ conversation, hasLocalAudio, hasLocalVideo, + amISpeaking, isInSpeakerView, presentingSource, remoteParticipants, @@ -501,13 +503,12 @@ export const CallScreen: React.FC = ({ onClick={hangUpActiveCall} /> -
+
{localPreviewNode} +
diff --git a/ts/components/CallingAudioIndicator.tsx b/ts/components/CallingAudioIndicator.tsx index 31487b571b..26d495bd03 100644 --- a/ts/components/CallingAudioIndicator.tsx +++ b/ts/components/CallingAudioIndicator.tsx @@ -1,25 +1,41 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only +import { noop } from 'lodash'; import type { ReactElement } from 'react'; -import React from 'react'; +import React, { useEffect, useState } from 'react'; import animationData from '../../images/lottie-animations/CallingSpeakingIndicator.json'; import { Lottie } from './Lottie'; +const SPEAKING_LINGER_MS = 100; + export function CallingAudioIndicator({ - hasRemoteAudio, + hasAudio, isSpeaking, -}: Readonly<{ - hasRemoteAudio: boolean; - isSpeaking: boolean; -}>): ReactElement { - if (!hasRemoteAudio) { +}: Readonly<{ hasAudio: boolean; isSpeaking: boolean }>): ReactElement { + const [shouldShowSpeaking, setShouldShowSpeaking] = useState(isSpeaking); + + useEffect(() => { + if (isSpeaking) { + setShouldShowSpeaking(true); + } else if (shouldShowSpeaking) { + const timeout = setTimeout(() => { + setShouldShowSpeaking(false); + }, SPEAKING_LINGER_MS); + return () => { + clearTimeout(timeout); + }; + } + return noop; + }, [isSpeaking, shouldShowSpeaking]); + + if (!hasAudio) { return (
); } - if (isSpeaking) { + if (shouldShowSpeaking) { return ( ); diff --git a/ts/components/CallingPip.stories.tsx b/ts/components/CallingPip.stories.tsx index 0457fa4f0e..14006ed1a0 100644 --- a/ts/components/CallingPip.stories.tsx +++ b/ts/components/CallingPip.stories.tsx @@ -39,6 +39,7 @@ const getCommonActiveCallData = () => ({ conversation, hasLocalAudio: boolean('hasLocalAudio', true), hasLocalVideo: boolean('hasLocalVideo', false), + amISpeaking: boolean('amISpeaking', false), isInSpeakerView: boolean('isInSpeakerView', false), joinedAt: Date.now(), outgoingRing: true, diff --git a/ts/components/GroupCallRemoteParticipant.tsx b/ts/components/GroupCallRemoteParticipant.tsx index 543fdd530f..92440794e1 100644 --- a/ts/components/GroupCallRemoteParticipant.tsx +++ b/ts/components/GroupCallRemoteParticipant.tsx @@ -285,7 +285,7 @@ export const GroupCallRemoteParticipant: React.FC = React.memo( title={title} />
diff --git a/ts/services/calling.ts b/ts/services/calling.ts index 4df018c73a..ef168f61cc 100644 --- a/ts/services/calling.ts +++ b/ts/services/calling.ts @@ -82,6 +82,7 @@ import type { ProcessedEnvelope } from '../textsecure/Types.d'; import { missingCaseError } from '../util/missingCaseError'; import { normalizeGroupCallTimestamp } from '../util/ringrtc/normalizeGroupCallTimestamp'; import { + AUDIO_LEVEL_INTERVAL_MS, REQUESTED_VIDEO_WIDTH, REQUESTED_VIDEO_HEIGHT, REQUESTED_VIDEO_FRAMERATE, @@ -624,7 +625,7 @@ export class CallingClass { groupIdBuffer, this.sfuUrl, Buffer.alloc(0), - 500, + AUDIO_LEVEL_INTERVAL_MS, { onLocalDeviceStateChanged: groupCall => { const localDeviceState = groupCall.getLocalDeviceState(); @@ -677,9 +678,11 @@ export class CallingClass { if (!remoteDeviceStates) { return; } + const localAudioLevel = groupCall.getLocalDeviceState().audioLevel; this.uxActions?.groupCallAudioLevelsChange({ conversationId, + localAudioLevel, remoteDeviceStates, }); }, @@ -1971,7 +1974,7 @@ export class CallingClass { hideIp: shouldRelayCalls || isContactUnknown, bandwidthMode: BandwidthMode.Normal, // TODO: DESKTOP-3101 - // audioLevelsIntervalMillis: 500, + // audioLevelsIntervalMillis: AUDIO_LEVEL_INTERVAL_MS, }; } diff --git a/ts/state/ducks/calling.ts b/ts/state/ducks/calling.ts index 7804e34a89..c19b246c1b 100644 --- a/ts/state/ducks/calling.ts +++ b/ts/state/ducks/calling.ts @@ -15,6 +15,7 @@ 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, @@ -102,6 +103,7 @@ export type ActiveCallStateType = { conversationId: string; hasLocalAudio: boolean; hasLocalVideo: boolean; + amISpeaking: boolean; isInSpeakerView: boolean; joinedAt?: number; outgoingRing: boolean; @@ -463,6 +465,7 @@ type DeclineCallActionType = { type GroupCallAudioLevelsChangeActionPayloadType = Readonly<{ conversationId: string; + localAudioLevel: number; remoteDeviceStates: ReadonlyArray<{ audioLevel: number; demuxId: number }>; }>; @@ -1431,6 +1434,7 @@ export function reducer( conversationId: action.payload.conversationId, hasLocalAudio: action.payload.hasLocalAudio, hasLocalVideo: action.payload.hasLocalVideo, + amISpeaking: false, isInSpeakerView: false, pip: false, safetyNumberChangedUuids: [], @@ -1458,6 +1462,7 @@ export function reducer( conversationId: action.payload.conversationId, hasLocalAudio: action.payload.hasLocalAudio, hasLocalVideo: action.payload.hasLocalVideo, + amISpeaking: false, isInSpeakerView: false, pip: false, safetyNumberChangedUuids: [], @@ -1480,6 +1485,7 @@ export function reducer( conversationId: action.payload.conversationId, hasLocalAudio: true, hasLocalVideo: action.payload.asVideoCall, + amISpeaking: false, isInSpeakerView: false, pip: false, safetyNumberChangedUuids: [], @@ -1633,6 +1639,7 @@ export function reducer( conversationId: action.payload.conversationId, hasLocalAudio: action.payload.hasLocalAudio, hasLocalVideo: action.payload.hasLocalVideo, + amISpeaking: false, isInSpeakerView: false, pip: false, safetyNumberChangedUuids: [], @@ -1690,24 +1697,36 @@ export function reducer( } if (action.type === GROUP_CALL_AUDIO_LEVELS_CHANGE) { - const { conversationId, remoteDeviceStates } = action.payload; + const { conversationId, localAudioLevel, remoteDeviceStates } = + action.payload; + const { activeCallState } = state; const existingCall = getGroupCall(conversationId, state); - if (!existingCall) { + + // The PiP check is an optimization. We don't need to update audio levels if the user + // cannot see them. + if (!activeCallState || activeCallState.pip || !existingCall) { return state; } + const amISpeaking = localAudioLevel > AUDIO_LEVEL_FOR_SPEAKING; + 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 > 0.25) { + if ( + typeof audioLevel === 'number' && + audioLevel > AUDIO_LEVEL_FOR_SPEAKING + ) { speakingDemuxIds.add(demuxId); } }); // This action is dispatched frequently. This equality check helps avoid re-renders. + const oldAmISpeaking = activeCallState.amISpeaking; const oldSpeakingDemuxIds = existingCall.speakingDemuxIds; if ( + oldAmISpeaking === amISpeaking && oldSpeakingDemuxIds && setUtil.isEqual(oldSpeakingDemuxIds, speakingDemuxIds) ) { @@ -1716,6 +1735,7 @@ export function reducer( return { ...state, + activeCallState: { ...activeCallState, amISpeaking }, callsByConversation: { ...callsByConversation, [conversationId]: { ...existingCall, speakingDemuxIds }, diff --git a/ts/state/smart/CallManager.tsx b/ts/state/smart/CallManager.tsx index c951ae7504..0fdd6b06f8 100644 --- a/ts/state/smart/CallManager.tsx +++ b/ts/state/smart/CallManager.tsx @@ -130,6 +130,7 @@ const mapStateToActiveCallProp = ( conversation, hasLocalAudio: activeCallState.hasLocalAudio, hasLocalVideo: activeCallState.hasLocalVideo, + amISpeaking: activeCallState.amISpeaking, isInSpeakerView: activeCallState.isInSpeakerView, joinedAt: activeCallState.joinedAt, outgoingRing: activeCallState.outgoingRing, diff --git a/ts/test-electron/state/ducks/calling_test.ts b/ts/test-electron/state/ducks/calling_test.ts index 4ac89ad32e..0bb3491e32 100644 --- a/ts/test-electron/state/ducks/calling_test.ts +++ b/ts/test-electron/state/ducks/calling_test.ts @@ -50,6 +50,7 @@ describe('calling duck', () => { conversationId: 'fake-direct-call-conversation-id', hasLocalAudio: true, hasLocalVideo: false, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -128,6 +129,7 @@ describe('calling duck', () => { conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, hasLocalVideo: false, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -433,6 +435,7 @@ describe('calling duck', () => { conversationId: 'fake-direct-call-conversation-id', hasLocalAudio: true, hasLocalVideo: true, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -525,6 +528,7 @@ describe('calling duck', () => { conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, hasLocalVideo: true, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -762,6 +766,7 @@ describe('calling duck', () => { it("does nothing if there's no relevant call", () => { const action = groupCallAudioLevelsChange({ conversationId: 'garbage', + localAudioLevel: 1, remoteDeviceStates, }); @@ -784,6 +789,7 @@ describe('calling duck', () => { }; const action = groupCallAudioLevelsChange({ conversationId: 'fake-group-call-conversation-id', + localAudioLevel: 0.1, remoteDeviceStates, }); @@ -792,13 +798,16 @@ describe('calling duck', () => { assert.strictEqual(result, state); }); - it('updates the set of speaking participants', () => { + it('updates the set of speaking participants, including yourself', () => { const action = groupCallAudioLevelsChange({ conversationId: 'fake-group-call-conversation-id', + localAudioLevel: 0.8, remoteDeviceStates, }); const result = reducer(stateWithActiveGroupCall, action); + assert.isTrue(result.activeCallState?.amISpeaking); + const call = result.callsByConversation['fake-group-call-conversation-id']; if (call?.callMode !== CallMode.Group) { @@ -1100,6 +1109,7 @@ describe('calling duck', () => { conversationId: 'fake-group-call-conversation-id', hasLocalAudio: true, hasLocalVideo: false, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -1628,6 +1638,7 @@ describe('calling duck', () => { conversationId: 'fake-conversation-id', hasLocalAudio: true, hasLocalVideo: true, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], @@ -1913,6 +1924,7 @@ describe('calling duck', () => { conversationId: 'fake-conversation-id', hasLocalAudio: true, hasLocalVideo: false, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], diff --git a/ts/test-electron/state/selectors/calling_test.ts b/ts/test-electron/state/selectors/calling_test.ts index a7a10587b6..16ce8d3745 100644 --- a/ts/test-electron/state/selectors/calling_test.ts +++ b/ts/test-electron/state/selectors/calling_test.ts @@ -1,4 +1,4 @@ -// Copyright 2019-2021 Signal Messenger, LLC +// Copyright 2019-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only import { assert } from 'chai'; @@ -51,6 +51,7 @@ describe('state/selectors/calling', () => { conversationId: 'fake-direct-call-conversation-id', hasLocalAudio: true, hasLocalVideo: false, + amISpeaking: false, isInSpeakerView: false, showParticipantsList: false, safetyNumberChangedUuids: [], diff --git a/ts/types/Calling.ts b/ts/types/Calling.ts index 1ea6057a1f..7825ec99fc 100644 --- a/ts/types/Calling.ts +++ b/ts/types/Calling.ts @@ -1,4 +1,4 @@ -// Copyright 2020-2021 Signal Messenger, LLC +// Copyright 2020-2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only import type { AudioDevice } from 'ringrtc'; @@ -28,6 +28,7 @@ type ActiveCallBaseType = { conversation: ConversationType; hasLocalAudio: boolean; hasLocalVideo: boolean; + amISpeaking: boolean; isInSpeakerView: boolean; isSharingScreen?: boolean; joinedAt?: number;