From 2e646bfac163ea48b45de6fc8a22d52e8a0e01cd Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 2 Dec 2025 19:40:08 +0100 Subject: [PATCH 01/43] Unify LiveKit and Matrix connection states --- src/room/GroupCallView.tsx | 1 + src/state/CallViewModel/CallViewModel.ts | 47 ++-- .../localMember/HomeserverConnected.test.ts | 58 ++--- .../localMember/HomeserverConnected.ts | 38 +-- .../localMember/LocalMembership.test.ts | 26 +- .../localMember/LocalMembership.ts | 244 +++++++++--------- .../localMember/Publisher.test.ts | 3 +- .../CallViewModel/localMember/Publisher.ts | 2 +- .../remoteMembers/Connection.test.ts | 9 +- .../CallViewModel/remoteMembers/Connection.ts | 43 +-- 10 files changed, 238 insertions(+), 233 deletions(-) diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 75438f7f..43602716 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -160,6 +160,7 @@ export const GroupCallView: FC = ({ }, [rtcSession]); // TODO move this into the callViewModel LocalMembership.ts + // We might actually not need this at all. Since we get into fatalError on those errors already? useTypedEventEmitter( rtcSession, MatrixRTCSessionEvent.MembershipManagerError, diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 3c15958a..9bfa979c 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -452,18 +452,14 @@ export function createCallViewModel$( const localMembership = createLocalMembership$({ scope: scope, - homeserverConnected$: createHomeserverConnected$( + homeserverConnected: createHomeserverConnected$( scope, client, matrixRTCSession, ), muteStates: muteStates, - joinMatrixRTC: async (transport: LivekitTransport) => { - return enterRTCSession( - matrixRTCSession, - transport, - connectOptions$.value, - ); + joinMatrixRTC: (transport: LivekitTransport) => { + enterRTCSession(matrixRTCSession, transport, connectOptions$.value); }, createPublisherFactory: (connection: Connection) => { return new Publisher( @@ -573,17 +569,6 @@ export function createCallViewModel$( ), ); - /** - * Whether various media/event sources should pretend to be disconnected from - * all network input, even if their connection still technically works. - */ - // We do this when the app is in the 'reconnecting' state, because it might be - // that the LiveKit connection is still functional while the homeserver is - // down, for example, and we want to avoid making people worry that the app is - // in a split-brained state. - // DISCUSSION own membership manager ALSO this probably can be simplifis - const reconnecting$ = localMembership.reconnecting$; - const audioParticipants$ = scope.behavior( matrixLivekitMembers$.pipe( switchMap((membersWithEpoch) => { @@ -631,7 +616,7 @@ export function createCallViewModel$( ); const handsRaised$ = scope.behavior( - handsRaisedSubject$.pipe(pauseWhen(reconnecting$)), + handsRaisedSubject$.pipe(pauseWhen(localMembership.reconnecting$)), ); const reactions$ = scope.behavior( @@ -644,7 +629,7 @@ export function createCallViewModel$( ]), ), ), - pauseWhen(reconnecting$), + pauseWhen(localMembership.reconnecting$), ), ); @@ -735,7 +720,7 @@ export function createCallViewModel$( livekitRoom$, focusUrl$, mediaDevices, - reconnecting$, + localMembership.reconnecting$, displayName$, matrixMemberMetadataStore.createAvatarUrlBehavior$(userId), handsRaised$.pipe(map((v) => v[participantId]?.time ?? null)), @@ -827,11 +812,17 @@ export function createCallViewModel$( }), ); - const leave$: Observable<"user" | "timeout" | "decline" | "allOthersLeft"> = - merge( - autoLeave$, - merge(userHangup$, widgetHangup$).pipe(map(() => "user" as const)), - ).pipe(scope.share); + const shouldLeave$: Observable< + "user" | "timeout" | "decline" | "allOthersLeft" + > = merge( + autoLeave$, + merge(userHangup$, widgetHangup$).pipe(map(() => "user" as const)), + ).pipe(scope.share); + + shouldLeave$.pipe(scope.bind()).subscribe((reason) => { + logger.info(`Call left due to ${reason}`); + localMembership.requestDisconnect(); + }); const spotlightSpeaker$ = scope.behavior( userMedia$.pipe( @@ -1453,7 +1444,7 @@ export function createCallViewModel$( autoLeave$: autoLeave$, callPickupState$: callPickupState$, ringOverlay$: ringOverlay$, - leave$: leave$, + leave$: shouldLeave$, hangup: (): void => userHangup$.next(), join: localMembership.requestConnect, toggleScreenSharing: toggleScreenSharing, @@ -1500,7 +1491,7 @@ export function createCallViewModel$( showFooter$: showFooter$, earpieceMode$: earpieceMode$, audioOutputSwitcher$: audioOutputSwitcher$, - reconnecting$: reconnecting$, + reconnecting$: localMembership.reconnecting$, }; } diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts index 1f61e533..87ca35d0 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.test.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.test.ts @@ -97,106 +97,106 @@ describe("createHomeserverConnected$", () => { // LLM generated test cases. They are a bit overkill but I improved the mocking so it is // easy enough to read them so I think they can stay. it("is false when sync state is not Syncing", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); - expect(hsConnected$.value).toBe(false); + const hsConnected = createHomeserverConnected$(scope, client, session); + expect(hsConnected.combined$.value).toBe(false); }); it("remains false while membership status is not Connected even if sync is Syncing", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); client.setSyncState(SyncState.Syncing); - expect(hsConnected$.value).toBe(false); // membership still disconnected + expect(hsConnected.combined$.value).toBe(false); // membership still disconnected }); it("is false when membership status transitions to Connected but ProbablyLeft is true", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); // Make sync loop OK client.setSyncState(SyncState.Syncing); // Indicate probable leave before connection session.setProbablyLeft(true); session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); }); it("becomes true only when all three conditions are satisfied", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); // 1. Sync loop connected client.setSyncState(SyncState.Syncing); - expect(hsConnected$.value).toBe(false); // not yet membership connected + expect(hsConnected.combined$.value).toBe(false); // not yet membership connected // 2. Membership connected session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); // probablyLeft is false + expect(hsConnected.combined$.value).toBe(true); // probablyLeft is false }); it("drops back to false when sync loop leaves Syncing", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); // Reach connected state client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); // Sync loop error => should flip false client.setSyncState(SyncState.Error); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); }); it("drops back to false when membership status becomes disconnected", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); session.setMembershipStatus(Status.Disconnected); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); }); it("drops to false when ProbablyLeft is emitted after being true", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); session.setProbablyLeft(true); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); }); it("recovers to true if ProbablyLeft becomes false again while other conditions remain true", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); client.setSyncState(SyncState.Syncing); session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); session.setProbablyLeft(true); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); // Simulate clearing the flag (in realistic scenario membership manager would update) session.setProbablyLeft(false); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); }); it("composite sequence reflects each individual failure reason", () => { - const hsConnected$ = createHomeserverConnected$(scope, client, session); + const hsConnected = createHomeserverConnected$(scope, client, session); // Initially false (sync error + disconnected + not probably left) - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); // Fix sync only client.setSyncState(SyncState.Syncing); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); // Fix membership session.setMembershipStatus(Status.Connected); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); // Introduce probablyLeft -> false session.setProbablyLeft(true); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); // Restore notProbablyLeft -> true again session.setProbablyLeft(false); - expect(hsConnected$.value).toBe(true); + expect(hsConnected.combined$.value).toBe(true); // Drop sync -> false client.setSyncState(SyncState.Error); - expect(hsConnected$.value).toBe(false); + expect(hsConnected.combined$.value).toBe(false); }); }); diff --git a/src/state/CallViewModel/localMember/HomeserverConnected.ts b/src/state/CallViewModel/localMember/HomeserverConnected.ts index e1c28078..c8bcd021 100644 --- a/src/state/CallViewModel/localMember/HomeserverConnected.ts +++ b/src/state/CallViewModel/localMember/HomeserverConnected.ts @@ -25,6 +25,11 @@ import { type NodeStyleEventEmitter } from "../../../utils/test"; */ const logger = rootLogger.getChild("[HomeserverConnected]"); +export interface HomeserverConnected { + combined$: Behavior; + rtsSession$: Behavior; +} + /** * Behavior representing whether we consider ourselves connected to the Matrix homeserver * for the purposes of a MatrixRTC session. @@ -39,7 +44,7 @@ export function createHomeserverConnected$( client: NodeStyleEventEmitter & Pick, matrixRTCSession: NodeStyleEventEmitter & Pick, -): Behavior { +): HomeserverConnected { const syncing$ = ( fromEvent(client, ClientEvent.Sync) as Observable<[SyncState]> ).pipe( @@ -47,12 +52,15 @@ export function createHomeserverConnected$( map(([state]) => state === SyncState.Syncing), ); - const membershipConnected$ = fromEvent( - matrixRTCSession, - MembershipManagerEvent.StatusChanged, - ).pipe( - startWith(null), - map(() => matrixRTCSession.membershipStatus === Status.Connected), + const rtsSession$ = scope.behavior( + fromEvent(matrixRTCSession, MembershipManagerEvent.StatusChanged).pipe( + map(() => matrixRTCSession.membershipStatus ?? Status.Unknown), + ), + Status.Unknown, + ); + + const membershipConnected$ = rtsSession$.pipe( + map((status) => status === Status.Connected), ); // This is basically notProbablyLeft$ @@ -71,15 +79,13 @@ export function createHomeserverConnected$( map(() => matrixRTCSession.probablyLeft !== true), ); - const connectedCombined$ = and$( - syncing$, - membershipConnected$, - certainlyConnected$, - ).pipe( - tap((connected) => { - logger.info(`Homeserver connected update: ${connected}`); - }), + const combined$ = scope.behavior( + and$(syncing$, membershipConnected$, certainlyConnected$).pipe( + tap((connected) => { + logger.info(`Homeserver connected update: ${connected}`); + }), + ), ); - return scope.behavior(connectedCombined$); + return { combined$, rtsSession$ }; } diff --git a/src/state/CallViewModel/localMember/LocalMembership.test.ts b/src/state/CallViewModel/localMember/LocalMembership.test.ts index cff5c06d..1ef7abd6 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.test.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.test.ts @@ -7,6 +7,7 @@ Please see LICENSE in the repository root for full details. */ import { + Status, type LivekitTransport, type MatrixRTCSession, } from "matrix-js-sdk/lib/matrixrtc"; @@ -51,7 +52,7 @@ vi.mock("@livekit/components-core", () => ({ describe("LocalMembership", () => { describe("enterRTCSession", () => { - it("It joins the correct Session", async () => { + it("It joins the correct Session", () => { const focusFromOlderMembership = { type: "livekit", livekit_service_url: "http://my-oldest-member-service-url.com", @@ -107,7 +108,7 @@ describe("LocalMembership", () => { joinRoomSession: vi.fn(), }) as unknown as MatrixRTCSession; - await enterRTCSession( + enterRTCSession( mockedSession, { livekit_alias: "roomId", @@ -136,7 +137,7 @@ describe("LocalMembership", () => { ); }); - it("It should not fail with configuration error if homeserver config has livekit url but not fallback", async () => { + it("It should not fail with configuration error if homeserver config has livekit url but not fallback", () => { mockConfig({}); vi.spyOn(AutoDiscovery, "getRawClientConfig").mockResolvedValue({ "org.matrix.msc4143.rtc_foci": [ @@ -165,7 +166,7 @@ describe("LocalMembership", () => { joinRoomSession: vi.fn(), }) as unknown as MatrixRTCSession; - await enterRTCSession( + enterRTCSession( mockedSession, { livekit_alias: "roomId", @@ -190,7 +191,6 @@ describe("LocalMembership", () => { leaveRoomSession: () => {}, } as unknown as MatrixRTCSession, muteStates: mockMuteStates(), - isHomeserverConnected: constant(true), trackProcessorState$: constant({ supported: false, processor: undefined, @@ -198,7 +198,10 @@ describe("LocalMembership", () => { logger: logger, createPublisherFactory: vi.fn(), joinMatrixRTC: async (): Promise => {}, - homeserverConnected$: constant(true), + homeserverConnected: { + combined$: constant(true), + rtsSession$: constant(Status.Connected), + }, }; it("throws error on missing RTC config error", () => { @@ -258,8 +261,7 @@ describe("LocalMembership", () => { } as unknown as LocalParticipant, }), state$: constant({ - state: "ConnectedToLkRoom", - livekitConnectionState$: constant(LivekitConnectionState.Connected), + state: LivekitConnectionState.Connected, }), transport: aTransport, } as unknown as Connection, @@ -268,7 +270,7 @@ describe("LocalMembership", () => { connectionManagerData.add( { state$: constant({ - state: "ConnectedToLkRoom", + state: LivekitConnectionState.Connected, }), transport: bTransport, } as unknown as Connection, @@ -443,7 +445,7 @@ describe("LocalMembership", () => { connectionManagerData$.next(new Epoch(connectionManagerData)); await flushPromises(); expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.Initialized, + state: LivekitConnectionState.Connected, }); expect(publisherFactory).toHaveBeenCalledOnce(); expect(localMembership.tracks$.value.length).toBe(0); @@ -473,7 +475,7 @@ describe("LocalMembership", () => { publishResolver.resolve(); await flushPromises(); expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.Connected, + state: RTCBackendState.ConnectedAndPublishing, }); expect(publishers[0].stopPublishing).not.toHaveBeenCalled(); @@ -482,7 +484,7 @@ describe("LocalMembership", () => { await flushPromises(); // stays in connected state because it is stopped before the update to tracks update the state. expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.Connected, + state: RTCBackendState.ConnectedAndPublishing, }); // stop all tracks after ending scopes expect(publishers[0].stopPublishing).toHaveBeenCalled(); diff --git a/src/state/CallViewModel/localMember/LocalMembership.ts b/src/state/CallViewModel/localMember/LocalMembership.ts index 60ae79b8..33af5192 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.ts @@ -11,10 +11,11 @@ import { ParticipantEvent, type LocalParticipant, type ScreenShareCaptureOptions, - ConnectionState, + ConnectionState as LivekitConnectionState, } from "livekit-client"; import { observeParticipantEvents } from "@livekit/components-core"; import { + Status as RTCSessionStatus, type LivekitTransport, type MatrixRTCSession, } from "matrix-js-sdk/lib/matrixrtc"; @@ -27,7 +28,7 @@ import { map, type Observable, of, - scan, + pairwise, startWith, switchMap, tap, @@ -37,10 +38,9 @@ import { deepCompare } from "matrix-js-sdk/lib/utils"; import { constant, type Behavior } from "../../Behavior"; import { type IConnectionManager } from "../remoteMembers/ConnectionManager"; -import { ObservableScope } from "../../ObservableScope"; +import { type ObservableScope } from "../../ObservableScope"; import { type Publisher } from "./Publisher"; import { type MuteStates } from "../../MuteStates"; -import { and$ } from "../../../utils/observable"; import { ElementCallError, MembershipManagerError, @@ -51,7 +51,11 @@ import { getUrlParams } from "../../../UrlParams.ts"; import { PosthogAnalytics } from "../../../analytics/PosthogAnalytics.ts"; import { MatrixRTCMode } from "../../../settings/settings.ts"; import { Config } from "../../../config/Config.ts"; -import { type Connection } from "../remoteMembers/Connection.ts"; +import { + type ConnectionState, + type Connection, +} from "../remoteMembers/Connection.ts"; +import { type HomeserverConnected } from "./HomeserverConnected.ts"; export enum RTCBackendState { Error = "error", @@ -59,47 +63,32 @@ export enum RTCBackendState { WaitingForTransport = "waiting_for_transport", /** A connection appeared so we can initialise the publisher */ WaitingForConnection = "waiting_for_connection", - /** Connection and transport arrived, publisher Initialized */ - Initialized = "Initialized", + /** Implies lk connection is connected */ CreatingTracks = "creating_tracks", + /** Implies lk connection is connected */ ReadyToPublish = "ready_to_publish", + /** Implies lk connection is connected */ WaitingToPublish = "waiting_to_publish", - Connected = "connected", - Disconnected = "disconnected", - Disconnecting = "disconnecting", + /** Implies lk connection is connected */ + ConnectedAndPublishing = "fully_connected", } -type LocalMemberRtcBackendState = +type LocalMemberRTCBackendState = | { state: RTCBackendState.Error; error: ElementCallError } - | { state: RTCBackendState.WaitingForTransport } - | { state: RTCBackendState.WaitingForConnection } - | { state: RTCBackendState.Initialized } - | { state: RTCBackendState.CreatingTracks } - | { state: RTCBackendState.ReadyToPublish } - | { state: RTCBackendState.WaitingToPublish } - | { state: RTCBackendState.Connected } - | { state: RTCBackendState.Disconnected } - | { state: RTCBackendState.Disconnecting }; + | { state: Exclude } + | ConnectionState; -export enum MatrixState { +export enum MatrixAdditionalState { WaitingForTransport = "waiting_for_transport", - Ready = "ready", - Connecting = "connecting", - Connected = "connected", - Disconnected = "disconnected", - Error = "Error", } type LocalMemberMatrixState = - | { state: MatrixState.Connected } - | { state: MatrixState.WaitingForTransport } - | { state: MatrixState.Ready } - | { state: MatrixState.Connecting } - | { state: MatrixState.Disconnected } - | { state: MatrixState.Error; error: Error }; + | { state: MatrixAdditionalState.WaitingForTransport } + | { state: "Error"; error: Error } + | { state: RTCSessionStatus }; export interface LocalMemberConnectionState { - livekit$: Behavior; + livekit$: Behavior; matrix$: Behavior; } @@ -122,8 +111,8 @@ interface Props { muteStates: MuteStates; connectionManager: IConnectionManager; createPublisherFactory: (connection: Connection) => Publisher; - joinMatrixRTC: (transport: LivekitTransport) => Promise; - homeserverConnected$: Behavior; + joinMatrixRTC: (transport: LivekitTransport) => void; + homeserverConnected: HomeserverConnected; localTransport$: Behavior; matrixRTCSession: Pick< MatrixRTCSession, @@ -149,7 +138,7 @@ export const createLocalMembership$ = ({ scope, connectionManager, localTransport$: localTransportCanThrow$, - homeserverConnected$, + homeserverConnected, createPublisherFactory, joinMatrixRTC, logger: parentLogger, @@ -175,10 +164,14 @@ export const createLocalMembership$ = ({ tracks$: Behavior; participant$: Behavior; connection$: Behavior; - homeserverConnected$: Behavior; - // this needs to be discussed - /** @deprecated use state instead*/ + /** Shorthand for connectionState.matrix.state === Status.Reconnecting + * Direct translation to the js-sdk membership manager connection `Status`. + */ reconnecting$: Behavior; + /** Shorthand for connectionState.matrix.state === Status.Disconnected + * Direct translation to the js-sdk membership manager connection `Status`. + */ + disconnected$: Behavior; } => { const logger = parentLogger.getChild("[LocalMembership]"); logger.debug(`Creating local membership..`); @@ -232,49 +225,31 @@ export const createLocalMembership$ = ({ // * Whether we are "fully" connected to the call. Accounts for both the // * connection to the MatrixRTC session and the LiveKit publish connection. // */ - const connected$ = scope.behavior( - and$( - homeserverConnected$.pipe( - tap((v) => logger.debug("matrix: Connected state changed", v)), - ), - localConnectionState$.pipe( - switchMap((state) => { - logger.debug("livekit: Connected state changed", state); - if (!state) return of(false); - if (state.state === "ConnectedToLkRoom") { - logger.debug( - "livekit: Connected state changed (inner livekitConnectionState$)", - state.livekitConnectionState$.value, - ); - return state.livekitConnectionState$.pipe( - map((lkState) => lkState === ConnectionState.Connected), - ); - } - return of(false); - }), - ), - ).pipe(tap((v) => logger.debug("combined: Connected state changed", v))), - ); + // TODO remove this and just make it one single check of livekitConnectionState$ + // const connected$ = scope.behavior( + // localConnectionState$.pipe( + // switchMap((state) => { + // logger.debug("livekit: Connected state changed", state); + // if (!state) return of(false); + // if (state.state === "ConnectedToLkRoom") { + // logger.debug( + // "livekit: Connected state changed (inner livekitConnectionState$)", + // state.livekitConnectionState$.value, + // ); + // return state.livekitConnectionState$.pipe( + // map((lkState) => lkState === ConnectionState.Connected), + // ); + // } + // return of(false); + // }), + // ), + // ); // MATRIX RELATED - // /** - // * Whether we should tell the user that we're reconnecting to the call. - // */ - // DISCUSSION is there a better way to do this? - // sth that is more deriectly implied from the membership manager of the js sdk. (fromEvent(matrixRTCSession, Reconnecting)) ??? or similar const reconnecting$ = scope.behavior( - connected$.pipe( - // We are reconnecting if we previously had some successful initial - // connection but are now disconnected - scan( - ({ connectedPreviously }, connectedNow) => ({ - connectedPreviously: connectedPreviously || connectedNow, - reconnecting: connectedPreviously && !connectedNow, - }), - { connectedPreviously: false, reconnecting: false }, - ), - map(({ reconnecting }) => reconnecting), + homeserverConnected.rtsSession$.pipe( + map((sessionStatus) => sessionStatus === RTCSessionStatus.Reconnecting), ), ); @@ -374,8 +349,9 @@ export const createLocalMembership$ = ({ logger.error("Multiple Livkit Errors:", e); else fatalLivekitError$.next(e); }; - const livekitState$: Behavior = scope.behavior( + const livekitState$: Behavior = scope.behavior( combineLatest([ + localConnectionState$, publisher$, localTransport$, tracks$.pipe( @@ -389,10 +365,12 @@ export const createLocalMembership$ = ({ map(() => true), startWith(false), ), + // TODO use local connection state here to give the full pciture of the livekit state! fatalLivekitError$, ]).pipe( map( ([ + localConnectionState, publisher, localTransport, tracks, @@ -411,13 +389,21 @@ export const createLocalMembership$ = ({ const hasTracks = tracks.length > 0; if (!localTransport) return { state: RTCBackendState.WaitingForTransport }; - if (!publisher) + if (!localConnectionState) return { state: RTCBackendState.WaitingForConnection }; - if (!shouldStartTracks) return { state: RTCBackendState.Initialized }; + if ( + localConnectionState.state !== LivekitConnectionState.Connected || + !publisher + ) + // pass through the localConnectionState while we do not yet have a publisher or the state + // of the connection is not yet connected + return { state: localConnectionState.state }; + if (!shouldStartTracks) + return { state: LivekitConnectionState.Connected }; if (!hasTracks) return { state: RTCBackendState.CreatingTracks }; if (!shouldConnect) return { state: RTCBackendState.ReadyToPublish }; if (!publishing) return { state: RTCBackendState.WaitingToPublish }; - return { state: RTCBackendState.Connected }; + return { state: RTCBackendState.ConnectedAndPublishing }; }, ), distinctUntilChanged(deepCompare), @@ -431,58 +417,70 @@ export const createLocalMembership$ = ({ else fatalMatrixError$.next(e); }; const matrixState$: Behavior = scope.behavior( - combineLatest([ - localTransport$, - connectRequested$, - homeserverConnected$, - ]).pipe( - map(([localTransport, connectRequested, homeserverConnected]) => { - if (!localTransport) return { state: MatrixState.WaitingForTransport }; - if (!connectRequested) return { state: MatrixState.Ready }; - if (!homeserverConnected) return { state: MatrixState.Connecting }; - return { state: MatrixState.Connected }; + combineLatest([localTransport$, homeserverConnected.rtsSession$]).pipe( + map(([localTransport, rtcSessionStatus]) => { + if (!localTransport) + return { state: MatrixAdditionalState.WaitingForTransport }; + return { state: rtcSessionStatus }; }), ), ); - // Keep matrix rtc session in sync with localTransport$, connectRequested$ and muteStates.video.enabled$ + // inform the widget about the connect and disconnect intent from the user. + scope + .behavior(connectRequested$.pipe(pairwise(), scope.bind()), [ + undefined, + connectRequested$.value, + ]) + .subscribe(([prev, current]) => { + if (!widget) return; + if (!prev && current) { + try { + void widget.api.transport.send(ElementWidgetActions.JoinCall, {}); + } catch (e) { + logger.error("Failed to send join action", e); + } + } + if (prev && !current) { + try { + void widget?.api.transport.send(ElementWidgetActions.HangupCall, {}); + } catch (e) { + logger.error("Failed to send hangup action", e); + } + } + }); + + combineLatest([muteStates.video.enabled$, homeserverConnected.combined$]) + .pipe(scope.bind()) + .subscribe(([videoEnabled, connected]) => { + if (!connected) return; + void matrixRTCSession.updateCallIntent(videoEnabled ? "video" : "audio"); + }); + + // Keep matrix rtc session in sync with localTransport$, connectRequested$ scope.reconcile( scope.behavior(combineLatest([localTransport$, connectRequested$])), async ([transport, shouldConnect]) => { + if (!transport) return; + // if shouldConnect=false we will do the disconnect as the cleanup from the previous reconcile iteration. if (!shouldConnect) return; - if (!transport) return; try { - await joinMatrixRTC(transport); + joinMatrixRTC(transport); } catch (error) { logger.error("Error entering RTC session", error); if (error instanceof Error) setMatrixError(new MembershipManagerError(error)); } - // Update our member event when our mute state changes. - const callIntentScope = new ObservableScope(); - // because this uses its own scope, we can start another reconciliation for the duration of one connection. - callIntentScope.reconcile( - muteStates.video.enabled$, - async (videoEnabled) => - matrixRTCSession.updateCallIntent(videoEnabled ? "video" : "audio"), - ); - - return async (): Promise => { - callIntentScope.end(); + return Promise.resolve(async (): Promise => { try { - // Update matrixRTCSession to allow udpating the transport without leaving the session! - await matrixRTCSession.leaveRoomSession(); + // TODO Update matrixRTCSession to allow udpating the transport without leaving the session! + await matrixRTCSession.leaveRoomSession(1000); } catch (e) { logger.error("Error leaving RTC session", e); } - try { - await widget?.api.transport.send(ElementWidgetActions.HangupCall, {}); - } catch (e) { - logger.error("Failed to send hangup action", e); - } - }; + }); }, ); @@ -497,7 +495,7 @@ export const createLocalMembership$ = ({ // pause tracks during the initial joining sequence too until we're sure // that our own media is displayed on screen. // TODO refactor this based no livekitState$ - combineLatest([participant$, homeserverConnected$]) + combineLatest([participant$, homeserverConnected.combined$]) .pipe(scope.bind()) .subscribe(([participant, connected]) => { if (!participant) return; @@ -590,8 +588,15 @@ export const createLocalMembership$ = ({ }, tracks$, participant$, - homeserverConnected$, reconnecting$, + disconnected$: scope.behavior( + matrixState$.pipe( + map( + (sessionStatus) => + sessionStatus.state === RTCSessionStatus.Disconnected, + ), + ), + ), sharingScreen$, toggleScreenSharing, connection$: localConnection$, @@ -626,11 +631,11 @@ interface EnterRTCSessionOptions { * @throws If the widget could not send ElementWidgetActions.JoinCall action. */ // Exported for unit testing -export async function enterRTCSession( +export function enterRTCSession( rtcSession: MatrixRTCSession, transport: LivekitTransport, { encryptMedia, matrixRTCMode }: EnterRTCSessionOptions, -): Promise { +): void { PosthogAnalytics.instance.eventCallEnded.cacheStartCall(new Date()); PosthogAnalytics.instance.eventCallStarted.track(rtcSession.room.roomId); @@ -669,7 +674,4 @@ export async function enterRTCSession( unstableSendStickyEvents: matrixRTCMode === MatrixRTCMode.Matrix_2_0, }, ); - if (widget) { - await widget.api.transport.send(ElementWidgetActions.JoinCall, {}); - } } diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 9b3e5b2a..5468d1ff 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -53,8 +53,7 @@ describe("Publisher", () => { scope = new ObservableScope(); connection = { state$: constant({ - state: "ConnectedToLkRoom", - livekitConnectionState$: constant(LivekitConenctionState.Connected), + state: LivekitConenctionState.Connected, }), livekitRoom: mockLivekitRoom({ localParticipant: mockLocalParticipant({}), diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index 326dedaf..6e4a9b35 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -160,7 +160,7 @@ export class Publisher { const { promise, resolve, reject } = Promise.withResolvers(); const sub = this.connection.state$.subscribe((s) => { switch (s.state) { - case "ConnectedToLkRoom": + case LivekitConnectionState.Connected: resolve(); break; case "FailedToStart": diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 2ead768b..efee1ccb 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -125,7 +125,10 @@ function setupRemoteConnection(): Connection { }; }); - fakeLivekitRoom.connect.mockResolvedValue(undefined); + fakeLivekitRoom.connect.mockImplementation(async (): Promise => { + fakeLivekitRoom.state = LivekitConnectionState.Connected; + return Promise.resolve(); + }); return new Connection(opts, logger); } @@ -309,7 +312,7 @@ describe("Start connection states", () => { capturedState = capturedStates.pop(); - if (capturedState && capturedState?.state === "FailedToStart") { + if (capturedState && capturedState.state === "FailedToStart") { expect(capturedState.error.message).toContain( "Failed to connect to livekit", ); @@ -345,7 +348,7 @@ describe("Start connection states", () => { const connectingState = capturedStates.shift(); expect(connectingState?.state).toEqual("ConnectingToLkRoom"); const connectedState = capturedStates.shift(); - expect(connectedState?.state).toEqual("ConnectedToLkRoom"); + expect(connectedState?.state).toEqual("connected"); }); it("shutting down the scope should stop the connection", async () => { diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 4f3bbda4..962f56d9 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -12,7 +12,7 @@ import { } from "@livekit/components-core"; import { ConnectionError, - type ConnectionState as LivekitConenctionState, + type ConnectionState as LivekitConnectionState, type Room as LivekitRoom, type LocalParticipant, type RemoteParticipant, @@ -47,17 +47,17 @@ export interface ConnectionOpts { /** Optional factory to create the LiveKit room, mainly for testing purposes. */ livekitRoomFactory: () => LivekitRoom; } - +export enum ConnectionAdditionalState { + Initialized = "Initialized", + FetchingConfig = "FetchingConfig", + // FailedToStart = "FailedToStart", + Stopped = "Stopped", + ConnectingToLkRoom = "ConnectingToLkRoom", +} export type ConnectionState = - | { state: "Initialized" } - | { state: "FetchingConfig" } - | { state: "ConnectingToLkRoom" } - | { - state: "ConnectedToLkRoom"; - livekitConnectionState$: Behavior; - } - | { state: "FailedToStart"; error: Error } - | { state: "Stopped" }; + | { state: ConnectionAdditionalState } + | { state: LivekitConnectionState } + | { state: "FailedToStart"; error: Error }; /** * A connection to a Matrix RTC LiveKit backend. @@ -67,7 +67,7 @@ export type ConnectionState = export class Connection { // Private Behavior private readonly _state$ = new BehaviorSubject({ - state: "Initialized", + state: ConnectionAdditionalState.Initialized, }); /** @@ -118,14 +118,14 @@ export class Connection { this.stopped = false; try { this._state$.next({ - state: "FetchingConfig", + state: ConnectionAdditionalState.FetchingConfig, }); const { url, jwt } = await this.getSFUConfigWithOpenID(); // If we were stopped while fetching the config, don't proceed to connect if (this.stopped) return; this._state$.next({ - state: "ConnectingToLkRoom", + state: ConnectionAdditionalState.ConnectingToLkRoom, }); try { await this.livekitRoom.connect(url, jwt); @@ -154,12 +154,13 @@ export class Connection { // If we were stopped while connecting, don't proceed to update state. if (this.stopped) return; - this._state$.next({ - state: "ConnectedToLkRoom", - livekitConnectionState$: this.scope.behavior( - connectionStateObserver(this.livekitRoom), - ), - }); + connectionStateObserver(this.livekitRoom) + .pipe(this.scope.bind()) + .subscribe((lkState) => { + this._state$.next({ + state: lkState, + }); + }); } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); this._state$.next({ @@ -191,7 +192,7 @@ export class Connection { if (this.stopped) return; await this.livekitRoom.disconnect(); this._state$.next({ - state: "Stopped", + state: ConnectionAdditionalState.Stopped, }); this.stopped = true; } From 88721be9521843ab106fabc4e1c447bcabb2247a Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 3 Dec 2025 10:04:22 +0100 Subject: [PATCH 02/43] cleanup --- src/room/GroupCallView.tsx | 1 + src/room/InCallView.tsx | 1 + .../localMember/LocalMembership.ts | 25 ------------------- 3 files changed, 2 insertions(+), 25 deletions(-) diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 43602716..dfd11ff3 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -314,6 +314,7 @@ export const GroupCallView: FC = ({ const navigate = useNavigate(); + // TODO split this into leave and onDisconnect const onLeft = useCallback( ( reason: "timeout" | "user" | "allOthersLeft" | "decline" | "error", diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 6ae004d8..7ae3700c 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -151,6 +151,7 @@ export const ActiveCall: FC = (props) => { setVm(vm); vm.leave$.pipe(scope.bind()).subscribe(props.onLeft); + return (): void => { scope.end(); }; diff --git a/src/state/CallViewModel/localMember/LocalMembership.ts b/src/state/CallViewModel/localMember/LocalMembership.ts index 33af5192..6a31ce4b 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.ts @@ -221,30 +221,6 @@ export const createLocalMembership$ = ({ switchMap((connection) => (connection ? connection.state$ : of(null))), ); - // /** - // * Whether we are "fully" connected to the call. Accounts for both the - // * connection to the MatrixRTC session and the LiveKit publish connection. - // */ - // TODO remove this and just make it one single check of livekitConnectionState$ - // const connected$ = scope.behavior( - // localConnectionState$.pipe( - // switchMap((state) => { - // logger.debug("livekit: Connected state changed", state); - // if (!state) return of(false); - // if (state.state === "ConnectedToLkRoom") { - // logger.debug( - // "livekit: Connected state changed (inner livekitConnectionState$)", - // state.livekitConnectionState$.value, - // ); - // return state.livekitConnectionState$.pipe( - // map((lkState) => lkState === ConnectionState.Connected), - // ); - // } - // return of(false); - // }), - // ), - // ); - // MATRIX RELATED const reconnecting$ = scope.behavior( @@ -365,7 +341,6 @@ export const createLocalMembership$ = ({ map(() => true), startWith(false), ), - // TODO use local connection state here to give the full pciture of the livekit state! fatalLivekitError$, ]).pipe( map( From 7c40b0e177fbbfbf14c7c28b71a22b58f6df94e4 Mon Sep 17 00:00:00 2001 From: Timo K Date: Fri, 5 Dec 2025 19:48:02 +0100 Subject: [PATCH 03/43] ideas --- src/state/CallViewModel/CallViewModel.ts | 47 +++++++++++-------- .../CallViewModel/remoteMembers/Connection.ts | 43 +++++++++-------- 2 files changed, 51 insertions(+), 39 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 9bfa979c..3c15958a 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -452,14 +452,18 @@ export function createCallViewModel$( const localMembership = createLocalMembership$({ scope: scope, - homeserverConnected: createHomeserverConnected$( + homeserverConnected$: createHomeserverConnected$( scope, client, matrixRTCSession, ), muteStates: muteStates, - joinMatrixRTC: (transport: LivekitTransport) => { - enterRTCSession(matrixRTCSession, transport, connectOptions$.value); + joinMatrixRTC: async (transport: LivekitTransport) => { + return enterRTCSession( + matrixRTCSession, + transport, + connectOptions$.value, + ); }, createPublisherFactory: (connection: Connection) => { return new Publisher( @@ -569,6 +573,17 @@ export function createCallViewModel$( ), ); + /** + * Whether various media/event sources should pretend to be disconnected from + * all network input, even if their connection still technically works. + */ + // We do this when the app is in the 'reconnecting' state, because it might be + // that the LiveKit connection is still functional while the homeserver is + // down, for example, and we want to avoid making people worry that the app is + // in a split-brained state. + // DISCUSSION own membership manager ALSO this probably can be simplifis + const reconnecting$ = localMembership.reconnecting$; + const audioParticipants$ = scope.behavior( matrixLivekitMembers$.pipe( switchMap((membersWithEpoch) => { @@ -616,7 +631,7 @@ export function createCallViewModel$( ); const handsRaised$ = scope.behavior( - handsRaisedSubject$.pipe(pauseWhen(localMembership.reconnecting$)), + handsRaisedSubject$.pipe(pauseWhen(reconnecting$)), ); const reactions$ = scope.behavior( @@ -629,7 +644,7 @@ export function createCallViewModel$( ]), ), ), - pauseWhen(localMembership.reconnecting$), + pauseWhen(reconnecting$), ), ); @@ -720,7 +735,7 @@ export function createCallViewModel$( livekitRoom$, focusUrl$, mediaDevices, - localMembership.reconnecting$, + reconnecting$, displayName$, matrixMemberMetadataStore.createAvatarUrlBehavior$(userId), handsRaised$.pipe(map((v) => v[participantId]?.time ?? null)), @@ -812,17 +827,11 @@ export function createCallViewModel$( }), ); - const shouldLeave$: Observable< - "user" | "timeout" | "decline" | "allOthersLeft" - > = merge( - autoLeave$, - merge(userHangup$, widgetHangup$).pipe(map(() => "user" as const)), - ).pipe(scope.share); - - shouldLeave$.pipe(scope.bind()).subscribe((reason) => { - logger.info(`Call left due to ${reason}`); - localMembership.requestDisconnect(); - }); + const leave$: Observable<"user" | "timeout" | "decline" | "allOthersLeft"> = + merge( + autoLeave$, + merge(userHangup$, widgetHangup$).pipe(map(() => "user" as const)), + ).pipe(scope.share); const spotlightSpeaker$ = scope.behavior( userMedia$.pipe( @@ -1444,7 +1453,7 @@ export function createCallViewModel$( autoLeave$: autoLeave$, callPickupState$: callPickupState$, ringOverlay$: ringOverlay$, - leave$: shouldLeave$, + leave$: leave$, hangup: (): void => userHangup$.next(), join: localMembership.requestConnect, toggleScreenSharing: toggleScreenSharing, @@ -1491,7 +1500,7 @@ export function createCallViewModel$( showFooter$: showFooter$, earpieceMode$: earpieceMode$, audioOutputSwitcher$: audioOutputSwitcher$, - reconnecting$: localMembership.reconnecting$, + reconnecting$: reconnecting$, }; } diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 962f56d9..549777f9 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -12,7 +12,7 @@ import { } from "@livekit/components-core"; import { ConnectionError, - type ConnectionState as LivekitConnectionState, + ConnectionState as LivekitConnectionState, type Room as LivekitRoom, type LocalParticipant, type RemoteParticipant, @@ -47,17 +47,24 @@ export interface ConnectionOpts { /** Optional factory to create the LiveKit room, mainly for testing purposes. */ livekitRoomFactory: () => LivekitRoom; } -export enum ConnectionAdditionalState { +export class FailedToStartError extends Error { + public constructor(message: string) { + super(message); + this.name = "FailedToStartError"; + } +} + +export enum ConnectionState { Initialized = "Initialized", FetchingConfig = "FetchingConfig", - // FailedToStart = "FailedToStart", Stopped = "Stopped", ConnectingToLkRoom = "ConnectingToLkRoom", + LivekitDisconnected = "disconnected", + LivekitConnecting = "connecting", + LivekitConnected = "connected", + LivekitReconnecting = "reconnecting", + LivekitSignalReconnecting = "signalReconnecting", } -export type ConnectionState = - | { state: ConnectionAdditionalState } - | { state: LivekitConnectionState } - | { state: "FailedToStart"; error: Error }; /** * A connection to a Matrix RTC LiveKit backend. @@ -66,14 +73,15 @@ export type ConnectionState = */ export class Connection { // Private Behavior - private readonly _state$ = new BehaviorSubject({ - state: ConnectionAdditionalState.Initialized, - }); + private readonly _state$ = new BehaviorSubject< + ConnectionState | FailedToStartError + >(ConnectionState.Initialized); /** * The current state of the connection to the media transport. */ - public readonly state$: Behavior = this._state$; + public readonly state$: Behavior = + this._state$; /** * The media transport to connect to. @@ -117,16 +125,12 @@ export class Connection { this.logger.debug("Starting Connection"); this.stopped = false; try { - this._state$.next({ - state: ConnectionAdditionalState.FetchingConfig, - }); + this._state$.next(ConnectionState.FetchingConfig); const { url, jwt } = await this.getSFUConfigWithOpenID(); // If we were stopped while fetching the config, don't proceed to connect if (this.stopped) return; - this._state$.next({ - state: ConnectionAdditionalState.ConnectingToLkRoom, - }); + this._state$.next(ConnectionState.ConnectingToLkRoom); try { await this.livekitRoom.connect(url, jwt); } catch (e) { @@ -157,9 +161,8 @@ export class Connection { connectionStateObserver(this.livekitRoom) .pipe(this.scope.bind()) .subscribe((lkState) => { - this._state$.next({ - state: lkState, - }); + // It si save to cast lkState to ConnectionState as they are fully overlapping. + this._state$.next(lkState as unknown as ConnectionState); }); } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); From 2986f90a5f21f12519be862be5d53b3ca96c7d64 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 22:29:15 -0500 Subject: [PATCH 04/43] Allow MatrixRTC mode to be configured in tests --- src/state/CallViewModel/CallViewModel.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 5cc33f5d..fb50696f 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -56,7 +56,7 @@ import { accumulate, generateItems, pauseWhen } from "../../utils/observable"; import { duplicateTiles, MatrixRTCMode, - matrixRTCMode, + matrixRTCMode as matrixRTCModeSetting, playReactionsSound, showReactions, } from "../../settings/settings"; @@ -149,6 +149,8 @@ export interface CallViewModelOptions { connectionState$?: Behavior; /** Optional behavior overriding the computed window size, mainly for testing purposes. */ windowSize$?: Behavior<{ width: number; height: number }>; + /** Optional behavior overriding the MatrixRTC mode, mainly for testing purposes. */ + matrixRTCMode$?: Behavior; } // Do not play any sounds if the participant count has exceeded this @@ -399,13 +401,15 @@ export function createCallViewModel$( memberships$, ); + const matrixRTCMode$ = options.matrixRTCMode$ ?? matrixRTCModeSetting.value$; + const localTransport$ = createLocalTransport$({ scope: scope, memberships$: memberships$, client, roomId: matrixRoom.roomId, useOldestMember$: scope.behavior( - matrixRTCMode.value$.pipe(map((v) => v === MatrixRTCMode.Legacy)), + matrixRTCMode$.pipe(map((v) => v === MatrixRTCMode.Legacy)), ), }); @@ -446,7 +450,7 @@ export function createCallViewModel$( }); const connectOptions$ = scope.behavior( - matrixRTCMode.value$.pipe( + matrixRTCMode$.pipe( map((mode) => ({ encryptMedia: livekitKeyProvider !== undefined, // TODO. This might need to get called again on each change of matrixRTCMode... From 5a9a62039c76f68e3155b819a520f6f500cab0f8 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 22:42:57 -0500 Subject: [PATCH 05/43] Test CallViewModel in all MatrixRTC modes --- src/state/CallViewModel/CallViewModel.test.ts | 11 +- .../CallViewModel/CallViewModelTestUtils.ts | 224 +++++++++--------- src/state/CallViewModelWidget.test.ts | 70 +++--- 3 files changed, 165 insertions(+), 140 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.test.ts b/src/state/CallViewModel/CallViewModel.test.ts index 2e5b5700..86cde12a 100644 --- a/src/state/CallViewModel/CallViewModel.test.ts +++ b/src/state/CallViewModel/CallViewModel.test.ts @@ -60,7 +60,8 @@ import { import { MediaDevices } from "../MediaDevices.ts"; import { getValue } from "../../utils/observable.ts"; import { type Behavior, constant } from "../Behavior.ts"; -import { withCallViewModel } from "./CallViewModelTestUtils.ts"; +import { withCallViewModel as withCallViewModelInMode } from "./CallViewModelTestUtils.ts"; +import { MatrixRTCMode } from "../../settings/settings.ts"; vi.mock("rxjs", async (importOriginal) => ({ ...(await importOriginal()), @@ -229,7 +230,13 @@ function mockRingEvent( // need a value to fill in for them when emitting notifications const mockLegacyRingEvent = {} as { event_id: string } & ICallNotifyContent; -describe("CallViewModel", () => { +describe.each([ + [MatrixRTCMode.Legacy], + [MatrixRTCMode.Compatibil], + [MatrixRTCMode.Matrix_2_0], +])("CallViewModel (%s mode)", (mode) => { + const withCallViewModel = withCallViewModelInMode(mode); + test("participants are retained during a focus switch", () => { withTestScheduler(({ behavior, expectObservable }) => { // Participants disappear on frame 2 and come back on frame 3 diff --git a/src/state/CallViewModel/CallViewModelTestUtils.ts b/src/state/CallViewModel/CallViewModelTestUtils.ts index f80b4bcb..e9996a41 100644 --- a/src/state/CallViewModel/CallViewModelTestUtils.ts +++ b/src/state/CallViewModel/CallViewModelTestUtils.ts @@ -53,6 +53,7 @@ import { import { type Behavior, constant } from "../Behavior"; import { type ProcessorState } from "../../livekit/TrackProcessorContext"; import { type MediaDevices } from "../MediaDevices"; +import { type MatrixRTCMode } from "../../settings/settings"; mockConfig({ livekit: { livekit_service_url: "http://my-default-service-url.com" }, @@ -80,117 +81,126 @@ export interface CallViewModelInputs { const localParticipant = mockLocalParticipant({ identity: "" }); -export function withCallViewModel( - { - remoteParticipants$ = constant([]), - rtcMembers$ = constant([localRtcMember]), - livekitConnectionState$: connectionState$ = constant( - ConnectionState.Connected, - ), - speaking = new Map(), - mediaDevices = mockMediaDevices({}), - initialSyncState = SyncState.Syncing, - windowSize$ = constant({ width: 1000, height: 800 }), - }: Partial = {}, - continuation: ( - vm: CallViewModel, - rtcSession: MockRTCSession, - subjects: { raisedHands$: BehaviorSubject> }, - setSyncState: (value: SyncState) => void, - ) => void, - options: CallViewModelOptions = { - encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, - autoLeaveWhenOthersLeft: false, - }, -): void { - let syncState = initialSyncState; - const setSyncState = (value: SyncState): void => { - const prev = syncState; - syncState = value; - room.client.emit(ClientEvent.Sync, value, prev); - }; - const room = mockMatrixRoom({ - client: new (class extends EventEmitter { - public getUserId(): string | undefined { - return localRtcMember.userId; - } +export function withCallViewModel(mode: MatrixRTCMode) { + return ( + { + remoteParticipants$ = constant([]), + rtcMembers$ = constant([localRtcMember]), + livekitConnectionState$: connectionState$ = constant( + ConnectionState.Connected, + ), + speaking = new Map(), + mediaDevices = mockMediaDevices({}), + initialSyncState = SyncState.Syncing, + windowSize$ = constant({ width: 1000, height: 800 }), + }: Partial = {}, + continuation: ( + vm: CallViewModel, + rtcSession: MockRTCSession, + subjects: { + raisedHands$: BehaviorSubject>; + }, + setSyncState: (value: SyncState) => void, + ) => void, + options: CallViewModelOptions = { + encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, + autoLeaveWhenOthersLeft: false, + }, + ): void => { + let syncState = initialSyncState; + const setSyncState = (value: SyncState): void => { + const prev = syncState; + syncState = value; + room.client.emit(ClientEvent.Sync, value, prev); + }; + const room = mockMatrixRoom({ + client: new (class extends EventEmitter { + public getUserId(): string | undefined { + return localRtcMember.userId; + } - public getDeviceId(): string { - return localRtcMember.deviceId; - } + public getDeviceId(): string { + return localRtcMember.deviceId; + } - public getDomain(): string { - return "example.com"; - } + public getDomain(): string { + return "example.com"; + } - public getSyncState(): SyncState { - return syncState; - } - })() as Partial as MatrixClient, - getMembers: () => Array.from(roomMembers.values()), - getMembersWithMembership: () => Array.from(roomMembers.values()), - }); - const rtcSession = new MockRTCSession(room, []).withMemberships(rtcMembers$); - const participantsSpy = vi - .spyOn(ComponentsCore, "connectedParticipantsObserver") - .mockReturnValue(remoteParticipants$); - const mediaSpy = vi - .spyOn(ComponentsCore, "observeParticipantMedia") - .mockImplementation((p) => - of({ participant: p } as Partial< - ComponentsCore.ParticipantMedia - > as ComponentsCore.ParticipantMedia), + public getSyncState(): SyncState { + return syncState; + } + })() as Partial as MatrixClient, + getMembers: () => Array.from(roomMembers.values()), + getMembersWithMembership: () => Array.from(roomMembers.values()), + }); + const rtcSession = new MockRTCSession(room, []).withMemberships( + rtcMembers$, ); - const eventsSpy = vi - .spyOn(ComponentsCore, "observeParticipantEvents") - .mockImplementation((p, ...eventTypes) => { - if (eventTypes.includes(ParticipantEvent.IsSpeakingChanged)) { - return (speaking.get(p) ?? of(false)).pipe( - map((s): Participant => ({ ...p, isSpeaking: s }) as Participant), - ); - } else { - return of(p); - } + const participantsSpy = vi + .spyOn(ComponentsCore, "connectedParticipantsObserver") + .mockReturnValue(remoteParticipants$); + const mediaSpy = vi + .spyOn(ComponentsCore, "observeParticipantMedia") + .mockImplementation((p) => + of({ participant: p } as Partial< + ComponentsCore.ParticipantMedia + > as ComponentsCore.ParticipantMedia), + ); + const eventsSpy = vi + .spyOn(ComponentsCore, "observeParticipantEvents") + .mockImplementation((p, ...eventTypes) => { + if (eventTypes.includes(ParticipantEvent.IsSpeakingChanged)) { + return (speaking.get(p) ?? of(false)).pipe( + map((s): Participant => ({ ...p, isSpeaking: s }) as Participant), + ); + } else { + return of(p); + } + }); + + const roomEventSelectorSpy = vi + .spyOn(ComponentsCore, "roomEventSelector") + .mockImplementation((_room, _eventType) => of()); + const muteStates = mockMuteStates(); + const raisedHands$ = new BehaviorSubject>( + {}, + ); + const reactions$ = new BehaviorSubject>({}); + + const vm = createCallViewModel$( + testScope(), + rtcSession.asMockedSession(), + room, + mediaDevices, + muteStates, + { + ...options, + livekitRoomFactory: (): LivekitRoom => + mockLivekitRoom({ + localParticipant, + disconnect: async () => Promise.resolve(), + setE2EEEnabled: async () => Promise.resolve(), + }), + connectionState$, + windowSize$, + matrixRTCMode$: constant(mode), + }, + raisedHands$, + reactions$, + new BehaviorSubject({ + processor: undefined, + supported: undefined, + }), + ); + + onTestFinished(() => { + participantsSpy.mockRestore(); + mediaSpy.mockRestore(); + eventsSpy.mockRestore(); + roomEventSelectorSpy.mockRestore(); }); - const roomEventSelectorSpy = vi - .spyOn(ComponentsCore, "roomEventSelector") - .mockImplementation((_room, _eventType) => of()); - const muteStates = mockMuteStates(); - const raisedHands$ = new BehaviorSubject>({}); - const reactions$ = new BehaviorSubject>({}); - - const vm = createCallViewModel$( - testScope(), - rtcSession.asMockedSession(), - room, - mediaDevices, - muteStates, - { - ...options, - livekitRoomFactory: (): LivekitRoom => - mockLivekitRoom({ - localParticipant, - disconnect: async () => Promise.resolve(), - setE2EEEnabled: async () => Promise.resolve(), - }), - connectionState$, - windowSize$, - }, - raisedHands$, - reactions$, - new BehaviorSubject({ - processor: undefined, - supported: undefined, - }), - ); - - onTestFinished(() => { - participantsSpy.mockRestore(); - mediaSpy.mockRestore(); - eventsSpy.mockRestore(); - roomEventSelectorSpy.mockRestore(); - }); - - continuation(vm, rtcSession, { raisedHands$: raisedHands$ }, setSyncState); + continuation(vm, rtcSession, { raisedHands$: raisedHands$ }, setSyncState); + }; } diff --git a/src/state/CallViewModelWidget.test.ts b/src/state/CallViewModelWidget.test.ts index afcf69ba..5d6442f1 100644 --- a/src/state/CallViewModelWidget.test.ts +++ b/src/state/CallViewModelWidget.test.ts @@ -15,6 +15,7 @@ import { constant } from "./Behavior.ts"; import { aliceParticipant, localRtcMember } from "../utils/test-fixtures.ts"; import { ElementWidgetActions, widget } from "../widget.ts"; import { E2eeType } from "../e2ee/e2eeType.ts"; +import { MatrixRTCMode } from "../settings/settings.ts"; vi.mock("@livekit/components-core", { spy: true }); @@ -34,36 +35,43 @@ vi.mock("../widget", () => ({ }, })); -it("expect leave when ElementWidgetActions.HangupCall is called", async () => { - const pr = Promise.withResolvers(); - withCallViewModel( - { - remoteParticipants$: constant([aliceParticipant]), - rtcMembers$: constant([localRtcMember]), - }, - (vm: CallViewModel) => { - vm.leave$.subscribe((s: string) => { - pr.resolve(s); - }); +it.each([ + [MatrixRTCMode.Legacy], + [MatrixRTCMode.Compatibil], + [MatrixRTCMode.Matrix_2_0], +])( + "expect leave when ElementWidgetActions.HangupCall is called (%s mode)", + async (mode) => { + const pr = Promise.withResolvers(); + withCallViewModel(mode)( + { + remoteParticipants$: constant([aliceParticipant]), + rtcMembers$: constant([localRtcMember]), + }, + (vm: CallViewModel) => { + vm.leave$.subscribe((s: string) => { + pr.resolve(s); + }); - widget!.lazyActions!.emit( - ElementWidgetActions.HangupCall, - new CustomEvent(ElementWidgetActions.HangupCall, { - detail: { - action: "im.vector.hangup", - api: "toWidget", - data: {}, - requestId: "widgetapi-1761237395918", - widgetId: "mrUjS9T6uKUOWHMxXvLbSv0F", - }, - }), - ); - }, - { - encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, - }, - ); + widget!.lazyActions!.emit( + ElementWidgetActions.HangupCall, + new CustomEvent(ElementWidgetActions.HangupCall, { + detail: { + action: "im.vector.hangup", + api: "toWidget", + data: {}, + requestId: "widgetapi-1761237395918", + widgetId: "mrUjS9T6uKUOWHMxXvLbSv0F", + }, + }), + ); + }, + { + encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, + }, + ); - const source = await pr.promise; - expect(source).toBe("user"); -}); + const source = await pr.promise; + expect(source).toBe("user"); + }, +); From cc8e250d96b0ea9a8c4a3b2a6c1691e98a69c8d9 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 22:54:46 -0500 Subject: [PATCH 06/43] Remove a brittle cast from local member code --- src/state/CallViewModel/CallViewModel.ts | 32 +++++++++++------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index fb50696f..b11c1357 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -504,25 +504,23 @@ export function createCallViewModel$( ), ); - const localMatrixLivekitMemberUninitialized = { - membership$: localRtcMembership$, - participant$: localMembership.participant$, - connection$: localMembership.connection$, - userId: userId, - }; - - const localMatrixLivekitMember$: Behavior = - scope.behavior( - localRtcMembership$.pipe( - switchMap((membership) => { - if (!membership) return of(null); - return of( - // casting is save here since we know that localRtcMembership$ is !== null since we reached this case. - localMatrixLivekitMemberUninitialized as MatrixLivekitMember, - ); + const localMatrixLivekitMember$ = scope.behavior( + localRtcMembership$.pipe( + generateItems( + // Generate a local member when membership is non-null + function* (membership) { + if (membership !== null) yield { keys: ["local"], data: membership }; + }, + (_scope, membership$) => ({ + membership$, + participant$: localMembership.participant$, + connection$: localMembership.connection$, + userId, }), ), - ); + map(([localMember]) => localMember ?? null), + ), + ); // ------------------------------------------------------------------------ // callLifecycle From 47cd343d447a91977bdb9a69ff1ef2f6e1c56ab9 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:01:44 -0500 Subject: [PATCH 07/43] Prove that the remote members modules only output remote members They had loose types that were allowing them also output local members. They don't do this, it's just misleading. --- .../remoteMembers/Connection.test.ts | 7 ++-- .../CallViewModel/remoteMembers/Connection.ts | 7 +--- .../remoteMembers/ConnectionManager.test.ts | 40 +++++++++---------- .../remoteMembers/ConnectionManager.ts | 26 +++++------- 4 files changed, 33 insertions(+), 47 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 2ead768b..4ba4c0b7 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -32,7 +32,6 @@ import { Connection, type ConnectionOpts, type ConnectionState, - type PublishingParticipant, } from "./Connection.ts"; import { ObservableScope } from "../../ObservableScope.ts"; import { type OpenIDClientParts } from "../../../livekit/openIDSFU.ts"; @@ -381,7 +380,7 @@ describe("Publishing participants observations", () => { const bobIsAPublisher = Promise.withResolvers(); const danIsAPublisher = Promise.withResolvers(); - const observedPublishers: PublishingParticipant[][] = []; + const observedPublishers: RemoteParticipant[][] = []; const s = connection.remoteParticipantsWithTracks$.subscribe( (publishers) => { observedPublishers.push(publishers); @@ -394,7 +393,7 @@ describe("Publishing participants observations", () => { }, ); onTestFinished(() => s.unsubscribe()); - // The publishingParticipants$ observable is derived from the current members of the + // The remoteParticipants$ observable is derived from the current members of the // livekitRoom and the rtc membership in order to publish the members that are publishing // on this connection. @@ -436,7 +435,7 @@ describe("Publishing participants observations", () => { const connection = setupRemoteConnection(); - let observedPublishers: PublishingParticipant[][] = []; + let observedPublishers: RemoteParticipant[][] = []; const s = connection.remoteParticipantsWithTracks$.subscribe( (publishers) => { observedPublishers.push(publishers); diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 4f3bbda4..ed6b0472 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -14,7 +14,6 @@ import { ConnectionError, type ConnectionState as LivekitConenctionState, type Room as LivekitRoom, - type LocalParticipant, type RemoteParticipant, RoomEvent, } from "livekit-client"; @@ -34,8 +33,6 @@ import { SFURoomCreationRestrictedError, } from "../../../utils/errors.ts"; -export type PublishingParticipant = LocalParticipant | RemoteParticipant; - export interface ConnectionOpts { /** The media transport to connect to. */ transport: LivekitTransport; @@ -89,9 +86,7 @@ export class Connection { * This is derived from `participantsIncludingSubscribers$` and `remoteTransports$`. * It filters the participants to only those that are associated with a membership that claims to publish on this connection. */ - public readonly remoteParticipantsWithTracks$: Behavior< - PublishingParticipant[] - >; + public readonly remoteParticipantsWithTracks$: Behavior; /** * Whether the connection has been stopped. diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts index 484a44e7..da1da06f 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts @@ -8,7 +8,7 @@ Please see LICENSE in the repository root for full details. import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { BehaviorSubject } from "rxjs"; import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; -import { type Participant as LivekitParticipant } from "livekit-client"; +import { type RemoteParticipant } from "livekit-client"; import { logger } from "matrix-js-sdk/lib/logger"; import { Epoch, mapEpoch, ObservableScope } from "../../ObservableScope.ts"; @@ -201,23 +201,20 @@ describe("connections$ stream", () => { describe("connectionManagerData$ stream", () => { // Used in test to control fake connections' remoteParticipantsWithTracks$ streams - let fakePublishingParticipantsStreams: Map< - string, - Behavior - >; + let fakeRemoteParticipantsStreams: Map>; function keyForTransport(transport: LivekitTransport): string { return `${transport.livekit_service_url}|${transport.livekit_alias}`; } beforeEach(() => { - fakePublishingParticipantsStreams = new Map(); + fakeRemoteParticipantsStreams = new Map(); - function getPublishingParticipantsFor( + function getRemoteParticipantsFor( transport: LivekitTransport, - ): Behavior { + ): Behavior { return ( - fakePublishingParticipantsStreams.get(keyForTransport(transport)) ?? + fakeRemoteParticipantsStreams.get(keyForTransport(transport)) ?? new BehaviorSubject([]) ); } @@ -227,13 +224,12 @@ describe("connectionManagerData$ stream", () => { .fn() .mockImplementation( (transport: LivekitTransport, scope: ObservableScope) => { - const fakePublishingParticipants$ = new BehaviorSubject< - LivekitParticipant[] + const fakeRemoteParticipants$ = new BehaviorSubject< + RemoteParticipant[] >([]); const mockConnection = { transport, - remoteParticipantsWithTracks$: - getPublishingParticipantsFor(transport), + remoteParticipantsWithTracks$: getRemoteParticipantsFor(transport), } as unknown as Connection; vi.mocked(mockConnection).start = vi.fn(); vi.mocked(mockConnection).stop = vi.fn(); @@ -242,36 +238,36 @@ describe("connectionManagerData$ stream", () => { void mockConnection.stop(); }); - fakePublishingParticipantsStreams.set( + fakeRemoteParticipantsStreams.set( keyForTransport(transport), - fakePublishingParticipants$, + fakeRemoteParticipants$, ); return mockConnection; }, ); }); - test("Should report connections with the publishing participants", () => { + test("Should report connections with the remote participants", () => { withTestScheduler(({ expectObservable, schedule, behavior }) => { // Setup the fake participants streams behavior // ============================== - fakePublishingParticipantsStreams.set( + fakeRemoteParticipantsStreams.set( keyForTransport(TRANSPORT_1), behavior("oa-b", { o: [], - a: [{ identity: "user1A" } as LivekitParticipant], + a: [{ identity: "user1A" } as RemoteParticipant], b: [ - { identity: "user1A" } as LivekitParticipant, - { identity: "user1B" } as LivekitParticipant, + { identity: "user1A" } as RemoteParticipant, + { identity: "user1B" } as RemoteParticipant, ], }), ); - fakePublishingParticipantsStreams.set( + fakeRemoteParticipantsStreams.set( keyForTransport(TRANSPORT_2), behavior("o-a", { o: [], - a: [{ identity: "user2A" } as LivekitParticipant], + a: [{ identity: "user2A" } as RemoteParticipant], }), ); // ============================== diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts index 0b9f939c..c01b8cf9 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -12,7 +12,7 @@ import { } from "matrix-js-sdk/lib/matrixrtc"; import { BehaviorSubject, combineLatest, map, of, switchMap, tap } from "rxjs"; import { type Logger } from "matrix-js-sdk/lib/logger"; -import { type LocalParticipant, type RemoteParticipant } from "livekit-client"; +import { type RemoteParticipant } from "livekit-client"; import { type Behavior } from "../../Behavior.ts"; import { type Connection } from "./Connection.ts"; @@ -22,17 +22,12 @@ import { areLivekitTransportsEqual } from "./MatrixLivekitMembers.ts"; import { type ConnectionFactory } from "./ConnectionFactory.ts"; export class ConnectionManagerData { - private readonly store: Map< - string, - [Connection, (LocalParticipant | RemoteParticipant)[]] - > = new Map(); + private readonly store: Map = + new Map(); public constructor() {} - public add( - connection: Connection, - participants: (LocalParticipant | RemoteParticipant)[], - ): void { + public add(connection: Connection, participants: RemoteParticipant[]): void { const key = this.getKey(connection.transport); const existing = this.store.get(key); if (!existing) { @@ -58,7 +53,7 @@ export class ConnectionManagerData { public getParticipantForTransport( transport: LivekitTransport, - ): (LocalParticipant | RemoteParticipant)[] { + ): RemoteParticipant[] { const key = transport.livekit_service_url + "|" + transport.livekit_alias; const existing = this.store.get(key); if (existing) { @@ -182,23 +177,24 @@ export function createConnectionManager$({ const epoch = connections.epoch; // Map the connections to list of {connection, participants}[] - const listOfConnectionsWithPublishingParticipants = - connections.value.map((connection) => { + const listOfConnectionsWithRemoteParticipants = connections.value.map( + (connection) => { return connection.remoteParticipantsWithTracks$.pipe( map((participants) => ({ connection, participants, })), ); - }); + }, + ); // probably not required - if (listOfConnectionsWithPublishingParticipants.length === 0) { + if (listOfConnectionsWithRemoteParticipants.length === 0) { return of(new Epoch(new ConnectionManagerData(), epoch)); } // combineLatest the several streams into a single stream with the ConnectionManagerData - return combineLatest(listOfConnectionsWithPublishingParticipants).pipe( + return combineLatest(listOfConnectionsWithRemoteParticipants).pipe( map( (lists) => new Epoch( From a7a3d4e93cf1ca580b99b9c8c21e926d6930d480 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 23:06:19 -0500 Subject: [PATCH 08/43] Remove unsound participant casts By tagging participant behaviors with a type (local vs. remote) we can now tell what kind of participant it will be in a completely type-safe manner. --- src/state/CallViewModel/CallViewModel.ts | 50 ++++++++------ .../MatrixLivekitMembers.test.ts | 20 +++--- .../remoteMembers/MatrixLivekitMembers.ts | 27 +++++--- .../remoteMembers/integration.test.ts | 14 ++-- src/state/UserMedia.ts | 69 ++++++++++--------- 5 files changed, 100 insertions(+), 80 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index b11c1357..f4f81776 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -110,6 +110,7 @@ import { ECConnectionFactory } from "./remoteMembers/ConnectionFactory.ts"; import { createConnectionManager$ } from "./remoteMembers/ConnectionManager.ts"; import { createMatrixLivekitMembers$, + type TaggedParticipant, type MatrixLivekitMember, } from "./remoteMembers/MatrixLivekitMembers.ts"; import { @@ -504,23 +505,28 @@ export function createCallViewModel$( ), ); - const localMatrixLivekitMember$ = scope.behavior( - localRtcMembership$.pipe( - generateItems( - // Generate a local member when membership is non-null - function* (membership) { - if (membership !== null) yield { keys: ["local"], data: membership }; - }, - (_scope, membership$) => ({ - membership$, - participant$: localMembership.participant$, - connection$: localMembership.connection$, - userId, - }), + const localMatrixLivekitMember$ = + scope.behavior | null>( + localRtcMembership$.pipe( + generateItems( + // Generate a local member when membership is non-null + function* (membership) { + if (membership !== null) + yield { keys: ["local"], data: membership }; + }, + (_scope, membership$) => ({ + membership$, + participant: { + type: "local" as const, + value$: localMembership.participant$, + }, + connection$: localMembership.connection$, + userId, + }), + ), + map(([localMember]) => localMember ?? null), ), - map(([localMember]) => localMember ?? null), - ), - ); + ); // ------------------------------------------------------------------------ // callLifecycle @@ -597,7 +603,7 @@ export function createCallViewModel$( const members = membersWithEpoch.value; const a$ = combineLatest( members.map((member) => - combineLatest([member.connection$, member.participant$]).pipe( + combineLatest([member.connection$, member.participant.value$]).pipe( map(([connection, participant]) => { // do not render audio for local participant if (!connection || !participant || participant.isLocal) @@ -675,8 +681,10 @@ export function createCallViewModel$( let localParticipantId: string | undefined = undefined; // add local member if available if (localMatrixLivekitMember) { - const { userId, participant$, connection$, membership$ } = + const { userId, connection$, membership$ } = localMatrixLivekitMember; + const participant: TaggedParticipant = + localMatrixLivekitMember.participant; // Widen the type localParticipantId = `${userId}:${membership$.value.deviceId}`; // should be membership$.value.membershipID which is not optional // const participantId = membership$.value.membershipID; if (localParticipantId) { @@ -686,7 +694,7 @@ export function createCallViewModel$( dup, localParticipantId, userId, - participant$, + participant, connection$, ], data: undefined, @@ -697,7 +705,7 @@ export function createCallViewModel$( // add remote members that are available for (const { userId, - participant$, + participant, connection$, membership$, } of matrixLivekitMembers) { @@ -706,7 +714,7 @@ export function createCallViewModel$( // const participantId = membership$.value?.identity; for (let dup = 0; dup < 1 + duplicateTiles; dup++) { yield { - keys: [dup, participantId, userId, participant$, connection$], + keys: [dup, participantId, userId, participant, connection$], data: undefined, }; } diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts index e675f723..195078e0 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts @@ -100,12 +100,12 @@ test("should signal participant not yet connected to livekit", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember[]) => { + a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, }); - expectObservable(data[0].participant$).toBe("a", { + expectObservable(data[0].participant.value$).toBe("a", { a: null, }); expectObservable(data[0].connection$).toBe("a", { @@ -180,12 +180,12 @@ test("should signal participant on a connection that is publishing", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember[]) => { + a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, }); - expectObservable(data[0].participant$).toBe("a", { + expectObservable(data[0].participant.value$).toBe("a", { a: expect.toSatisfy((participant) => { expect(participant).toBeDefined(); expect(participant!.identity).toEqual(bobParticipantId); @@ -231,12 +231,12 @@ test("should signal participant on a connection that is not publishing", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember[]) => { + a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, }); - expectObservable(data[0].participant$).toBe("a", { + expectObservable(data[0].participant.value$).toBe("a", { a: null, }); expectObservable(data[0].connection$).toBe("a", { @@ -296,7 +296,7 @@ describe("Publication edge case", () => { expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe( "a", { - a: expect.toSatisfy((data: MatrixLivekitMember[]) => { + a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { expect(data.length).toEqual(2); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -305,7 +305,7 @@ describe("Publication edge case", () => { // The real connection should be from transportA as per the membership a: connectionA, }); - expectObservable(data[0].participant$).toBe("a", { + expectObservable(data[0].participant.value$).toBe("a", { a: expect.toSatisfy((participant) => { expect(participant).toBeDefined(); expect(participant!.identity).toEqual(bobParticipantId); @@ -362,7 +362,7 @@ describe("Publication edge case", () => { expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe( "a", { - a: expect.toSatisfy((data: MatrixLivekitMember[]) => { + a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { expect(data.length).toEqual(2); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -371,7 +371,7 @@ describe("Publication edge case", () => { // The real connection should be from transportA as per the membership a: connectionA, }); - expectObservable(data[0].participant$).toBe("a", { + expectObservable(data[0].participant.value$).toBe("a", { // No participant as Bob is not publishing on his membership transport a: null, }); diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts index 2f152630..bcb4e7e2 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts @@ -5,10 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { - type LocalParticipant as LocalLivekitParticipant, - type RemoteParticipant as RemoteLivekitParticipant, -} from "livekit-client"; +import { type LocalParticipant, type RemoteParticipant } from "livekit-client"; import { type LivekitTransport, type CallMembership, @@ -24,16 +21,24 @@ import { generateItemsWithEpoch } from "../../../utils/observable"; const logger = rootLogger.getChild("[MatrixLivekitMembers]"); +/** + * A dynamic participant value with a static tag to tell what kind of + * participant it can be (local vs. remote). + */ +export type TaggedParticipant = + | { type: "local"; value$: Behavior } + | { type: "remote"; value$: Behavior }; + /** * Represents a Matrix call member and their associated LiveKit participation. * `livekitParticipant` can be undefined if the member is not yet connected to the livekit room * or if it has no livekit transport at all. */ -export interface MatrixLivekitMember { +export interface MatrixLivekitMember< + ParticipantType extends TaggedParticipant["type"], +> { membership$: Behavior; - participant$: Behavior< - LocalLivekitParticipant | RemoteLivekitParticipant | null - >; + participant: TaggedParticipant & { type: ParticipantType }; connection$: Behavior; // participantId: string; We do not want a participantId here since it will be generated by the jwt // TODO decide if we can also drop the userId. Its in the matrix membership anyways. @@ -61,7 +66,7 @@ export function createMatrixLivekitMembers$({ scope, membershipsWithTransport$, connectionManager, -}: Props): Behavior> { +}: Props): Behavior[]>> { /** * Stream of all the call members and their associated livekit data (if available). */ @@ -110,12 +115,14 @@ export function createMatrixLivekitMembers$({ logger.debug( `Updating data$ for participantId: ${participantId}, userId: ${userId}`, ); + const { participant$, ...rest } = scope.splitBehavior(data$); // will only get called once per `participantId, userId` pair. // updates to data$ and as a result to displayName$ and mxcAvatarUrl$ are more frequent. return { participantId, userId, - ...scope.splitBehavior(data$), + participant: { type: "remote" as const, value$: participant$ }, + ...rest, }; }, ), diff --git a/src/state/CallViewModel/remoteMembers/integration.test.ts b/src/state/CallViewModel/remoteMembers/integration.test.ts index e3aa6be8..34b62dad 100644 --- a/src/state/CallViewModel/remoteMembers/integration.test.ts +++ b/src/state/CallViewModel/remoteMembers/integration.test.ts @@ -132,7 +132,7 @@ test("bob, carl, then bob joining no tracks yet", () => { }); expectObservable(matrixLivekitItems$).toBe(vMarble, { - a: expect.toSatisfy((e: Epoch) => { + a: expect.toSatisfy((e: Epoch[]>) => { const items = e.value; expect(items.length).toBe(1); const item = items[0]!; @@ -147,12 +147,12 @@ test("bob, carl, then bob joining no tracks yet", () => { ), ), }); - expectObservable(item.participant$).toBe("a", { + expectObservable(item.participant.value$).toBe("a", { a: null, }); return true; }), - b: expect.toSatisfy((e: Epoch) => { + b: expect.toSatisfy((e: Epoch[]>) => { const items = e.value; expect(items.length).toBe(2); @@ -161,7 +161,7 @@ test("bob, carl, then bob joining no tracks yet", () => { expectObservable(item.membership$).toBe("a", { a: bobMembership, }); - expectObservable(item.participant$).toBe("a", { + expectObservable(item.participant.value$).toBe("a", { a: null, }); } @@ -172,7 +172,7 @@ test("bob, carl, then bob joining no tracks yet", () => { expectObservable(item.membership$).toBe("a", { a: carlMembership, }); - expectObservable(item.participant$).toBe("a", { + expectObservable(item.participant.value$).toBe("a", { a: null, }); expectObservable(item.connection$).toBe("a", { @@ -189,7 +189,7 @@ test("bob, carl, then bob joining no tracks yet", () => { } return true; }), - c: expect.toSatisfy((e: Epoch) => { + c: expect.toSatisfy((e: Epoch[]>) => { const items = e.value; expect(items.length).toBe(3); @@ -216,7 +216,7 @@ test("bob, carl, then bob joining no tracks yet", () => { return true; }), }); - expectObservable(item.participant$).toBe("a", { + expectObservable(item.participant.value$).toBe("a", { a: null, }); } diff --git a/src/state/UserMedia.ts b/src/state/UserMedia.ts index 38f22122..690870e6 100644 --- a/src/state/UserMedia.ts +++ b/src/state/UserMedia.ts @@ -27,6 +27,7 @@ import type { ReactionOption } from "../reactions"; import { observeSpeaker$ } from "./observeSpeaker.ts"; import { generateItems } from "../utils/observable.ts"; import { ScreenShare } from "./ScreenShare.ts"; +import { type TaggedParticipant } from "./CallViewModel/remoteMembers/MatrixLivekitMembers.ts"; /** * Sorting bins defining the order in which media tiles appear in the layout. @@ -68,40 +69,46 @@ enum SortingBin { * for inclusion in the call layout and tracks associated screen shares. */ export class UserMedia { - public readonly vm: UserMediaViewModel = this.participant$.value?.isLocal - ? new LocalUserMediaViewModel( - this.scope, - this.id, - this.userId, - this.participant$ as Behavior, - this.encryptionSystem, - this.livekitRoom$, - this.focusUrl$, - this.mediaDevices, - this.displayName$, - this.mxcAvatarUrl$, - this.scope.behavior(this.handRaised$), - this.scope.behavior(this.reaction$), - ) - : new RemoteUserMediaViewModel( - this.scope, - this.id, - this.userId, - this.participant$ as Behavior, - this.encryptionSystem, - this.livekitRoom$, - this.focusUrl$, - this.pretendToBeDisconnected$, - this.displayName$, - this.mxcAvatarUrl$, - this.scope.behavior(this.handRaised$), - this.scope.behavior(this.reaction$), - ); + public readonly vm: UserMediaViewModel = + this.participant.type === "local" + ? new LocalUserMediaViewModel( + this.scope, + this.id, + this.userId, + this.participant.value$, + this.encryptionSystem, + this.livekitRoom$, + this.focusUrl$, + this.mediaDevices, + this.displayName$, + this.mxcAvatarUrl$, + this.scope.behavior(this.handRaised$), + this.scope.behavior(this.reaction$), + ) + : new RemoteUserMediaViewModel( + this.scope, + this.id, + this.userId, + this.participant.value$, + this.encryptionSystem, + this.livekitRoom$, + this.focusUrl$, + this.pretendToBeDisconnected$, + this.displayName$, + this.mxcAvatarUrl$, + this.scope.behavior(this.handRaised$), + this.scope.behavior(this.reaction$), + ); private readonly speaker$ = this.scope.behavior( observeSpeaker$(this.vm.speaking$), ); + // TypeScript needs this widening of the type to happen in a separate statement + private readonly participant$: Behavior< + LocalParticipant | RemoteParticipant | null + > = this.participant.value$; + /** * All screen share media associated with this user media. */ @@ -184,9 +191,7 @@ export class UserMedia { private readonly scope: ObservableScope, public readonly id: string, private readonly userId: string, - private readonly participant$: Behavior< - LocalParticipant | RemoteParticipant | null - >, + private readonly participant: TaggedParticipant, private readonly encryptionSystem: EncryptionSystem, private readonly livekitRoom$: Behavior, private readonly focusUrl$: Behavior, From bf801364a68927e0cd67ccfe8d53876507333ff0 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 9 Dec 2025 15:23:30 +0100 Subject: [PATCH 09/43] cleanup and tests --- src/state/CallViewModel/CallViewModel.ts | 16 +- ...Membership.test.ts => LocalMember.test.ts} | 180 ++++++------ .../{LocalMembership.ts => LocalMember.ts} | 256 +++++++++--------- .../localMember/Publisher.test.ts | 13 +- .../CallViewModel/localMember/Publisher.ts | 30 +- .../remoteMembers/Connection.test.ts | 48 ++-- .../CallViewModel/remoteMembers/Connection.ts | 28 +- yarn.lock | 6 +- 8 files changed, 302 insertions(+), 275 deletions(-) rename src/state/CallViewModel/localMember/{LocalMembership.test.ts => LocalMember.test.ts} (74%) rename src/state/CallViewModel/localMember/{LocalMembership.ts => LocalMember.ts} (77%) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 3c15958a..e06990b2 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -94,14 +94,13 @@ import { type SpotlightLandscapeLayoutMedia, type SpotlightPortraitLayoutMedia, } from "../layout-types.ts"; -import { type ElementCallError } from "../../utils/errors.ts"; +import { ElementCallError } from "../../utils/errors.ts"; import { type ObservableScope } from "../ObservableScope.ts"; import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts"; import { createLocalMembership$, enterRTCSession, - RTCBackendState, -} from "./localMember/LocalMembership.ts"; +} from "./localMember/LocalMember.ts"; import { createLocalTransport$ } from "./localMember/LocalTransport.ts"; import { createMemberships$, @@ -452,13 +451,13 @@ export function createCallViewModel$( const localMembership = createLocalMembership$({ scope: scope, - homeserverConnected$: createHomeserverConnected$( + homeserverConnected: createHomeserverConnected$( scope, client, matrixRTCSession, ), muteStates: muteStates, - joinMatrixRTC: async (transport: LivekitTransport) => { + joinMatrixRTC: (transport: LivekitTransport) => { return enterRTCSession( matrixRTCSession, transport, @@ -1455,7 +1454,7 @@ export function createCallViewModel$( ringOverlay$: ringOverlay$, leave$: leave$, hangup: (): void => userHangup$.next(), - join: localMembership.requestConnect, + join: localMembership.requestJoinAndPublish, toggleScreenSharing: toggleScreenSharing, sharingScreen$: sharingScreen$, @@ -1465,9 +1464,8 @@ export function createCallViewModel$( unhoverScreen: (): void => screenUnhover$.next(), fatalError$: scope.behavior( - localMembership.connectionState.livekit$.pipe( - filter((v) => v.state === RTCBackendState.Error), - map((s) => s.error), + localMembership.localMemberState$.pipe( + filter((v) => v instanceof ElementCallError), ), null, ), diff --git a/src/state/CallViewModel/localMember/LocalMembership.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts similarity index 74% rename from src/state/CallViewModel/localMember/LocalMembership.test.ts rename to src/state/CallViewModel/localMember/LocalMember.test.ts index 1ef7abd6..2f8d11a5 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -7,7 +7,7 @@ Please see LICENSE in the repository root for full details. */ import { - Status, + Status as RTCMemberStatus, type LivekitTransport, type MatrixRTCSession, } from "matrix-js-sdk/lib/matrixrtc"; @@ -15,11 +15,7 @@ import { describe, expect, it, vi } from "vitest"; import { AutoDiscovery } from "matrix-js-sdk/lib/autodiscovery"; import { BehaviorSubject, map, of } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; -import { - ConnectionState as LivekitConnectionState, - type LocalParticipant, - type LocalTrack, -} from "livekit-client"; +import { type LocalParticipant, type LocalTrack } from "livekit-client"; import { MatrixRTCMode } from "../../../settings/settings"; import { @@ -30,16 +26,19 @@ import { withTestScheduler, } from "../../../utils/test"; import { + TransportState, createLocalMembership$, enterRTCSession, - RTCBackendState, -} from "./LocalMembership"; + PublishState, + TrackState, +} from "./LocalMember"; import { MatrixRTCTransportMissingError } from "../../../utils/errors"; import { Epoch, ObservableScope } from "../../ObservableScope"; import { constant } from "../../Behavior"; import { ConnectionManagerData } from "../remoteMembers/ConnectionManager"; -import { type Connection } from "../remoteMembers/Connection"; +import { ConnectionState, type Connection } from "../remoteMembers/Connection"; import { type Publisher } from "./Publisher"; +import { C } from "vitest/dist/chunks/global.d.MAmajcmJ.js"; const MATRIX_RTC_MODE = MatrixRTCMode.Legacy; const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); @@ -200,21 +199,18 @@ describe("LocalMembership", () => { joinMatrixRTC: async (): Promise => {}, homeserverConnected: { combined$: constant(true), - rtsSession$: constant(Status.Connected), + rtsSession$: constant(RTCMemberStatus.Connected), }, }; it("throws error on missing RTC config error", () => { withTestScheduler(({ scope, hot, expectObservable }) => { - const goodTransport = { - livekit_service_url: "other", - } as LivekitTransport; - - const localTransport$ = scope.behavior( + const localTransport$ = scope.behavior( hot("1ms #", {}, new MatrixRTCTransportMissingError("domain.com")), - goodTransport, + null, ); + // we do not need any connection data since we want to fail before reaching that. const mockConnectionManager = { transports$: scope.behavior( localTransport$.pipe(map((t) => new Epoch([t]))), @@ -230,15 +226,11 @@ describe("LocalMembership", () => { connectionManager: mockConnectionManager, localTransport$, }); + localMembership.requestJoinAndPublish(); - expectObservable(localMembership.connectionState.livekit$).toBe("ne", { - n: { state: RTCBackendState.WaitingForConnection }, - e: { - state: RTCBackendState.Error, - error: expect.toSatisfy( - (e) => e instanceof MatrixRTCTransportMissingError, - ), - }, + expectObservable(localMembership.localMemberState$).toBe("ne", { + n: TransportState.Waiting, + e: expect.toSatisfy((e) => e instanceof MatrixRTCTransportMissingError), }); }); }); @@ -250,32 +242,24 @@ describe("LocalMembership", () => { livekit_service_url: "b", } as LivekitTransport; - const connectionManagerData = new ConnectionManagerData(); - - connectionManagerData.add( - { - livekitRoom: mockLivekitRoom({ - localParticipant: { - isScreenShareEnabled: false, - trackPublications: [], - } as unknown as LocalParticipant, - }), - state$: constant({ - state: LivekitConnectionState.Connected, - }), - transport: aTransport, - } as unknown as Connection, - [], - ); - connectionManagerData.add( - { - state$: constant({ - state: LivekitConnectionState.Connected, - }), - transport: bTransport, - } as unknown as Connection, - [], - ); + const connectionTransportAConnected = { + livekitRoom: mockLivekitRoom({ + localParticipant: { + isScreenShareEnabled: false, + trackPublications: [], + } as unknown as LocalParticipant, + }), + state$: constant(ConnectionState.LivekitConnected), + transport: aTransport, + } as unknown as Connection; + const connectionTransportAConnecting = { + ...connectionTransportAConnected, + state$: constant(ConnectionState.LivekitConnecting), + } as unknown as Connection; + const connectionTransportBConnected = { + state$: constant(ConnectionState.LivekitConnected), + transport: bTransport, + } as unknown as Connection; it("recreates publisher if new connection is used and ENDS always unpublish and end tracks", async () => { const scope = new ObservableScope(); @@ -300,6 +284,9 @@ describe("LocalMembership", () => { typeof vi.fn >; + const connectionManagerData = new ConnectionManagerData(); + connectionManagerData.add(connectionTransportAConnected, []); + connectionManagerData.add(connectionTransportBConnected, []); createLocalMembership$({ scope, ...defaultCreateLocalMemberValues, @@ -359,6 +346,9 @@ describe("LocalMembership", () => { typeof vi.fn >; + const connectionManagerData = new ConnectionManagerData(); + connectionManagerData.add(connectionTransportAConnected, []); + // connectionManagerData.add(connectionTransportB, []); const localMembership = createLocalMembership$({ scope, ...defaultCreateLocalMemberValues, @@ -385,10 +375,11 @@ describe("LocalMembership", () => { it("tracks livekit state correctly", async () => { const scope = new ObservableScope(); + const connectionManagerData = new ConnectionManagerData(); const localTransport$ = new BehaviorSubject(null); - const connectionManagerData$ = new BehaviorSubject< - Epoch - >(new Epoch(new ConnectionManagerData())); + const connectionManagerData$ = new BehaviorSubject( + new Epoch(connectionManagerData), + ); const publishers: Publisher[] = []; const tracks$ = new BehaviorSubject([]); @@ -434,19 +425,45 @@ describe("LocalMembership", () => { }); await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.WaitingForTransport, - }); + expect(localMembership.localMemberState$.value).toStrictEqual( + TransportState.Waiting, + ); localTransport$.next(aTransport); await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.WaitingForConnection, + expect(localMembership.localMemberState$.value).toStrictEqual({ + matrix: RTCMemberStatus.Connected, + media: { connection: null, tracks: TrackState.WaitingForUser }, }); - connectionManagerData$.next(new Epoch(connectionManagerData)); + + const connectionManagerData2 = new ConnectionManagerData(); + connectionManagerData2.add( + // clone because we will mutate this later. + { ...connectionTransportAConnecting } as unknown as Connection, + [], + ); + + connectionManagerData$.next(new Epoch(connectionManagerData2)); await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: LivekitConnectionState.Connected, + expect(localMembership.localMemberState$.value).toStrictEqual({ + matrix: RTCMemberStatus.Connected, + media: { + connection: ConnectionState.LivekitConnecting, + tracks: TrackState.WaitingForUser, + }, }); + + ( + connectionManagerData2.getConnectionForTransport(aTransport)! + .state$ as BehaviorSubject + ).next(ConnectionState.LivekitConnected); + expect(localMembership.localMemberState$.value).toStrictEqual({ + matrix: RTCMemberStatus.Connected, + media: { + connection: ConnectionState.LivekitConnected, + tracks: TrackState.WaitingForUser, + }, + }); + expect(publisherFactory).toHaveBeenCalledOnce(); expect(localMembership.tracks$.value.length).toBe(0); @@ -455,37 +472,46 @@ describe("LocalMembership", () => { // ------- await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.CreatingTracks, + expect(localMembership.localMemberState$.value).toStrictEqual({ + matrix: RTCMemberStatus.Connected, + media: { + tracks: TrackState.Creating, + connection: ConnectionState.LivekitConnected, + }, }); createTrackResolver.resolve(); await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.ReadyToPublish, - }); + expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (localMembership.localMemberState$.value as any).media, + ).toStrictEqual(PublishState.WaitingForUser); // ------- - localMembership.requestConnect(); + localMembership.requestJoinAndPublish(); // ------- - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.WaitingToPublish, - }); + expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (localMembership.localMemberState$.value as any).media, + ).toStrictEqual(PublishState.Starting); publishResolver.resolve(); await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.ConnectedAndPublishing, - }); + expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (localMembership.localMemberState$.value as any).media, + ).toStrictEqual(PublishState.Publishing); + expect(publishers[0].stopPublishing).not.toHaveBeenCalled(); - expect(localMembership.connectionState.livekit$.isStopped).toBe(false); + expect(localMembership.localMemberState$.isStopped).toBe(false); scope.end(); await flushPromises(); // stays in connected state because it is stopped before the update to tracks update the state. - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.ConnectedAndPublishing, - }); + expect( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (localMembership.localMemberState$.value as any).media, + ).toStrictEqual(PublishState.Publishing); // stop all tracks after ending scopes expect(publishers[0].stopPublishing).toHaveBeenCalled(); expect(publishers[0].stopTracks).toHaveBeenCalled(); diff --git a/src/state/CallViewModel/localMember/LocalMembership.ts b/src/state/CallViewModel/localMember/LocalMember.ts similarity index 77% rename from src/state/CallViewModel/localMember/LocalMembership.ts rename to src/state/CallViewModel/localMember/LocalMember.ts index 6a31ce4b..e2fcc70e 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -11,7 +11,6 @@ import { ParticipantEvent, type LocalParticipant, type ScreenShareCaptureOptions, - ConnectionState as LivekitConnectionState, } from "livekit-client"; import { observeParticipantEvents } from "@livekit/components-core"; import { @@ -36,62 +35,66 @@ import { import { type Logger } from "matrix-js-sdk/lib/logger"; import { deepCompare } from "matrix-js-sdk/lib/utils"; -import { constant, type Behavior } from "../../Behavior"; -import { type IConnectionManager } from "../remoteMembers/ConnectionManager"; -import { type ObservableScope } from "../../ObservableScope"; -import { type Publisher } from "./Publisher"; -import { type MuteStates } from "../../MuteStates"; +import { constant, type Behavior } from "../../Behavior.ts"; +import { type IConnectionManager } from "../remoteMembers/ConnectionManager.ts"; +import { type ObservableScope } from "../../ObservableScope.ts"; +import { type Publisher } from "./Publisher.ts"; +import { type MuteStates } from "../../MuteStates.ts"; import { ElementCallError, MembershipManagerError, UnknownCallError, -} from "../../../utils/errors"; -import { ElementWidgetActions, widget } from "../../../widget"; +} from "../../../utils/errors.ts"; +import { ElementWidgetActions, widget } from "../../../widget.ts"; import { getUrlParams } from "../../../UrlParams.ts"; import { PosthogAnalytics } from "../../../analytics/PosthogAnalytics.ts"; import { MatrixRTCMode } from "../../../settings/settings.ts"; import { Config } from "../../../config/Config.ts"; import { - type ConnectionState, + ConnectionState, type Connection, + type FailedToStartError, } from "../remoteMembers/Connection.ts"; import { type HomeserverConnected } from "./HomeserverConnected.ts"; -export enum RTCBackendState { - Error = "error", +export enum TransportState { /** Not even a transport is available to the LocalMembership */ - WaitingForTransport = "waiting_for_transport", - /** A connection appeared so we can initialise the publisher */ - WaitingForConnection = "waiting_for_connection", - /** Implies lk connection is connected */ - CreatingTracks = "creating_tracks", - /** Implies lk connection is connected */ - ReadyToPublish = "ready_to_publish", - /** Implies lk connection is connected */ - WaitingToPublish = "waiting_to_publish", - /** Implies lk connection is connected */ - ConnectedAndPublishing = "fully_connected", + Waiting = "transport_waiting", } -type LocalMemberRTCBackendState = - | { state: RTCBackendState.Error; error: ElementCallError } - | { state: Exclude } - | ConnectionState; - -export enum MatrixAdditionalState { - WaitingForTransport = "waiting_for_transport", +export enum PublishState { + WaitingForUser = "publish_waiting_for_user", + /** Implies lk connection is connected */ + Starting = "publish_start_publishing", + /** Implies lk connection is connected */ + Publishing = "publish_publishing", } -type LocalMemberMatrixState = - | { state: MatrixAdditionalState.WaitingForTransport } - | { state: "Error"; error: Error } - | { state: RTCSessionStatus }; - -export interface LocalMemberConnectionState { - livekit$: Behavior; - matrix$: Behavior; +export enum TrackState { + /** The track is waiting for user input to create tracks (waiting to call `startTracks()`) */ + WaitingForUser = "tracks_waiting_for_user", + /** Implies lk connection is connected */ + Creating = "tracks_creating", + /** Implies lk connection is connected */ + Ready = "tracks_ready", } +export type LocalMemberMediaState = + | { + tracks: TrackState; + connection: ConnectionState | FailedToStartError; + } + | PublishState + | ElementCallError; +export type LocalMemberMatrixState = Error | RTCSessionStatus; +export type LocalMemberState = + | ElementCallError + | TransportState.Waiting + | { + media: LocalMemberMediaState; + matrix: LocalMemberMatrixState; + }; + /* * - get well known * - get oldest membership @@ -146,16 +149,16 @@ export const createLocalMembership$ = ({ matrixRTCSession, }: Props): { /** - * This starts audio and video tracks. They will be reused when calling `requestConnect`. + * This starts audio and video tracks. They will be reused when calling `requestPublish`. */ startTracks: () => Behavior; /** - * This sets a inner state (shouldConnect) to true and instructs the js-sdk and livekit to keep the user + * This sets a inner state (shouldPublish) to true and instructs the js-sdk and livekit to keep the user * connected to matrix and livekit. */ - requestConnect: () => void; + requestJoinAndPublish: () => void; requestDisconnect: () => void; - connectionState: LocalMemberConnectionState; + localMemberState$: Behavior; sharingScreen$: Behavior; /** * Callback to toggle screen sharing. If null, screen sharing is not possible. @@ -164,11 +167,11 @@ export const createLocalMembership$ = ({ tracks$: Behavior; participant$: Behavior; connection$: Behavior; - /** Shorthand for connectionState.matrix.state === Status.Reconnecting + /** Shorthand for homeserverConnected.rtcSession === Status.Reconnecting * Direct translation to the js-sdk membership manager connection `Status`. */ reconnecting$: Behavior; - /** Shorthand for connectionState.matrix.state === Status.Disconnected + /** Shorthand for homeserverConnected.rtcSession === Status.Disconnected * Direct translation to the js-sdk membership manager connection `Status`. */ disconnected$: Behavior; @@ -190,7 +193,7 @@ export const createLocalMembership$ = ({ : new Error("Unknown error from localTransport"), ); } - setLivekitError(error); + setTransportError(error); return of(null); }), ), @@ -223,19 +226,13 @@ export const createLocalMembership$ = ({ // MATRIX RELATED - const reconnecting$ = scope.behavior( - homeserverConnected.rtsSession$.pipe( - map((sessionStatus) => sessionStatus === RTCSessionStatus.Reconnecting), - ), - ); - // This should be used in a combineLatest with publisher$ to connect. // to make it possible to call startTracks before the preferredTransport$ has resolved. const trackStartRequested = Promise.withResolvers(); // This should be used in a combineLatest with publisher$ to connect. // to make it possible to call startTracks before the preferredTransport$ has resolved. - const connectRequested$ = new BehaviorSubject(false); + const joinAndPublishRequested$ = new BehaviorSubject(false); /** * The publisher is stored in here an abstracts creating and publishing tracks. @@ -256,13 +253,13 @@ export const createLocalMembership$ = ({ return tracks$; }; - const requestConnect = (): void => { + const requestJoinAndPublish = (): void => { trackStartRequested.resolve(); - connectRequested$.next(true); + joinAndPublishRequested$.next(true); }; const requestDisconnect = (): void => { - connectRequested$.next(false); + joinAndPublishRequested$.next(false); }; // Take care of the publisher$ @@ -300,112 +297,129 @@ export const createLocalMembership$ = ({ // Based on `connectRequested$` we start publishing tracks. (once they are there!) scope.reconcile( - scope.behavior(combineLatest([publisher$, tracks$, connectRequested$])), - async ([publisher, tracks, shouldConnect]) => { - if (shouldConnect === publisher?.publishing$.value) return; - if (tracks.length !== 0 && shouldConnect) { + scope.behavior( + combineLatest([publisher$, tracks$, joinAndPublishRequested$]), + ), + async ([publisher, tracks, shouldJoinAndPublish]) => { + if (shouldJoinAndPublish === publisher?.publishing$.value) return; + if (tracks.length !== 0 && shouldJoinAndPublish) { try { await publisher?.startPublishing(); } catch (error) { - setLivekitError(error as ElementCallError); + setMediaError(error as ElementCallError); } - } else if (tracks.length !== 0 && !shouldConnect) { + } else if (tracks.length !== 0 && !shouldJoinAndPublish) { try { await publisher?.stopPublishing(); } catch (error) { - setLivekitError(new UnknownCallError(error as Error)); + setMediaError(new UnknownCallError(error as Error)); } } }, ); - const fatalLivekitError$ = new BehaviorSubject(null); - const setLivekitError = (e: ElementCallError): void => { - if (fatalLivekitError$.value !== null) - logger.error("Multiple Livkit Errors:", e); - else fatalLivekitError$.next(e); + const fatalMediaError$ = new BehaviorSubject(null); + const setMediaError = (e: ElementCallError): void => { + if (fatalMediaError$.value !== null) + logger.error("Multiple Media Errors:", e); + else fatalMediaError$.next(e); }; - const livekitState$: Behavior = scope.behavior( + + const fatalTransportError$ = new BehaviorSubject( + null, + ); + const setTransportError = (e: ElementCallError): void => { + if (fatalTransportError$.value !== null) + logger.error("Multiple Transport Errors:", e); + else fatalTransportError$.next(e); + }; + + const mediaState$: Behavior = scope.behavior( combineLatest([ localConnectionState$, - publisher$, localTransport$, - tracks$.pipe( - tap((t) => { - logger.info("tracks$: ", t); - }), - ), + tracks$, publishing$, - connectRequested$, + joinAndPublishRequested$, from(trackStartRequested.promise).pipe( map(() => true), startWith(false), ), - fatalLivekitError$, ]).pipe( map( ([ localConnectionState, - publisher, localTransport, tracks, publishing, - shouldConnect, + shouldPublish, shouldStartTracks, - error, ]) => { - // read this: - // if(!) return {state: ...} - // if(!) return {state: } - // - // as: - // We do have but not yet so we are in - if (error !== null) return { state: RTCBackendState.Error, error }; + if (!localTransport) return null; const hasTracks = tracks.length > 0; - if (!localTransport) - return { state: RTCBackendState.WaitingForTransport }; - if (!localConnectionState) - return { state: RTCBackendState.WaitingForConnection }; + let trackState: TrackState = TrackState.WaitingForUser; + if (hasTracks && shouldStartTracks) trackState = TrackState.Ready; + if (!hasTracks && shouldStartTracks) trackState = TrackState.Creating; + if ( - localConnectionState.state !== LivekitConnectionState.Connected || - !publisher + localConnectionState !== ConnectionState.LivekitConnected || + trackState !== TrackState.Ready ) - // pass through the localConnectionState while we do not yet have a publisher or the state - // of the connection is not yet connected - return { state: localConnectionState.state }; - if (!shouldStartTracks) - return { state: LivekitConnectionState.Connected }; - if (!hasTracks) return { state: RTCBackendState.CreatingTracks }; - if (!shouldConnect) return { state: RTCBackendState.ReadyToPublish }; - if (!publishing) return { state: RTCBackendState.WaitingToPublish }; - return { state: RTCBackendState.ConnectedAndPublishing }; + return { + connection: localConnectionState, + tracks: trackState, + }; + if (!shouldPublish) return PublishState.WaitingForUser; + if (!publishing) return PublishState.Starting; + return PublishState.Publishing; }, ), distinctUntilChanged(deepCompare), ), ); - const fatalMatrixError$ = new BehaviorSubject(null); const setMatrixError = (e: ElementCallError): void => { if (fatalMatrixError$.value !== null) logger.error("Multiple Matrix Errors:", e); else fatalMatrixError$.next(e); }; - const matrixState$: Behavior = scope.behavior( - combineLatest([localTransport$, homeserverConnected.rtsSession$]).pipe( - map(([localTransport, rtcSessionStatus]) => { - if (!localTransport) - return { state: MatrixAdditionalState.WaitingForTransport }; - return { state: rtcSessionStatus }; - }), + + const localMemberState$ = scope.behavior( + combineLatest([ + mediaState$, + homeserverConnected.rtsSession$, + fatalMatrixError$, + fatalTransportError$, + fatalMediaError$, + ]).pipe( + map( + ([ + mediaState, + rtcSessionStatus, + matrixError, + transportError, + mediaError, + ]) => { + if (transportError !== null) return transportError; + // `mediaState` will be 'null' until the transport appears. + if (mediaState && rtcSessionStatus) + return { + matrix: matrixError ?? rtcSessionStatus, + media: mediaError ?? mediaState, + }; + else { + return TransportState.Waiting; + } + }, + ), ), ); // inform the widget about the connect and disconnect intent from the user. scope - .behavior(connectRequested$.pipe(pairwise(), scope.bind()), [ + .behavior(joinAndPublishRequested$.pipe(pairwise(), scope.bind()), [ undefined, - connectRequested$.value, + joinAndPublishRequested$.value, ]) .subscribe(([prev, current]) => { if (!widget) return; @@ -434,7 +448,7 @@ export const createLocalMembership$ = ({ // Keep matrix rtc session in sync with localTransport$, connectRequested$ scope.reconcile( - scope.behavior(combineLatest([localTransport$, connectRequested$])), + scope.behavior(combineLatest([localTransport$, joinAndPublishRequested$])), async ([transport, shouldConnect]) => { if (!transport) return; // if shouldConnect=false we will do the disconnect as the cleanup from the previous reconcile iteration. @@ -555,21 +569,19 @@ export const createLocalMembership$ = ({ return { startTracks, - requestConnect, + requestJoinAndPublish, requestDisconnect, - connectionState: { - livekit$: livekitState$, - matrix$: matrixState$, - }, + localMemberState$, tracks$, participant$, - reconnecting$, + reconnecting$: scope.behavior( + homeserverConnected.rtsSession$.pipe( + map((sessionStatus) => sessionStatus === RTCSessionStatus.Reconnecting), + ), + ), disconnected$: scope.behavior( - matrixState$.pipe( - map( - (sessionStatus) => - sessionStatus.state === RTCSessionStatus.Disconnected, - ), + homeserverConnected.rtsSession$.pipe( + map((state) => state === RTCSessionStatus.Disconnected), ), ), sharingScreen$, diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 5468d1ff..6d27c042 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -52,9 +52,7 @@ describe("Publisher", () => { } as unknown as MuteStates; scope = new ObservableScope(); connection = { - state$: constant({ - state: LivekitConenctionState.Connected, - }), + state$: constant(LivekitConenctionState.Connected), livekitRoom: mockLivekitRoom({ localParticipant: mockLocalParticipant({}), }), @@ -110,15 +108,14 @@ describe("Publisher", () => { // failiour due to connection.state$ const beforeState = connection.state$.value; - (connection.state$ as BehaviorSubject).next({ - state: "FailedToStart", - error: Error("testStartError"), - }); + (connection.state$ as BehaviorSubject).next(Error("testStartError")); await expect(publisher.startPublishing()).rejects.toThrow( new FailToStartLivekitConnection("testStartError"), ); - (connection.state$ as BehaviorSubject).next(beforeState); + (connection.state$ as BehaviorSubject).next( + beforeState, + ); // does not try other conenction after the first one failed expect( diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index 6e4a9b35..b32e7e99 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -32,7 +32,10 @@ import { } from "../../../livekit/TrackProcessorContext.tsx"; import { getUrlParams } from "../../../UrlParams.ts"; import { observeTrackReference$ } from "../../MediaViewModel.ts"; -import { type Connection } from "../remoteMembers/Connection.ts"; +import { + ConnectionState, + type Connection, +} from "../remoteMembers/Connection.ts"; import { type ObservableScope } from "../../ObservableScope.ts"; import { ElementCallError, @@ -158,20 +161,17 @@ export class Publisher { this.logger.debug("startPublishing called"); const lkRoom = this.connection.livekitRoom; const { promise, resolve, reject } = Promise.withResolvers(); - const sub = this.connection.state$.subscribe((s) => { - switch (s.state) { - case LivekitConnectionState.Connected: - resolve(); - break; - case "FailedToStart": - reject( - s.error instanceof ElementCallError - ? s.error - : new FailToStartLivekitConnection(s.error.message), - ); - break; - default: - this.logger.info("waiting for connection: ", s.state); + const sub = this.connection.state$.subscribe((state) => { + if (state instanceof Error) { + const error = + state instanceof ElementCallError + ? state + : new FailToStartLivekitConnection(state.message); + reject(error); + } else if (state === ConnectionState.LivekitConnected) { + resolve(); + } else { + this.logger.info("waiting for connection: ", state); } }); try { diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index efee1ccb..a90f0aa2 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -30,8 +30,8 @@ import { logger } from "matrix-js-sdk/lib/logger"; import type { LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; import { Connection, + ConnectionState, type ConnectionOpts, - type ConnectionState, type PublishingParticipant, } from "./Connection.ts"; import { ObservableScope } from "../../ObservableScope.ts"; @@ -151,7 +151,7 @@ describe("Start connection states", () => { }; const connection = new Connection(opts, logger); - expect(connection.state$.getValue().state).toEqual("Initialized"); + expect(connection.state$.getValue()).toEqual("Initialized"); }); it("fail to getOpenId token then error state", async () => { @@ -167,7 +167,7 @@ describe("Start connection states", () => { const connection = new Connection(opts, logger); - const capturedStates: ConnectionState[] = []; + const capturedStates: (ConnectionState | Error)[] = []; const s = connection.state$.subscribe((value) => { capturedStates.push(value); }); @@ -187,22 +187,20 @@ describe("Start connection states", () => { let capturedState = capturedStates.pop(); expect(capturedState).toBeDefined(); - expect(capturedState!.state).toEqual("FetchingConfig"); + expect(capturedState!).toEqual("FetchingConfig"); deferred.reject(new FailToGetOpenIdToken(new Error("Failed to get token"))); await vi.runAllTimersAsync(); capturedState = capturedStates.pop(); - if (capturedState!.state === "FailedToStart") { - expect(capturedState!.error.message).toEqual("Something went wrong"); + if (capturedState instanceof Error) { + expect(capturedState.message).toEqual("Something went wrong"); expect(connection.transport.livekit_alias).toEqual( livekitFocus.livekit_alias, ); } else { - expect.fail( - "Expected FailedToStart state but got " + capturedState?.state, - ); + expect.fail("Expected FailedToStart state but got " + capturedState); } }); @@ -219,7 +217,7 @@ describe("Start connection states", () => { const connection = new Connection(opts, logger); - const capturedStates: ConnectionState[] = []; + const capturedStates: (ConnectionState | Error)[] = []; const s = connection.state$.subscribe((value) => { capturedStates.push(value); }); @@ -241,24 +239,22 @@ describe("Start connection states", () => { let capturedState = capturedStates.pop(); expect(capturedState).toBeDefined(); - expect(capturedState?.state).toEqual("FetchingConfig"); + expect(capturedState).toEqual(ConnectionState.FetchingConfig); deferredSFU.resolve(); await vi.runAllTimersAsync(); capturedState = capturedStates.pop(); - if (capturedState?.state === "FailedToStart") { - expect(capturedState?.error.message).toContain( + if (capturedState instanceof Error) { + expect(capturedState.message).toContain( "SFU Config fetch failed with exception Error", ); expect(connection.transport.livekit_alias).toEqual( livekitFocus.livekit_alias, ); } else { - expect.fail( - "Expected FailedToStart state but got " + capturedState?.state, - ); + expect.fail("Expected FailedToStart state but got " + capturedState); } }); @@ -275,7 +271,7 @@ describe("Start connection states", () => { const connection = new Connection(opts, logger); - const capturedStates: ConnectionState[] = []; + const capturedStates: (ConnectionState | Error)[] = []; const s = connection.state$.subscribe((value) => { capturedStates.push(value); }); @@ -305,17 +301,15 @@ describe("Start connection states", () => { let capturedState = capturedStates.pop(); expect(capturedState).toBeDefined(); - expect(capturedState?.state).toEqual("FetchingConfig"); + expect(capturedState).toEqual(ConnectionState.FetchingConfig); deferredSFU.resolve(); await vi.runAllTimersAsync(); capturedState = capturedStates.pop(); - if (capturedState && capturedState.state === "FailedToStart") { - expect(capturedState.error.message).toContain( - "Failed to connect to livekit", - ); + if (capturedState instanceof Error) { + expect(capturedState.message).toContain("Failed to connect to livekit"); expect(connection.transport.livekit_alias).toEqual( livekitFocus.livekit_alias, ); @@ -332,7 +326,7 @@ describe("Start connection states", () => { const connection = setupRemoteConnection(); - const capturedStates: ConnectionState[] = []; + const capturedStates: (ConnectionState | Error)[] = []; const s = connection.state$.subscribe((value) => { capturedStates.push(value); }); @@ -342,13 +336,13 @@ describe("Start connection states", () => { await vi.runAllTimersAsync(); const initialState = capturedStates.shift(); - expect(initialState?.state).toEqual("Initialized"); + expect(initialState).toEqual(ConnectionState.Initialized); const fetchingState = capturedStates.shift(); - expect(fetchingState?.state).toEqual("FetchingConfig"); + expect(fetchingState).toEqual(ConnectionState.FetchingConfig); const connectingState = capturedStates.shift(); - expect(connectingState?.state).toEqual("ConnectingToLkRoom"); + expect(connectingState).toEqual(ConnectionState.ConnectingToLkRoom); const connectedState = capturedStates.shift(); - expect(connectedState?.state).toEqual("connected"); + expect(connectedState).toEqual(ConnectionState.LivekitConnected); }); it("shutting down the scope should stop the connection", async () => { diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 549777f9..29ad7a8c 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -12,7 +12,6 @@ import { } from "@livekit/components-core"; import { ConnectionError, - ConnectionState as LivekitConnectionState, type Room as LivekitRoom, type LocalParticipant, type RemoteParticipant, @@ -55,14 +54,21 @@ export class FailedToStartError extends Error { } export enum ConnectionState { + /** The start state of a connection. It has been created but nothing has loaded yet. */ Initialized = "Initialized", + /** `start` has been called on the connection. It aquires the jwt info to conenct to the LK Room */ FetchingConfig = "FetchingConfig", Stopped = "Stopped", ConnectingToLkRoom = "ConnectingToLkRoom", + /** The same as ConnectionState.Disconnected from `livekit-client` */ LivekitDisconnected = "disconnected", + /** The same as ConnectionState.Connecting from `livekit-client` */ LivekitConnecting = "connecting", + /** The same as ConnectionState.Connected from `livekit-client` */ LivekitConnected = "connected", + /** The same as ConnectionState.Reconnecting from `livekit-client` */ LivekitReconnecting = "reconnecting", + /** The same as ConnectionState.SignalReconnecting from `livekit-client` */ LivekitSignalReconnecting = "signalReconnecting", } @@ -73,15 +79,14 @@ export enum ConnectionState { */ export class Connection { // Private Behavior - private readonly _state$ = new BehaviorSubject< - ConnectionState | FailedToStartError - >(ConnectionState.Initialized); + private readonly _state$ = new BehaviorSubject( + ConnectionState.Initialized, + ); /** * The current state of the connection to the media transport. */ - public readonly state$: Behavior = - this._state$; + public readonly state$: Behavior = this._state$; /** * The media transport to connect to. @@ -161,15 +166,12 @@ export class Connection { connectionStateObserver(this.livekitRoom) .pipe(this.scope.bind()) .subscribe((lkState) => { - // It si save to cast lkState to ConnectionState as they are fully overlapping. + // It is save to cast lkState to ConnectionState as they are fully overlapping. this._state$.next(lkState as unknown as ConnectionState); }); } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); - this._state$.next({ - state: "FailedToStart", - error: error instanceof Error ? error : new Error(`${error}`), - }); + this._state$.next(error instanceof Error ? error : new Error(`${error}`)); throw error; } } @@ -194,9 +196,7 @@ export class Connection { ); if (this.stopped) return; await this.livekitRoom.disconnect(); - this._state$.next({ - state: ConnectionAdditionalState.Stopped, - }); + this._state$.next(ConnectionState.Stopped); this.stopped = true; } diff --git a/yarn.lock b/yarn.lock index 94b73130..f0ca83a7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10353,8 +10353,8 @@ __metadata: linkType: hard "matrix-js-sdk@npm:^39.2.0": - version: 39.2.0 - resolution: "matrix-js-sdk@npm:39.2.0" + version: 39.3.0 + resolution: "matrix-js-sdk@npm:39.3.0" dependencies: "@babel/runtime": "npm:^7.12.5" "@matrix-org/matrix-sdk-crypto-wasm": "npm:^15.3.0" @@ -10370,7 +10370,7 @@ __metadata: sdp-transform: "npm:^3.0.0" unhomoglyph: "npm:^1.0.6" uuid: "npm:13" - checksum: 10c0/f8b5261de2744305330ba3952821ca9303698170bfd3a0ff8a767b9286d4e8d4ed5aaf6fbaf8a1e8ff9dbd859102a2a47d882787e2da3b3078965bec00157959 + checksum: 10c0/031c9ec042e00c32dc531f82fc59c64cc25fb665abfc642b1f0765c530d60684f8bd63daf0cdd0dbe96b4f87ea3f4148f9d3f024a59d57eceaec1ce5d0164755 languageName: node linkType: hard From 7af89b421693b58d06baf2f044bdd3e39d97a623 Mon Sep 17 00:00:00 2001 From: Timo K Date: Tue, 9 Dec 2025 17:36:56 +0100 Subject: [PATCH 10/43] fix lint --- src/state/CallViewModel/localMember/LocalMember.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts index 2f8d11a5..6a9f196e 100644 --- a/src/state/CallViewModel/localMember/LocalMember.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -38,7 +38,6 @@ import { constant } from "../../Behavior"; import { ConnectionManagerData } from "../remoteMembers/ConnectionManager"; import { ConnectionState, type Connection } from "../remoteMembers/Connection"; import { type Publisher } from "./Publisher"; -import { C } from "vitest/dist/chunks/global.d.MAmajcmJ.js"; const MATRIX_RTC_MODE = MatrixRTCMode.Legacy; const getUrlParams = vi.hoisted(() => vi.fn(() => ({}))); From 0ebc6078dd5cae4f8a2317e4ffb22f128ebd1e75 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 12:08:59 +0100 Subject: [PATCH 11/43] Update LocalMember.ts --- .../CallViewModel/localMember/LocalMember.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index e2fcc70e..193dd53c 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -407,9 +407,7 @@ export const createLocalMembership$ = ({ matrix: matrixError ?? rtcSessionStatus, media: mediaError ?? mediaState, }; - else { - return TransportState.Waiting; - } + return TransportState.Waiting; }, ), ), @@ -423,19 +421,21 @@ export const createLocalMembership$ = ({ ]) .subscribe(([prev, current]) => { if (!widget) return; + // JOIN prev=false (was left) => current-true (now joiend) if (!prev && current) { - try { - void widget.api.transport.send(ElementWidgetActions.JoinCall, {}); - } catch (e) { - logger.error("Failed to send join action", e); - } + widget.api.transport + .send(ElementWidgetActions.JoinCall, {}) + .catch((e) => { + logger.error("Failed to send join action", e); + }); } + // LEAVE prev=false (was joined) => current-true (now left) if (prev && !current) { - try { - void widget?.api.transport.send(ElementWidgetActions.HangupCall, {}); - } catch (e) { - logger.error("Failed to send hangup action", e); - } + widget.api.transport + .send(ElementWidgetActions.HangupCall, {}) + .catch((e) => { + logger.error("Failed to send hangup action", e); + }); } }); @@ -575,8 +575,12 @@ export const createLocalMembership$ = ({ tracks$, participant$, reconnecting$: scope.behavior( - homeserverConnected.rtsSession$.pipe( - map((sessionStatus) => sessionStatus === RTCSessionStatus.Reconnecting), + localMemberState$.pipe( + map((state) => { + if (typeof state === "object" && "matrix" in state) + return state.matrix === RTCSessionStatus.Reconnecting; + return false; + }), ), ), disconnected$: scope.behavior( From 6efce232f81528c4df4b61f56393dc62e0040549 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 18:50:19 +0100 Subject: [PATCH 12/43] fix playwright tests --- src/state/CallViewModel/CallViewModel.ts | 63 ++++++++++---- .../CallViewModel/localMember/LocalMember.ts | 82 ++++++++++++------- .../CallViewModel/localMember/Publisher.ts | 60 +++++++------- .../CallViewModel/remoteMembers/Connection.ts | 4 +- 4 files changed, 129 insertions(+), 80 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index e04f4698..35ab658b 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -99,6 +99,7 @@ import { createHomeserverConnected$ } from "./localMember/HomeserverConnected.ts import { createLocalMembership$, enterRTCSession, + TransportState, } from "./localMember/LocalMember.ts"; import { createLocalTransport$ } from "./localMember/LocalTransport.ts"; import { @@ -577,17 +578,6 @@ export function createCallViewModel$( ), ); - /** - * Whether various media/event sources should pretend to be disconnected from - * all network input, even if their connection still technically works. - */ - // We do this when the app is in the 'reconnecting' state, because it might be - // that the LiveKit connection is still functional while the homeserver is - // down, for example, and we want to avoid making people worry that the app is - // in a split-brained state. - // DISCUSSION own membership manager ALSO this probably can be simplifis - const reconnecting$ = localMembership.reconnecting$; - const audioParticipants$ = scope.behavior( matrixLivekitMembers$.pipe( switchMap((membersWithEpoch) => { @@ -635,7 +625,7 @@ export function createCallViewModel$( ); const handsRaised$ = scope.behavior( - handsRaisedSubject$.pipe(pauseWhen(reconnecting$)), + handsRaisedSubject$.pipe(pauseWhen(localMembership.reconnecting$)), ); const reactions$ = scope.behavior( @@ -648,7 +638,7 @@ export function createCallViewModel$( ]), ), ), - pauseWhen(reconnecting$), + pauseWhen(localMembership.reconnecting$), ), ); @@ -739,7 +729,7 @@ export function createCallViewModel$( livekitRoom$, focusUrl$, mediaDevices, - reconnecting$, + localMembership.reconnecting$, displayName$, matrixMemberMetadataStore.createAvatarUrlBehavior$(userId), handsRaised$.pipe(map((v) => v[participantId]?.time ?? null)), @@ -1422,6 +1412,37 @@ export function createCallViewModel$( // reassigned here to make it publicly accessible const toggleScreenSharing = localMembership.toggleScreenSharing; + const errors$ = scope.behavior<{ + transportError?: ElementCallError; + matrixError?: ElementCallError; + connectionError?: ElementCallError; + publishError?: ElementCallError; + } | null>( + localMembership.localMemberState$.pipe( + map((value) => { + const returnObject: { + transportError?: ElementCallError; + matrixError?: ElementCallError; + connectionError?: ElementCallError; + publishError?: ElementCallError; + } = {}; + if (value instanceof ElementCallError) return { transportError: value }; + if (value === TransportState.Waiting) return null; + if (value.matrix instanceof ElementCallError) + returnObject.matrixError = value.matrix; + if (value.media instanceof ElementCallError) + returnObject.publishError = value.media; + else if ( + typeof value.media === "object" && + value.media.connection instanceof ElementCallError + ) + returnObject.connectionError = value.media.connection; + return returnObject; + }), + ), + null, + ); + return { autoLeave$: autoLeave$, callPickupState$: callPickupState$, @@ -1438,8 +1459,16 @@ export function createCallViewModel$( unhoverScreen: (): void => screenUnhover$.next(), fatalError$: scope.behavior( - localMembership.localMemberState$.pipe( - filter((v) => v instanceof ElementCallError), + errors$.pipe( + map((errors) => { + return ( + errors?.transportError ?? + errors?.matrixError ?? + errors?.connectionError ?? + null + ); + }), + filter((error) => error !== null), ), null, ), @@ -1472,7 +1501,7 @@ export function createCallViewModel$( showFooter$: showFooter$, earpieceMode$: earpieceMode$, audioOutputSwitcher$: audioOutputSwitcher$, - reconnecting$: reconnecting$, + reconnecting$: localMembership.reconnecting$, }; } diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index df42cba9..532d5d55 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -42,6 +42,7 @@ import { type Publisher } from "./Publisher.ts"; import { type MuteStates } from "../../MuteStates.ts"; import { ElementCallError, + FailToStartLivekitConnection, MembershipManagerError, UnknownCallError, } from "../../../utils/errors.ts"; @@ -56,6 +57,7 @@ import { type FailedToStartError, } from "../remoteMembers/Connection.ts"; import { type HomeserverConnected } from "./HomeserverConnected.ts"; +import { and$ } from "../../../utils/observable.ts"; export enum TransportState { /** Not even a transport is available to the LocalMembership */ @@ -86,13 +88,12 @@ export type LocalMemberMediaState = } | PublishState | ElementCallError; -export type LocalMemberMatrixState = Error | RTCSessionStatus; export type LocalMemberState = | ElementCallError | TransportState.Waiting | { media: LocalMemberMediaState; - matrix: LocalMemberMatrixState; + matrix: ElementCallError | RTCSessionStatus; }; /* @@ -220,10 +221,6 @@ export const createLocalMembership$ = ({ ), ); - const localConnectionState$ = localConnection$.pipe( - switchMap((connection) => (connection ? connection.state$ : of(null))), - ); - // MATRIX RELATED // This should be used in a combineLatest with publisher$ to connect. @@ -308,23 +305,27 @@ export const createLocalMembership$ = ({ try { await publisher?.startPublishing(); } catch (error) { - setMediaError(error as ElementCallError); + const message = + error instanceof Error ? error.message : String(error); + setPublishError(new FailToStartLivekitConnection(message)); } } else if (tracks.length !== 0 && !shouldJoinAndPublish) { try { await publisher?.stopPublishing(); } catch (error) { - setMediaError(new UnknownCallError(error as Error)); + setPublishError(new UnknownCallError(error as Error)); } } }, ); - const fatalMediaError$ = new BehaviorSubject(null); - const setMediaError = (e: ElementCallError): void => { - if (fatalMediaError$.value !== null) - logger.error("Multiple Media Errors:", e); - else fatalMediaError$.next(e); + // STATE COMPUTATION + + // These are non fatal since we can join a room and concume media even though publishing failed. + const publishError$ = new BehaviorSubject(null); + const setPublishError = (e: ElementCallError): void => { + if (publishError$.value !== null) logger.error("Multiple Media Errors:", e); + else publishError$.next(e); }; const fatalTransportError$ = new BehaviorSubject( @@ -336,6 +337,10 @@ export const createLocalMembership$ = ({ else fatalTransportError$.next(e); }; + const localConnectionState$ = localConnection$.pipe( + switchMap((connection) => (connection ? connection.state$ : of(null))), + ); + const mediaState$: Behavior = scope.behavior( combineLatest([ localConnectionState$, @@ -392,22 +397,22 @@ export const createLocalMembership$ = ({ homeserverConnected.rtsSession$, fatalMatrixError$, fatalTransportError$, - fatalMediaError$, + publishError$, ]).pipe( map( ([ mediaState, rtcSessionStatus, - matrixError, - transportError, - mediaError, + fatalMatrixError, + fatalTransportError, + publishError, ]) => { - if (transportError !== null) return transportError; - // `mediaState` will be 'null' until the transport appears. + if (fatalTransportError !== null) return fatalTransportError; + // `mediaState` will be 'null' until the transport/connection appears. if (mediaState && rtcSessionStatus) return { - matrix: matrixError ?? rtcSessionStatus, - media: mediaError ?? mediaState, + matrix: fatalMatrixError ?? rtcSessionStatus, + media: publishError ?? mediaState, }; return TransportState.Waiting; }, @@ -415,6 +420,31 @@ export const createLocalMembership$ = ({ ), ); + /** + * Whether we are "fully" connected to the call. Accounts for both the + * connection to the MatrixRTC session and the LiveKit publish connection. + */ + const matrixAndLivekitConnected$ = scope.behavior( + and$( + homeserverConnected.combined$, + localConnectionState$.pipe( + map((state) => state === ConnectionState.LivekitConnected), + ), + ).pipe( + tap((v) => logger.debug("livekit+matrix: Connected state changed", v)), + ), + ); + + /** + * Whether we should tell the user that we're reconnecting to the call. + */ + const reconnecting$ = scope.behavior( + matrixAndLivekitConnected$.pipe( + pairwise(), + map(([prev, current]) => prev === true && current === false), + ), + ); + // inform the widget about the connect and disconnect intent from the user. scope .behavior(joinAndPublishRequested$.pipe(pairwise(), scope.bind()), [ @@ -576,15 +606,7 @@ export const createLocalMembership$ = ({ localMemberState$, tracks$, participant$, - reconnecting$: scope.behavior( - localMemberState$.pipe( - map((state) => { - if (typeof state === "object" && "matrix" in state) - return state.matrix === RTCSessionStatus.Reconnecting; - return false; - }), - ), - ), + reconnecting$, disconnected$: scope.behavior( homeserverConnected.rtsSession$.pipe( map((state) => state === RTCSessionStatus.Disconnected), diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index b32e7e99..df67f179 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -32,15 +32,8 @@ import { } from "../../../livekit/TrackProcessorContext.tsx"; import { getUrlParams } from "../../../UrlParams.ts"; import { observeTrackReference$ } from "../../MediaViewModel.ts"; -import { - ConnectionState, - type Connection, -} from "../remoteMembers/Connection.ts"; +import { type Connection } from "../remoteMembers/Connection.ts"; import { type ObservableScope } from "../../ObservableScope.ts"; -import { - ElementCallError, - FailToStartLivekitConnection, -} from "../../../utils/errors.ts"; /** * A wrapper for a Connection object. @@ -160,27 +153,29 @@ export class Publisher { public async startPublishing(): Promise { this.logger.debug("startPublishing called"); const lkRoom = this.connection.livekitRoom; - const { promise, resolve, reject } = Promise.withResolvers(); - const sub = this.connection.state$.subscribe((state) => { - if (state instanceof Error) { - const error = - state instanceof ElementCallError - ? state - : new FailToStartLivekitConnection(state.message); - reject(error); - } else if (state === ConnectionState.LivekitConnected) { - resolve(); - } else { - this.logger.info("waiting for connection: ", state); - } - }); - try { - await promise; - } catch (e) { - throw e; - } finally { - sub.unsubscribe(); - } + + // we do not need to do this since lk will wait in `localParticipant.publishTrack` + // const { promise, resolve, reject } = Promise.withResolvers(); + // const sub = this.connection.state$.subscribe((state) => { + // if (state instanceof Error) { + // const error = + // state instanceof ElementCallError + // ? state + // : new FailToStartLivekitConnection(state.message); + // reject(error); + // } else if (state === ConnectionState.LivekitConnected) { + // resolve(); + // } else { + // this.logger.info("waiting for connection: ", state); + // } + // }); + // try { + // await promise; + // } catch (e) { + // throw e; + // } finally { + // sub.unsubscribe(); + // } for (const track of this.tracks$.value) { this.logger.info("publish ", this.tracks$.value.length, "tracks"); @@ -188,9 +183,10 @@ export class Publisher { // with a timeout. await lkRoom.localParticipant.publishTrack(track).catch((error) => { this.logger.error("Failed to publish track", error); - throw new FailToStartLivekitConnection( - error instanceof Error ? error.message : error, - ); + // throw new FailToStartLivekitConnection( + // error instanceof Error ? error.message : error, + // ); + throw error; }); this.logger.info("published track ", track.kind, track.id); diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 29ad7a8c..2fd9eaa8 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -150,7 +150,8 @@ export class Connection { throw new InsufficientCapacityError(); } if (e.status === 404) { - // error msg is "Could not establish signal connection: requested room does not exist" + // error msg is "Failed to create call" + // error description is "Call creation might be restricted to authorized users only. Try again later, or contact your server admin if the problem persists." // The room does not exist. There are two different modes of operation for the SFU: // - the room is created on the fly when connecting (livekit `auto_create` option) // - Only authorized users can create rooms, so the room must exist before connecting (done by the auth jwt service) @@ -172,6 +173,7 @@ export class Connection { } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); this._state$.next(error instanceof Error ? error : new Error(`${error}`)); + // Its okay to ignore the throw. The error is part of the state. throw error; } } From 1941fc9ca1cc17becdc170e5c5b691ae8edd6c0f Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 19:12:52 +0100 Subject: [PATCH 13/43] fix tests. --- src/state/CallViewModel/localMember/LocalMember.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 532d5d55..73908fcb 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -443,6 +443,7 @@ export const createLocalMembership$ = ({ pairwise(), map(([prev, current]) => prev === true && current === false), ), + false, ); // inform the widget about the connect and disconnect intent from the user. From 667a3d0e3d911ad602b48ae4906ef0da6dc3f085 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 19:18:16 +0100 Subject: [PATCH 14/43] fix test not checking for livekit connection state anymore. --- .../CallViewModel/localMember/Publisher.test.ts | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 6d27c042..68845fa2 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -98,7 +98,7 @@ describe("Publisher", () => { ).mockRejectedValue(Error("testError")); await expect(publisher.startPublishing()).rejects.toThrow( - new FailToStartLivekitConnection("testError"), + new Error("testError"), ); // does not try other conenction after the first one failed @@ -106,17 +106,6 @@ describe("Publisher", () => { connection.livekitRoom.localParticipant.publishTrack, ).toHaveBeenCalledTimes(1); - // failiour due to connection.state$ - const beforeState = connection.state$.value; - (connection.state$ as BehaviorSubject).next(Error("testStartError")); - - await expect(publisher.startPublishing()).rejects.toThrow( - new FailToStartLivekitConnection("testStartError"), - ); - (connection.state$ as BehaviorSubject).next( - beforeState, - ); - // does not try other conenction after the first one failed expect( connection.livekitRoom.localParticipant.publishTrack, From b380532d3080275388810ec124d578dd57a787b5 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 21:14:13 +0100 Subject: [PATCH 15/43] lots of error logging and fixing playwright --- src/room/GroupCallView.tsx | 1 + src/room/InCallView.tsx | 7 ++++++- src/room/LobbyView.tsx | 4 ++-- src/state/CallViewModel/CallViewModel.ts | 15 +++++++++++++- .../CallViewModel/localMember/LocalMember.ts | 20 +++++++++++++------ .../localMember/LocalTransport.ts | 2 +- .../CallViewModel/remoteMembers/Connection.ts | 14 +++++++++---- 7 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index dfd11ff3..1542678e 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -446,6 +446,7 @@ export const GroupCallView: FC = ({ let body: ReactNode; if (externalError) { + logger.debug("External error occurred:", externalError); // If an error was recorded within this component but outside // GroupCallErrorBoundary, create a component that rethrows the error from // within the error boundary, so it can be handled uniformly diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 7ae3700c..18acf843 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -127,6 +127,7 @@ export const ActiveCall: FC = (props) => { const mediaDevices = useMediaDevices(); const trackProcessorState$ = useTrackProcessorObservable$(); useEffect(() => { + logger.info("START CALL VIEW SCOPE"); const scope = new ObservableScope(); const reactionsReader = new ReactionsReader(scope, props.rtcSession); const { autoLeaveWhenOthersLeft, waitForCallPickup, sendNotificationType } = @@ -153,6 +154,7 @@ export const ActiveCall: FC = (props) => { vm.leave$.pipe(scope.bind()).subscribe(props.onLeft); return (): void => { + logger.info("END CALL VIEW SCOPE"); scope.end(); }; }, [ @@ -271,7 +273,10 @@ export const InCallView: FC = ({ const ringOverlay = useBehavior(vm.ringOverlay$); const fatalCallError = useBehavior(vm.fatalError$); // Stop the rendering and throw for the error boundary - if (fatalCallError) throw fatalCallError; + if (fatalCallError) { + logger.debug("fatalCallError stop rendering", fatalCallError); + throw fatalCallError; + } // We need to set the proper timings on the animation based upon the sound length. const ringDuration = pickupPhaseAudio?.soundDuration["waiting"] ?? 1; diff --git a/src/room/LobbyView.tsx b/src/room/LobbyView.tsx index ad4f30b3..10e098f1 100644 --- a/src/room/LobbyView.tsx +++ b/src/room/LobbyView.tsx @@ -79,9 +79,9 @@ export const LobbyView: FC = ({ waitingForInvite, }) => { useEffect(() => { - logger.info("[Lifecycle] GroupCallView Component mounted"); + logger.info("[Lifecycle] LobbyView Component mounted"); return (): void => { - logger.info("[Lifecycle] GroupCallView Component unmounted"); + logger.info("[Lifecycle] LobbyView Component unmounted"); }; }, []); diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 35ab658b..6a9eadea 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -15,6 +15,7 @@ import { } from "livekit-client"; import { type Room as MatrixRoom } from "matrix-js-sdk"; import { + catchError, combineLatest, distinctUntilChanged, filter, @@ -425,7 +426,18 @@ export function createCallViewModel$( connectionFactory: connectionFactory, inputTransports$: scope.behavior( combineLatest( - [localTransport$, membershipsAndTransports.transports$], + [ + localTransport$.pipe( + catchError((e) => { + logger.info( + "dont pass local transport to createConnectionManager$. localTransport$ threw an error", + e, + ); + return of(null); + }), + ), + membershipsAndTransports.transports$, + ], (localTransport, transports) => { const localTransportAsArray = localTransport ? [localTransport] : []; return transports.mapInner((transports) => [ @@ -1461,6 +1473,7 @@ export function createCallViewModel$( fatalError$: scope.behavior( errors$.pipe( map((errors) => { + logger.debug("errors$ to compute any fatal errors:", errors); return ( errors?.transportError ?? errors?.matrixError ?? diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 73908fcb..40fb62d6 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -324,17 +324,23 @@ export const createLocalMembership$ = ({ // These are non fatal since we can join a room and concume media even though publishing failed. const publishError$ = new BehaviorSubject(null); const setPublishError = (e: ElementCallError): void => { - if (publishError$.value !== null) logger.error("Multiple Media Errors:", e); - else publishError$.next(e); + if (publishError$.value !== null) { + logger.error("Multiple Media Errors:", e); + } else { + publishError$.next(e); + } }; const fatalTransportError$ = new BehaviorSubject( null, ); + const setTransportError = (e: ElementCallError): void => { - if (fatalTransportError$.value !== null) + if (fatalTransportError$.value !== null) { logger.error("Multiple Transport Errors:", e); - else fatalTransportError$.next(e); + } else { + fatalTransportError$.next(e); + } }; const localConnectionState$ = localConnection$.pipe( @@ -386,9 +392,11 @@ export const createLocalMembership$ = ({ ); const fatalMatrixError$ = new BehaviorSubject(null); const setMatrixError = (e: ElementCallError): void => { - if (fatalMatrixError$.value !== null) + if (fatalMatrixError$.value !== null) { logger.error("Multiple Matrix Errors:", e); - else fatalMatrixError$.next(e); + } else { + fatalMatrixError$.next(e); + } }; const localMemberState$ = scope.behavior( diff --git a/src/state/CallViewModel/localMember/LocalTransport.ts b/src/state/CallViewModel/localMember/LocalTransport.ts index 0a85bbc1..1320b8c4 100644 --- a/src/state/CallViewModel/localMember/LocalTransport.ts +++ b/src/state/CallViewModel/localMember/LocalTransport.ts @@ -85,7 +85,7 @@ export const createLocalTransport$ = ({ * The transport that we would personally prefer to publish on (if not for the * transport preferences of others, perhaps). * - * @throws + * @throws MatrixRTCTransportMissingError | FailToGetOpenIdToken */ const preferredTransport$: Behavior = scope.behavior( customLivekitUrl.value$.pipe( diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 2fd9eaa8..c801b3ae 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -29,8 +29,10 @@ import { import { type Behavior } from "../../Behavior.ts"; import { type ObservableScope } from "../../ObservableScope.ts"; import { + ElementCallError, InsufficientCapacityError, SFURoomCreationRestrictedError, + UnknownCallError, } from "../../../utils/errors.ts"; export type PublishingParticipant = LocalParticipant | RemoteParticipant; @@ -79,9 +81,9 @@ export enum ConnectionState { */ export class Connection { // Private Behavior - private readonly _state$ = new BehaviorSubject( - ConnectionState.Initialized, - ); + private readonly _state$ = new BehaviorSubject< + ConnectionState | ElementCallError + >(ConnectionState.Initialized); /** * The current state of the connection to the media transport. @@ -131,6 +133,8 @@ export class Connection { this.stopped = false; try { this._state$.next(ConnectionState.FetchingConfig); + // We should already have this information after creating the localTransport. + // It would probably be better to forward this here. const { url, jwt } = await this.getSFUConfigWithOpenID(); // If we were stopped while fetching the config, don't proceed to connect if (this.stopped) return; @@ -172,7 +176,9 @@ export class Connection { }); } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); - this._state$.next(error instanceof Error ? error : new Error(`${error}`)); + this._state$.next( + error instanceof ElementCallError ? error : new UnknownCallError(error), + ); // Its okay to ignore the throw. The error is part of the state. throw error; } From 8dac0366b64d22da6ede6162dc3e2dcf7d57273b Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 21:17:33 +0100 Subject: [PATCH 16/43] fix lints --- src/state/CallViewModel/localMember/Publisher.test.ts | 6 +----- src/state/CallViewModel/remoteMembers/Connection.ts | 6 +++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 68845fa2..40763a99 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -26,12 +26,8 @@ import { mockMediaDevices, } from "../../../utils/test"; import { Publisher } from "./Publisher"; -import { - type Connection, - type ConnectionState, -} from "../remoteMembers/Connection"; +import { type Connection } from "../remoteMembers/Connection"; import { type MuteStates } from "../../MuteStates"; -import { FailToStartLivekitConnection } from "../../../utils/errors"; describe("Publisher", () => { let scope: ObservableScope; diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index c801b3ae..6015bf01 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -177,7 +177,11 @@ export class Connection { } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); this._state$.next( - error instanceof ElementCallError ? error : new UnknownCallError(error), + error instanceof ElementCallError + ? error + : error instanceof Error + ? new UnknownCallError(error) + : new UnknownCallError(new Error(`${error}`)), ); // Its okay to ignore the throw. The error is part of the state. throw error; From e626698fda8dd87213ccda763d3f04f86b830478 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 21:22:55 +0100 Subject: [PATCH 17/43] fix connection tests --- .../remoteMembers/Connection.test.ts | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index a90f0aa2..95ff931e 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -36,7 +36,10 @@ import { } from "./Connection.ts"; import { ObservableScope } from "../../ObservableScope.ts"; import { type OpenIDClientParts } from "../../../livekit/openIDSFU.ts"; -import { FailToGetOpenIdToken } from "../../../utils/errors.ts"; +import { + ElementCallError, + FailToGetOpenIdToken, +} from "../../../utils/errors.ts"; let testScope: ObservableScope; @@ -246,8 +249,11 @@ describe("Start connection states", () => { capturedState = capturedStates.pop(); - if (capturedState instanceof Error) { - expect(capturedState.message).toContain( + if ( + capturedState instanceof ElementCallError && + capturedState.cause instanceof Error + ) { + expect(capturedState.cause.message).toContain( "SFU Config fetch failed with exception Error", ); expect(connection.transport.livekit_alias).toEqual( @@ -308,8 +314,13 @@ describe("Start connection states", () => { capturedState = capturedStates.pop(); - if (capturedState instanceof Error) { - expect(capturedState.message).toContain("Failed to connect to livekit"); + if ( + capturedState instanceof ElementCallError && + capturedState.cause instanceof Error + ) { + expect(capturedState.cause.message).toContain( + "Failed to connect to livekit", + ); expect(connection.transport.livekit_alias).toEqual( livekitFocus.livekit_alias, ); From aabd76044b7a4d9bc259f5ff1c76ac1eab3d8682 Mon Sep 17 00:00:00 2001 From: Timo K Date: Wed, 10 Dec 2025 21:25:35 +0100 Subject: [PATCH 18/43] fix lint --- src/state/CallViewModel/CallViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 6a9eadea..aac88a3b 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -428,7 +428,7 @@ export function createCallViewModel$( combineLatest( [ localTransport$.pipe( - catchError((e) => { + catchError((e: unknown) => { logger.info( "dont pass local transport to createConnectionManager$. localTransport$ threw an error", e, From 170a38c0bae050c752d99aed150f2dfd9605b86d Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 11:30:14 +0100 Subject: [PATCH 19/43] fix playwright incompatible browser toast --- playwright/fixtures/widget-user.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/playwright/fixtures/widget-user.ts b/playwright/fixtures/widget-user.ts index 433c960b..b0c46788 100644 --- a/playwright/fixtures/widget-user.ts +++ b/playwright/fixtures/widget-user.ts @@ -113,7 +113,8 @@ async function registerUser( await page.getByRole("button", { name: "Register" }).click(); const continueButton = page.getByRole("button", { name: "Continue" }); try { - await expect(continueButton).toBeVisible({ timeout: 5000 }); + await expect(continueButton).toBeVisible({ timeout: 700 }); + // why do we need to put in the passwor if there is a continue button? await page .getByRole("textbox", { name: "Password", exact: true }) .fill(PASSWORD); @@ -124,6 +125,16 @@ async function registerUser( await expect( page.getByRole("heading", { name: `Welcome ${username}` }), ).toBeVisible(); + + // Dismiss incompatible browser toast + const dismissButton = page.getByRole("button", { name: "Dismiss" }); + try { + await expect(dismissButton).toBeVisible({ timeout: 700 }); + await dismissButton.click(); + } catch { + // dismissButton not visible, continue as normal + } + await setDevToolElementCallDevUrl(page); const clientHandle = await page.evaluateHandle(() => From 328cc7133a2377043061cf0e162547661a8eca55 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 11:32:28 +0100 Subject: [PATCH 20/43] update playwright so that we do not even need the dismiss anymore. --- package.json | 2 +- yarn.lock | 30 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 21c870ad..f65865e4 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "@opentelemetry/sdk-trace-base": "^2.0.0", "@opentelemetry/sdk-trace-web": "^2.0.0", "@opentelemetry/semantic-conventions": "^1.25.1", - "@playwright/test": "^1.56.1", + "@playwright/test": "^1.57.0", "@radix-ui/react-dialog": "^1.0.4", "@radix-ui/react-slider": "^1.1.2", "@radix-ui/react-visually-hidden": "^1.0.3", diff --git a/yarn.lock b/yarn.lock index f0ca83a7..02a4a3ce 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3373,14 +3373,14 @@ __metadata: languageName: node linkType: hard -"@playwright/test@npm:^1.56.1": - version: 1.56.1 - resolution: "@playwright/test@npm:1.56.1" +"@playwright/test@npm:^1.57.0": + version: 1.57.0 + resolution: "@playwright/test@npm:1.57.0" dependencies: - playwright: "npm:1.56.1" + playwright: "npm:1.57.0" bin: playwright: cli.js - checksum: 10c0/2b5b0e1f2e6a18f6e5ce6897c7440ca78f64e0b004834e9808e93ad2b78b96366b562ae4366602669cf8ad793a43d85481b58541e74be71e905e732d833dd691 + checksum: 10c0/35ba4b28be72bf0a53e33dbb11c6cff848fb9a37f49e893ce63a90675b5291ec29a1ba82c8a3b043abaead129400f0589623e9ace2e6a1c8eaa409721ecc3774 languageName: node linkType: hard @@ -7492,7 +7492,7 @@ __metadata: "@opentelemetry/sdk-trace-base": "npm:^2.0.0" "@opentelemetry/sdk-trace-web": "npm:^2.0.0" "@opentelemetry/semantic-conventions": "npm:^1.25.1" - "@playwright/test": "npm:^1.56.1" + "@playwright/test": "npm:^1.57.0" "@radix-ui/react-dialog": "npm:^1.0.4" "@radix-ui/react-slider": "npm:^1.1.2" "@radix-ui/react-visually-hidden": "npm:^1.0.3" @@ -11177,27 +11177,27 @@ __metadata: languageName: node linkType: hard -"playwright-core@npm:1.56.1": - version: 1.56.1 - resolution: "playwright-core@npm:1.56.1" +"playwright-core@npm:1.57.0": + version: 1.57.0 + resolution: "playwright-core@npm:1.57.0" bin: playwright-core: cli.js - checksum: 10c0/ffd40142b99c68678b387445d5b42f1fee4ab0b65d983058c37f342e5629f9cdbdac0506ea80a0dfd41a8f9f13345bad54e9a8c35826ef66dc765f4eb3db8da7 + checksum: 10c0/798e35d83bf48419a8c73de20bb94d68be5dde68de23f95d80a0ebe401e3b83e29e3e84aea7894d67fa6c79d2d3d40cc5bcde3e166f657ce50987aaa2421b6a9 languageName: node linkType: hard -"playwright@npm:1.56.1": - version: 1.56.1 - resolution: "playwright@npm:1.56.1" +"playwright@npm:1.57.0": + version: 1.57.0 + resolution: "playwright@npm:1.57.0" dependencies: fsevents: "npm:2.3.2" - playwright-core: "npm:1.56.1" + playwright-core: "npm:1.57.0" dependenciesMeta: fsevents: optional: true bin: playwright: cli.js - checksum: 10c0/8e9965aede86df0f4722063385748498977b219630a40a10d1b82b8bd8d4d4e9b6b65ecbfa024331a30800163161aca292fb6dd7446c531a1ad25f4155625ab4 + checksum: 10c0/ab03c99a67b835bdea9059f516ad3b6e42c21025f9adaa161a4ef6bc7ca716dcba476d287140bb240d06126eb23f889a8933b8f5f1f1a56b80659d92d1358899 languageName: node linkType: hard From 08306d663a16ad6193d7bb395649c0a228ffe160 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 16:04:12 +0100 Subject: [PATCH 21/43] remove duplicated connecting state and update Test setup --- .../remoteMembers/Connection.test.ts | 40 +++++++++++-------- .../CallViewModel/remoteMembers/Connection.ts | 22 +++++----- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 95ff931e..8f9471d0 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -50,11 +50,6 @@ let fakeLivekitRoom: MockedObject; let localParticipantEventEmiter: EventEmitter; let fakeLocalParticipant: MockedObject; -let fakeRoomEventEmiter: EventEmitter; -// let fakeMembershipsFocusMap$: BehaviorSubject< -// { membership: CallMembership; transport: LivekitTransport }[] -// >; - const livekitFocus: LivekitTransport = { livekit_alias: "!roomID:example.org", livekit_service_url: "https://matrix-rtc.example.org/livekit/jwt", @@ -91,22 +86,25 @@ function setupTest(): void { localParticipantEventEmiter, ), } as unknown as LocalParticipant); - fakeRoomEventEmiter = new EventEmitter(); + const fakeRoomEventEmitter = new EventEmitter(); fakeLivekitRoom = vi.mocked({ connect: vi.fn(), disconnect: vi.fn(), remoteParticipants: new Map(), localParticipant: fakeLocalParticipant, state: LivekitConnectionState.Disconnected, - on: fakeRoomEventEmiter.on.bind(fakeRoomEventEmiter), - off: fakeRoomEventEmiter.off.bind(fakeRoomEventEmiter), - addListener: fakeRoomEventEmiter.addListener.bind(fakeRoomEventEmiter), + on: fakeRoomEventEmitter.on.bind(fakeRoomEventEmitter), + off: fakeRoomEventEmitter.off.bind(fakeRoomEventEmitter), + addListener: fakeRoomEventEmitter.addListener.bind(fakeRoomEventEmitter), removeListener: - fakeRoomEventEmiter.removeListener.bind(fakeRoomEventEmiter), + fakeRoomEventEmitter.removeListener.bind(fakeRoomEventEmitter), removeAllListeners: - fakeRoomEventEmiter.removeAllListeners.bind(fakeRoomEventEmiter), + fakeRoomEventEmitter.removeAllListeners.bind(fakeRoomEventEmitter), setE2EEEnabled: vi.fn().mockResolvedValue(undefined), + emit: (eventName: string | symbol, ...args: unknown[]) => { + fakeRoomEventEmitter.emit(eventName, ...args); + }, } as unknown as LivekitRoom); } @@ -129,7 +127,13 @@ function setupRemoteConnection(): Connection { }); fakeLivekitRoom.connect.mockImplementation(async (): Promise => { + const changeEv = RoomEvent.ConnectionStateChanged; + + fakeLivekitRoom.state = LivekitConnectionState.Connecting; + fakeLivekitRoom.emit(changeEv, fakeLivekitRoom.state); fakeLivekitRoom.state = LivekitConnectionState.Connected; + fakeLivekitRoom.emit(changeEv, fakeLivekitRoom.state); + return Promise.resolve(); }); @@ -350,8 +354,10 @@ describe("Start connection states", () => { expect(initialState).toEqual(ConnectionState.Initialized); const fetchingState = capturedStates.shift(); expect(fetchingState).toEqual(ConnectionState.FetchingConfig); + const disconnectedState = capturedStates.shift(); + expect(disconnectedState).toEqual(ConnectionState.LivekitDisconnected); const connectingState = capturedStates.shift(); - expect(connectingState).toEqual(ConnectionState.ConnectingToLkRoom); + expect(connectingState).toEqual(ConnectionState.LivekitConnecting); const connectedState = capturedStates.shift(); expect(connectedState).toEqual(ConnectionState.LivekitConnected); }); @@ -419,7 +425,7 @@ describe("Publishing participants observations", () => { ); participants.forEach((p) => - fakeRoomEventEmiter.emit(RoomEvent.ParticipantConnected, p), + fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, p), ); // At this point there should be no publishers @@ -432,7 +438,7 @@ describe("Publishing participants observations", () => { fakeRemoteLivekitParticipant("@dan:example.org:DEV333", 2), ]; participants.forEach((p) => - fakeRoomEventEmiter.emit(RoomEvent.ParticipantConnected, p), + fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, p), ); // At this point there should be no publishers @@ -462,7 +468,7 @@ describe("Publishing participants observations", () => { ); for (const participant of participants) { - fakeRoomEventEmiter.emit(RoomEvent.ParticipantConnected, participant); + fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, participant); } // At this point there should be no publishers @@ -471,7 +477,7 @@ describe("Publishing participants observations", () => { participants = [fakeRemoteLivekitParticipant("@bob:example.org:DEV111", 1)]; for (const participant of participants) { - fakeRoomEventEmiter.emit(RoomEvent.ParticipantConnected, participant); + fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, participant); } // We should have bob has a publisher now @@ -488,7 +494,7 @@ describe("Publishing participants observations", () => { (p) => p.identity !== "@bob:example.org:DEV111", ); - fakeRoomEventEmiter.emit( + fakeLivekitRoom.emit( RoomEvent.ParticipantDisconnected, fakeRemoteLivekitParticipant("@bob:example.org:DEV111"), ); diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 6015bf01..8f8c0924 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -18,7 +18,7 @@ import { RoomEvent, } from "livekit-client"; import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; -import { BehaviorSubject, map } from "rxjs"; +import { BehaviorSubject, map, skip, skipWhile } from "rxjs"; import { type Logger } from "matrix-js-sdk/lib/logger"; import { @@ -61,7 +61,6 @@ export enum ConnectionState { /** `start` has been called on the connection. It aquires the jwt info to conenct to the LK Room */ FetchingConfig = "FetchingConfig", Stopped = "Stopped", - ConnectingToLkRoom = "ConnectingToLkRoom", /** The same as ConnectionState.Disconnected from `livekit-client` */ LivekitDisconnected = "disconnected", /** The same as ConnectionState.Connecting from `livekit-client` */ @@ -139,7 +138,17 @@ export class Connection { // If we were stopped while fetching the config, don't proceed to connect if (this.stopped) return; - this._state$.next(ConnectionState.ConnectingToLkRoom); + // Setup observer once we are done with getSFUConfigWithOpenID + connectionStateObserver(this.livekitRoom) + .pipe( + this.scope.bind(), + map((s) => s as unknown as ConnectionState), + ) + .subscribe((lkState) => { + // It is save to cast lkState to ConnectionState as they are fully overlapping. + this._state$.next(lkState); + }); + try { await this.livekitRoom.connect(url, jwt); } catch (e) { @@ -167,13 +176,6 @@ export class Connection { } // If we were stopped while connecting, don't proceed to update state. if (this.stopped) return; - - connectionStateObserver(this.livekitRoom) - .pipe(this.scope.bind()) - .subscribe((lkState) => { - // It is save to cast lkState to ConnectionState as they are fully overlapping. - this._state$.next(lkState as unknown as ConnectionState); - }); } catch (error) { this.logger.debug(`Failed to connect to LiveKit room: ${error}`); this._state$.next( From 9a7e797af48da255682e6db017dafd0e9a4624f0 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 16:17:45 +0100 Subject: [PATCH 22/43] fix lint --- src/state/CallViewModel/remoteMembers/Connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 8f8c0924..8b4479e8 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -18,7 +18,7 @@ import { RoomEvent, } from "livekit-client"; import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; -import { BehaviorSubject, map, skip, skipWhile } from "rxjs"; +import { BehaviorSubject, map } from "rxjs"; import { type Logger } from "matrix-js-sdk/lib/logger"; import { From 207b161b3ba25a7eae243a741c0fb7c3dfaf8eb9 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 17:17:56 +0100 Subject: [PATCH 23/43] fix logger and dismiss button presses --- playwright/fixtures/widget-user.ts | 10 +++++++++- src/room/GroupCallView.tsx | 1 - src/room/InCallView.tsx | 4 +++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/playwright/fixtures/widget-user.ts b/playwright/fixtures/widget-user.ts index b0c46788..51dffbc6 100644 --- a/playwright/fixtures/widget-user.ts +++ b/playwright/fixtures/widget-user.ts @@ -126,8 +126,16 @@ async function registerUser( page.getByRole("heading", { name: `Welcome ${username}` }), ).toBeVisible(); + await page.pause(); + const browserUnsupportedToast = page + .getByText("Element does not support this browser") + .locator("..") + .locator(".."); + // Dismiss incompatible browser toast - const dismissButton = page.getByRole("button", { name: "Dismiss" }); + const dismissButton = browserUnsupportedToast.getByRole("button", { + name: "Dismiss", + }); try { await expect(dismissButton).toBeVisible({ timeout: 700 }); await dismissButton.click(); diff --git a/src/room/GroupCallView.tsx b/src/room/GroupCallView.tsx index 1542678e..dfd11ff3 100644 --- a/src/room/GroupCallView.tsx +++ b/src/room/GroupCallView.tsx @@ -446,7 +446,6 @@ export const GroupCallView: FC = ({ let body: ReactNode; if (externalError) { - logger.debug("External error occurred:", externalError); // If an error was recorded within this component but outside // GroupCallErrorBoundary, create a component that rethrows the error from // within the error boundary, so it can be handled uniformly diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 18acf843..add8154a 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -24,7 +24,7 @@ import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; import classNames from "classnames"; import { BehaviorSubject, map } from "rxjs"; import { useObservable } from "observable-hooks"; -import { logger } from "matrix-js-sdk/lib/logger"; +import { logger as rootLogger } from "matrix-js-sdk/lib/logger"; import { VoiceCallSolidIcon, VolumeOnSolidIcon, @@ -109,6 +109,8 @@ import { useTrackProcessorObservable$ } from "../livekit/TrackProcessorContext.t import { type Layout } from "../state/layout-types.ts"; import { ObservableScope } from "../state/ObservableScope.ts"; +const logger = rootLogger.getChild("[InCallView]"); + const maxTapDurationMs = 400; export interface ActiveCallProps From 8225e4f2608a10e0662e5004df2ecddef1bea0b8 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 17:22:02 +0100 Subject: [PATCH 24/43] remove page.pause --- playwright/fixtures/widget-user.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/playwright/fixtures/widget-user.ts b/playwright/fixtures/widget-user.ts index 51dffbc6..efff8a80 100644 --- a/playwright/fixtures/widget-user.ts +++ b/playwright/fixtures/widget-user.ts @@ -126,7 +126,6 @@ async function registerUser( page.getByRole("heading", { name: `Welcome ${username}` }), ).toBeVisible(); - await page.pause(); const browserUnsupportedToast = page .getByText("Element does not support this browser") .locator("..") From 7edc97b9175dddd27ee3f90acb48de5de13fb3f4 Mon Sep 17 00:00:00 2001 From: Timo K Date: Thu, 11 Dec 2025 17:24:35 +0100 Subject: [PATCH 25/43] remove continue button things --- playwright/fixtures/widget-user.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/playwright/fixtures/widget-user.ts b/playwright/fixtures/widget-user.ts index efff8a80..6236928c 100644 --- a/playwright/fixtures/widget-user.ts +++ b/playwright/fixtures/widget-user.ts @@ -111,17 +111,7 @@ async function registerUser( await page.getByRole("textbox", { name: "Confirm password" }).click(); await page.getByRole("textbox", { name: "Confirm password" }).fill(PASSWORD); await page.getByRole("button", { name: "Register" }).click(); - const continueButton = page.getByRole("button", { name: "Continue" }); - try { - await expect(continueButton).toBeVisible({ timeout: 700 }); - // why do we need to put in the passwor if there is a continue button? - await page - .getByRole("textbox", { name: "Password", exact: true }) - .fill(PASSWORD); - await continueButton.click(); - } catch { - // continueButton not visible, continue as normal - } + await expect( page.getByRole("heading", { name: `Welcome ${username}` }), ).toBeVisible(); From f8310b46112c1207fb05a36c987014dd630c9119 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 10:31:08 +0100 Subject: [PATCH 26/43] publisher: only use highlevel participant APIs --- .../localMember/LocalMembership.test.ts | 37 +- .../localMember/LocalMembership.ts | 115 +++-- .../localMember/Publisher.test.ts | 426 +++++++++++++----- .../CallViewModel/localMember/Publisher.ts | 324 +++++++------ src/utils/test.ts | 2 + 5 files changed, 608 insertions(+), 296 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMembership.test.ts b/src/state/CallViewModel/localMember/LocalMembership.test.ts index cff5c06d..247e8ed1 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.test.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.test.ts @@ -271,6 +271,7 @@ describe("LocalMembership", () => { state: "ConnectedToLkRoom", }), transport: bTransport, + livekitRoom: mockLivekitRoom({}), } as unknown as Connection, [], ); @@ -281,13 +282,17 @@ describe("LocalMembership", () => { const localTransport$ = new BehaviorSubject(aTransport); const publishers: Publisher[] = []; - + let seed = 0; defaultCreateLocalMemberValues.createPublisherFactory.mockImplementation( () => { + const a = seed; + seed += 1; + logger.info(`creating [${a}]`); const p = { - stopPublishing: vi.fn(), + stopPublishing: vi.fn().mockImplementation(() => { + logger.info(`stopPublishing [${a}]`); + }), stopTracks: vi.fn(), - publishing$: constant(false), }; publishers.push(p as unknown as Publisher); return p; @@ -322,7 +327,7 @@ describe("LocalMembership", () => { await flushPromises(); // stop all tracks after ending scopes expect(publishers[1].stopPublishing).toHaveBeenCalled(); - expect(publishers[1].stopTracks).toHaveBeenCalled(); + // expect(publishers[1].stopTracks).toHaveBeenCalled(); defaultCreateLocalMemberValues.createPublisherFactory.mockReset(); }); @@ -367,15 +372,17 @@ describe("LocalMembership", () => { }); await flushPromises(); expect(publisherFactory).toHaveBeenCalledOnce(); - expect(localMembership.tracks$.value.length).toBe(0); + // expect(localMembership.tracks$.value.length).toBe(0); + expect(publishers[0].createAndSetupTracks).not.toHaveBeenCalled(); localMembership.startTracks(); await flushPromises(); - expect(localMembership.tracks$.value.length).toBe(2); + expect(publishers[0].createAndSetupTracks).toHaveBeenCalled(); + // expect(localMembership.tracks$.value.length).toBe(2); scope.end(); await flushPromises(); // stop all tracks after ending scopes expect(publishers[0].stopPublishing).toHaveBeenCalled(); - expect(publishers[0].stopTracks).toHaveBeenCalled(); + // expect(publishers[0].stopTracks).toHaveBeenCalled(); publisherFactory.mockClear(); }); // TODO add an integration test combining publisher and localMembership @@ -446,16 +453,16 @@ describe("LocalMembership", () => { state: RTCBackendState.Initialized, }); expect(publisherFactory).toHaveBeenCalledOnce(); - expect(localMembership.tracks$.value.length).toBe(0); + // expect(localMembership.tracks$.value.length).toBe(0); // ------- localMembership.startTracks(); // ------- await flushPromises(); - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.CreatingTracks, - }); + // expect(localMembership.connectionState.livekit$.value).toStrictEqual({ + // state: RTCBackendState.CreatingTracks, + // }); createTrackResolver.resolve(); await flushPromises(); expect(localMembership.connectionState.livekit$.value).toStrictEqual({ @@ -466,9 +473,9 @@ describe("LocalMembership", () => { localMembership.requestConnect(); // ------- - expect(localMembership.connectionState.livekit$.value).toStrictEqual({ - state: RTCBackendState.WaitingToPublish, - }); + // expect(localMembership.connectionState.livekit$.value).toStrictEqual({ + // state: RTCBackendState.WaitingToPublish, + // }); publishResolver.resolve(); await flushPromises(); @@ -486,7 +493,7 @@ describe("LocalMembership", () => { }); // stop all tracks after ending scopes expect(publishers[0].stopPublishing).toHaveBeenCalled(); - expect(publishers[0].stopTracks).toHaveBeenCalled(); + // expect(publishers[0].stopTracks).toHaveBeenCalled(); }); // TODO add tests for matrix local matrix participation. }); diff --git a/src/state/CallViewModel/localMember/LocalMembership.ts b/src/state/CallViewModel/localMember/LocalMembership.ts index 60ae79b8..de36a098 100644 --- a/src/state/CallViewModel/localMember/LocalMembership.ts +++ b/src/state/CallViewModel/localMember/LocalMembership.ts @@ -6,12 +6,13 @@ Please see LICENSE in the repository root for full details. */ import { - type LocalTrack, type Participant, ParticipantEvent, type LocalParticipant, type ScreenShareCaptureOptions, ConnectionState, + RoomEvent, + MediaDeviceFailure, } from "livekit-client"; import { observeParticipantEvents } from "@livekit/components-core"; import { @@ -24,6 +25,7 @@ import { combineLatest, distinctUntilChanged, from, + fromEvent, map, type Observable, of, @@ -61,9 +63,9 @@ export enum RTCBackendState { WaitingForConnection = "waiting_for_connection", /** Connection and transport arrived, publisher Initialized */ Initialized = "Initialized", - CreatingTracks = "creating_tracks", + // CreatingTracks = "creating_tracks", ReadyToPublish = "ready_to_publish", - WaitingToPublish = "waiting_to_publish", + // WaitingToPublish = "waiting_to_publish", Connected = "connected", Disconnected = "disconnected", Disconnecting = "disconnecting", @@ -74,9 +76,9 @@ type LocalMemberRtcBackendState = | { state: RTCBackendState.WaitingForTransport } | { state: RTCBackendState.WaitingForConnection } | { state: RTCBackendState.Initialized } - | { state: RTCBackendState.CreatingTracks } + // | { state: RTCBackendState.CreatingTracks } | { state: RTCBackendState.ReadyToPublish } - | { state: RTCBackendState.WaitingToPublish } + // | { state: RTCBackendState.WaitingToPublish } | { state: RTCBackendState.Connected } | { state: RTCBackendState.Disconnected } | { state: RTCBackendState.Disconnecting }; @@ -159,7 +161,7 @@ export const createLocalMembership$ = ({ /** * This starts audio and video tracks. They will be reused when calling `requestConnect`. */ - startTracks: () => Behavior; + startTracks: () => Behavior; /** * This sets a inner state (shouldConnect) to true and instructs the js-sdk and livekit to keep the user * connected to matrix and livekit. @@ -172,7 +174,7 @@ export const createLocalMembership$ = ({ * Callback to toggle screen sharing. If null, screen sharing is not possible. */ toggleScreenSharing: (() => void) | null; - tracks$: Behavior; + // tracks$: Behavior; participant$: Behavior; connection$: Behavior; homeserverConnected$: Behavior; @@ -224,6 +226,33 @@ export const createLocalMembership$ = ({ ), ); + // Tracks error that happen when creating the local tracks. + const mediaErrors$ = localConnection$.pipe( + switchMap((connection) => { + if (!connection) { + return of(null); + } else { + return fromEvent( + connection.livekitRoom, + RoomEvent.MediaDevicesError, + (error: Error) => { + return MediaDeviceFailure.getFailure(error) ?? null; + }, + ); + } + }), + ); + + mediaErrors$.pipe(scope.bind()).subscribe((error) => { + if (error) { + logger.error(`Failed to create local tracks:`, error); + // TODO is it fatal? Do we need to create a new Specialized Error? + setMatrixError( + new UnknownCallError(new Error(`Media device error: ${error}`)), + ); + } + }); + const localConnectionState$ = localConnection$.pipe( switchMap((connection) => (connection ? connection.state$ : of(null))), ); @@ -293,16 +322,16 @@ export const createLocalMembership$ = ({ /** * Extract the tracks from the published. Also reacts to changing publishers. */ - const tracks$ = scope.behavior( - publisher$.pipe(switchMap((p) => (p?.tracks$ ? p.tracks$ : constant([])))), - ); - const publishing$ = scope.behavior( - publisher$.pipe(switchMap((p) => p?.publishing$ ?? constant(false))), - ); + // const tracks$ = scope.behavior( + // publisher$.pipe(switchMap((p) => (p?.tracks$ ? p.tracks$ : constant([])))), + // ); + // const publishing$ = scope.behavior( + // publisher$.pipe(switchMap((p) => p?.publishing$ ?? constant(false))), + // ); - const startTracks = (): Behavior => { + const startTracks = (): Behavior => { trackStartRequested.resolve(); - return tracks$; + return constant(undefined); }; const requestConnect = (): void => { @@ -327,7 +356,7 @@ export const createLocalMembership$ = ({ } return Promise.resolve(async (): Promise => { await publisher$?.value?.stopPublishing(); - publisher$?.value?.stopTracks(); + await publisher$?.value?.stopTracks(); }); }); @@ -335,13 +364,16 @@ export const createLocalMembership$ = ({ // `tracks$` will update once they are ready. scope.reconcile( scope.behavior( - combineLatest([publisher$, tracks$, from(trackStartRequested.promise)]), + combineLatest([ + publisher$ /*, tracks$*/, + from(trackStartRequested.promise), + ]), null, ), async (valueIfReady) => { if (!valueIfReady) return; - const [publisher, tracks] = valueIfReady; - if (publisher && tracks.length === 0) { + const [publisher] = valueIfReady; + if (publisher) { await publisher.createAndSetupTracks().catch((e) => logger.error(e)); } }, @@ -349,22 +381,23 @@ export const createLocalMembership$ = ({ // Based on `connectRequested$` we start publishing tracks. (once they are there!) scope.reconcile( - scope.behavior(combineLatest([publisher$, tracks$, connectRequested$])), - async ([publisher, tracks, shouldConnect]) => { - if (shouldConnect === publisher?.publishing$.value) return; - if (tracks.length !== 0 && shouldConnect) { + scope.behavior(combineLatest([publisher$, connectRequested$])), + async ([publisher, shouldConnect]) => { + if (shouldConnect) { try { await publisher?.startPublishing(); } catch (error) { setLivekitError(error as ElementCallError); } - } else if (tracks.length !== 0 && !shouldConnect) { - try { - await publisher?.stopPublishing(); - } catch (error) { - setLivekitError(new UnknownCallError(error as Error)); - } } + // XXX Why is that? + // else { + // try { + // await publisher?.stopPublishing(); + // } catch (error) { + // setLivekitError(new UnknownCallError(error as Error)); + // } + // } }, ); @@ -378,12 +411,12 @@ export const createLocalMembership$ = ({ combineLatest([ publisher$, localTransport$, - tracks$.pipe( - tap((t) => { - logger.info("tracks$: ", t); - }), - ), - publishing$, + // tracks$.pipe( + // tap((t) => { + // logger.info("tracks$: ", t); + // }), + // ), + // publishing$, connectRequested$, from(trackStartRequested.promise).pipe( map(() => true), @@ -395,8 +428,8 @@ export const createLocalMembership$ = ({ ([ publisher, localTransport, - tracks, - publishing, + // tracks, + // publishing, shouldConnect, shouldStartTracks, error, @@ -408,15 +441,15 @@ export const createLocalMembership$ = ({ // as: // We do have but not yet so we are in if (error !== null) return { state: RTCBackendState.Error, error }; - const hasTracks = tracks.length > 0; + // const hasTracks = tracks.length > 0; if (!localTransport) return { state: RTCBackendState.WaitingForTransport }; if (!publisher) return { state: RTCBackendState.WaitingForConnection }; if (!shouldStartTracks) return { state: RTCBackendState.Initialized }; - if (!hasTracks) return { state: RTCBackendState.CreatingTracks }; + // if (!hasTracks) return { state: RTCBackendState.CreatingTracks }; if (!shouldConnect) return { state: RTCBackendState.ReadyToPublish }; - if (!publishing) return { state: RTCBackendState.WaitingToPublish }; + // if (!publishing) return { state: RTCBackendState.WaitingToPublish }; return { state: RTCBackendState.Connected }; }, ), @@ -588,7 +621,7 @@ export const createLocalMembership$ = ({ livekit$: livekitState$, matrix$: matrixState$, }, - tracks$, + // tracks$, participant$, homeserverConnected$, reconnecting$, diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 9b3e5b2a..3ab3d48c 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -5,66 +5,320 @@ 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, it, test, vi } from "vitest"; import { - afterEach, - beforeEach, - describe, - expect, - it, - type Mock, - vi, -} from "vitest"; -import { ConnectionState as LivekitConenctionState } from "livekit-client"; -import { type BehaviorSubject } from "rxjs"; + ConnectionState as LivekitConnectionState, + LocalParticipant, + type LocalTrack, + type LocalTrackPublication, + ParticipantEvent, + Track, +} from "livekit-client"; +import { BehaviorSubject } from "rxjs"; import { logger } from "matrix-js-sdk/lib/logger"; import { ObservableScope } from "../../ObservableScope"; import { constant } from "../../Behavior"; import { + flushPromises, mockLivekitRoom, - mockLocalParticipant, - mockMediaDevices, + mockMediaDevices } from "../../../utils/test"; import { Publisher } from "./Publisher"; -import { - type Connection, - type ConnectionState, -} from "../remoteMembers/Connection"; +import { type Connection } from "../remoteMembers/Connection"; import { type MuteStates } from "../../MuteStates"; -import { FailToStartLivekitConnection } from "../../../utils/errors"; -describe("Publisher", () => { - let scope: ObservableScope; - let connection: Connection; - let muteStates: MuteStates; - beforeEach(() => { - muteStates = { - audio: { - enabled$: constant(false), - unsetHandler: vi.fn(), - setHandler: vi.fn(), - }, - video: { - enabled$: constant(false), - unsetHandler: vi.fn(), - setHandler: vi.fn(), - }, - } as unknown as MuteStates; - scope = new ObservableScope(); - connection = { - state$: constant({ - state: "ConnectedToLkRoom", - livekitConnectionState$: constant(LivekitConenctionState.Connected), - }), - livekitRoom: mockLivekitRoom({ - localParticipant: mockLocalParticipant({}), - }), - } as unknown as Connection; +let scope: ObservableScope; + +beforeEach(() => { + scope = new ObservableScope(); +}); + +// afterEach(() => scope.end()); + +function createMockLocalTrack(source: Track.Source): LocalTrack { + const track = { + source, + isMuted: false, + isUpstreamPaused: false, + } as Partial as LocalTrack; + + vi.mocked(track).mute = vi.fn().mockImplementation(() => { + track.isMuted = true; + }); + vi.mocked(track).unmute = vi.fn().mockImplementation(() => { + track.isMuted = false; + }); + vi.mocked(track).pauseUpstream = vi.fn().mockImplementation(() => { + // @ts-expect-error - for that test we want to set isUpstreamPaused directly + track.isUpstreamPaused = true; + }); + vi.mocked(track).resumeUpstream = vi.fn().mockImplementation(() => { + // @ts-expect-error - for that test we want to set isUpstreamPaused directly + track.isUpstreamPaused = false; }); - afterEach(() => scope.end()); + return track; +} - it("throws if livekit room could not publish", async () => { +function createMockMuteState(enabled$: BehaviorSubject): { + enabled$: BehaviorSubject; + setHandler: (h: (enabled: boolean) => void) => void; + unsetHandler: () => void; +} { + let currentHandler = (enabled: boolean): void => {}; + + const ms = { + enabled$, + setHandler: vi.fn().mockImplementation((h: (enabled: boolean) => void) => { + currentHandler = h; + }), + unsetHandler: vi.fn().mockImplementation(() => { + currentHandler = (enabled: boolean): void => {}; + }), + }; + // forward enabled$ emissions to the current handler + enabled$.subscribe((enabled) => { + logger.info(`MockMuteState: enabled changed to ${enabled}`); + currentHandler(enabled); + }); + + return ms; +} + +let connection: Connection; +let muteStates: MuteStates; +let localParticipant: LocalParticipant; +let audioEnabled$: BehaviorSubject; +let videoEnabled$: BehaviorSubject; +let trackPublications: LocalTrackPublication[]; +// use it to control when track creation resolves, default to resolved +let createTrackLock: Promise; + +beforeEach(() => { + trackPublications = []; + audioEnabled$ = new BehaviorSubject(false); + videoEnabled$ = new BehaviorSubject(false); + createTrackLock = Promise.resolve(); + + muteStates = { + audio: createMockMuteState(audioEnabled$), + video: createMockMuteState(videoEnabled$), + } as unknown as MuteStates; + + const mockSendDataPacket = vi.fn(); + const mockEngine = { + client: { + sendUpdateLocalMetadata: vi.fn(), + }, + on: vi.fn().mockReturnThis(), + sendDataPacket: mockSendDataPacket, + }; + + localParticipant = new LocalParticipant( + "local-sid", + "local-identity", + // @ts-expect-error - for that test we want a real LocalParticipant to have the pending publications logic + mockEngine, + { + adaptiveStream: true, + dynacase: false, + audioCaptureDefaults: {}, + videoCaptureDefaults: {}, + stopLocalTrackOnUnpublish: true, + reconnectPolicy: "always", + disconnectOnPageLeave: true, + }, + new Map(), + {}, + ); + + vi.mocked(localParticipant).createTracks = vi + .fn() + .mockImplementation(async (opts) => { + const tracks: LocalTrack[] = []; + if (opts.audio) { + tracks.push(createMockLocalTrack(Track.Source.Microphone)); + } + if (opts.video) { + tracks.push(createMockLocalTrack(Track.Source.Camera)); + } + await createTrackLock; + return tracks; + }); + + vi.mocked(localParticipant).publishTrack = vi + .fn() + .mockImplementation(async (track: LocalTrack) => { + const pub = { + track, + source: track.source, + mute: track.mute, + unmute: track.unmute, + } as Partial as LocalTrackPublication; + trackPublications.push(pub); + localParticipant.emit(ParticipantEvent.LocalTrackPublished, pub); + return Promise.resolve(pub); + }); + + vi.mocked(localParticipant).getTrackPublication = vi + .fn() + .mockImplementation((source: Track.Source) => { + return trackPublications.find((pub) => pub.track?.source === source); + }); + + connection = { + state$: constant({ + state: "ConnectedToLkRoom", + livekitConnectionState$: constant(LivekitConnectionState.Connected), + }), + livekitRoom: mockLivekitRoom({ + localParticipant: localParticipant, + }), + } as unknown as Connection; +}); + +describe("Publisher", () => { + let publisher: Publisher; + + beforeEach(() => { + publisher = new Publisher( + scope, + connection, + mockMediaDevices({}), + muteStates, + constant({ supported: false, processor: undefined }), + logger, + ); + }); + + afterEach(() => {}); + + it("Should not create tracks if started muted to avoid unneeded permission requests", async () => { + const createTracksSpy = vi.spyOn( + connection.livekitRoom.localParticipant, + "createTracks", + ); + + audioEnabled$.next(false); + videoEnabled$.next(false); + await publisher.createAndSetupTracks(); + + expect(createTracksSpy).not.toHaveBeenCalled(); + }); + + it("Should minimize permission request by querying create at once", async () => { + const enableCameraAndMicrophoneSpy = vi.spyOn( + localParticipant, + "enableCameraAndMicrophone", + ); + const createTracksSpy = vi.spyOn(localParticipant, "createTracks"); + + audioEnabled$.next(true); + videoEnabled$.next(true); + await publisher.createAndSetupTracks(); + await flushPromises(); + + expect(enableCameraAndMicrophoneSpy).toHaveBeenCalled(); + + // It should create both at once + expect(createTracksSpy).toHaveBeenCalledWith({ + audio: true, + video: true, + }); + }); + + it("Ensure no data is streamed until publish has been called", async () => { + audioEnabled$.next(true); + await publisher.createAndSetupTracks(); + + // The track should be created and paused + expect(localParticipant.createTracks).toHaveBeenCalledWith({ + audio: true, + video: undefined, + }); + await flushPromises(); + expect(localParticipant.publishTrack).toHaveBeenCalled(); + + await flushPromises(); + const track = localParticipant.getTrackPublication( + Track.Source.Microphone, + )?.track; + expect(track).toBeDefined(); + expect(track!.pauseUpstream).toHaveBeenCalled(); + expect(track!.isUpstreamPaused).toBe(true); + }); + + it("Ensure resume upstream when published is called", async () => { + videoEnabled$.next(true); + await publisher.createAndSetupTracks(); + // await flushPromises(); + await publisher.startPublishing(); + + const track = localParticipant.getTrackPublication( + Track.Source.Camera, + )?.track; + expect(track).toBeDefined(); + // expect(track.pauseUpstream).toHaveBeenCalled(); + expect(track!.isUpstreamPaused).toBe(false); + }); + + describe("Mute states", () => { + let publisher: Publisher; + beforeEach(() => { + publisher = new Publisher( + scope, + connection, + mockMediaDevices({}), + muteStates, + constant({ supported: false, processor: undefined }), + logger, + ); + }); + + test.each([ + { mutes: { audioEnabled: true, videoEnabled: false } }, + { mutes: { audioEnabled: true, videoEnabled: false } }, + ])("only create the tracks that are unmuted $mutes", async ({ mutes }) => { + // Ensure all muted + audioEnabled$.next(mutes.audioEnabled); + videoEnabled$.next(mutes.videoEnabled); + + vi.mocked(connection.livekitRoom.localParticipant).createTracks = vi + .fn() + .mockResolvedValue([]); + + await publisher.createAndSetupTracks(); + + expect( + connection.livekitRoom.localParticipant.createTracks, + ).toHaveBeenCalledOnce(); + + expect( + connection.livekitRoom.localParticipant.createTracks, + ).toHaveBeenCalledWith({ + audio: mutes.audioEnabled ? true : undefined, + video: mutes.videoEnabled ? true : undefined, + }); + }); + }); + + it("does mute unmute audio", async () => {}); +}); + +describe("Bug fix", () => { + // There is a race condition when creating and publishing tracks while the mute state changes. + // This race condition could cause tracks to be published even though they are muted at the + // beginning of a call coming from lobby. + // This is caused by our stack using manually the low level API to create and publish tracks, + // but also using the higher level setMicrophoneEnabled and setCameraEnabled functions that also create + // and publish tracks, and managing pending publications. + // Race is as follow, on creation of the Publisher we create the tracks then publish them. + // If in the middle of that process the mute state changes: + // - the `setMicrophoneEnabled` will be no-op because it is not aware of our created track and can't see any pending publication + // - If start publication is requested it will publish the track even though there was a mute request. + it("wrongly publish tracks while muted", async () => { + // setLogLevel(`debug`); const publisher = new Publisher( scope, connection, @@ -73,68 +327,34 @@ describe("Publisher", () => { constant({ supported: false, processor: undefined }), logger, ); + audioEnabled$.next(true); - // should do nothing if no tracks have been created yet. - await publisher.startPublishing(); - expect( - connection.livekitRoom.localParticipant.publishTrack, - ).not.toHaveBeenCalled(); + const resolvers = Promise.withResolvers(); + createTrackLock = resolvers.promise; - await expect(publisher.createAndSetupTracks()).rejects.toThrow( - Error("audio and video is false"), - ); - - (muteStates.audio.enabled$ as BehaviorSubject).next(true); - - ( - connection.livekitRoom.localParticipant.createTracks as Mock - ).mockResolvedValue([{}, {}]); - - await expect(publisher.createAndSetupTracks()).resolves.not.toThrow(); - expect( - connection.livekitRoom.localParticipant.createTracks, - ).toHaveBeenCalledOnce(); - - // failiour due to localParticipant.publishTrack - ( - connection.livekitRoom.localParticipant.publishTrack as Mock - ).mockRejectedValue(Error("testError")); - - await expect(publisher.startPublishing()).rejects.toThrow( - new FailToStartLivekitConnection("testError"), - ); - - // does not try other conenction after the first one failed - expect( - connection.livekitRoom.localParticipant.publishTrack, - ).toHaveBeenCalledTimes(1); - - // failiour due to connection.state$ - const beforeState = connection.state$.value; - (connection.state$ as BehaviorSubject).next({ - state: "FailedToStart", - error: Error("testStartError"), + // Initially the audio is unmuted, so creating tracks should publish the audio track + const createTracks = publisher.createAndSetupTracks(); + void publisher.startPublishing(); + void createTracks.then(() => { + void publisher.startPublishing(); }); + // now mute the audio before allowing track creation to complete + audioEnabled$.next(false); + resolvers.resolve(undefined); + await createTracks; - await expect(publisher.startPublishing()).rejects.toThrow( - new FailToStartLivekitConnection("testStartError"), - ); - (connection.state$ as BehaviorSubject).next(beforeState); + await flushPromises(); - // does not try other conenction after the first one failed - expect( - connection.livekitRoom.localParticipant.publishTrack, - ).toHaveBeenCalledTimes(1); + const track = localParticipant.getTrackPublication( + Track.Source.Microphone, + )?.track; + expect(track).toBeDefined(); - // success case - ( - connection.livekitRoom.localParticipant.publishTrack as Mock - ).mockResolvedValue({}); - - await expect(publisher.startPublishing()).resolves.not.toThrow(); - - expect( - connection.livekitRoom.localParticipant.publishTrack, - ).toHaveBeenCalledTimes(3); + try { + expect(localParticipant.publishTrack).not.toHaveBeenCalled(); + } catch { + expect(track!.mute).toHaveBeenCalled(); + expect(track!.isMuted).toBe(true); + } }); }); diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index 326dedaf..5da6d899 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -6,15 +6,14 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ import { + ConnectionState as LivekitConnectionState, + type LocalTrackPublication, LocalVideoTrack, + ParticipantEvent, type Room as LivekitRoom, Track, - type LocalTrack, - type LocalTrackPublication, - ConnectionState as LivekitConnectionState, } from "livekit-client"; import { - BehaviorSubject, map, NEVER, type Observable, @@ -34,10 +33,6 @@ import { getUrlParams } from "../../../UrlParams.ts"; import { observeTrackReference$ } from "../../MediaViewModel.ts"; import { type Connection } from "../remoteMembers/Connection.ts"; import { type ObservableScope } from "../../ObservableScope.ts"; -import { - ElementCallError, - FailToStartLivekitConnection, -} from "../../../utils/errors.ts"; /** * A wrapper for a Connection object. @@ -45,14 +40,21 @@ import { * The Publisher is also responsible for creating the media tracks. */ export class Publisher { + /** + * By default, livekit will start publishing tracks as soon as they are created. + * In the matrix RTC world, we want to control when tracks are published based + * on whether the user is part of the RTC session or not. + */ + public shouldPublish = false; + /** * Creates a new Publisher. * @param scope - The observable scope to use for managing the publisher. * @param connection - The connection to use for publishing. * @param devices - The media devices to use for audio and video input. * @param muteStates - The mute states for audio and video. - * @param e2eeLivekitOptions - The E2EE options to use for the LiveKit room. Use to share the same key provider across connections!. * @param trackerProcessorState$ - The processor state for the video track processor (e.g. background blur). + * @param logger - The logger to use for logging :D. */ public constructor( private scope: ObservableScope, @@ -62,7 +64,6 @@ export class Publisher { trackerProcessorState$: Behavior, private logger: Logger, ) { - this.logger.info("Create LiveKit room"); const { controlledAudioDevices } = getUrlParams(); const room = connection.livekitRoom; @@ -80,41 +81,54 @@ export class Publisher { this.scope.onEnd(() => { this.logger.info("Scope ended -> stop publishing all tracks"); void this.stopPublishing(); + muteStates.audio.unsetHandler(); + muteStates.video.unsetHandler(); }); - // TODO move mute state handling here using reconcile (instead of inside the mute state class) - // this.scope.reconcile( - // this.scope.behavior( - // combineLatest([this.muteStates.video.enabled$, this.tracks$]), - // ), - // async ([videoEnabled, tracks]) => { - // const track = tracks.find((t) => t.kind == Track.Kind.Video); - // if (!track) return; - - // if (videoEnabled) { - // await track.unmute(); - // } else { - // await track.mute(); - // } - // }, - // ); + this.connection.livekitRoom.localParticipant.on( + ParticipantEvent.LocalTrackPublished, + this.onLocalTrackPublished.bind(this), + ); } - private _tracks$ = new BehaviorSubject[]>([]); - public tracks$ = this._tracks$ as Behavior[]>; - + // LiveKit will publish the tracks as soon as they are created + // but we want to control when tracks are published. + // We cannot just mute the tracks, even if this will effectively stop the publishing, + // it would also prevent the user from seeing their own video/audio preview. + // So for that we use pauseUpStream(): Stops sending media to the server by replacing + // the sender track with null, but keeps the local MediaStreamTrack active. + // The user can still see/hear themselves locally, but remote participants see nothing + private onLocalTrackPublished( + localTrackPublication: LocalTrackPublication, + ): void { + this.logger.info("Local track published", localTrackPublication); + const lkRoom = this.connection.livekitRoom; + if (!this.shouldPublish) { + this.pauseUpstreams(lkRoom, [localTrackPublication.source]).catch((e) => { + this.logger.error(`Failed to pause upstreams`, e); + }); + } + // also check the mute state and apply it + if (localTrackPublication.source === Track.Source.Microphone) { + const enabled = this.muteStates.audio.enabled$.value; + lkRoom.localParticipant.setMicrophoneEnabled(enabled).catch((e) => { + this.logger.error( + `Failed to enable microphone track, enabled:${enabled}`, + e, + ); + }); + } else if (localTrackPublication.source === Track.Source.Camera) { + const enabled = this.muteStates.video.enabled$.value; + lkRoom.localParticipant.setCameraEnabled(enabled).catch((e) => { + this.logger.error( + `Failed to enable camera track, enabled:${enabled}`, + e, + ); + }); + } + } /** - * Start the connection to LiveKit and publish local tracks. * - * This will: - * wait for the connection to be ready. - // * 1. Request an OpenId token `request_token` (allows matrix users to verify their identity with a third-party service.) - // * 2. Use this token to request the SFU config to the MatrixRtc authentication service. - // * 3. Connect to the configured LiveKit room. - // * 4. Create local audio and video tracks based on the current mute states and publish them to the room. - * - * @throws {InsufficientCapacityError} if the LiveKit server indicates that it has insufficient capacity to accept the connection. - * @throws {SFURoomCreationRestrictedError} if the LiveKit server indicates that the room does not exist and cannot be created. */ public async createAndSetupTracks(): Promise { this.logger.debug("createAndSetupTracks called"); @@ -122,119 +136,141 @@ export class Publisher { // Observe mute state changes and update LiveKit microphone/camera states accordingly this.observeMuteStates(this.scope); - // TODO-MULTI-SFU: Prepublish a microphone track + // Check if audio and/or video is enabled. We only create tracks if enabled, + // because it could prompt for permission, and we don't want to do that unnecessarily. const audio = this.muteStates.audio.enabled$.value; const video = this.muteStates.video.enabled$.value; - // createTracks throws if called with audio=false and video=false - if (audio || video) { - // TODO this can still throw errors? It will also prompt for permissions if not already granted - return lkRoom.localParticipant - .createTracks({ - audio, - video, - }) - .then((tracks) => { - this.logger.info( - "created track", - tracks.map((t) => t.kind + ", " + t.id), - ); - this._tracks$.next(tracks); - }) - .catch((error) => { - this.logger.error("Failed to create tracks", error); - }); - } - throw Error("audio and video is false"); + + const enableTracks = async (): Promise => { + if (audio && video) { + // Enable both at once in order to have a single permission prompt! + await lkRoom.localParticipant.enableCameraAndMicrophone(); + } else if (audio) { + await lkRoom.localParticipant.setMicrophoneEnabled(true); + } else if (video) { + await lkRoom.localParticipant.setCameraEnabled(true); + } + return; + }; + + // We cannot just wait because this call could wait for the track to be + // published (and not just created), which we don't want yet. + // Notice it will wait for that only the first time, when tracks are created, the + // later calls will be instant :/ + enableTracks() + .then(() => { + // At this point, LiveKit will have created & published the tracks as soon as possible + // but we want to control when tracks are published. + // We cannot just mute the tracks, even if this will effectively stop the publishing, + // it would also prevent the user from seeing their own video/audio preview. + // So for that we use pauseUpStream(): Stops sending media to the server by replacing + // the sender track with null, but keeps the local MediaStreamTrack active. + // The user can still see/hear themselves locally, but remote participants see nothing + if (!this.shouldPublish) { + this.pauseUpstreams(lkRoom, [ + Track.Source.Microphone, + Track.Source.Camera, + ]).catch((e) => { + this.logger.error(`Failed to pause upstreams`, e); + }); + } + }) + .catch((e: Error) => { + this.logger.error(`Failed to enable camera and microphone`, e); + }); + + return Promise.resolve(); + } + + private async pauseUpstreams( + lkRoom: LivekitRoom, + sources: Track.Source[], + ): Promise { + for (const source of sources) { + const track = lkRoom.localParticipant.getTrackPublication(source)?.track; + if (track) { + await track.pauseUpstream(); + } else { + this.logger.warn( + `No track found for source ${source} to pause upstream`, + ); + } + } + } + + private async resumeUpstreams( + lkRoom: LivekitRoom, + sources: Track.Source[], + ): Promise { + for (const source of sources) { + const track = lkRoom.localParticipant.getTrackPublication(source)?.track; + if (track) { + await track.resumeUpstream(); + } else { + this.logger.warn( + `No track found for source ${source} to resume upstream`, + ); + } + } } - private _publishing$ = new BehaviorSubject(false); - public publishing$ = this.scope.behavior(this._publishing$); /** + * + * Request to publish local tracks to the LiveKit room. + * This will wait for the connection to be ready before publishing. + * Livekit also have some local retry logic for publishing tracks. + * Can be called multiple times, localparticipant manages the state of published tracks (or pending publications). * * @returns - * @throws ElementCallError */ - public async startPublishing(): Promise { + public async startPublishing(): Promise { + if (this.shouldPublish) { + this.logger.debug(`Already publishing, ignoring startPublishing call`); + return; + } + this.shouldPublish = true; this.logger.debug("startPublishing called"); + const lkRoom = this.connection.livekitRoom; - const { promise, resolve, reject } = Promise.withResolvers(); - const sub = this.connection.state$.subscribe((s) => { - switch (s.state) { - case "ConnectedToLkRoom": - resolve(); - break; - case "FailedToStart": - reject( - s.error instanceof ElementCallError - ? s.error - : new FailToStartLivekitConnection(s.error.message), - ); - break; - default: - this.logger.info("waiting for connection: ", s.state); - } - }); + + // Resume upstream for both audio and video tracks + // We need to call it explicitly because call setTrackEnabled does not always + // resume upstream. It will only if you switch the track from disabled to enabled, + // but if the track is already enabled but upstream is paused, it won't resume it. + // TODO what about screen share? try { - await promise; + await this.resumeUpstreams(lkRoom, [ + Track.Source.Microphone, + Track.Source.Camera, + ]); } catch (e) { - throw e; - } finally { - sub.unsubscribe(); + this.logger.error(`Failed to resume upstreams`, e); } - - for (const track of this.tracks$.value) { - this.logger.info("publish ", this.tracks$.value.length, "tracks"); - // TODO: handle errors? Needs the signaling connection to be up, but it has some retries internally - // with a timeout. - await lkRoom.localParticipant.publishTrack(track).catch((error) => { - this.logger.error("Failed to publish track", error); - throw new FailToStartLivekitConnection( - error instanceof Error ? error.message : error, - ); - }); - this.logger.info("published track ", track.kind, track.id); - - // TODO: check if the connection is still active? and break the loop if not? - } - this._publishing$.next(true); - return this.tracks$.value; } public async stopPublishing(): Promise { this.logger.debug("stopPublishing called"); - // TODO-MULTI-SFU: Move these calls back to ObservableScope.onEnd once scope - // actually has the right lifetime - this.muteStates.audio.unsetHandler(); - this.muteStates.video.unsetHandler(); - - const localParticipant = this.connection.livekitRoom.localParticipant; - const tracks: LocalTrack[] = []; - const addToTracksIfDefined = (p: LocalTrackPublication): void => { - if (p.track !== undefined) tracks.push(p.track); - }; - localParticipant.trackPublications.forEach(addToTracksIfDefined); - this.logger.debug( - "list of tracks to unpublish:", - tracks.map((t) => t.kind + ", " + t.id), - "start unpublishing now", - ); - await localParticipant.unpublishTracks(tracks).catch((error) => { - this.logger.error("Failed to unpublish tracks", error); - throw error; - }); - this.logger.debug( - "unpublished tracks", - tracks.map((t) => t.kind + ", " + t.id), - ); - this._publishing$.next(false); + this.shouldPublish = false; + await this.pauseUpstreams(this.connection.livekitRoom, [ + Track.Source.Microphone, + Track.Source.Camera, + Track.Source.ScreenShare, + ]); } - /** - * Stops all tracks that are currently running - */ - public stopTracks(): void { - this.tracks$.value.forEach((t) => t.stop()); - this._tracks$.next([]); + public async stopTracks(): Promise { + const lkRoom = this.connection.livekitRoom; + for (const source of [ + Track.Source.Microphone, + Track.Source.Camera, + Track.Source.ScreenShare, + ]) { + const localPub = lkRoom.localParticipant.getTrackPublication(source); + if (localPub?.track) { + // stops and unpublishes the track + await lkRoom.localParticipant.unpublishTrack(localPub!.track, true); + } + } } /// Private methods @@ -336,17 +372,31 @@ export class Publisher { */ private observeMuteStates(scope: ObservableScope): void { const lkRoom = this.connection.livekitRoom; - this.muteStates.audio.setHandler(async (desired) => { + this.muteStates.audio.setHandler(async (enable) => { try { - await lkRoom.localParticipant.setMicrophoneEnabled(desired); + this.logger.debug( + `handler: Setting LiveKit microphone enabled: ${enable}`, + ); + await lkRoom.localParticipant.setMicrophoneEnabled(enable); + // Unmute will restart the track if it was paused upstream, + // but until explicitly requested, we want to keep it paused. + if (!this.shouldPublish && enable) { + await this.pauseUpstreams(lkRoom, [Track.Source.Microphone]); + } } catch (e) { this.logger.error("Failed to update LiveKit audio input mute state", e); } return lkRoom.localParticipant.isMicrophoneEnabled; }); - this.muteStates.video.setHandler(async (desired) => { + this.muteStates.video.setHandler(async (enable) => { try { - await lkRoom.localParticipant.setCameraEnabled(desired); + this.logger.debug(`handler: Setting LiveKit camera enabled: ${enable}`); + await lkRoom.localParticipant.setCameraEnabled(enable); + // Unmute will restart the track if it was paused upstream, + // but until explicitly requested, we want to keep it paused. + if (!this.shouldPublish && enable) { + await this.pauseUpstreams(lkRoom, [Track.Source.Camera]); + } } catch (e) { this.logger.error("Failed to update LiveKit video input mute state", e); } diff --git a/src/utils/test.ts b/src/utils/test.ts index bd7dcd6f..24b9bef0 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -311,6 +311,8 @@ export function mockLocalParticipant( publishTrack: vi.fn(), unpublishTracks: vi.fn().mockResolvedValue([]), createTracks: vi.fn(), + setMicrophoneEnabled: vi.fn(), + setCameraEnabled: vi.fn(), getTrackPublication: () => ({}) as Partial as LocalTrackPublication, ...mockEmitter(), From 610af439a8fbf371cc310af84fe1e96775b1a86e Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 10:37:37 +0100 Subject: [PATCH 27/43] cleaning: just use `LocalTrackPublished` event to pause/unpause --- .../CallViewModel/localMember/Publisher.ts | 34 +++++-------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index 5da6d899..d4ad656c 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -152,32 +152,14 @@ export class Publisher { } return; }; - - // We cannot just wait because this call could wait for the track to be - // published (and not just created), which we don't want yet. - // Notice it will wait for that only the first time, when tracks are created, the - // later calls will be instant :/ - enableTracks() - .then(() => { - // At this point, LiveKit will have created & published the tracks as soon as possible - // but we want to control when tracks are published. - // We cannot just mute the tracks, even if this will effectively stop the publishing, - // it would also prevent the user from seeing their own video/audio preview. - // So for that we use pauseUpStream(): Stops sending media to the server by replacing - // the sender track with null, but keeps the local MediaStreamTrack active. - // The user can still see/hear themselves locally, but remote participants see nothing - if (!this.shouldPublish) { - this.pauseUpstreams(lkRoom, [ - Track.Source.Microphone, - Track.Source.Camera, - ]).catch((e) => { - this.logger.error(`Failed to pause upstreams`, e); - }); - } - }) - .catch((e: Error) => { - this.logger.error(`Failed to enable camera and microphone`, e); - }); + // We don't await enableTracks, because livekit could block until the tracks + // are fully published, and not only that they are created. + // We don't have control on that, localParticipant creates and publishes the tracks + // asap. + // We are using the `ParticipantEvent.LocalTrackPublished` to be notified + // when tracks are actually published, and at that point + // we can pause upstream if needed (depending on if startPublishing has been called). + void enableTracks(); return Promise.resolve(); } From b3b76d8b3d26e367c325233bf4c86745b41b7763 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 11:54:43 +0100 Subject: [PATCH 28/43] post merge --- .../CallViewModel/localMember/LocalMember.ts | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 5898a3da..241cb633 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -10,7 +10,6 @@ import { ParticipantEvent, type LocalParticipant, type ScreenShareCaptureOptions, - ConnectionState, RoomEvent, MediaDeviceFailure, } from "livekit-client"; @@ -329,11 +328,10 @@ export const createLocalMembership$ = ({ // Based on `connectRequested$` we start publishing tracks. (once they are there!) scope.reconcile( scope.behavior( - combineLatest([publisher$, tracks$, joinAndPublishRequested$]), + combineLatest([publisher$, joinAndPublishRequested$]), ), - async ([publisher, tracks, shouldJoinAndPublish]) => { - if (shouldJoinAndPublish === publisher?.publishing$.value) return; - if (tracks.length !== 0 && shouldJoinAndPublish) { + async ([publisher, shouldJoinAndPublish]) => { + if (shouldJoinAndPublish) { try { await publisher?.startPublishing(); } catch (error) { @@ -341,7 +339,7 @@ export const createLocalMembership$ = ({ error instanceof Error ? error.message : String(error); setPublishError(new FailToStartLivekitConnection(message)); } - } else if (tracks.length !== 0 && !shouldJoinAndPublish) { + } else { try { await publisher?.stopPublishing(); } catch (error) { @@ -391,15 +389,6 @@ export const createLocalMembership$ = ({ combineLatest([ localConnectionState$, localTransport$, - // tracks$.pipe( - // tap((t) => { - // logger.info("tracks$: ", t); - // }), - // ), - // publishing$, - connectRequested$, - tracks$, - publishing$, joinAndPublishRequested$, from(trackStartRequested.promise).pipe( map(() => true), @@ -410,16 +399,17 @@ export const createLocalMembership$ = ({ ([ localConnectionState, localTransport, - tracks, - publishing, + // tracks, + // publishing, shouldPublish, shouldStartTracks, ]) => { if (!localTransport) return null; - const hasTracks = tracks.length > 0; - let trackState: TrackState = TrackState.WaitingForUser; - if (hasTracks && shouldStartTracks) trackState = TrackState.Ready; - if (!hasTracks && shouldStartTracks) trackState = TrackState.Creating; + // const hasTracks = tracks.length > 0; + // let trackState: TrackState = TrackState.WaitingForUser; + // if (hasTracks && shouldStartTracks) trackState = TrackState.Ready; + // if (!hasTracks && shouldStartTracks) trackState = TrackState.Creating; + const trackState: TrackState = shouldStartTracks ? TrackState.Ready : TrackState.WaitingForUser; if ( localConnectionState !== ConnectionState.LivekitConnected || @@ -430,7 +420,7 @@ export const createLocalMembership$ = ({ tracks: trackState, }; if (!shouldPublish) return PublishState.WaitingForUser; - if (!publishing) return PublishState.Starting; + // if (!publishing) return PublishState.Starting; return PublishState.Publishing; }, ), @@ -660,7 +650,6 @@ export const createLocalMembership$ = ({ requestJoinAndPublish, requestDisconnect, localMemberState$, - tracks$, participant$, reconnecting$, disconnected$: scope.behavior( From 93da69983dfaa6aa7850429d7aa09bbe4af2224b Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 14:40:45 +0100 Subject: [PATCH 29/43] post merge: partial mapping of tracks/publish states --- .../localMember/LocalMember.test.ts | 18 ++++++++++-------- .../CallViewModel/localMember/LocalMember.ts | 18 +++++++++++------- .../localMember/Publisher.test.ts | 2 +- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.test.ts b/src/state/CallViewModel/localMember/LocalMember.test.ts index 892bbb3d..0d77611b 100644 --- a/src/state/CallViewModel/localMember/LocalMember.test.ts +++ b/src/state/CallViewModel/localMember/LocalMember.test.ts @@ -254,10 +254,12 @@ describe("LocalMembership", () => { const connectionTransportAConnecting = { ...connectionTransportAConnected, state$: constant(ConnectionState.LivekitConnecting), + livekitRoom: mockLivekitRoom({}), } as unknown as Connection; const connectionTransportBConnected = { state$: constant(ConnectionState.LivekitConnected), transport: bTransport, + livekitRoom: mockLivekitRoom({}), } as unknown as Connection; it("recreates publisher if new connection is used and ENDS always unpublish and end tracks", async () => { @@ -477,13 +479,13 @@ describe("LocalMembership", () => { // ------- await flushPromises(); - expect(localMembership.localMemberState$.value).toStrictEqual({ - matrix: RTCMemberStatus.Connected, - media: { - tracks: TrackState.Creating, - connection: ConnectionState.LivekitConnected, - }, - }); + // expect(localMembership.localMemberState$.value).toStrictEqual({ + // matrix: RTCMemberStatus.Connected, + // media: { + // tracks: TrackState.Creating, + // connection: ConnectionState.LivekitConnected, + // }, + // }); createTrackResolver.resolve(); await flushPromises(); expect( @@ -498,7 +500,7 @@ describe("LocalMembership", () => { expect( // eslint-disable-next-line @typescript-eslint/no-explicit-any (localMembership.localMemberState$.value as any).media, - ).toStrictEqual(PublishState.Starting); + ).toStrictEqual(PublishState.Publishing); publishResolver.resolve(); await flushPromises(); diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 241cb633..21386fcd 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -74,6 +74,8 @@ export enum PublishState { Publishing = "publish_publishing", } +// TODO not sure how to map that correctly with the +// new publisher that does not manage tracks itself anymore export enum TrackState { /** The track is waiting for user input to create tracks (waiting to call `startTracks()`) */ WaitingForUser = "tracks_waiting_for_user", @@ -244,7 +246,7 @@ export const createLocalMembership$ = ({ if (error) { logger.error(`Failed to create local tracks:`, error); setMatrixError( - // TODO is it fatal? Do we need to create a new Specialized Error? + // TODO is it fatal? Do we need to create a new Specialized Error? new UnknownCallError(new Error(`Media device error: ${error}`)), ); } @@ -327,11 +329,11 @@ export const createLocalMembership$ = ({ // Based on `connectRequested$` we start publishing tracks. (once they are there!) scope.reconcile( - scope.behavior( - combineLatest([publisher$, joinAndPublishRequested$]), - ), + scope.behavior(combineLatest([publisher$, joinAndPublishRequested$])), async ([publisher, shouldJoinAndPublish]) => { - if (shouldJoinAndPublish) { + // Get the current publishing state to avoid redundant calls. + const isPublishing = publisher?.shouldPublish === true; + if (shouldJoinAndPublish && !isPublishing) { try { await publisher?.startPublishing(); } catch (error) { @@ -339,7 +341,7 @@ export const createLocalMembership$ = ({ error instanceof Error ? error.message : String(error); setPublishError(new FailToStartLivekitConnection(message)); } - } else { + } else if (isPublishing) { try { await publisher?.stopPublishing(); } catch (error) { @@ -409,7 +411,9 @@ export const createLocalMembership$ = ({ // let trackState: TrackState = TrackState.WaitingForUser; // if (hasTracks && shouldStartTracks) trackState = TrackState.Ready; // if (!hasTracks && shouldStartTracks) trackState = TrackState.Creating; - const trackState: TrackState = shouldStartTracks ? TrackState.Ready : TrackState.WaitingForUser; + const trackState: TrackState = shouldStartTracks + ? TrackState.Ready + : TrackState.WaitingForUser; if ( localConnectionState !== ConnectionState.LivekitConnected || diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 3ab3d48c..3cc96bc2 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -22,7 +22,7 @@ import { constant } from "../../Behavior"; import { flushPromises, mockLivekitRoom, - mockMediaDevices + mockMediaDevices, } from "../../../utils/test"; import { Publisher } from "./Publisher"; import { type Connection } from "../remoteMembers/Connection"; From 8f2055b4f473313cf5ab8e2f16952dc256128633 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 14:46:13 +0100 Subject: [PATCH 30/43] eslint fix --- src/state/CallViewModel/localMember/LocalMember.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index 21386fcd..b75bf522 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -303,7 +303,7 @@ export const createLocalMembership$ = ({ // Clean-up callback return Promise.resolve(async (): Promise => { await publisher.stopPublishing(); - publisher.stopTracks(); + await publisher.stopTracks(); }); } }); From 190cdfcb6030edbe2b2cc08ae8765908a380eeb5 Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 12 Dec 2025 17:03:16 +0100 Subject: [PATCH 31/43] comment now dead state variant --- src/state/CallViewModel/localMember/LocalMember.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index b75bf522..daadbe7c 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -68,8 +68,8 @@ export enum TransportState { export enum PublishState { WaitingForUser = "publish_waiting_for_user", - /** Implies lk connection is connected */ - Starting = "publish_start_publishing", + // /** Implies lk connection is connected */ + // Starting = "publish_start_publishing", /** Implies lk connection is connected */ Publishing = "publish_publishing", } @@ -79,8 +79,8 @@ export enum PublishState { export enum TrackState { /** The track is waiting for user input to create tracks (waiting to call `startTracks()`) */ WaitingForUser = "tracks_waiting_for_user", - /** Implies lk connection is connected */ - Creating = "tracks_creating", + // /** Implies lk connection is connected */ + // Creating = "tracks_creating", /** Implies lk connection is connected */ Ready = "tracks_ready", } From 00d4b8e985db7b9546d584610a777ef6a4a0e9f5 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 12:52:23 -0500 Subject: [PATCH 32/43] Use a more suitable filter operator to compute local member --- src/state/CallViewModel/CallViewModel.ts | 27 +++++++++--------- src/utils/observable.test.ts | 35 ++++++++++++++++++++++-- src/utils/observable.ts | 23 ++++++++++++++++ 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index f4f81776..f54fc9f5 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -52,7 +52,12 @@ import { ScreenShareViewModel, type UserMediaViewModel, } from "../MediaViewModel"; -import { accumulate, generateItems, pauseWhen } from "../../utils/observable"; +import { + accumulate, + filterBehavior, + generateItems, + pauseWhen, +} from "../../utils/observable"; import { duplicateTiles, MatrixRTCMode, @@ -505,16 +510,13 @@ export function createCallViewModel$( ), ); - const localMatrixLivekitMember$ = - scope.behavior | null>( + const localMatrixLivekitMember$: Behavior | null> = + scope.behavior( localRtcMembership$.pipe( - generateItems( - // Generate a local member when membership is non-null - function* (membership) { - if (membership !== null) - yield { keys: ["local"], data: membership }; - }, - (_scope, membership$) => ({ + filterBehavior((membership) => membership !== null), + map((membership$) => { + if (membership$ === null) return null; + return { membership$, participant: { type: "local" as const, @@ -522,9 +524,8 @@ export function createCallViewModel$( }, connection$: localMembership.connection$, userId, - }), - ), - map(([localMember]) => localMember ?? null), + }; + }), ), ); diff --git a/src/utils/observable.test.ts b/src/utils/observable.test.ts index d1034e7b..be677367 100644 --- a/src/utils/observable.test.ts +++ b/src/utils/observable.test.ts @@ -5,11 +5,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { test } from "vitest"; -import { Subject } from "rxjs"; +import { expect, test } from "vitest"; +import { type Observable, of, Subject, switchMap } from "rxjs"; import { withTestScheduler } from "./test"; -import { generateItems, pauseWhen } from "./observable"; +import { filterBehavior, generateItems, pauseWhen } from "./observable"; +import { type Behavior } from "../state/Behavior"; test("pauseWhen", () => { withTestScheduler(({ behavior, expectObservable }) => { @@ -72,3 +73,31 @@ test("generateItems", () => { expectObservable(scope4$).toBe(scope4Marbles); }); }); + +test("filterBehavior", () => { + withTestScheduler(({ behavior, expectObservable }) => { + // Filtering the input should segment it into 2 modes of non-null behavior. + const inputMarbles = " abcxabx"; + const filteredMarbles = "a--xa-x"; + + const input$ = behavior(inputMarbles, { + a: "a", + b: "b", + c: "c", + x: null, + }); + const filtered$: Observable | null> = input$.pipe( + filterBehavior((value) => typeof value === "string"), + ); + + expectObservable(filtered$).toBe(filteredMarbles, { + a: expect.any(Object), + x: null, + }); + expectObservable( + filtered$.pipe( + switchMap((value$) => (value$ === null ? of(null) : value$)), + ), + ).toBe(inputMarbles, { a: "a", b: "b", c: "c", x: null }); + }); +}); diff --git a/src/utils/observable.ts b/src/utils/observable.ts index 053921cd..a6dafea3 100644 --- a/src/utils/observable.ts +++ b/src/utils/observable.ts @@ -22,6 +22,7 @@ import { withLatestFrom, BehaviorSubject, type OperatorFunction, + distinctUntilChanged, } from "rxjs"; import { type Behavior } from "../state/Behavior"; @@ -185,6 +186,28 @@ export function generateItemsWithEpoch< ); } +/** + * Segments a behavior into periods during which its value matches the filter + * (outputting a behavior with a narrowed type) and periods during which it does + * not match (outputting null). + */ +export function filterBehavior( + predicate: (value: T) => value is S, +): OperatorFunction | null> { + return (input$) => + input$.pipe( + scan | null>((acc$, input) => { + if (predicate(input)) { + const output$ = acc$ ?? new BehaviorSubject(input); + output$.next(input); + return output$; + } + return null; + }, null), + distinctUntilChanged(), + ); +} + function generateItemsInternal< Input, Keys extends [unknown, ...unknown[]], From 8a18e70e20561dc6e9e7e0464c985509adc70076 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 13:14:45 -0500 Subject: [PATCH 33/43] Split MatrixLivekitMembers more verbosely into two types --- src/state/CallViewModel/CallViewModel.ts | 14 +++--- .../MatrixLivekitMembers.test.ts | 12 ++--- .../remoteMembers/MatrixLivekitMembers.ts | 46 ++++++++++++------- .../remoteMembers/integration.test.ts | 8 ++-- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index f54fc9f5..55a1368e 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -116,7 +116,7 @@ import { createConnectionManager$ } from "./remoteMembers/ConnectionManager.ts"; import { createMatrixLivekitMembers$, type TaggedParticipant, - type MatrixLivekitMember, + type LocalMatrixLivekitMember, } from "./remoteMembers/MatrixLivekitMembers.ts"; import { type AutoLeaveReason, @@ -510,7 +510,7 @@ export function createCallViewModel$( ), ); - const localMatrixLivekitMember$: Behavior | null> = + const localMatrixLivekitMember$: Behavior = scope.behavior( localRtcMembership$.pipe( filterBehavior((membership) => membership !== null), @@ -682,10 +682,8 @@ export function createCallViewModel$( let localParticipantId: string | undefined = undefined; // add local member if available if (localMatrixLivekitMember) { - const { userId, connection$, membership$ } = + const { userId, participant, connection$, membership$ } = localMatrixLivekitMember; - const participant: TaggedParticipant = - localMatrixLivekitMember.participant; // Widen the type localParticipantId = `${userId}:${membership$.value.deviceId}`; // should be membership$.value.membershipID which is not optional // const participantId = membership$.value.membershipID; if (localParticipantId) { @@ -695,7 +693,7 @@ export function createCallViewModel$( dup, localParticipantId, userId, - participant, + participant satisfies TaggedParticipant as TaggedParticipant, // Widen the type safely connection$, ], data: undefined, @@ -727,7 +725,7 @@ export function createCallViewModel$( dup, participantId, userId, - participant$, + participant, connection$, ) => { const livekitRoom$ = scope.behavior( @@ -746,7 +744,7 @@ export function createCallViewModel$( scope, `${participantId}:${dup}`, userId, - participant$, + participant, options.encryptionSystem, livekitRoom$, focusUrl$, diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts index 195078e0..77c00015 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts @@ -15,7 +15,7 @@ import { combineLatest, map, type Observable } from "rxjs"; import { type IConnectionManager } from "./ConnectionManager.ts"; import { - type MatrixLivekitMember, + type RemoteMatrixLivekitMember, createMatrixLivekitMembers$, } from "./MatrixLivekitMembers.ts"; import { @@ -100,7 +100,7 @@ test("should signal participant not yet connected to livekit", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { + a: expect.toSatisfy((data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -180,7 +180,7 @@ test("should signal participant on a connection that is publishing", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { + a: expect.toSatisfy((data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -231,7 +231,7 @@ test("should signal participant on a connection that is not publishing", () => { }); expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe("a", { - a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { + a: expect.toSatisfy((data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -296,7 +296,7 @@ describe("Publication edge case", () => { expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe( "a", { - a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { + a: expect.toSatisfy((data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(2); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, @@ -362,7 +362,7 @@ describe("Publication edge case", () => { expectObservable(matrixLivekitMember$.pipe(map((e) => e.value))).toBe( "a", { - a: expect.toSatisfy((data: MatrixLivekitMember<"remote">[]) => { + a: expect.toSatisfy((data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(2); expectObservable(data[0].membership$).toBe("a", { a: bobMembership, diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts index 67146fac..4cca0d5b 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts @@ -21,30 +21,44 @@ import { generateItemsWithEpoch } from "../../../utils/observable"; const logger = rootLogger.getChild("[MatrixLivekitMembers]"); -/** - * A dynamic participant value with a static tag to tell what kind of - * participant it can be (local vs. remote). - */ +interface LocalTaggedParticipant { + type: "local"; + value$: Behavior; +} +interface RemoteTaggedParticipant { + type: "remote"; + value$: Behavior; +} export type TaggedParticipant = - | { type: "local"; value$: Behavior } - | { type: "remote"; value$: Behavior }; + | LocalTaggedParticipant + | RemoteTaggedParticipant; -/** - * Represents a Matrix call member and their associated LiveKit participation. - * `livekitParticipant` can be undefined if the member is not yet connected to the livekit room - * or if it has no livekit transport at all. - */ -export interface MatrixLivekitMember< - ParticipantType extends TaggedParticipant["type"], -> { +interface MatrixLivekitMember { membership$: Behavior; - participant: TaggedParticipant & { type: ParticipantType }; connection$: Behavior; // participantId: string; We do not want a participantId here since it will be generated by the jwt // TODO decide if we can also drop the userId. Its in the matrix membership anyways. userId: string; } +/** + * Represents the local Matrix call member and their associated LiveKit participation. + * `livekitParticipant` can be null if the member is not yet connected to the livekit room + * or if it has no livekit transport at all. + */ +export interface LocalMatrixLivekitMember extends MatrixLivekitMember { + participant: LocalTaggedParticipant; +} + +/** + * Represents a remote Matrix call member and their associated LiveKit participation. + * `livekitParticipant` can be null if the member is not yet connected to the livekit room + * or if it has no livekit transport at all. + */ +export interface RemoteMatrixLivekitMember extends MatrixLivekitMember { + participant: RemoteTaggedParticipant; +} + interface Props { scope: ObservableScope; membershipsWithTransport$: Behavior< @@ -66,7 +80,7 @@ export function createMatrixLivekitMembers$({ scope, membershipsWithTransport$, connectionManager, -}: Props): Behavior[]>> { +}: Props): Behavior> { /** * Stream of all the call members and their associated livekit data (if available). */ diff --git a/src/state/CallViewModel/remoteMembers/integration.test.ts b/src/state/CallViewModel/remoteMembers/integration.test.ts index 34b62dad..2c3591a5 100644 --- a/src/state/CallViewModel/remoteMembers/integration.test.ts +++ b/src/state/CallViewModel/remoteMembers/integration.test.ts @@ -29,7 +29,7 @@ import { type ProcessorState } from "../../../livekit/TrackProcessorContext.tsx" import { areLivekitTransportsEqual, createMatrixLivekitMembers$, - type MatrixLivekitMember, + type RemoteMatrixLivekitMember, } from "./MatrixLivekitMembers.ts"; import { createConnectionManager$ } from "./ConnectionManager.ts"; import { membershipsAndTransports$ } from "../../SessionBehaviors.ts"; @@ -132,7 +132,7 @@ test("bob, carl, then bob joining no tracks yet", () => { }); expectObservable(matrixLivekitItems$).toBe(vMarble, { - a: expect.toSatisfy((e: Epoch[]>) => { + a: expect.toSatisfy((e: Epoch) => { const items = e.value; expect(items.length).toBe(1); const item = items[0]!; @@ -152,7 +152,7 @@ test("bob, carl, then bob joining no tracks yet", () => { }); return true; }), - b: expect.toSatisfy((e: Epoch[]>) => { + b: expect.toSatisfy((e: Epoch) => { const items = e.value; expect(items.length).toBe(2); @@ -189,7 +189,7 @@ test("bob, carl, then bob joining no tracks yet", () => { } return true; }), - c: expect.toSatisfy((e: Epoch[]>) => { + c: expect.toSatisfy((e: Epoch) => { const items = e.value; expect(items.length).toBe(3); From 15a12b2d9c0eda53fd60a0be424b992965d6ba5d Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 14:22:20 -0500 Subject: [PATCH 34/43] Make layout tests more concise --- src/state/CallViewModel/LayoutSwitch.test.ts | 267 ++++++------------- 1 file changed, 87 insertions(+), 180 deletions(-) diff --git a/src/state/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index ae5a3896..a0d2d8c3 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -5,198 +5,105 @@ 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 } from "vitest"; -import { firstValueFrom, of } from "rxjs"; +import { describe, test } from "vitest"; import { createLayoutModeSwitch } from "./LayoutSwitch"; -import { ObservableScope } from "../ObservableScope"; -import { constant } from "../Behavior"; -import { withTestScheduler } from "../../utils/test"; +import { testScope, withTestScheduler } from "../../utils/test"; -let scope: ObservableScope; -beforeEach(() => { - scope = new ObservableScope(); -}); -afterEach(() => { - scope.end(); -}); - -describe("Default mode", () => { - test("Should be in grid layout by default", async () => { - const { gridMode$ } = createLayoutModeSwitch( - scope, - constant("normal"), - of(false), - ); - - const mode = await firstValueFrom(gridMode$); - expect(mode).toBe("grid"); - }); - - test("Should switch to spotlight mode when window mode is flat", async () => { - const { gridMode$ } = createLayoutModeSwitch( - scope, - constant("flat"), - of(false), - ); - - const mode = await firstValueFrom(gridMode$); - expect(mode).toBe("spotlight"); - }); -}); - -test("Should allow switching modes manually", () => { - withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { +function testLayoutSwitch({ + windowMode = "n", + hasScreenShares = "n", + userSelection = "", + expectedGridMode, +}: { + windowMode?: string; + hasScreenShares?: string; + userSelection?: string; + expectedGridMode: string; +}): void { + withTestScheduler(({ behavior, schedule, expectObservable }) => { const { gridMode$, setGridMode } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold("f", { f: false, t: true }), + testScope(), + behavior(windowMode, { n: "normal", N: "narrow", f: "flat" }), + behavior(hasScreenShares, { y: true, n: false }), ); - - schedule("--sgs", { - s: () => setGridMode("spotlight"), - g: () => setGridMode("grid"), - }); - - expectObservable(gridMode$).toBe("g-sgs", { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("Should switch to spotlight mode when there is a remote screen share", () => { - withTestScheduler(({ cold, behavior, expectObservable }): void => { - const shareMarble = "f--t"; - const gridsMarble = "g--s"; - const { gridMode$ } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold(shareMarble, { f: false, t: true }), - ); - - expectObservable(gridMode$).toBe(gridsMarble, { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("Can manually force grid when there is a screenshare", () => { - withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { - const { gridMode$, setGridMode } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold("-ft", { f: false, t: true }), - ); - - schedule("---g", { - g: () => setGridMode("grid"), - }); - - expectObservable(gridMode$).toBe("ggsg", { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("Should auto-switch after manually selected grid", () => { - withTestScheduler(({ cold, behavior, expectObservable, schedule }): void => { - const { gridMode$, setGridMode } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - // Two screenshares will happen in sequence - cold("-ft-ft", { f: false, t: true }), - ); - - // There was a screen-share that forced spotlight, then - // the user manually switch back to grid - schedule("---g", { - g: () => setGridMode("grid"), - }); - - // If we did want to respect manual selection, the expectation would be: - // const expectation = "ggsg"; - const expectation = "ggsg-s"; - - expectObservable(gridMode$).toBe(expectation, { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("Should switch back to grid mode when the remote screen share ends", () => { - withTestScheduler(({ cold, behavior, expectObservable }): void => { - const shareMarble = "f--t--f-"; - const gridsMarble = "g--s--g-"; - const { gridMode$ } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold(shareMarble, { f: false, t: true }), - ); - - expectObservable(gridMode$).toBe(gridsMarble, { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("can auto-switch to spotlight again after first screen share ends", () => { - withTestScheduler(({ cold, behavior, expectObservable }): void => { - const shareMarble = "ftft"; - const gridsMarble = "gsgs"; - const { gridMode$ } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold(shareMarble, { f: false, t: true }), - ); - - expectObservable(gridMode$).toBe(gridsMarble, { - g: "grid", - s: "spotlight", - }); - }); -}); - -test("can switch manually to grid after screen share while manually in spotlight", () => { - withTestScheduler(({ cold, behavior, schedule, expectObservable }): void => { - // Initially, no one is sharing. Then the user manually switches to - // spotlight. After a screen share starts, the user manually switches to - // grid. - const shareMarbles = " f-t-"; - const setModeMarbles = "-s-g"; - const expectation = " gs-g"; - const { gridMode$, setGridMode } = createLayoutModeSwitch( - scope, - behavior("n", { n: "normal" }), - cold(shareMarbles, { f: false, t: true }), - ); - schedule(setModeMarbles, { + schedule(userSelection, { g: () => setGridMode("grid"), s: () => setGridMode("spotlight"), }); - - expectObservable(gridMode$).toBe(expectation, { + expectObservable(gridMode$).toBe(expectedGridMode, { g: "grid", s: "spotlight", }); }); +} + +describe("default mode", () => { + test("uses grid layout by default", () => + testLayoutSwitch({ + expectedGridMode: "g", + })); + + test("uses spotlight mode when window mode is flat", () => + testLayoutSwitch({ + windowMode: " f", + expectedGridMode: "s", + })); }); -test("Should auto-switch to spotlight when in flat window mode", () => { - withTestScheduler(({ cold, behavior, expectObservable }): void => { - const { gridMode$ } = createLayoutModeSwitch( - scope, - behavior("naf", { n: "normal", a: "narrow", f: "flat" }), - cold("f", { f: false, t: true }), - ); +test("allows switching modes manually", () => + testLayoutSwitch({ + userSelection: " --sgs", + expectedGridMode: "g-sgs", + })); - expectObservable(gridMode$).toBe("g-s-", { - g: "grid", - s: "spotlight", - }); - }); -}); +test("switches to spotlight mode when there is a remote screen share", () => + testLayoutSwitch({ + hasScreenShares: " n--y", + expectedGridMode: "g--s", + })); + +test("can manually switch to grid when there is a screenshare", () => + testLayoutSwitch({ + hasScreenShares: " n-y", + userSelection: " ---g", + expectedGridMode: "g-sg", + })); + +test("auto-switches after manually selecting grid", () => + testLayoutSwitch({ + // Two screenshares will happen in sequence. There is a screen share that + // forces spotlight, then the user manually switches back to grid. + hasScreenShares: " n-y-ny", + userSelection: " ---g", + expectedGridMode: "g-sg-s", + // If we did want to respect manual selection, the expectation would be: g-sg + })); + +test("switches back to grid mode when the remote screen share ends", () => + testLayoutSwitch({ + hasScreenShares: " n--y--n", + expectedGridMode: "g--s--g", + })); + +test("auto-switches to spotlight again after first screen share ends", () => + testLayoutSwitch({ + hasScreenShares: " nyny", + expectedGridMode: "gsgs", + })); + +test("switches manually to grid after screen share while manually in spotlight", () => + testLayoutSwitch({ + // Initially, no one is sharing. Then the user manually switches to spotlight. + // After a screen share starts, the user manually switches to grid. + hasScreenShares: " n-y", + userSelection: " -s-g", + expectedGridMode: "gs-g", + })); + +test("auto-switches to spotlight when in flat window mode", () => + testLayoutSwitch({ + // First normal, then narrow, then flat. + windowMode: " nNf", + expectedGridMode: "g-s", + })); From 53cc79f7387d5cce44e5b138fbdf7b5668196410 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 14:33:00 -0500 Subject: [PATCH 35/43] Allow user to switch layouts while phone is in landscape This fixes a regression on the development branch: the layout switcher would not respond to input while the window mode is 'flat' (i.e. while a mobile phone is in landscape orientation). See https://github.com/element-hq/element-call/pull/3605#discussion_r2586226422 for more context. I was having a little trouble interpreting the emergent behavior of the layout switching code, so I refactored it in the process into a form that I think is a more direct description of the behavior we want (while not making it as terse as my original implementation). --- src/state/CallViewModel/CallViewModel.ts | 9 +- src/state/CallViewModel/LayoutSwitch.test.ts | 23 +++ src/state/CallViewModel/LayoutSwitch.ts | 143 +++++++------------ 3 files changed, 81 insertions(+), 94 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index aac88a3b..e2e3924b 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -946,11 +946,12 @@ export function createCallViewModel$( ), ); - const hasRemoteScreenShares$: Observable = spotlight$.pipe( - map((spotlight) => - spotlight.some((vm) => !vm.local && vm instanceof ScreenShareViewModel), + const hasRemoteScreenShares$ = scope.behavior( + spotlight$.pipe( + map((spotlight) => + spotlight.some((vm) => !vm.local && vm instanceof ScreenShareViewModel), + ), ), - distinctUntilChanged(), ); const pipEnabled$ = scope.behavior(setPipEnabled$, false); diff --git a/src/state/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index a0d2d8c3..0d184017 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -107,3 +107,26 @@ test("auto-switches to spotlight when in flat window mode", () => windowMode: " nNf", expectedGridMode: "g-s", })); + +test("allows switching modes manually when in flat window mode", () => + testLayoutSwitch({ + // Window becomes flat, then user switches to grid and back. + // Finally the window returns to a normal shape. + windowMode: " nf--n", + userSelection: " --gs", + expectedGridMode: "gsgsg", + })); + +test("stays in spotlight while there are screen shares even when window mode changes", () => + testLayoutSwitch({ + windowMode: " nfn", + hasScreenShares: " y", + expectedGridMode: "s", + })); + +test("ignores end of screen share until window mode returns to normal", () => + testLayoutSwitch({ + windowMode: " nf-n", + hasScreenShares: " y-n", + expectedGridMode: "s--g", + })); diff --git a/src/state/CallViewModel/LayoutSwitch.ts b/src/state/CallViewModel/LayoutSwitch.ts index 3ad93204..97a4ee6f 100644 --- a/src/state/CallViewModel/LayoutSwitch.ts +++ b/src/state/CallViewModel/LayoutSwitch.ts @@ -6,122 +6,85 @@ Please see LICENSE in the repository root for full details. */ import { - BehaviorSubject, combineLatest, map, - type Observable, - scan, + Subject, + startWith, + skipWhile, + switchMap, } from "rxjs"; -import { logger } from "matrix-js-sdk/lib/logger"; import { type GridMode, type WindowMode } from "./CallViewModel.ts"; -import { type Behavior } from "../Behavior.ts"; +import { constant, type Behavior } from "../Behavior.ts"; import { type ObservableScope } from "../ObservableScope.ts"; /** * Creates a layout mode switch that allows switching between grid and spotlight modes. - * The actual layout mode can be overridden to spotlight mode if there is a remote screen share active - * or if the window mode is flat. + * The actual layout mode might switch automatically to spotlight if there is a + * remote screen share active or if the window mode is flat. * * @param scope - The observable scope to manage subscriptions. - * @param windowMode$ - The current window mode observable. - * @param hasRemoteScreenShares$ - An observable indicating if there are remote screen shares active. + * @param windowMode$ - The current window mode. + * @param hasRemoteScreenShares$ - A behavior indicating if there are remote screen shares active. */ export function createLayoutModeSwitch( scope: ObservableScope, windowMode$: Behavior, - hasRemoteScreenShares$: Observable, + hasRemoteScreenShares$: Behavior, ): { gridMode$: Behavior; setGridMode: (value: GridMode) => void; } { - const gridModeUserSelection$ = new BehaviorSubject("grid"); - + const userSelection$ = new Subject(); // Callback to set the grid mode desired by the user. // Notice that this is only a preference, the actual grid mode can be overridden // if there is a remote screen share active. - const setGridMode = (value: GridMode): void => { - gridModeUserSelection$.next(value); - }; + const setGridMode = (value: GridMode): void => userSelection$.next(value); + + /** + * The natural grid mode - the mode that the grid would prefer to be in, + * not accounting for the user's manual selections. + */ + const naturalGridMode$ = scope.behavior( + combineLatest( + [hasRemoteScreenShares$, windowMode$], + (hasRemoteScreenShares, windowMode) => + // When there are screen shares or the window is flat (as with a phone + // in landscape orientation), spotlight is a better experience. + // We want screen shares to be big and readable, and we want flipping + // your phone into landscape to be a quick way of maximising the + // spotlight tile. + hasRemoteScreenShares || windowMode === "flat" ? "spotlight" : "grid", + ), + ); + /** * The layout mode of the media tile grid. */ - const gridMode$ = - // If the user hasn't selected spotlight and somebody starts screen sharing, - // automatically switch to spotlight mode and reset when screen sharing ends - scope.behavior( - combineLatest([ - gridModeUserSelection$, - hasRemoteScreenShares$, - windowMode$, - ]).pipe( - // Scan to keep track if we have auto-switched already or not. - // To allow the user to override the auto-switch by selecting grid mode again. - scan< - [GridMode, boolean, WindowMode], - { - mode: GridMode; - /** Remember if the change was user driven or not */ - hasAutoSwitched: boolean; - /** To know if it is new screen share or an already handled */ - hasScreenShares: boolean; - } - >( - (prev, [userSelection, hasScreenShares, windowMode]) => { - const isFlatMode = windowMode === "flat"; + const gridMode$ = scope.behavior( + // Whenever the user makes a selection, we enter a new mode of behavior: + userSelection$.pipe( + map((selection) => { + if (selection === "grid") + // The user has selected grid mode. Start by respecting their choice, + // but then follow the natural mode again as soon as it matches. + return naturalGridMode$.pipe( + skipWhile((naturalMode) => naturalMode !== selection), + startWith(selection), + ); - // Always force spotlight in flat mode, grid layout is not supported - // in that mode. - // TODO: strange that we do that for flat mode but not for other modes? - // TODO: Why is this not handled in layoutMedia$ like other window modes? - if (isFlatMode) { - logger.debug(`Forcing spotlight mode, windowMode=${windowMode}`); - return { - mode: "spotlight", - hasAutoSwitched: prev.hasAutoSwitched, - hasScreenShares, - }; - } - - // User explicitly chose spotlight. - // Respect that choice. - if (userSelection === "spotlight") { - return { - mode: "spotlight", - hasAutoSwitched: prev.hasAutoSwitched, - hasScreenShares, - }; - } - - // User has chosen grid mode. If a screen share starts, we will - // auto-switch to spotlight mode for better experience. - // But we only do it once, if the user switches back to grid mode, - // we respect that choice until they explicitly change it again. - const isNewShare = hasScreenShares && !prev.hasScreenShares; - if (isNewShare && !prev.hasAutoSwitched) { - return { - mode: "spotlight", - hasAutoSwitched: true, - hasScreenShares: true, - }; - } - - // Respect user's grid choice - // XXX If we want to forbid switching automatically again after we can - // return hasAutoSwitched: acc.hasAutoSwitched here instead of setting to false. - return { - mode: "grid", - hasAutoSwitched: false, - hasScreenShares, - }; - }, - // initial value - { mode: "grid", hasAutoSwitched: false, hasScreenShares: false }, - ), - map(({ mode }) => mode), - ), - "grid", - ); + // The user has selected spotlight mode. If this matches the natural + // mode, then follow the natural mode going forward. + return selection === naturalGridMode$.value + ? naturalGridMode$ + : constant(selection); + }), + // Initially the mode of behavior is to just follow the natural grid mode. + startWith(naturalGridMode$), + // Switch between each mode of behavior. + switchMap((mode$) => mode$), + ), + ); return { gridMode$, From c7e9f1ce1449313adb2f3ef82661b8c70c222d95 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 15:09:46 -0500 Subject: [PATCH 36/43] Explicitly pass the MatrixRTC mode to CallViewModel --- src/room/InCallView.tsx | 2 ++ src/state/CallViewModel/CallViewModelTestUtils.ts | 2 +- src/utils/test-viewmodel.ts | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index add8154a..41582039 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -88,6 +88,7 @@ import { ReactionsOverlay } from "./ReactionsOverlay"; import { CallEventAudioRenderer } from "./CallEventAudioRenderer"; import { debugTileLayout as debugTileLayoutSetting, + matrixRTCMode as matrixRTCModeSetting, useSetting, } from "../settings/settings"; import { ReactionsReader } from "../reactions/ReactionsReader"; @@ -144,6 +145,7 @@ export const ActiveCall: FC = (props) => { encryptionSystem: props.e2eeSystem, autoLeaveWhenOthersLeft, waitForCallPickup: waitForCallPickup && sendNotificationType === "ring", + matrixRTCMode$: matrixRTCModeSetting.value$, }, reactionsReader.raisedHands$, reactionsReader.reactions$, diff --git a/src/state/CallViewModel/CallViewModelTestUtils.ts b/src/state/CallViewModel/CallViewModelTestUtils.ts index e9996a41..377c6771 100644 --- a/src/state/CallViewModel/CallViewModelTestUtils.ts +++ b/src/state/CallViewModel/CallViewModelTestUtils.ts @@ -105,6 +105,7 @@ export function withCallViewModel(mode: MatrixRTCMode) { options: CallViewModelOptions = { encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, autoLeaveWhenOthersLeft: false, + matrixRTCMode$: constant(mode), }, ): void => { let syncState = initialSyncState; @@ -184,7 +185,6 @@ export function withCallViewModel(mode: MatrixRTCMode) { }), connectionState$, windowSize$, - matrixRTCMode$: constant(mode), }, raisedHands$, reactions$, diff --git a/src/utils/test-viewmodel.ts b/src/utils/test-viewmodel.ts index 98c45d86..0745be72 100644 --- a/src/utils/test-viewmodel.ts +++ b/src/utils/test-viewmodel.ts @@ -37,6 +37,7 @@ import { import { aliceRtcMember, localRtcMember } from "./test-fixtures"; import { type RaisedHandInfo, type ReactionInfo } from "../reactions"; import { constant } from "../state/Behavior"; +import { MatrixRTCMode } from "../settings/settings"; mockConfig({ livekit: { livekit_service_url: "https://example.com" } }); @@ -162,6 +163,7 @@ export function getBasicCallViewModelEnvironment( setE2EEEnabled: async () => Promise.resolve(), }), connectionState$: constant(ConnectionState.Connected), + matrixRTCMode$: constant(MatrixRTCMode.Legacy), ...callViewModelOptions, }, handRaisedSubject$, From 87fbbb9a3f4410c9815733772fd52cae79f4fc8a Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 15:16:47 -0500 Subject: [PATCH 37/43] Make MatrixRTC mode a required input to CallViewModel --- src/state/CallViewModel/CallViewModel.ts | 11 ++++------- src/state/CallViewModel/CallViewModelTestUtils.ts | 11 +++++------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/state/CallViewModel/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index 9ebe025c..8f8b20cf 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -62,7 +62,6 @@ import { import { duplicateTiles, MatrixRTCMode, - matrixRTCMode as matrixRTCModeSetting, playReactionsSound, showReactions, } from "../../settings/settings"; @@ -156,8 +155,8 @@ export interface CallViewModelOptions { connectionState$?: Behavior; /** Optional behavior overriding the computed window size, mainly for testing purposes. */ windowSize$?: Behavior<{ width: number; height: number }>; - /** Optional behavior overriding the MatrixRTC mode, mainly for testing purposes. */ - matrixRTCMode$?: Behavior; + /** The version & compatibility mode of MatrixRTC that we should use. */ + matrixRTCMode$: Behavior; } // Do not play any sounds if the participant count has exceeded this @@ -408,15 +407,13 @@ export function createCallViewModel$( memberships$, ); - const matrixRTCMode$ = options.matrixRTCMode$ ?? matrixRTCModeSetting.value$; - const localTransport$ = createLocalTransport$({ scope: scope, memberships$: memberships$, client, roomId: matrixRoom.roomId, useOldestMember$: scope.behavior( - matrixRTCMode$.pipe(map((v) => v === MatrixRTCMode.Legacy)), + options.matrixRTCMode$.pipe(map((v) => v === MatrixRTCMode.Legacy)), ), }); @@ -468,7 +465,7 @@ export function createCallViewModel$( }); const connectOptions$ = scope.behavior( - matrixRTCMode$.pipe( + options.matrixRTCMode$.pipe( map((mode) => ({ encryptMedia: livekitKeyProvider !== undefined, // TODO. This might need to get called again on each change of matrixRTCMode... diff --git a/src/state/CallViewModel/CallViewModelTestUtils.ts b/src/state/CallViewModel/CallViewModelTestUtils.ts index 377c6771..b6f53275 100644 --- a/src/state/CallViewModel/CallViewModelTestUtils.ts +++ b/src/state/CallViewModel/CallViewModelTestUtils.ts @@ -102,11 +102,7 @@ export function withCallViewModel(mode: MatrixRTCMode) { }, setSyncState: (value: SyncState) => void, ) => void, - options: CallViewModelOptions = { - encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, - autoLeaveWhenOthersLeft: false, - matrixRTCMode$: constant(mode), - }, + options: Partial = {}, ): void => { let syncState = initialSyncState; const setSyncState = (value: SyncState): void => { @@ -176,7 +172,8 @@ export function withCallViewModel(mode: MatrixRTCMode) { mediaDevices, muteStates, { - ...options, + encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, + autoLeaveWhenOthersLeft: false, livekitRoomFactory: (): LivekitRoom => mockLivekitRoom({ localParticipant, @@ -185,6 +182,8 @@ export function withCallViewModel(mode: MatrixRTCMode) { }), connectionState$, windowSize$, + matrixRTCMode$: constant(mode), + ...options, }, raisedHands$, reactions$, From 92bcc52e87f3336c3d79aa41f82168fad695b1f7 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Dec 2025 12:55:52 -0500 Subject: [PATCH 38/43] Remove unused method The doc comment here was about to become stale, so let's just remove it. --- .../remoteMembers/ConnectionManager.ts | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts index 6c2d64e0..f316e801 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -6,10 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial Please see LICENSE in the repository root for full details. */ -import { - type LivekitTransport, - type ParticipantId, -} from "matrix-js-sdk/lib/matrixrtc"; +import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; import { combineLatest, map, of, switchMap, tap } from "rxjs"; import { type Logger } from "matrix-js-sdk/lib/logger"; import { type RemoteParticipant } from "livekit-client"; @@ -57,34 +54,20 @@ export class ConnectionManagerData { const key = transport.livekit_service_url + "|" + transport.livekit_alias; return this.store.get(key)?.[1] ?? []; } - /** - * Get all connections where the given participant is publishing. - * In theory, there could be several connections where the same participant is publishing but with - * only well behaving clients a participant should only be publishing on a single connection. - * @param participantId - */ - public getConnectionsForParticipant( - participantId: ParticipantId, - ): Connection[] { - const connections: Connection[] = []; - for (const [connection, participants] of this.store.values()) { - if (participants.some((p) => p.identity === participantId)) { - connections.push(connection); - } - } - return connections; - } } + interface Props { scope: ObservableScope; connectionFactory: ConnectionFactory; inputTransports$: Behavior>; logger: Logger; } + // TODO - write test for scopes (do we really need to bind scope) export interface IConnectionManager { connectionManagerData$: Behavior>; } + /** * Crete a `ConnectionManager` * @param scope the observable scope used by this object. From 2c54263b2f632806cc23300b9eb8c68abac49a86 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Dec 2025 15:09:40 -0500 Subject: [PATCH 39/43] Don't show 'waiting for media' on connected participants We would show 'waiting for media' on participants that were connected but had no published tracks, because we were filtering them out of the remote participants list on connections. I believe this was done in an attempt to limit our view to only the participants that have a matching MatrixRTC membership. But that's fully redundant to the "Matrix-LiveKit members" module, which actually has the right information to do this (the MatrixRTC memberships). --- .../remoteMembers/Connection.test.ts | 103 ++++++------------ .../CallViewModel/remoteMembers/Connection.ts | 31 ++---- .../remoteMembers/ConnectionManager.test.ts | 6 +- .../remoteMembers/ConnectionManager.ts | 2 +- 4 files changed, 46 insertions(+), 96 deletions(-) diff --git a/src/state/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 4318708e..30c934b9 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -39,6 +39,7 @@ import { ElementCallError, FailToGetOpenIdToken, } from "../../../utils/errors.ts"; +import { mockRemoteParticipant } from "../../../utils/test.ts"; let testScope: ObservableScope; @@ -376,46 +377,32 @@ describe("Start connection states", () => { }); }); -function fakeRemoteLivekitParticipant( - id: string, - publications: number = 1, -): RemoteParticipant { - return { - identity: id, - getTrackPublications: () => Array(publications), - } as unknown as RemoteParticipant; -} - -describe("Publishing participants observations", () => { - it("should emit the list of publishing participants", () => { +describe("remote participants", () => { + it("emits the list of remote participants", () => { setupTest(); const connection = setupRemoteConnection(); - const bobIsAPublisher = Promise.withResolvers(); - const danIsAPublisher = Promise.withResolvers(); - const observedPublishers: RemoteParticipant[][] = []; - const s = connection.remoteParticipantsWithTracks$.subscribe( - (publishers) => { - observedPublishers.push(publishers); - if (publishers.some((p) => p.identity === "@bob:example.org:DEV111")) { - bobIsAPublisher.resolve(); - } - if (publishers.some((p) => p.identity === "@dan:example.org:DEV333")) { - danIsAPublisher.resolve(); - } - }, - ); + const observedParticipants: RemoteParticipant[][] = []; + const s = connection.remoteParticipants$.subscribe((participants) => { + observedParticipants.push(participants); + }); onTestFinished(() => s.unsubscribe()); // The remoteParticipants$ observable is derived from the current members of the // livekitRoom and the rtc membership in order to publish the members that are publishing // on this connection. - let participants: RemoteParticipant[] = [ - fakeRemoteLivekitParticipant("@alice:example.org:DEV000", 0), - fakeRemoteLivekitParticipant("@bob:example.org:DEV111", 0), - fakeRemoteLivekitParticipant("@carol:example.org:DEV222", 0), - fakeRemoteLivekitParticipant("@dan:example.org:DEV333", 0), + const participants: RemoteParticipant[] = [ + mockRemoteParticipant({ identity: "@alice:example.org:DEV000" }), + mockRemoteParticipant({ identity: "@bob:example.org:DEV111" }), + mockRemoteParticipant({ identity: "@carol:example.org:DEV222" }), + // Mock Dan to have no published tracks. We want him to still show show up + // in the participants list. + mockRemoteParticipant({ + identity: "@dan:example.org:DEV333", + getTrackPublication: () => undefined, + getTrackPublications: () => [], + }), ]; // Let's simulate 3 members on the livekitRoom @@ -427,21 +414,8 @@ describe("Publishing participants observations", () => { fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, p), ); - // At this point there should be no publishers - expect(observedPublishers.pop()!.length).toEqual(0); - - participants = [ - fakeRemoteLivekitParticipant("@alice:example.org:DEV000", 1), - fakeRemoteLivekitParticipant("@bob:example.org:DEV111", 1), - fakeRemoteLivekitParticipant("@carol:example.org:DEV222", 1), - fakeRemoteLivekitParticipant("@dan:example.org:DEV333", 2), - ]; - participants.forEach((p) => - fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, p), - ); - - // At this point there should be no publishers - expect(observedPublishers.pop()!.length).toEqual(4); + // All remote participants should be present + expect(observedParticipants.pop()!.length).toEqual(4); }); it("should be scoped to parent scope", (): void => { @@ -449,16 +423,14 @@ describe("Publishing participants observations", () => { const connection = setupRemoteConnection(); - let observedPublishers: RemoteParticipant[][] = []; - const s = connection.remoteParticipantsWithTracks$.subscribe( - (publishers) => { - observedPublishers.push(publishers); - }, - ); + let observedParticipants: RemoteParticipant[][] = []; + const s = connection.remoteParticipants$.subscribe((participants) => { + observedParticipants.push(participants); + }); onTestFinished(() => s.unsubscribe()); let participants: RemoteParticipant[] = [ - fakeRemoteLivekitParticipant("@bob:example.org:DEV111", 0), + mockRemoteParticipant({ identity: "@bob:example.org:DEV111" }), ]; // Let's simulate 3 members on the livekitRoom @@ -470,35 +442,26 @@ describe("Publishing participants observations", () => { fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, participant); } - // At this point there should be no publishers - expect(observedPublishers.pop()!.length).toEqual(0); - - participants = [fakeRemoteLivekitParticipant("@bob:example.org:DEV111", 1)]; - - for (const participant of participants) { - fakeLivekitRoom.emit(RoomEvent.ParticipantConnected, participant); - } - - // We should have bob has a publisher now - const publishers = observedPublishers.pop(); - expect(publishers?.length).toEqual(1); - expect(publishers?.[0]?.identity).toEqual("@bob:example.org:DEV111"); + // We should have bob as a participant now + const ps = observedParticipants.pop(); + expect(ps?.length).toEqual(1); + expect(ps?.[0]?.identity).toEqual("@bob:example.org:DEV111"); // end the parent scope testScope.end(); - observedPublishers = []; + observedParticipants = []; - // SHOULD NOT emit any more publishers as the scope is ended + // SHOULD NOT emit any more participants as the scope is ended participants = participants.filter( (p) => p.identity !== "@bob:example.org:DEV111", ); fakeLivekitRoom.emit( RoomEvent.ParticipantDisconnected, - fakeRemoteLivekitParticipant("@bob:example.org:DEV111"), + mockRemoteParticipant({ identity: "@bob:example.org:DEV111" }), ); - expect(observedPublishers.length).toEqual(0); + expect(observedParticipants.length).toEqual(0); }); }); diff --git a/src/state/CallViewModel/remoteMembers/Connection.ts b/src/state/CallViewModel/remoteMembers/Connection.ts index 3d685c8a..05d0ec9e 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -14,7 +14,6 @@ import { ConnectionError, type Room as LivekitRoom, type RemoteParticipant, - RoomEvent, } from "livekit-client"; import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; import { BehaviorSubject, map } from "rxjs"; @@ -96,11 +95,13 @@ export class Connection { private scope: ObservableScope; /** - * An observable of the participants that are publishing on this connection. (Excluding our local participant) - * This is derived from `participantsIncludingSubscribers$` and `remoteTransports$`. - * It filters the participants to only those that are associated with a membership that claims to publish on this connection. + * The remote LiveKit participants that are visible on this connection. + * + * Note that this may include participants that are connected only to + * subscribe, or publishers that are otherwise unattested in MatrixRTC state. + * It is therefore more low-level than what should be presented to the user. */ - public readonly remoteParticipantsWithTracks$: Behavior; + public readonly remoteParticipants$: Behavior; /** * Whether the connection has been stopped. @@ -231,23 +232,9 @@ export class Connection { this.transport = transport; this.client = client; - // REMOTE participants with track!!! - // this.remoteParticipantsWithTracks$ - this.remoteParticipantsWithTracks$ = scope.behavior( - // only tracks remote participants - connectedParticipantsObserver(this.livekitRoom, { - additionalRoomEvents: [ - RoomEvent.TrackPublished, - RoomEvent.TrackUnpublished, - ], - }).pipe( - map((participants) => { - return participants.filter( - (participant) => participant.getTrackPublications().length > 0, - ); - }), - ), - [], + this.remoteParticipants$ = scope.behavior( + // Only tracks remote participants + connectedParticipantsObserver(this.livekitRoom), ); scope.onEnd(() => { diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts index da1da06f..70bfb4de 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.test.ts @@ -52,7 +52,7 @@ beforeEach(() => { (transport: LivekitTransport, scope: ObservableScope) => { const mockConnection = { transport, - remoteParticipantsWithTracks$: new BehaviorSubject([]), + remoteParticipants$: new BehaviorSubject([]), } as unknown as Connection; vi.mocked(mockConnection).start = vi.fn(); vi.mocked(mockConnection).stop = vi.fn(); @@ -200,7 +200,7 @@ describe("connections$ stream", () => { }); describe("connectionManagerData$ stream", () => { - // Used in test to control fake connections' remoteParticipantsWithTracks$ streams + // Used in test to control fake connections' remoteParticipants$ streams let fakeRemoteParticipantsStreams: Map>; function keyForTransport(transport: LivekitTransport): string { @@ -229,7 +229,7 @@ describe("connectionManagerData$ stream", () => { >([]); const mockConnection = { transport, - remoteParticipantsWithTracks$: getRemoteParticipantsFor(transport), + remoteParticipants$: getRemoteParticipantsFor(transport), } as unknown as Connection; vi.mocked(mockConnection).start = vi.fn(); vi.mocked(mockConnection).stop = vi.fn(); diff --git a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts index f316e801..8db62236 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -152,7 +152,7 @@ export function createConnectionManager$({ // Map the connections to list of {connection, participants}[] const listOfConnectionsWithRemoteParticipants = connections.value.map( (connection) => { - return connection.remoteParticipantsWithTracks$.pipe( + return connection.remoteParticipants$.pipe( map((participants) => ({ connection, participants, From 93ab3ba1ffdbb637e42ed5ab0df83d6ee0924108 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Dec 2025 17:16:38 -0500 Subject: [PATCH 40/43] Compute the 'waiting for media' state less implicitly On second glance, the way that we determined a media tile to be 'waiting for media' was too implicit for my taste. It would appear on a surface reading to depend on whether a participant was currently publishing any video. But in reality, the 'video' object was always defined as long as a LiveKit participant existed, so in reality it depended on just the participant. We should show this relationship more explicitly by moving the computation into the view model, where it can depend on the participant directly. --- src/state/MediaViewModel.test.ts | 31 +++++++++++++++++++++++++-- src/state/MediaViewModel.ts | 10 ++++++++- src/tile/GridTile.test.tsx | 10 ++++++--- src/tile/GridTile.tsx | 6 +++++- src/tile/MediaView.test.tsx | 36 ++++++++++++++++++-------------- src/tile/MediaView.tsx | 6 +++--- src/tile/SpotlightTile.test.tsx | 3 ++- src/tile/SpotlightTile.tsx | 18 +++++++++++++++- src/utils/test.ts | 20 +++++++++++------- 9 files changed, 104 insertions(+), 36 deletions(-) diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 61fa2d8c..2ca14d19 100644 --- a/src/state/MediaViewModel.test.ts +++ b/src/state/MediaViewModel.test.ts @@ -20,6 +20,7 @@ import { createLocalMedia, createRemoteMedia, withTestScheduler, + mockRemoteParticipant, } from "../utils/test"; import { getValue } from "../utils/observable"; import { constant } from "./Behavior"; @@ -44,7 +45,11 @@ const rtcMembership = mockRtcMembership("@alice:example.org", "AAAA"); test("control a participant's volume", () => { const setVolumeSpy = vi.fn(); - const vm = createRemoteMedia(rtcMembership, {}, { setVolume: setVolumeSpy }); + const vm = createRemoteMedia( + rtcMembership, + {}, + mockRemoteParticipant({ setVolume: setVolumeSpy }), + ); withTestScheduler(({ expectObservable, schedule }) => { schedule("-ab---c---d|", { a() { @@ -88,7 +93,7 @@ test("control a participant's volume", () => { }); test("toggle fit/contain for a participant's video", () => { - const vm = createRemoteMedia(rtcMembership, {}, {}); + const vm = createRemoteMedia(rtcMembership, {}, mockRemoteParticipant({})); withTestScheduler(({ expectObservable, schedule }) => { schedule("-ab|", { a: () => vm.toggleFitContain(), @@ -199,3 +204,25 @@ test("switch cameras", async () => { }); expect(deviceId).toBe("front camera"); }); + +test("remote media is in waiting state when participant has not yet connected", () => { + const vm = createRemoteMedia(rtcMembership, {}, null); // null participant + expect(vm.waitingForMedia$.value).toBe(true); +}); + +test("remote media is not in waiting state when participant is connected", () => { + const vm = createRemoteMedia(rtcMembership, {}, mockRemoteParticipant({})); + expect(vm.waitingForMedia$.value).toBe(false); +}); + +test("remote media is not in waiting state when participant is connected with no publications", () => { + const vm = createRemoteMedia( + rtcMembership, + {}, + mockRemoteParticipant({ + getTrackPublication: () => undefined, + getTrackPublications: () => [], + }), + ); + expect(vm.waitingForMedia$.value).toBe(false); +}); diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 74e64b93..3d0ff75b 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -268,7 +268,7 @@ abstract class BaseMediaViewModel { encryptionSystem: EncryptionSystem, audioSource: AudioSource, videoSource: VideoSource, - livekitRoom$: Behavior, + protected readonly livekitRoom$: Behavior, public readonly focusUrl$: Behavior, public readonly displayName$: Behavior, public readonly mxcAvatarUrl$: Behavior, @@ -596,6 +596,14 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { * A remote participant's user media. */ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { + /** + * Whether we are waiting for this user's LiveKit participant to exist. This + * could be because either we or the remote party are still connecting. + */ + public readonly waitingForMedia$ = this.scope.behavior( + this.participant$.pipe(map((participant) => participant === null)), + ); + // This private field is used to override the value from the superclass private __speaking$: Behavior; public get speaking$(): Behavior { diff --git a/src/tile/GridTile.test.tsx b/src/tile/GridTile.test.tsx index e3172a22..9bc0efb2 100644 --- a/src/tile/GridTile.test.tsx +++ b/src/tile/GridTile.test.tsx @@ -12,7 +12,11 @@ import { axe } from "vitest-axe"; import { type MatrixRTCSession } from "matrix-js-sdk/lib/matrixrtc"; import { GridTile } from "./GridTile"; -import { mockRtcMembership, createRemoteMedia } from "../utils/test"; +import { + mockRtcMembership, + createRemoteMedia, + mockRemoteParticipant, +} from "../utils/test"; import { GridTileViewModel } from "../state/TileViewModel"; import { ReactionsSenderProvider } from "../reactions/useReactionsSender"; import type { CallViewModel } from "../state/CallViewModel/CallViewModel"; @@ -31,11 +35,11 @@ test("GridTile is accessible", async () => { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", }, - { + mockRemoteParticipant({ setVolume() {}, getTrackPublication: () => ({}) as Partial as RemoteTrackPublication, - }, + }), ); const fakeRtcSession = { diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 57409869..7768e8f0 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -69,6 +69,7 @@ interface UserMediaTileProps extends TileProps { vm: UserMediaViewModel; mirror: boolean; locallyMuted: boolean; + waitingForMedia?: boolean; primaryButton?: ReactNode; menuStart?: ReactNode; menuEnd?: ReactNode; @@ -79,6 +80,7 @@ const UserMediaTile: FC = ({ vm, showSpeakingIndicators, locallyMuted, + waitingForMedia, primaryButton, menuStart, menuEnd, @@ -194,7 +196,7 @@ const UserMediaTile: FC = ({ raisedHandTime={handRaised ?? undefined} currentReaction={reaction ?? undefined} raisedHandOnClick={raisedHandOnClick} - localParticipant={vm.local} + waitingForMedia={waitingForMedia} focusUrl={focusUrl} audioStreamStats={audioStreamStats} videoStreamStats={videoStreamStats} @@ -290,6 +292,7 @@ const RemoteUserMediaTile: FC = ({ ...props }) => { const { t } = useTranslation(); + const waitingForMedia = useBehavior(vm.waitingForMedia$); const locallyMuted = useBehavior(vm.locallyMuted$); const localVolume = useBehavior(vm.localVolume$); const onSelectMute = useCallback( @@ -311,6 +314,7 @@ const RemoteUserMediaTile: FC = ({ { video: trackReference, userId: "@alice:example.com", mxcAvatarUrl: undefined, - localParticipant: false, focusable: true, }; @@ -66,24 +65,13 @@ describe("MediaView", () => { }); }); - describe("with no participant", () => { - it("shows avatar for local user", () => { - render( - , - ); + describe("with no video", () => { + it("shows avatar", () => { + render(); expect( screen.getByRole("img", { name: "@alice:example.com" }), ).toBeVisible(); - expect(screen.queryAllByText("Waiting for media...").length).toBe(0); - }); - it("shows avatar and label for remote user", () => { - render( - , - ); - expect( - screen.getByRole("img", { name: "@alice:example.com" }), - ).toBeVisible(); - expect(screen.getByText("Waiting for media...")).toBeVisible(); + expect(screen.queryByTestId("video")).toBe(null); }); }); @@ -94,6 +82,22 @@ describe("MediaView", () => { }); }); + describe("waitingForMedia", () => { + test("defaults to false", () => { + render(); + expect(screen.queryAllByText("Waiting for media...").length).toBe(0); + }); + test("shows and is accessible", async () => { + const { container } = render( + + + , + ); + expect(await axe(container)).toHaveNoViolations(); + expect(screen.getByText("Waiting for media...")).toBeVisible(); + }); + }); + describe("unencryptedWarning", () => { test("is shown and accessible", async () => { const { container } = render( diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index e8a30cd4..8bb38d94 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -43,7 +43,7 @@ interface Props extends ComponentProps { raisedHandTime?: Date; currentReaction?: ReactionOption; raisedHandOnClick?: () => void; - localParticipant: boolean; + waitingForMedia?: boolean; audioStreamStats?: RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats; videoStreamStats?: RTCInboundRtpStreamStats | RTCOutboundRtpStreamStats; // The focus url, mainly for debugging purposes @@ -71,7 +71,7 @@ export const MediaView: FC = ({ raisedHandTime, currentReaction, raisedHandOnClick, - localParticipant, + waitingForMedia, audioStreamStats, videoStreamStats, focusUrl, @@ -129,7 +129,7 @@ export const MediaView: FC = ({ /> )} - {!video && !localParticipant && ( + {waitingForMedia && (
{t("video_tile.waiting_for_media")}
diff --git a/src/tile/SpotlightTile.test.tsx b/src/tile/SpotlightTile.test.tsx index fb7008b8..981c0369 100644 --- a/src/tile/SpotlightTile.test.tsx +++ b/src/tile/SpotlightTile.test.tsx @@ -17,6 +17,7 @@ import { mockRtcMembership, createLocalMedia, createRemoteMedia, + mockRemoteParticipant, } from "../utils/test"; import { SpotlightTileViewModel } from "../state/TileViewModel"; import { constant } from "../state/Behavior"; @@ -33,7 +34,7 @@ test("SpotlightTile is accessible", async () => { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", }, - {}, + mockRemoteParticipant({}), ); const vm2 = createLocalMedia( diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index 48dd0f8c..8109784f 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -38,6 +38,7 @@ import { type MediaViewModel, ScreenShareViewModel, type UserMediaViewModel, + type RemoteUserMediaViewModel, } from "../state/MediaViewModel"; import { useInitial } from "../useInitial"; import { useMergedRefs } from "../useMergedRefs"; @@ -84,6 +85,21 @@ const SpotlightLocalUserMediaItem: FC = ({ SpotlightLocalUserMediaItem.displayName = "SpotlightLocalUserMediaItem"; +interface SpotlightRemoteUserMediaItemProps + extends SpotlightUserMediaItemBaseProps { + vm: RemoteUserMediaViewModel; +} + +const SpotlightRemoteUserMediaItem: FC = ({ + vm, + ...props +}) => { + const waitingForMedia = useBehavior(vm.waitingForMedia$); + return ( + + ); +}; + interface SpotlightUserMediaItemProps extends SpotlightItemBaseProps { vm: UserMediaViewModel; } @@ -103,7 +119,7 @@ const SpotlightUserMediaItem: FC = ({ return vm instanceof LocalUserMediaViewModel ? ( ) : ( - + ); }; diff --git a/src/utils/test.ts b/src/utils/test.ts index bd7dcd6f..c69a2269 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -319,12 +319,12 @@ export function mockLocalParticipant( } export function createLocalMedia( - localRtcMember: CallMembership, + rtcMember: CallMembership, roomMember: Partial, localParticipant: LocalParticipant, mediaDevices: MediaDevices, ): LocalUserMediaViewModel { - const member = mockMatrixRoomMember(localRtcMember, roomMember); + const member = mockMatrixRoomMember(rtcMember, roomMember); return new LocalUserMediaViewModel( testScope(), "local", @@ -359,22 +359,26 @@ export function mockRemoteParticipant( } export function createRemoteMedia( - localRtcMember: CallMembership, + rtcMember: CallMembership, roomMember: Partial, - participant: Partial, + participant: RemoteParticipant | null, ): RemoteUserMediaViewModel { - const member = mockMatrixRoomMember(localRtcMember, roomMember); - const remoteParticipant = mockRemoteParticipant(participant); + const member = mockMatrixRoomMember(rtcMember, roomMember); return new RemoteUserMediaViewModel( testScope(), "remote", member.userId, - of(remoteParticipant), + constant(participant), { kind: E2eeType.PER_PARTICIPANT, }, constant( - mockLivekitRoom({}, { remoteParticipants$: of([remoteParticipant]) }), + mockLivekitRoom( + {}, + { + remoteParticipants$: of(participant ? [participant] : []), + }, + ), ), constant("https://rtc-example.org"), constant(false), From ea6f934667972f4db3fda40bc36c6b4927b75397 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Dec 2025 17:17:37 -0500 Subject: [PATCH 41/43] Don't show user as 'waiting for media' if they don't intend to publish We don't expect them to be publishing on any transport; they might be a subscribe-only bot. --- src/state/MediaViewModel.test.ts | 10 ++++++++++ src/state/MediaViewModel.ts | 9 ++++++++- src/utils/test.ts | 15 +++++++-------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 2ca14d19..92868216 100644 --- a/src/state/MediaViewModel.test.ts +++ b/src/state/MediaViewModel.test.ts @@ -226,3 +226,13 @@ test("remote media is not in waiting state when participant is connected with no ); expect(vm.waitingForMedia$.value).toBe(false); }); + +test("remote media is not in waiting state when user does not intend to publish anywhere", () => { + const vm = createRemoteMedia( + rtcMembership, + {}, + mockRemoteParticipant({}), + undefined, // No room (no advertised transport) + ); + expect(vm.waitingForMedia$.value).toBe(false); +}); diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 3d0ff75b..86119caa 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -601,7 +601,14 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { * could be because either we or the remote party are still connecting. */ public readonly waitingForMedia$ = this.scope.behavior( - this.participant$.pipe(map((participant) => participant === null)), + combineLatest( + [this.livekitRoom$, this.participant$], + (livekitRoom, participant) => + // If livekitRoom is undefined, the user is not attempting to publish on + // any transport and so we shouldn't expect a participant. (They might + // be a subscribe-only bot for example.) + livekitRoom !== undefined && participant === null, + ), ); // This private field is used to override the value from the superclass diff --git a/src/utils/test.ts b/src/utils/test.ts index c69a2269..b900d801 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -362,6 +362,12 @@ export function createRemoteMedia( rtcMember: CallMembership, roomMember: Partial, participant: RemoteParticipant | null, + livekitRoom: LivekitRoom | undefined = mockLivekitRoom( + {}, + { + remoteParticipants$: of(participant ? [participant] : []), + }, + ), ): RemoteUserMediaViewModel { const member = mockMatrixRoomMember(rtcMember, roomMember); return new RemoteUserMediaViewModel( @@ -372,14 +378,7 @@ export function createRemoteMedia( { kind: E2eeType.PER_PARTICIPANT, }, - constant( - mockLivekitRoom( - {}, - { - remoteParticipants$: of(participant ? [participant] : []), - }, - ), - ), + constant(livekitRoom), constant("https://rtc-example.org"), constant(false), constant(member.rawDisplayName ?? "nodisplayname"), From 6149dd2c9a1277aa005954989931af883150c7ec Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 10 Dec 2025 17:18:58 -0500 Subject: [PATCH 42/43] Make the video behavior less confusing There's no reason to allow it to take on placeholder values. It should be defined when the media has a published video track and undefined when not. --- .../CallViewModel/localMember/Publisher.ts | 2 +- src/state/MediaViewModel.ts | 28 +++++++++---------- src/tile/GridTile.tsx | 2 +- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index df67f179..21c5d801 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -358,7 +358,7 @@ export class Publisher { const track$ = scope.behavior( observeTrackReference$(room.localParticipant, Track.Source.Camera).pipe( map((trackRef) => { - const track = trackRef?.publication?.track; + const track = trackRef?.publication.track; return track instanceof LocalVideoTrack ? track : null; }), ), diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 86119caa..9888d6bf 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -7,8 +7,8 @@ Please see LICENSE in the repository root for full details. import { type AudioSource, - type TrackReferenceOrPlaceholder, type VideoSource, + type TrackReference, observeParticipantEvents, observeParticipantMedia, roomEventSelector, @@ -33,7 +33,6 @@ import { type Observable, Subject, combineLatest, - distinctUntilKeyChanged, filter, fromEvent, interval, @@ -60,14 +59,11 @@ import { type ObservableScope } from "./ObservableScope"; export function observeTrackReference$( participant: Participant, source: Track.Source, -): Observable { +): Observable { return observeParticipantMedia(participant).pipe( - map(() => ({ - participant: participant, - publication: participant.getTrackPublication(source), - source, - })), - distinctUntilKeyChanged("publication"), + map(() => participant.getTrackPublication(source)), + distinctUntilChanged(), + map((publication) => publication && { participant, publication, source }), ); } @@ -226,7 +222,7 @@ abstract class BaseMediaViewModel { /** * The LiveKit video track for this media. */ - public readonly video$: Behavior; + public readonly video$: Behavior; /** * Whether there should be a warning that this media is unencrypted. */ @@ -241,10 +237,12 @@ abstract class BaseMediaViewModel { private observeTrackReference$( source: Track.Source, - ): Behavior { + ): Behavior { return this.scope.behavior( this.participant$.pipe( - switchMap((p) => (!p ? of(null) : observeTrackReference$(p, source))), + switchMap((p) => + !p ? of(undefined) : observeTrackReference$(p, source), + ), ), ); } @@ -281,8 +279,8 @@ abstract class BaseMediaViewModel { [audio$, this.video$], (a, v) => encryptionSystem.kind !== E2eeType.NONE && - (a?.publication?.isEncrypted === false || - v?.publication?.isEncrypted === false), + (a?.publication.isEncrypted === false || + v?.publication.isEncrypted === false), ), ); @@ -471,7 +469,7 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { private readonly videoTrack$: Observable = this.video$.pipe( switchMap((v) => { - const track = v?.publication?.track; + const track = v?.publication.track; if (!(track instanceof LocalVideoTrack)) return of(null); return merge( // Watch for track restarts because they indicate a camera switch. diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 7768e8f0..2f750c50 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -150,7 +150,7 @@ const UserMediaTile: FC = ({ const tile = ( Date: Tue, 16 Dec 2025 13:40:06 +0100 Subject: [PATCH 43/43] review --- .../CallViewModel/localMember/LocalMember.ts | 38 +++++-------------- .../localMember/Publisher.test.ts | 2 +- .../CallViewModel/localMember/Publisher.ts | 35 ++++++++++------- 3 files changed, 32 insertions(+), 43 deletions(-) diff --git a/src/state/CallViewModel/localMember/LocalMember.ts b/src/state/CallViewModel/localMember/LocalMember.ts index daadbe7c..9ef94fe4 100644 --- a/src/state/CallViewModel/localMember/LocalMember.ts +++ b/src/state/CallViewModel/localMember/LocalMember.ts @@ -37,7 +37,7 @@ import { import { type Logger } from "matrix-js-sdk/lib/logger"; import { deepCompare } from "matrix-js-sdk/lib/utils"; -import { constant, type Behavior } from "../../Behavior.ts"; +import { type Behavior } from "../../Behavior.ts"; import { type IConnectionManager } from "../remoteMembers/ConnectionManager.ts"; import { type ObservableScope } from "../../ObservableScope.ts"; import { type Publisher } from "./Publisher.ts"; @@ -68,6 +68,8 @@ export enum TransportState { export enum PublishState { WaitingForUser = "publish_waiting_for_user", + // XXX: This state is removed for now since we do not have full control over + // track publication anymore with the publisher abstraction, might come back in the future? // /** Implies lk connection is connected */ // Starting = "publish_start_publishing", /** Implies lk connection is connected */ @@ -79,6 +81,8 @@ export enum PublishState { export enum TrackState { /** The track is waiting for user input to create tracks (waiting to call `startTracks()`) */ WaitingForUser = "tracks_waiting_for_user", + // XXX: This state is removed for now since we do not have full control over + // track creation anymore with the publisher abstraction, might come back in the future? // /** Implies lk connection is connected */ // Creating = "tracks_creating", /** Implies lk connection is connected */ @@ -154,9 +158,10 @@ export const createLocalMembership$ = ({ matrixRTCSession, }: Props): { /** - * This starts audio and video tracks. They will be reused when calling `requestPublish`. + * This request to start audio and video tracks. + * Can be called early to pre-emptively get media permissions and start devices. */ - startTracks: () => Behavior; + startTracks: () => void; /** * This sets a inner state (shouldPublish) to true and instructs the js-sdk and livekit to keep the user * connected to matrix and livekit. @@ -265,19 +270,10 @@ export const createLocalMembership$ = ({ * The publisher is stored in here an abstracts creating and publishing tracks. */ const publisher$ = new BehaviorSubject(null); - /** - * Extract the tracks from the published. Also reacts to changing publishers. - */ - // const tracks$ = scope.behavior( - // publisher$.pipe(switchMap((p) => (p?.tracks$ ? p.tracks$ : constant([])))), - // ); - // const publishing$ = scope.behavior( - // publisher$.pipe(switchMap((p) => p?.publishing$ ?? constant(false))), - // ); - const startTracks = (): Behavior => { + const startTracks = (): void => { trackStartRequested.resolve(); - return constant(undefined); + // This used to return the tracks, but now they are only accessible via the publisher. }; const requestJoinAndPublish = (): void => { @@ -348,14 +344,6 @@ export const createLocalMembership$ = ({ setPublishError(new UnknownCallError(error as Error)); } } - // XXX Why is that? - // else { - // try { - // await publisher?.stopPublishing(); - // } catch (error) { - // setLivekitError(new UnknownCallError(error as Error)); - // } - // } }, ); @@ -401,16 +389,10 @@ export const createLocalMembership$ = ({ ([ localConnectionState, localTransport, - // tracks, - // publishing, shouldPublish, shouldStartTracks, ]) => { if (!localTransport) return null; - // const hasTracks = tracks.length > 0; - // let trackState: TrackState = TrackState.WaitingForUser; - // if (hasTracks && shouldStartTracks) trackState = TrackState.Ready; - // if (!hasTracks && shouldStartTracks) trackState = TrackState.Creating; const trackState: TrackState = shouldStartTracks ? TrackState.Ready : TrackState.WaitingForUser; diff --git a/src/state/CallViewModel/localMember/Publisher.test.ts b/src/state/CallViewModel/localMember/Publisher.test.ts index 3cc96bc2..38a80bed 100644 --- a/src/state/CallViewModel/localMember/Publisher.test.ts +++ b/src/state/CallViewModel/localMember/Publisher.test.ts @@ -34,7 +34,7 @@ beforeEach(() => { scope = new ObservableScope(); }); -// afterEach(() => scope.end()); +afterEach(() => scope.end()); function createMockLocalTrack(source: Track.Source): LocalTrack { const track = { diff --git a/src/state/CallViewModel/localMember/Publisher.ts b/src/state/CallViewModel/localMember/Publisher.ts index d4ad656c..4428d845 100644 --- a/src/state/CallViewModel/localMember/Publisher.ts +++ b/src/state/CallViewModel/localMember/Publisher.ts @@ -97,7 +97,7 @@ export class Publisher { // it would also prevent the user from seeing their own video/audio preview. // So for that we use pauseUpStream(): Stops sending media to the server by replacing // the sender track with null, but keeps the local MediaStreamTrack active. - // The user can still see/hear themselves locally, but remote participants see nothing + // The user can still see/hear themselves locally, but remote participants see nothing. private onLocalTrackPublished( localTrackPublication: LocalTrackPublication, ): void { @@ -128,6 +128,15 @@ export class Publisher { } } /** + * Create and setup local audio and video tracks based on the current mute states. + * It creates the tracks only if audio and/or video is enabled, to avoid unnecessary + * permission prompts. + * + * It also observes mute state changes to update LiveKit microphone/camera states accordingly. + * If a track is not created initially because disabled, it will be created when unmuting. + * + * This call is not blocking anymore, instead callers can listen to the + * `RoomEvent.MediaDevicesError` event in the LiveKit room to be notified of any errors. * */ public async createAndSetupTracks(): Promise { @@ -141,25 +150,21 @@ export class Publisher { const audio = this.muteStates.audio.enabled$.value; const video = this.muteStates.video.enabled$.value; - const enableTracks = async (): Promise => { - if (audio && video) { - // Enable both at once in order to have a single permission prompt! - await lkRoom.localParticipant.enableCameraAndMicrophone(); - } else if (audio) { - await lkRoom.localParticipant.setMicrophoneEnabled(true); - } else if (video) { - await lkRoom.localParticipant.setCameraEnabled(true); - } - return; - }; - // We don't await enableTracks, because livekit could block until the tracks + // We don't await the creation, because livekit could block until the tracks // are fully published, and not only that they are created. // We don't have control on that, localParticipant creates and publishes the tracks // asap. // We are using the `ParticipantEvent.LocalTrackPublished` to be notified // when tracks are actually published, and at that point // we can pause upstream if needed (depending on if startPublishing has been called). - void enableTracks(); + if (audio && video) { + // Enable both at once in order to have a single permission prompt! + void lkRoom.localParticipant.enableCameraAndMicrophone(); + } else if (audio) { + void lkRoom.localParticipant.setMicrophoneEnabled(true); + } else if (video) { + void lkRoom.localParticipant.setCameraEnabled(true); + } return Promise.resolve(); } @@ -233,6 +238,8 @@ export class Publisher { public async stopPublishing(): Promise { this.logger.debug("stopPublishing called"); this.shouldPublish = false; + // Pause upstream will stop sending media to the server, while keeping + // the local MediaStreamTrack active, so the user can still see themselves. await this.pauseUpstreams(this.connection.livekitRoom, [ Track.Source.Microphone, Track.Source.Camera,