Mute state improvements including muting if skipLobby=true in SPA (#2834)

* Mute state improvements including muting if skipLobby=true in SPA

If skipLobby=true then always default to muted.
Use the mute states from the config file.
Refactor logic so that defaults are handled by useMuteStates().
Test cases.

* Refactor config mocking to shared function

* Update docs/url-params.md

Co-authored-by: Timo <16718859+toger5@users.noreply.github.com>

* Lint

* Add TODO for future comment

---------

Co-authored-by: Timo <16718859+toger5@users.noreply.github.com>

docs/url-params.md

@@ -55,7 +55,7 @@ There are two formats for Element Call urls.
 | `returnToLobby`           | `true` or `false`                                                                                    | No, defaults to `false` | Not applicable          | Displays the lobby in widget mode after leaving a call; shows a blank page if set to `false`. Useful for video rooms.                                                                                                                                                                                                                                                                               |
 | `roomId`                  | [Matrix Room ID](https://spec.matrix.org/v1.12/appendices/#room-ids)                                 | Yes                     | No                      | Anything about what room we're pointed to should be from useRoomIdentifier which parses the path and resolves alias with respect to the default server name, however roomId is an exception as we need the room ID in embedded widget mode, and not the room alias (or even the via params because we are not trying to join it). This is also not validated, where it is in `useRoomIdentifier()`. |
 | `showControls`            | `true` or `false`                                                                                    | No, defaults to `true`  | No, defaults to `true`  | Displays controls like mute, screen-share, invite, and hangup buttons during a call.                                                                                                                                                                                                                                                                                                                |
-| `skipLobby`               | `true` or `false`                                                                                    | No, defaults to `false` | No, defaults to `false` | Skips the lobby to join a call directly, can be combined with preload in widget.                                                                                                                                                                                                                                                                                                                    |
+| `skipLobby`               | `true` or `false`                                                                                    | No, defaults to `false` | No, defaults to `false` | Skips the lobby to join a call directly, can be combined with preload in widget. When `true` the audio and video inputs will be muted by default. (This means there currently is no way to start without muted video if one wants to skip the lobby. Also not in widget mode.)                                                                                                                      |
 | `theme`                   | One of: `light`, `dark`, `light-high-contrast`, `dark-high-contrast`                                 | No, defaults to `dark`  | No, defaults to `dark`  | UI theme to use.                                                                                                                                                                                                                                                                                                                                                                                    |
 | `userId`                  | [Matrix User Identifier](https://spec.matrix.org/v1.12/appendices/#user-identifiers)                 | Yes                     | Not applicable          | The Matrix user ID.                                                                                                                                                                                                                                                                                                                                                                                 |
 | `viaServers`              | Comma separated list of [Matrix Server Names](https://spec.matrix.org/v1.12/appendices/#server-name) | Not applicable          | No                      | Homeserver for joining a room, non-empty value required for rooms not on the user’s default homeserver.                                                                                                                                                                                                                                                                                             |

src/UrlParams.test.ts

@@ -7,7 +7,7 @@ Please see LICENSE in the repository root for full details.
 
 import { describe, expect, it } from "vitest";
 
-import { getRoomIdentifierFromUrl } from "../src/UrlParams";
+import { getRoomIdentifierFromUrl, getUrlParams } from "../src/UrlParams";
 
 const ROOM_NAME = "roomNameHere";
 const ROOM_ID = "!d45f138fsd";
@@ -86,4 +86,18 @@ describe("UrlParams", () => {
         .roomAlias,
     ).toBeFalsy();
   });
+
+  describe("preload", () => {
+    it("defaults to false", () => {
+      expect(getUrlParams().preload).toBe(false);
+    });
+
+    it("ignored in SPA mode", () => {
+      expect(getUrlParams("?preload=true").preload).toBe(false);
+    });
+
+    it("respected in widget mode", () => {
+      expect(getUrlParams("?preload=true&widgetId=12345").preload).toBe(true);
+    });
+  });
 });

src/UrlParams.ts

@@ -211,8 +211,11 @@ export const getUrlParams = (
 
   const fontScale = parseFloat(parser.getParam("fontScale") ?? "");
 
+  const widgetId = parser.getParam("widgetId");
+  const isWidget = !!widgetId;
+
   return {
-    widgetId: parser.getParam("widgetId"),
+    widgetId,
     parentUrl: parser.getParam("parentUrl"),
 
     // NB. we don't validate roomId here as we do in getRoomIdentifierFromUrl:
@@ -224,7 +227,7 @@ export const getUrlParams = (
     confineToRoom:
       parser.getFlagParam("confineToRoom") || parser.getFlagParam("embed"),
     appPrompt: parser.getFlagParam("appPrompt", true),
-    preload: parser.getFlagParam("preload"),
+    preload: isWidget ? parser.getFlagParam("preload") : false,
     hideHeader: parser.getFlagParam("hideHeader"),
     showControls: parser.getFlagParam("showControls", true),
     hideScreensharing: parser.getFlagParam("hideScreensharing"),

src/livekit/MediaDevicesContext.tsx

@@ -128,7 +128,7 @@ const devicesStub: MediaDevices = {
   stopUsingDeviceNames: () => {},
 };
 
-const MediaDevicesContext = createContext<MediaDevices>(devicesStub);
+export const MediaDevicesContext = createContext<MediaDevices>(devicesStub);
 
 interface Props {
   children: JSX.Element;

src/room/GroupCallView.tsx

@@ -131,48 +131,46 @@ export const GroupCallView: FC<Props> = ({
   const latestDevices = useRef<MediaDevices>();
   latestDevices.current = deviceContext;
 
+  // TODO: why do we use a ref here instead of using muteStates directly?
   const latestMuteStates = useRef<MuteStates>();
   latestMuteStates.current = muteStates;
 
   useEffect(() => {
-    const defaultDeviceSetup = async (
-      requestedDeviceData: JoinCallData,
-    ): Promise<void> => {
+    const defaultDeviceSetup = async ({
+      audioInput,
+      videoInput,
+    }: JoinCallData): Promise<void> => {
       // XXX: I think this is broken currently - LiveKit *won't* request
       // permissions and give you device names unless you specify a kind, but
       // here we want all kinds of devices. This needs a fix in livekit-client
       // for the following name-matching logic to do anything useful.
       const devices = await Room.getLocalDevices(undefined, true);
-      const { audioInput, videoInput } = requestedDeviceData;
-      if (audioInput === null) {
-        latestMuteStates.current!.audio.setEnabled?.(false);
-      } else {
+
+      if (audioInput) {
         const deviceId = findDeviceByName(audioInput, "audioinput", devices);
         if (!deviceId) {
           logger.warn("Unknown audio input: " + audioInput);
+          // override the default mute state
           latestMuteStates.current!.audio.setEnabled?.(false);
         } else {
           logger.debug(
             `Found audio input ID ${deviceId} for name ${audioInput}`,
           );
           latestDevices.current!.audioInput.select(deviceId);
-          latestMuteStates.current!.audio.setEnabled?.(true);
         }
       }
 
-      if (videoInput === null) {
-        latestMuteStates.current!.video.setEnabled?.(false);
-      } else {
+      if (videoInput) {
         const deviceId = findDeviceByName(videoInput, "videoinput", devices);
         if (!deviceId) {
           logger.warn("Unknown video input: " + videoInput);
+          // override the default mute state
           latestMuteStates.current!.video.setEnabled?.(false);
         } else {
           logger.debug(
             `Found video input ID ${deviceId} for name ${videoInput}`,
           );
           latestDevices.current!.videoInput.select(deviceId);
-          latestMuteStates.current!.video.setEnabled?.(true);
         }
       }
     };
@@ -199,7 +197,6 @@ export const GroupCallView: FC<Props> = ({
         } else {
           // No lobby and no preload: we enter the rtc session right away
           (async (): Promise<void> => {
-            await defaultDeviceSetup({ audioInput: null, videoInput: null });
             await enterRTCSession(rtcSession, perParticipantE2EE);
           })().catch((e) => {
             logger.error("Error joining RTC session", e);

src/room/MuteStates.test.tsx

@@ -0,0 +1,172 @@
+/*
+Copyright 2024 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only
+Please see LICENSE in the repository root for full details.
+*/
+
+import { afterAll, afterEach, describe, expect, it, vi } from "vitest";
+import React, { ReactNode } from "react";
+import { beforeEach } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { MemoryRouter } from "react-router-dom";
+
+import { useMuteStates } from "./MuteStates";
+import {
+  MediaDevice,
+  MediaDevices,
+  MediaDevicesContext,
+} from "../livekit/MediaDevicesContext";
+import { mockConfig } from "../utils/test";
+
+function TestComponent(): ReactNode {
+  const muteStates = useMuteStates();
+  return (
+    <div>
+      <div data-testid="audio-enabled">
+        {muteStates.audio.enabled.toString()}
+      </div>
+      <div data-testid="video-enabled">
+        {muteStates.video.enabled.toString()}
+      </div>
+    </div>
+  );
+}
+
+const mockMicrophone: MediaDeviceInfo = {
+  deviceId: "",
+  kind: "audioinput",
+  label: "",
+  groupId: "",
+  toJSON() {
+    return {};
+  },
+};
+
+const mockSpeaker: MediaDeviceInfo = {
+  deviceId: "",
+  kind: "audiooutput",
+  label: "",
+  groupId: "",
+  toJSON() {
+    return {};
+  },
+};
+
+const mockCamera: MediaDeviceInfo = {
+  deviceId: "",
+  kind: "videoinput",
+  label: "",
+  groupId: "",
+  toJSON() {
+    return {};
+  },
+};
+
+function mockDevices(available: MediaDeviceInfo[]): MediaDevice {
+  return {
+    available,
+    selectedId: "",
+    select: (): void => {},
+  };
+}
+
+function mockMediaDevices(
+  {
+    microphone,
+    speaker,
+    camera,
+  }: {
+    microphone?: boolean;
+    speaker?: boolean;
+    camera?: boolean;
+  } = { microphone: true, speaker: true, camera: true },
+): MediaDevices {
+  return {
+    audioInput: mockDevices(microphone ? [mockMicrophone] : []),
+    audioOutput: mockDevices(speaker ? [mockSpeaker] : []),
+    videoInput: mockDevices(camera ? [mockCamera] : []),
+    startUsingDeviceNames: (): void => {},
+    stopUsingDeviceNames: (): void => {},
+  };
+}
+
+describe("useMuteStates", () => {
+  beforeEach(() => {
+    vi.spyOn(React, "useContext").mockReturnValue({});
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  afterAll(() => {
+    vi.clearAllMocks();
+  });
+
+  it("disabled when no input devices", () => {
+    mockConfig();
+
+    render(
+      <MemoryRouter>
+        <MediaDevicesContext.Provider
+          value={mockMediaDevices({
+            microphone: false,
+            camera: false,
+          })}
+        >
+          <TestComponent />
+        </MediaDevicesContext.Provider>
+      </MemoryRouter>,
+    );
+    expect(screen.getByTestId("audio-enabled").textContent).toBe("false");
+    expect(screen.getByTestId("video-enabled").textContent).toBe("false");
+  });
+
+  it("should be enabled by default", () => {
+    mockConfig();
+
+    render(
+      <MemoryRouter>
+        <MediaDevicesContext.Provider value={mockMediaDevices()}>
+          <TestComponent />
+        </MediaDevicesContext.Provider>
+      </MemoryRouter>,
+    );
+    expect(screen.getByTestId("audio-enabled").textContent).toBe("true");
+    expect(screen.getByTestId("video-enabled").textContent).toBe("true");
+  });
+
+  it("uses defaults from config", () => {
+    mockConfig({
+      media_devices: {
+        enable_audio: false,
+        enable_video: false,
+      },
+    });
+
+    render(
+      <MemoryRouter>
+        <MediaDevicesContext.Provider value={mockMediaDevices()}>
+          <TestComponent />
+        </MediaDevicesContext.Provider>
+      </MemoryRouter>,
+    );
+    expect(screen.getByTestId("audio-enabled").textContent).toBe("false");
+    expect(screen.getByTestId("video-enabled").textContent).toBe("false");
+  });
+
+  it("skipLobby mutes inputs", () => {
+    mockConfig();
+
+    render(
+      <MemoryRouter initialEntries={["/room/?skipLobby=true"]}>
+        <MediaDevicesContext.Provider value={mockMediaDevices()}>
+          <TestComponent />
+        </MediaDevicesContext.Provider>
+      </MemoryRouter>,
+    );
+    expect(screen.getByTestId("audio-enabled").textContent).toBe("false");
+    expect(screen.getByTestId("video-enabled").textContent).toBe("false");
+  });
+});

src/room/MuteStates.ts

@@ -19,6 +19,7 @@ import { MediaDevice, useMediaDevices } from "../livekit/MediaDevicesContext";
 import { useReactiveState } from "../useReactiveState";
 import { ElementWidgetActions, widget } from "../widget";
 import { Config } from "../config/Config";
+import { useUrlParams } from "../UrlParams";
 
 /**
  * If there already are this many participants in the call, we automatically mute
@@ -72,13 +73,14 @@ function useMuteState(
 export function useMuteStates(): MuteStates {
   const devices = useMediaDevices();
 
-  const audio = useMuteState(
-    devices.audioInput,
-    () => Config.get().media_devices.enable_audio,
-  );
+  const { skipLobby } = useUrlParams();
+
+  const audio = useMuteState(devices.audioInput, () => {
+    return Config.get().media_devices.enable_audio && !skipLobby;
+  });
   const video = useMuteState(
     devices.videoInput,
-    () => Config.get().media_devices.enable_video,
+    () => Config.get().media_devices.enable_video && !skipLobby,
   );
 
   useEffect(() => {

src/rtcSessionHelper.test.ts

@@ -9,8 +9,7 @@ import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession";
 import { expect, test, vi } from "vitest";
 
 import { enterRTCSession } from "../src/rtcSessionHelpers";
-import { Config } from "../src/config/Config";
-import { DEFAULT_CONFIG } from "./config/ConfigOptions";
+import { mockConfig } from "./utils/test";
 
 test("It joins the correct Session", async () => {
   const focusFromOlderMembership = {
@@ -34,8 +33,7 @@ test("It joins the correct Session", async () => {
     ],
   };
 
-  vi.spyOn(Config, "get").mockReturnValue({
-    ...DEFAULT_CONFIG,
+  mockConfig({
     livekit: { livekit_service_url: "http://my-default-service-url.com" },
   });
   const mockedSession = vi.mocked({

src/utils/test.ts

@@ -21,6 +21,8 @@ import {
   RemoteUserMediaViewModel,
 } from "../state/MediaViewModel";
 import { E2eeType } from "../e2ee/e2eeType";
+import { DEFAULT_CONFIG, ResolvedConfigOptions } from "../config/ConfigOptions";
+import { Config } from "../config/Config";
 
 export function withFakeTimers(continuation: () => void): void {
   vi.useFakeTimers();
@@ -197,3 +199,10 @@ export async function withRemoteMedia(
     vm.destroy();
   }
 }
+
+export function mockConfig(config: Partial<ResolvedConfigOptions> = {}): void {
+  vi.spyOn(Config, "get").mockReturnValue({
+    ...DEFAULT_CONFIG,
+    ...config,
+  });
+}