From 2986f90a5f21f12519be862be5d53b3ca96c7d64 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 8 Dec 2025 22:29:15 -0500 Subject: [PATCH 01/16] 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 02/16] 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 03/16] 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 04/16] 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 05/16] 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 00d4b8e985db7b9546d584610a777ef6a4a0e9f5 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 15 Dec 2025 12:52:23 -0500 Subject: [PATCH 06/16] 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 07/16] 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 08/16] 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 09/16] 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 10/16] 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 11/16] 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 12/16] 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 13/16] 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 14/16] 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 15/16] 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 16/16] 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 = (