From 2012b0984515595402ed6d292542b9f89f017040 Mon Sep 17 00:00:00 2001 From: Timo Date: Fri, 16 May 2025 11:32:32 +0200 Subject: [PATCH] review cleanup --- docs/controls.md | 2 +- src/controls.ts | 6 +-- src/livekit/MediaDevicesContext.tsx | 60 +++++++++++++---------------- src/room/GroupCallView.tsx | 13 ++----- src/room/InCallView.tsx | 11 ++---- src/settings/settings.ts | 2 +- src/state/MuteAllAudioModel.ts | 22 +++++++++++ src/useAudioContext.test.tsx | 2 +- 8 files changed, 61 insertions(+), 57 deletions(-) create mode 100644 src/state/MuteAllAudioModel.ts diff --git a/docs/controls.md b/docs/controls.md index 4405a9f3..7e84d78d 100644 --- a/docs/controls.md +++ b/docs/controls.md @@ -13,7 +13,7 @@ A few aspects of Element Call's interface can be controlled through a global API These functions must be used in conjunction with the `controlledOutput` URL parameter in order to have any effect. - `controls.setAvailableOutputDevices(devices: { id: string, name: string, forEarpiece?: boolean }[]): void` Sets the list of available audio outputs. `forEarpiece` is used on ios only. - It flags the device that should be used if the user selects earpice mode. This should be the main (stereo loudspeaker) of the device. + It flags the device that should be used if the user selects earpiece mode. This should be the main stereo loudspeaker of the device. - `controls.onOutputDeviceSelect: ((id: string) => void) | undefined` Callback called whenever the user or application selects a new audio output. - `controls.setOutputDevice(id: string): void` Sets the selected audio device in EC menu. This should be used if the os decides to automatically switch to bluetooth. - `controls.setOutputEnabled(enabled: boolean)` Enables/disables all audio output from the application. This can be useful for temporarily pausing audio while the controlling application is switching output devices. Output is enabled by default. diff --git a/src/controls.ts b/src/controls.ts index bff5f47d..cc45f881 100644 --- a/src/controls.ts +++ b/src/controls.ts @@ -26,7 +26,7 @@ export interface OutputDevice { export const setPipEnabled$ = new Subject(); export const setAvailableOutputDevices$ = new Subject(); export const setOutputDevice$ = new Subject(); -export const setOutputDisabled$ = new Subject(); +export const setOutputEnabled$ = new Subject(); window.controls = { canEnterPip(): boolean { @@ -51,8 +51,8 @@ window.controls = { setOutputDevice$.next(id); }, setOutputEnabled(enabled: boolean): void { - if (!setOutputDisabled$.observed) + if (!setOutputEnabled$.observed) throw new Error("Output controls are disabled"); - setOutputDisabled$.next(!enabled); + setOutputEnabled$.next(!enabled); }, }; diff --git a/src/livekit/MediaDevicesContext.tsx b/src/livekit/MediaDevicesContext.tsx index 319f492c..4260e040 100644 --- a/src/livekit/MediaDevicesContext.tsx +++ b/src/livekit/MediaDevicesContext.tsx @@ -77,6 +77,27 @@ export interface MediaDevices extends Omit { audioOutput: MediaDeviceHandle; } +function useSelectedId( + available: Map, + preferredId: string | undefined, +): string | undefined { + return useMemo(() => { + if (available.size) { + // If the preferred device is available, use it. Or if every available + // device ID is falsy, the browser is probably just being paranoid about + // fingerprinting and we should still try using the preferred device. + // Worst case it is not available and the browser will gracefully fall + // back to some other device for us when requesting the media stream. + // Otherwise, select the first available device. + return (preferredId !== undefined && available.has(preferredId)) || + (available.size === 1 && available.has("")) + ? preferredId + : available.keys().next().value; + } + return undefined; + }, [available, preferredId]); +} + /** * Hook to get access to a mediaDevice handle for a kind. This allows to list * the available devices, read and set the selected device. @@ -84,17 +105,17 @@ export interface MediaDevices extends Omit { * @param setting The setting this handles selection should be synced with. * @param usingNames If the hook should query device names for the associated * list. - * @returns A handle for the choosen kind. + * @returns A handle for the chosen kind. */ function useMediaDeviceHandle( kind: MediaDeviceKind, setting: Setting, usingNames: boolean, ): MediaDeviceHandle { - // Make sure we don't needlessly reset to a device observer without names, - // once permissions are already given const hasRequestedPermissions = useRef(false); const requestPermissions = usingNames || hasRequestedPermissions.current; + // Make sure we don't needlessly reset to a device observer without names, + // once permissions are already given hasRequestedPermissions.current ||= usingNames; // We use a bare device observer here rather than one of the fancy device @@ -153,22 +174,7 @@ function useMediaDeviceHandle( ); const [preferredId, select] = useSetting(setting); - const selectedId = useMemo(() => { - if (available.size) { - // If the preferred device is available, use it. Or if every available - // device ID is falsy, the browser is probably just being paranoid about - // fingerprinting and we should still try using the preferred device. - // Worst case it is not available and the browser will gracefully fall - // back to some other device for us when requesting the media stream. - // Otherwise, select the first available device. - return (preferredId !== undefined && available.has(preferredId)) || - (available.size === 1 && available.has("")) - ? preferredId - : available.keys().next().value; - } - return undefined; - }, [available, preferredId]); - + const selectedId = useSelectedId(available, preferredId); const selectedGroupId = useObservableEagerState( useMemo( () => @@ -337,21 +343,7 @@ function useControlledOutput(): MediaDeviceHandle { setOutputDevice$.subscribe((id) => setPreferredId(id)); }, [setPreferredId]); - const selectedId = useMemo(() => { - if (available.size) { - // If the preferred device is available, use it. Or if every available - // device ID is falsy, the browser is probably just being paranoid about - // fingerprinting and we should still try using the preferred device. - // Worst case it is not available and the browser will gracefully fall - // back to some other device for us when requesting the media stream. - // Otherwise, select the first available device. - return (preferredId !== undefined && available.has(preferredId)) || - (available.size === 1 && available.has("")) - ? preferredId - : available.keys().next().value; - } - return undefined; - }, [available, preferredId]); + const selectedId = useSelectedId(available, preferredId); const [asEarpice, setAsEarpiece] = useState(false); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index ef63be35..a9a421d9 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -24,8 +24,7 @@ import { type MatrixRTCSession, } from "matrix-js-sdk/lib/matrixrtc"; import { useNavigate } from "react-router-dom"; -import { useObservable, useObservableEagerState } from "observable-hooks"; -import { startWith } from "rxjs"; +import { useObservableEagerState } from "observable-hooks"; import type { IWidgetApiRequest } from "matrix-widget-api"; import { @@ -66,11 +65,10 @@ import { GroupCallErrorBoundary } from "./GroupCallErrorBoundary.tsx"; import { useNewMembershipManager as useNewMembershipManagerSetting, useExperimentalToDeviceTransport as useExperimentalToDeviceTransportSetting, - muteAllAudio as muteAllAudioSetting, useSetting, } from "../settings/settings"; import { useTypedEventEmitter } from "../useEvents"; -import { setOutputDisabled$ } from "../controls.ts"; +import { muteAllAudio$ } from "../state/MuteAllAudioModel.ts"; declare global { interface Window { @@ -107,12 +105,9 @@ export const GroupCallView: FC = ({ const [externalError, setExternalError] = useState( null, ); - const muteAllAudioControlled = useObservableEagerState( - useObservable(() => setOutputDisabled$.pipe(startWith(false))), - ); - const [muteAllAudioFromSetting] = useSetting(muteAllAudioSetting); - const muteAllAudio = muteAllAudioControlled || muteAllAudioFromSetting; const memberships = useMatrixRTCSessionMemberships(rtcSession); + + const muteAllAudio = useObservableEagerState(muteAllAudio$); const leaveSoundContext = useLatest( useAudioContext({ sounds: callEventAudioSounds, diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index b1eb10b3..de7a38ef 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -25,7 +25,7 @@ import { import useMeasure from "react-use-measure"; import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; import classNames from "classnames"; -import { BehaviorSubject, map, startWith } from "rxjs"; +import { BehaviorSubject, map } from "rxjs"; import { useObservable, useObservableEagerState } from "observable-hooks"; import { logger } from "matrix-js-sdk/lib/logger"; import { RoomAndToDeviceEvents } from "matrix-js-sdk/lib/matrixrtc/RoomAndToDeviceKeyTransport"; @@ -96,7 +96,6 @@ import { CallEventAudioRenderer } from "./CallEventAudioRenderer"; import { debugTileLayout as debugTileLayoutSetting, useExperimentalToDeviceTransport as useExperimentalToDeviceTransportSetting, - muteAllAudio as muteAllAudioSetting, developerMode as developerModeSetting, useSetting, } from "../settings/settings"; @@ -104,7 +103,7 @@ import { ReactionsReader } from "../reactions/ReactionsReader"; import { ConnectionLostError } from "../utils/errors.ts"; import { useTypedEventEmitter } from "../useEvents.ts"; import { MatrixAudioRenderer } from "../livekit/MatrixAudioRenderer.tsx"; -import { setOutputDisabled$ } from "../controls.ts"; +import { muteAllAudio$ } from "../state/MuteAllAudioModel.ts"; const canScreenshare = "getDisplayMedia" in (navigator.mediaDevices ?? {}); @@ -223,11 +222,7 @@ export const InCallView: FC = ({ room: livekitRoom, }); - const muteAllAudioControlled = useObservableEagerState( - useObservable(() => setOutputDisabled$.pipe(startWith(false))), - ); - const [muteAllAudioFromSetting] = useSetting(muteAllAudioSetting); - const muteAllAudio = muteAllAudioControlled || muteAllAudioFromSetting; + const muteAllAudio = useObservableEagerState(muteAllAudio$); // This seems like it might be enough logic to use move it into the call view model? const [didFallbackToRoomKey, setDidFallbackToRoomKey] = useState(false); diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 0c7b9191..50e70671 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -133,6 +133,6 @@ export const muteAllAudio = new Setting("mute-all-audio", false); export const alwaysShowSelf = new Setting("always-show-self", true); export const alwaysShowIphoneEarpiece = new Setting( - "always-show-iphone-earpice", + "always-show-iphone-earpiece", false, ); diff --git a/src/state/MuteAllAudioModel.ts b/src/state/MuteAllAudioModel.ts new file mode 100644 index 00000000..22365e0a --- /dev/null +++ b/src/state/MuteAllAudioModel.ts @@ -0,0 +1,22 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE in the repository root for full details. +*/ + +import { combineLatest, map, startWith } from "rxjs"; + +import { setOutputEnabled$ } from "../controls"; +import { muteAllAudio as muteAllAudioSetting } from "../settings/settings"; + +/** + * This can transition into sth more complete: `GroupCallViewModel.ts` + */ +export const muteAllAudio$ = combineLatest([ + setOutputEnabled$, + muteAllAudioSetting.value$, +]).pipe( + startWith([false, muteAllAudioSetting.getValue()]), + map(([outputEndabled, settingsMute]) => !outputEndabled || settingsMute), +); diff --git a/src/useAudioContext.test.tsx b/src/useAudioContext.test.tsx index dd3c3b0c..f2e2efdb 100644 --- a/src/useAudioContext.test.tsx +++ b/src/useAudioContext.test.tsx @@ -140,7 +140,7 @@ test("will use the correct volume level", async () => { expect(testAudioContext.pan.pan.setValueAtTime).toHaveBeenCalledWith(0, 0); }); -test("will use the pan if earpice is selected", async () => { +test("will use the pan if earpiece is selected", async () => { const { findByText } = render(