Merge pull request #4249 from element-hq/matthew/audio-renderer-warn-spam

Stop MatrixAudioRenderer warning on every render

src/livekit/MatrixAudioRenderer.test.tsx

@@ -24,10 +24,12 @@ import { testAudioContext } from "../useAudioContext.test";
 import * as MediaDevicesContext from "../MediaDevicesContext";
 import { LivekitRoomAudioRenderer } from "./MatrixAudioRenderer";
 import {
+  mockLocalParticipant,
   mockMediaDevices,
   mockRemoteParticipant,
   mockTrack,
 } from "../utils/test";
+import { logger as rootLogger } from "matrix-js-sdk/lib/logger";
 import { initializeWidget } from "../widget";
 initializeWidget();
 export const TestAudioContextConstructor = vi.fn(
@@ -131,6 +133,57 @@ it("should render for member", () => {
   expect(queryAllByTestId("audio")).toHaveLength(1);
 });
 
+function spyOnWarn(): ReturnType<typeof vi.fn> {
+  const warn = vi.fn();
+  vi.spyOn(rootLogger, "getChild").mockReturnValue({
+    warn,
+  } as unknown as typeof rootLogger);
+  return warn;
+}
+
+it("should not render or warn for the local participant", () => {
+  const warn = spyOnWarn();
+  const local = mockLocalParticipant({ identity: "@alice:DEV0" });
+  vi.mocked(useTracks).mockReturnValue([mockTrack(local)]);
+  const { queryAllByTestId } = render(
+    <MediaDevicesProvider value={mockMediaDevices({})}>
+      <LivekitRoomAudioRenderer
+        validIdentities={[]}
+        livekitRoom={{ remoteParticipants: new Map() } as unknown as Room}
+        url={""}
+      />
+    </MediaDevicesProvider>,
+  );
+  expect(queryAllByTestId("audio")).toHaveLength(0);
+  expect(warn).not.toHaveBeenCalled();
+});
+
+it("should warn only once per unexpected participant", () => {
+  const warn = spyOnWarn();
+  const { rerender } = renderTestComponent(
+    [{ userId: "@bob", deviceId: "DEV0" }],
+    ["@alice:DEV0"],
+    [
+      {
+        participantId: "@alice:DEV0",
+        kind: Track.Kind.Audio,
+        source: Track.Source.Microphone,
+      },
+    ],
+  );
+  expect(warn).toHaveBeenCalledTimes(1);
+  rerender(
+    <MediaDevicesProvider value={mockMediaDevices({})}>
+      <LivekitRoomAudioRenderer
+        validIdentities={[]}
+        livekitRoom={{ remoteParticipants: new Map() } as unknown as Room}
+        url={""}
+      />
+    </MediaDevicesProvider>,
+  );
+  expect(warn).toHaveBeenCalledTimes(1);
+});
+
 it("should not render without member", () => {
   const { container, queryAllByTestId } = renderTestComponent(
     [{ userId: "@bob", deviceId: "DEV0" }],

src/livekit/MatrixAudioRenderer.tsx

@@ -8,7 +8,7 @@ Please see LICENSE in the repository root for full details.
 import { getTrackReferenceId } from "@livekit/components-core";
 import { type Room as LivekitRoom } from "livekit-client";
 import { type RemoteAudioTrack, Track } from "livekit-client";
-import { useEffect, useMemo, useState, type ReactNode } from "react";
+import { useEffect, useMemo, useRef, useState, type ReactNode } from "react";
 import {
   useTracks,
   AudioTrack,
@@ -60,6 +60,9 @@ export function LivekitRoomAudioRenderer({
   muted,
 }: MatrixAudioRendererProps): ReactNode {
   const logger = rootLogger.getChild("[MatrixAudioRenderer]");
+  // Identities we have already warned about, so that re-renders (which happen
+  // on every active speaker update) don't repeat the warning.
+  const warnedIdentities = useRef(new Set<string>());
   const tracks = useTracks(
     [
       Track.Source.Microphone,
@@ -72,22 +75,27 @@ export function LivekitRoomAudioRenderer({
       room: livekitRoom,
     },
   )
-    // Only keep audio tracks
+    // Only keep remote audio tracks (we never render our own audio)
-    .filter((ref) => ref.publication.kind === Track.Kind.Audio)
+    .filter(
+      (ref) =>
+        ref.publication.kind === Track.Kind.Audio && !ref.participant.isLocal,
+    )
     // Only keep tracks from participants that are in the validIdentities list
     .filter((ref) => {
-      const isValid = validIdentities.includes(ref.participant.identity);
+      const { identity } = ref.participant;
-      if (!isValid) {
+      const isValid = validIdentities.includes(identity);
-        // TODO make sure to also skip the warn logging for the local identity
+      if (!isValid && !warnedIdentities.current.has(identity)) {
+        warnedIdentities.current.add(identity);
         // Log that there is an invalid identity, that means that someone is publishing audio that is not expected to be in the call.
         logger.warn(
-          `Audio track ${ref.participant.identity} from ${url} has no matching matrix call member`,
+          `Audio track ${identity} from ${url} has no matching matrix call member`,
           `current members: ${validIdentities.join()}`,
           `track will not get rendered`,
         );
-        return false;
+      } else if (isValid) {
+        warnedIdentities.current.delete(identity);
       }
-      return true;
+      return isValid;
     });
 
   // This component is also (in addition to the "only play audio for connected members" logic above)