-
Notifications
You must be signed in to change notification settings - Fork 133
Fix: Camera is not muted when the earpiece mode is enabled #3596
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: livekit
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,212 @@ | ||
| /* | ||
| Copyright 2025 Element Creations Ltd. | ||
|
|
||
| SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial | ||
| Please see LICENSE in the repository root for full details. | ||
| */ | ||
|
|
||
| import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; | ||
| import { BehaviorSubject } from "rxjs"; | ||
| import { logger } from "matrix-js-sdk/lib/logger"; | ||
|
|
||
| import { MuteStates, MuteState } from "./MuteStates"; | ||
| import { | ||
| type AudioOutputDeviceLabel, | ||
| type DeviceLabel, | ||
| type MediaDevice, | ||
| type SelectedAudioOutputDevice, | ||
| type SelectedDevice, | ||
| } from "./MediaDevices"; | ||
| import { constant } from "./Behavior"; | ||
| import { ObservableScope } from "./ObservableScope"; | ||
| import { flushPromises, mockMediaDevices } from "../utils/test"; | ||
|
|
||
| const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); | ||
| vi.mock("../UrlParams", () => ({ getUrlParams })); | ||
|
|
||
| let testScope: ObservableScope; | ||
|
|
||
| beforeEach(() => { | ||
| testScope = new ObservableScope(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| testScope.end(); | ||
| }); | ||
|
|
||
| describe("MuteState", () => { | ||
| test("should automatically mute if force mute is set", async () => { | ||
| const forceMute$ = new BehaviorSubject<boolean>(false); | ||
|
|
||
| const deviceStub = { | ||
| available$: constant( | ||
| new Map<string, DeviceLabel>([ | ||
| ["fbac11", { type: "name", name: "HD Camera" }], | ||
| ]), | ||
| ), | ||
| selected$: constant({ id: "fbac11" }), | ||
| select(): void {}, | ||
| } as unknown as MediaDevice<DeviceLabel, SelectedDevice>; | ||
|
|
||
| const muteState = new MuteState( | ||
| testScope, | ||
| deviceStub, | ||
| constant(true), | ||
| true, | ||
| forceMute$, | ||
| ); | ||
| let lastEnabled: boolean = false; | ||
| muteState.enabled$.subscribe((enabled) => { | ||
| lastEnabled = enabled; | ||
| }); | ||
| let setEnabled: ((enabled: boolean) => void) | null = null; | ||
| muteState.setEnabled$.subscribe((setter) => { | ||
| setEnabled = setter; | ||
| }); | ||
|
|
||
| await flushPromises(); | ||
|
|
||
| setEnabled!(true); | ||
| await flushPromises(); | ||
| expect(lastEnabled).toBe(true); | ||
|
|
||
| // Now force mute | ||
| forceMute$.next(true); | ||
| await flushPromises(); | ||
| // Should automatically mute | ||
| expect(lastEnabled).toBe(false); | ||
|
|
||
| // Try to unmute can not work | ||
| expect(setEnabled).toBeNull(); | ||
|
|
||
| // Disable force mute | ||
| forceMute$.next(false); | ||
| await flushPromises(); | ||
|
|
||
| // TODO I'd expect it to go back to previous state (enabled) | ||
| // but actually it goes back to the initial state from construction (disabled) | ||
| // Should go back to previous state (enabled) | ||
| // Skip for now | ||
| // expect(lastEnabled).toBe(true); | ||
|
|
||
| // But yet it can be unmuted now | ||
| expect(setEnabled).not.toBeNull(); | ||
|
|
||
| setEnabled!(true); | ||
| await flushPromises(); | ||
| expect(lastEnabled).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("MuteStates", () => { | ||
| function aAudioOutputDevices(): MediaDevice< | ||
| AudioOutputDeviceLabel, | ||
| SelectedAudioOutputDevice | ||
| > { | ||
| const selected$ = new BehaviorSubject< | ||
| SelectedAudioOutputDevice | undefined | ||
| >({ | ||
| id: "default", | ||
| virtualEarpiece: false, | ||
| }); | ||
| return { | ||
| available$: constant( | ||
| new Map<string, AudioOutputDeviceLabel>([ | ||
| ["default", { type: "speaker" }], | ||
| ["0000", { type: "speaker" }], | ||
| ["1111", { type: "earpiece" }], | ||
| ["222", { type: "name", name: "Bluetooth Speaker" }], | ||
| ]), | ||
| ), | ||
| selected$, | ||
| select(id: string): void { | ||
| if (!this.available$.getValue().has(id)) { | ||
| logger.warn(`Attempted to select unknown device id: ${id}`); | ||
| return; | ||
| } | ||
| selected$.next({ | ||
| id, | ||
| /** For test purposes we ignore this */ | ||
| virtualEarpiece: false, | ||
| }); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function aVideoInput(): MediaDevice<DeviceLabel, SelectedDevice> { | ||
| const selected$ = new BehaviorSubject<SelectedDevice | undefined>( | ||
| undefined, | ||
| ); | ||
| return { | ||
| available$: constant( | ||
| new Map<string, DeviceLabel>([ | ||
| ["0000", { type: "name", name: "HD Camera" }], | ||
| ["1111", { type: "name", name: "WebCam Pro" }], | ||
| ]), | ||
| ), | ||
| selected$, | ||
| select(id: string): void { | ||
| if (!this.available$.getValue().has(id)) { | ||
| logger.warn(`Attempted to select unknown device id: ${id}`); | ||
| return; | ||
| } | ||
| selected$.next({ id }); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| test("should mute camera when in earpiece mode", async () => { | ||
| const audioOutputDevice = aAudioOutputDevices(); | ||
|
|
||
| const mediaDevices = mockMediaDevices({ | ||
| audioOutput: audioOutputDevice, | ||
| videoInput: aVideoInput(), | ||
| // other devices are not relevant for this test | ||
| }); | ||
| const muteStates = new MuteStates( | ||
| testScope, | ||
| mediaDevices, | ||
| // consider joined | ||
| constant(true), | ||
| ); | ||
|
|
||
| let latestSyncedState: boolean | null = null; | ||
| muteStates.video.setHandler(async (enabled: boolean): Promise<boolean> => { | ||
| logger.info(`Video mute state set to: ${enabled}`); | ||
| latestSyncedState = enabled; | ||
| return Promise.resolve(enabled); | ||
| }); | ||
|
|
||
| let lastVideoEnabled: boolean = false; | ||
| muteStates.video.enabled$.subscribe((enabled) => { | ||
| lastVideoEnabled = enabled; | ||
| }); | ||
|
|
||
| expect(muteStates.video.setEnabled$.value).toBeDefined(); | ||
| muteStates.video.setEnabled$.value?.(true); | ||
| await flushPromises(); | ||
|
|
||
| expect(lastVideoEnabled).toBe(true); | ||
|
|
||
| // Select earpiece audio output | ||
| audioOutputDevice.select("1111"); | ||
| await flushPromises(); | ||
| // Video should be automatically muted | ||
| expect(lastVideoEnabled).toBe(false); | ||
| expect(latestSyncedState).toBe(false); | ||
|
|
||
| // Try to switch to speaker | ||
| audioOutputDevice.select("0000"); | ||
| await flushPromises(); | ||
| // TODO I'd expect it to go back to previous state (enabled)?? | ||
| // But maybe not? If you move the phone away from your ear you may not want it | ||
| // to automatically enable video? | ||
| expect(lastVideoEnabled).toBe(false); | ||
|
|
||
| // But yet it can be unmuted now | ||
| expect(muteStates.video.setEnabled$.value).toBeDefined(); | ||
| muteStates.video.setEnabled$.value?.(true); | ||
| await flushPromises(); | ||
| expect(lastVideoEnabled).toBe(true); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,7 +27,7 @@ import { ElementWidgetActions, widget } from "../widget"; | |
| import { Config } from "../config/Config"; | ||
| import { getUrlParams } from "../UrlParams"; | ||
| import { type ObservableScope } from "./ObservableScope"; | ||
| import { type Behavior } from "./Behavior"; | ||
| import { type Behavior, constant } from "./Behavior"; | ||
|
|
||
| interface MuteStateData { | ||
| enabled$: Observable<boolean>; | ||
|
|
@@ -38,31 +38,58 @@ interface MuteStateData { | |
| export type Handler = (desired: boolean) => Promise<boolean>; | ||
| const defaultHandler: Handler = async (desired) => Promise.resolve(desired); | ||
|
|
||
| class MuteState<Label, Selected> { | ||
| /** | ||
| * Internal class - exported only for testing purposes. | ||
| * Do not use directly outside of tests. | ||
| */ | ||
| export class MuteState<Label, Selected> { | ||
| // TODO: rewrite this to explain behavior, it is not understandable, and cannot add logging | ||
| private readonly enabledByDefault$ = | ||
| this.enabledByConfig && !getUrlParams().skipLobby | ||
| ? this.joined$.pipe(map((isJoined) => !isJoined)) | ||
| : of(false); | ||
|
|
||
| private readonly handler$ = new BehaviorSubject(defaultHandler); | ||
|
|
||
| public setHandler(handler: Handler): void { | ||
| if (this.handler$.value !== defaultHandler) | ||
| throw new Error("Multiple mute state handlers are not supported"); | ||
| this.handler$.next(handler); | ||
| } | ||
|
|
||
| public unsetHandler(): void { | ||
| this.handler$.next(defaultHandler); | ||
| } | ||
|
|
||
| private readonly devicesConnected$ = combineLatest([ | ||
| this.device.available$, | ||
| this.forceMute$, | ||
| ]).pipe( | ||
| map(([available, forceMute]) => { | ||
| return !forceMute && available.size > 0; | ||
| }), | ||
| ); | ||
|
|
||
| private readonly data$ = this.scope.behavior<MuteStateData>( | ||
| this.device.available$.pipe( | ||
| map((available) => available.size > 0), | ||
| this.devicesConnected$.pipe( | ||
| distinctUntilChanged(), | ||
| withLatestFrom( | ||
| this.enabledByDefault$, | ||
| (devicesConnected, enabledByDefault) => { | ||
| if (!devicesConnected) | ||
| logger.info( | ||
| `MuteState: devices connected: ${devicesConnected}, enabled by default: ${enabledByDefault}`, | ||
| ); | ||
| if (!devicesConnected) { | ||
| logger.info( | ||
| `MuteState: devices connected: ${devicesConnected}, disabling`, | ||
| ); | ||
| // We need to sync the mute state with the handler | ||
| // to ensure nothing is beeing published. | ||
| this.handler$.value(false).catch((err) => { | ||
| logger.error("MuteState-disable: handler error", err); | ||
| }); | ||
| return { enabled$: of(false), set: null, toggle: null }; | ||
| } | ||
|
|
||
| // Assume the default value only once devices are actually connected | ||
| let enabled = enabledByDefault; | ||
|
|
@@ -135,21 +162,45 @@ class MuteState<Label, Selected> { | |
| private readonly device: MediaDevice<Label, Selected>, | ||
| private readonly joined$: Observable<boolean>, | ||
| private readonly enabledByConfig: boolean, | ||
| /** | ||
| * An optional observable which, when it emits `true`, will force the mute. | ||
| * Used for video to stop camera when earpiece mode is on. | ||
| * @private | ||
| */ | ||
| private readonly forceMute$: Observable<boolean>, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is also what we should use if we run have an error for any of the devices. Add the force mute flag and then render the button as in disabled state.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes we have to check for these errors and when they occur. But maybe now it will just make the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An error while muting/unmuting. But I think I mixed things up since if we use the lower level api |
||
| ) {} | ||
| } | ||
|
|
||
| export class MuteStates { | ||
| /** | ||
| * True if the selected audio output device is an earpiece. | ||
| * Used to force-disable video when on earpiece. | ||
| */ | ||
| private readonly isEarpiece$ = combineLatest( | ||
| this.mediaDevices.audioOutput.available$, | ||
| this.mediaDevices.audioOutput.selected$, | ||
| ).pipe( | ||
| map(([available, selected]) => { | ||
| if (!selected?.id) return false; | ||
| const device = available.get(selected.id); | ||
| logger.info(`MuteStates: selected audio output device:`, device); | ||
| return device?.type === "earpiece"; | ||
| }), | ||
| ); | ||
|
|
||
| public readonly audio = new MuteState( | ||
| this.scope, | ||
| this.mediaDevices.audioInput, | ||
| this.joined$, | ||
| Config.get().media_devices.enable_audio, | ||
| constant(false), | ||
| ); | ||
| public readonly video = new MuteState( | ||
| this.scope, | ||
| this.mediaDevices.videoInput, | ||
| this.joined$, | ||
| Config.get().media_devices.enable_video, | ||
| this.isEarpiece$, | ||
| ); | ||
|
|
||
| public constructor( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I like this name.
devicesConnectednow is implied by force mute?I would expect the other way around, that no connected device implies force muting.
But force mute = true -> no devices connected feels like it messes with causality.