From 7cbb1ec1e8069453d987c215ec7a9cd9a2045064 Mon Sep 17 00:00:00 2001 From: Valere Date: Thu, 9 Oct 2025 15:33:25 +0200 Subject: [PATCH] Simplify AudioRenderer and add more tests --- src/livekit/MatrixAudioRenderer.test.tsx | 110 ++++++++++++++++++++--- src/livekit/MatrixAudioRenderer.tsx | 83 ++++++----------- src/room/InCallView.tsx | 2 +- src/utils/test.ts | 12 ++- 4 files changed, 133 insertions(+), 74 deletions(-) diff --git a/src/livekit/MatrixAudioRenderer.test.tsx b/src/livekit/MatrixAudioRenderer.test.tsx index 83b3f73a..049add97 100644 --- a/src/livekit/MatrixAudioRenderer.test.tsx +++ b/src/livekit/MatrixAudioRenderer.test.tsx @@ -14,8 +14,8 @@ import { import { type Participant, type RemoteAudioTrack, - type RemoteParticipant, type Room, + Track, } from "livekit-client"; import { type ReactNode } from "react"; import { useTracks } from "@livekit/components-react"; @@ -23,7 +23,11 @@ import { useTracks } from "@livekit/components-react"; import { testAudioContext } from "../useAudioContext.test"; import * as MediaDevicesContext from "../MediaDevicesContext"; import { LivekitRoomAudioRenderer } from "./MatrixAudioRenderer"; -import { mockMediaDevices, mockTrack } from "../utils/test"; +import { + mockMediaDevices, + mockRemoteParticipant, + mockTrack, +} from "../utils/test"; export const TestAudioContextConstructor = vi.fn(() => testAudioContext); @@ -61,17 +65,20 @@ let tracks: TrackReference[] = []; * * @param rtcMembers - Array of active rtc members with userId and deviceId. * @param livekitParticipantIdentities - Array of livekit participant (that are publishing). + * @param explicitTracks - Array of tracks available in livekit, if not provided, one audio track per livekitParticipantIdentities will be created. * */ function renderTestComponent( rtcMembers: { userId: string; deviceId: string }[], livekitParticipantIdentities: string[], + explicitTracks?: { + participantId: string; + kind: Track.Kind; + source: Track.Source; + }[], ): RenderResult { - const liveKitParticipants = livekitParticipantIdentities.map( - (identity) => - ({ - identity, - }) as unknown as RemoteParticipant, + const liveKitParticipants = livekitParticipantIdentities.map((identity) => + mockRemoteParticipant({ identity }), ); const participants = rtcMembers.flatMap(({ userId, deviceId }) => { const p = liveKitParticipants.find( @@ -85,13 +92,22 @@ function renderTestComponent( ), } as unknown as Room; - tracks = participants.map((p) => mockTrack(p)); + if (explicitTracks?.length ?? 0 > 0) { + tracks = explicitTracks!.map(({ participantId, source, kind }) => { + const participant = + liveKitParticipants.find((p) => p.identity === participantId) ?? + mockRemoteParticipant({ identity: participantId }); + return mockTrack(participant, kind, source); + }); + } else { + tracks = participants.map((p) => mockTrack(p)); + } vi.mocked(useTracks).mockReturnValue(tracks); return render( p.identity)} livekitRoom={livekitRoom} url={""} /> @@ -118,11 +134,18 @@ it("should not render without member", () => { }); const TEST_CASES: { + name: string; rtcUsers: { userId: string; deviceId: string }[]; livekitParticipantIdentities: string[]; + explicitTracks?: { + participantId: string; + kind: Track.Kind; + source: Track.Source; + }[]; expectedAudioTracks: number; }[] = [ { + name: "single user single device", rtcUsers: [ { userId: "@alice", deviceId: "DEV0" }, { userId: "@alice", deviceId: "DEV1" }, @@ -133,6 +156,7 @@ const TEST_CASES: { }, // Charlie is a rtc member but not in livekit { + name: "Charlie is rtc member but not in livekit", rtcUsers: [ { userId: "@alice", deviceId: "DEV0" }, { userId: "@bob", deviceId: "DEV0" }, @@ -143,6 +167,7 @@ const TEST_CASES: { }, // Charlie is in livekit but not rtc member { + name: "Charlie is in livekit but not rtc member", rtcUsers: [ { userId: "@alice", deviceId: "DEV0" }, { userId: "@bob", deviceId: "DEV0" }, @@ -150,14 +175,77 @@ const TEST_CASES: { livekitParticipantIdentities: ["@alice:DEV0", "@bob:DEV0", "@charlie:DEV0"], expectedAudioTracks: 2, }, + { + name: "no audio track, only video track", + rtcUsers: [{ userId: "@alice", deviceId: "DEV0" }], + livekitParticipantIdentities: ["@alice:DEV0"], + explicitTracks: [ + { + participantId: "@alice:DEV0", + kind: Track.Kind.Video, + source: Track.Source.Camera, + }, + ], + expectedAudioTracks: 0, + }, + { + name: "Audio track from unknown source", + rtcUsers: [{ userId: "@alice", deviceId: "DEV0" }], + livekitParticipantIdentities: ["@alice:DEV0"], + explicitTracks: [ + { + participantId: "@alice:DEV0", + kind: Track.Kind.Audio, + source: Track.Source.Unknown, + }, + ], + expectedAudioTracks: 1, + }, + { + name: "Audio track from other device", + rtcUsers: [{ userId: "@alice", deviceId: "DEV0" }], + livekitParticipantIdentities: ["@alice:DEV0"], + explicitTracks: [ + { + participantId: "@alice:DEV1", + kind: Track.Kind.Audio, + source: Track.Source.Microphone, + }, + ], + expectedAudioTracks: 0, + }, + { + name: "two audio tracks, microphone and screenshare", + rtcUsers: [{ userId: "@alice", deviceId: "DEV0" }], + livekitParticipantIdentities: ["@alice:DEV0"], + explicitTracks: [ + { + participantId: "@alice:DEV0", + kind: Track.Kind.Audio, + source: Track.Source.Microphone, + }, + { + participantId: "@alice:DEV0", + kind: Track.Kind.Audio, + source: Track.Source.ScreenShareAudio, + }, + ], + expectedAudioTracks: 2, + }, ]; it.each(TEST_CASES)( - `should render sound test cases %s`, - ({ rtcUsers, livekitParticipantIdentities, expectedAudioTracks }) => { + `should render sound test cases $name`, + ({ + rtcUsers, + livekitParticipantIdentities, + explicitTracks, + expectedAudioTracks, + }) => { const { queryAllByTestId } = renderTestComponent( rtcUsers, livekitParticipantIdentities, + explicitTracks, ); expect(queryAllByTestId("audio")).toHaveLength(expectedAudioTracks); }, diff --git a/src/livekit/MatrixAudioRenderer.tsx b/src/livekit/MatrixAudioRenderer.tsx index fb1400b4..5b1149e9 100644 --- a/src/livekit/MatrixAudioRenderer.tsx +++ b/src/livekit/MatrixAudioRenderer.tsx @@ -6,23 +6,20 @@ Please see LICENSE in the repository root for full details. */ import { getTrackReferenceId } from "@livekit/components-core"; -import { - type RemoteParticipant, - type Room as LivekitRoom, -} from "livekit-client"; +import { type Room as LivekitRoom } from "livekit-client"; import { type RemoteAudioTrack, Track } from "livekit-client"; -import { useEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { useEffect, useMemo, useState, type ReactNode } from "react"; import { useTracks, AudioTrack, type AudioTrackProps, } from "@livekit/components-react"; import { logger } from "matrix-js-sdk/lib/logger"; +import { type ParticipantId } from "matrix-js-sdk/lib/matrixrtc"; import { useEarpieceAudioConfig } from "../MediaDevicesContext"; import { useReactiveState } from "../useReactiveState"; import * as controls from "../controls"; -import {} from "@livekit/components-core"; export interface MatrixAudioRendererProps { /** @@ -31,11 +28,11 @@ export interface MatrixAudioRendererProps { url: string; livekitRoom: LivekitRoom; /** - * The list of participants to render audio for. + * The list of participant identities to render audio for. * This list needs to be composed based on the matrixRTC members so that we do not play audio from users - * that are not expected to be in the rtc session. + * that are not expected to be in the rtc session (local user is excluded). */ - participants: RemoteParticipant[]; + validIdentities: ParticipantId[]; /** * If set to `true`, mutes all audio tracks rendered by the component. * @remarks @@ -44,6 +41,7 @@ export interface MatrixAudioRendererProps { muted?: boolean; } +const prefixedLogger = logger.getChild("[MatrixAudioRenderer]"); /** * Takes care of handling remote participants’ audio tracks and makes sure that microphones and screen share are audible. * @@ -60,35 +58,9 @@ export interface MatrixAudioRendererProps { export function LivekitRoomAudioRenderer({ url, livekitRoom, - participants, + validIdentities, muted, }: MatrixAudioRendererProps): ReactNode { - // This is the list of valid identities that are allowed to play audio. - // It is derived from the list of matrix rtc members. - const validIdentities = useMemo( - () => new Set(participants.map((p) => p.identity)), - [participants], - ); - - const loggedInvalidIdentities = useRef(new Set()); - - /** - * Log an invalid livekit track identity. - * A invalid identity is one that does not match any of the matrix rtc members. - * - * @param identity The identity of the track that is invalid - * @param validIdentities The list of valid identities - */ - const logInvalid = (identity: string): void => { - if (loggedInvalidIdentities.current.has(identity)) return; - logger.warn( - `[MatrixAudioRenderer] Audio track ${identity} from ${url} has no matching matrix call member`, - `current members: ${participants.map((p) => p.identity)}`, - `track will not get rendered`, - ); - loggedInvalidIdentities.current.add(identity); - }; - const tracks = useTracks( [ Track.Source.Microphone, @@ -100,28 +72,23 @@ export function LivekitRoomAudioRenderer({ onlySubscribed: true, room: livekitRoom, }, - ).filter((ref) => { - const isValid = validIdentities.has(ref.participant.identity); - if (!isValid && !ref.participant.isLocal) - logInvalid(ref.participant.identity); - return ( - !ref.participant.isLocal && - ref.publication.kind === Track.Kind.Audio && - isValid - ); - }); - - useEffect(() => { - if ( - loggedInvalidIdentities.current.size && - tracks.every((t) => validIdentities.has(t.participant.identity)) - ) { - logger.debug( - `[MatrixAudioRenderer] All audio tracks from ${url} have a matching matrix call member identity.`, - ); - loggedInvalidIdentities.current.clear(); - } - }, [tracks, validIdentities, url]); + ) + // Only keep audio tracks + .filter((ref) => ref.publication.kind === Track.Kind.Audio) + // Only keep tracks from participants that are in the validIdentities list + .filter((ref) => { + const isValid = validIdentities.includes(ref.participant.identity); + if (!isValid) { + // Log that there is an invalid identity, that means that someone is publishing audio that is not expected to be in the call. + prefixedLogger.warn( + `Audio track ${ref.participant.identity} from ${url} has no matching matrix call member`, + `current members: ${validIdentities.join()}`, + `track will not get rendered`, + ); + return false; + } + return true; + }); // This component is also (in addition to the "only play audio for connected members" logic above) // responsible for mimicking earpiece audio on iPhones. diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 658f9fbe..fd631bae 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -865,7 +865,7 @@ export const InCallView: FC = ({ key={url} url={url} livekitRoom={livekitRoom} - participants={participants} + validIdentities={participants.map((p) => p.identity)} muted={muteAllAudio} /> ))} diff --git a/src/utils/test.ts b/src/utils/test.ts index 6e0e95c9..508559c2 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -393,13 +393,17 @@ export class MockRTCSession extends TypedEventEmitter< } } -export const mockTrack = (participant: Participant): TrackReference => +export const mockTrack = ( + participant: Participant, + kind?: Track.Kind, + source?: Track.Source, +): TrackReference => ({ participant, publication: { - kind: Track.Kind.Audio, - source: "mic", - trackSid: "123", + kind: kind ?? Track.Kind.Audio, + source: source ?? Track.Source.Microphone, + trackSid: `123##${participant.identity}`, track: { attach: vi.fn(), detach: vi.fn(),