diff --git a/src/e2ee/matrixKeyProvider.ts b/src/e2ee/matrixKeyProvider.ts index 1d1c4588..a499f45c 100644 --- a/src/e2ee/matrixKeyProvider.ts +++ b/src/e2ee/matrixKeyProvider.ts @@ -12,11 +12,12 @@ import { MatrixRTCSessionEvent, } from "matrix-js-sdk/lib/matrixrtc"; import { type CallMembershipIdentityParts } from "matrix-js-sdk/lib/matrixrtc/EncryptionManager"; +import { firstValueFrom } from "rxjs"; import { - computeLivekitParticipantIdentity, + computeLivekitParticipantIdentity$, livekitIdentityInput, -} from "../state/CallViewModel/remoteMembers/MatrixLivekitMembers"; +} from "../state/CallViewModel/remoteMembers/LivekitParticipantIdentity"; export class MatrixKeyProvider extends BaseKeyProvider { private rtcSession?: MatrixRTCSession; @@ -69,7 +70,7 @@ export class MatrixKeyProvider extends BaseKeyProvider { "deriveBits", "deriveKey", ]), - computeLivekitParticipantIdentity(membership, kind), + firstValueFrom(computeLivekitParticipantIdentity$(membership, kind)), ]).then( ([keyMaterial, livekitParticipantId]) => { this.onSetEncryptionKey( diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index a767388a..89a151ae 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/CallViewModel.test.ts b/src/state/CallViewModel/CallViewModel.test.ts index 58814bbe..505cb19f 100644 --- a/src/state/CallViewModel/CallViewModel.test.ts +++ b/src/state/CallViewModel/CallViewModel.test.ts @@ -61,7 +61,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()), @@ -241,7 +242,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/CallViewModel.ts b/src/state/CallViewModel/CallViewModel.ts index bab77eaf..e1869cf6 100644 --- a/src/state/CallViewModel/CallViewModel.ts +++ b/src/state/CallViewModel/CallViewModel.ts @@ -53,11 +53,15 @@ import { ScreenShareViewModel, type UserMediaViewModel, } from "../MediaViewModel"; -import { accumulate, generateItems, pauseWhen } from "../../utils/observable"; +import { + accumulate, + filterBehavior, + generateItems, + pauseWhen, +} from "../../utils/observable"; import { duplicateTiles, MatrixRTCMode, - matrixRTCMode, playReactionsSound, showReactions, } from "../../settings/settings"; @@ -111,7 +115,8 @@ import { ECConnectionFactory } from "./remoteMembers/ConnectionFactory.ts"; import { createConnectionManager$ } from "./remoteMembers/ConnectionManager.ts"; import { createMatrixLivekitMembers$, - type MatrixLivekitMember, + type TaggedParticipant, + type LocalMatrixLivekitMember, } from "./remoteMembers/MatrixLivekitMembers.ts"; import { type AutoLeaveReason, @@ -150,6 +155,8 @@ export interface CallViewModelOptions { connectionState$?: Behavior; /** Optional behavior overriding the computed window size, mainly for testing purposes. */ windowSize$?: Behavior<{ width: number; height: number }>; + /** The version & compatibility mode of MatrixRTC that we should use. */ + matrixRTCMode$: Behavior; } // Do not play any sounds if the participant count has exceeded this @@ -406,7 +413,7 @@ export function createCallViewModel$( client, roomId: matrixRoom.roomId, useOldestMember$: scope.behavior( - matrixRTCMode.value$.pipe(map((v) => v === MatrixRTCMode.Legacy)), + options.matrixRTCMode$.pipe(map((v) => v === MatrixRTCMode.Legacy)), ), }); @@ -458,7 +465,7 @@ export function createCallViewModel$( }); const connectOptions$ = scope.behavior( - matrixRTCMode.value$.pipe( + options.matrixRTCMode$.pipe( map((mode) => ({ encryptMedia: livekitKeyProvider !== undefined, // TODO. This might need to get called again on each change of matrixRTCMode... @@ -512,22 +519,21 @@ export function createCallViewModel$( ), ); - const localMatrixLivekitMemberUninitialized = { - membership$: localRtcMembership$, - participant$: localMembership.participant$, - connection$: localMembership.connection$, - userId: userId, - }; - - const localMatrixLivekitMember$: Behavior = + 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, - ); + filterBehavior((membership) => membership !== null), + map((membership$) => { + if (membership$ === null) return null; + return { + membership$, + participant: { + type: "local" as const, + value$: localMembership.participant$, + }, + connection$: localMembership.connection$, + userId, + }; }), ), ); @@ -595,7 +601,7 @@ export function createCallViewModel$( switchMap((members) => { const a$ = combineLatest( members.value.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) @@ -673,7 +679,7 @@ export function createCallViewModel$( let localUserMediaId: string | undefined = undefined; // add local member if available if (localMatrixLivekitMember) { - const { userId, participant$, connection$, membership$ } = + const { userId, participant, connection$, membership$ } = localMatrixLivekitMember; localUserMediaId = `${userId}:${membership$.value.deviceId}`; // should be membership$.value.membershipID which is not optional @@ -684,7 +690,7 @@ export function createCallViewModel$( dup, localUserMediaId, userId, - participant$, + participant satisfies TaggedParticipant as TaggedParticipant, // Widen the type safely connection$, ], data: undefined, @@ -695,7 +701,7 @@ export function createCallViewModel$( // add remote members that are available for (const { userId, - participant$, + participant, connection$, membership$, } of matrixLivekitMembers.value) { @@ -704,7 +710,7 @@ export function createCallViewModel$( // const participantId = membership$.value?.identity; for (let dup = 0; dup < 1 + duplicateTiles; dup++) { yield { - keys: [dup, userMediaId, userId, participant$, connection$], + keys: [dup, userMediaId, userId, participant, connection$], data: undefined, }; } @@ -716,7 +722,7 @@ export function createCallViewModel$( dup, participantId, userId, - participant$, + participant, connection$, ) => { const livekitRoom$ = scope.behavior( @@ -735,7 +741,7 @@ export function createCallViewModel$( scope, `${participantId}:${dup}`, userId, - participant$, + participant, options.encryptionSystem, livekitRoom$, focusUrl$, @@ -945,11 +951,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/CallViewModelTestUtils.ts b/src/state/CallViewModel/CallViewModelTestUtils.ts index f80b4bcb..b6f53275 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,125 @@ 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: Partial = {}, + ): 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, + { + encryptionSystem: { kind: E2eeType.PER_PARTICIPANT }, + autoLeaveWhenOthersLeft: false, + livekitRoomFactory: (): LivekitRoom => + mockLivekitRoom({ + localParticipant, + disconnect: async () => Promise.resolve(), + setE2EEEnabled: async () => Promise.resolve(), + }), + connectionState$, + windowSize$, + matrixRTCMode$: constant(mode), + ...options, + }, + 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/CallViewModel/LayoutSwitch.test.ts b/src/state/CallViewModel/LayoutSwitch.test.ts index ae5a3896..0d184017 100644 --- a/src/state/CallViewModel/LayoutSwitch.test.ts +++ b/src/state/CallViewModel/LayoutSwitch.test.ts @@ -5,198 +5,128 @@ 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", + })); + +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$, 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/CallViewModel/remoteMembers/Connection.test.ts b/src/state/CallViewModel/remoteMembers/Connection.test.ts index 8f9471d0..30c934b9 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.test.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.test.ts @@ -32,7 +32,6 @@ import { Connection, ConnectionState, type ConnectionOpts, - type PublishingParticipant, } from "./Connection.ts"; import { ObservableScope } from "../../ObservableScope.ts"; import { type OpenIDClientParts } from "../../../livekit/openIDSFU.ts"; @@ -40,6 +39,7 @@ import { ElementCallError, FailToGetOpenIdToken, } from "../../../utils/errors.ts"; +import { mockRemoteParticipant } from "../../../utils/test.ts"; let testScope: ObservableScope; @@ -377,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: PublishingParticipant[][] = []; - 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 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. - 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 @@ -428,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 => { @@ -450,16 +423,14 @@ describe("Publishing participants observations", () => { const connection = setupRemoteConnection(); - let observedPublishers: PublishingParticipant[][] = []; - 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 @@ -471,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 8b4479e8..05d0ec9e 100644 --- a/src/state/CallViewModel/remoteMembers/Connection.ts +++ b/src/state/CallViewModel/remoteMembers/Connection.ts @@ -13,9 +13,7 @@ import { import { ConnectionError, type Room as LivekitRoom, - type LocalParticipant, type RemoteParticipant, - RoomEvent, } from "livekit-client"; import { type LivekitTransport } from "matrix-js-sdk/lib/matrixrtc"; import { BehaviorSubject, map } from "rxjs"; @@ -35,8 +33,6 @@ import { UnknownCallError, } from "../../../utils/errors.ts"; -export type PublishingParticipant = LocalParticipant | RemoteParticipant; - export interface ConnectionOpts { /** The media transport to connect to. */ transport: LivekitTransport; @@ -99,13 +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< - PublishingParticipant[] - >; + public readonly remoteParticipants$: Behavior; /** * Whether the connection has been stopped. @@ -236,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 484a44e7..70bfb4de 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"; @@ -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,24 +200,21 @@ describe("connections$ stream", () => { }); describe("connectionManagerData$ stream", () => { - // Used in test to control fake connections' remoteParticipantsWithTracks$ streams - let fakePublishingParticipantsStreams: Map< - string, - Behavior - >; + // Used in test to control fake connections' remoteParticipants$ streams + 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), + remoteParticipants$: 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 6660df62..4303d50a 100644 --- a/src/state/CallViewModel/remoteMembers/ConnectionManager.ts +++ b/src/state/CallViewModel/remoteMembers/ConnectionManager.ts @@ -9,7 +9,7 @@ Please see LICENSE in the repository root for full details. 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 LocalParticipant, type RemoteParticipant } from "livekit-client"; +import { type RemoteParticipant } from "livekit-client"; import { type Behavior } from "../../Behavior.ts"; import { type Connection } from "./Connection.ts"; @@ -19,17 +19,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) { @@ -55,7 +50,7 @@ export class ConnectionManagerData { public getParticipantForTransport( transport: LivekitTransport, - ): (LocalParticipant | RemoteParticipant)[] { + ): RemoteParticipant[] { const key = transport.livekit_service_url + "|" + transport.livekit_alias; return this.store.get(key)?.[1] ?? []; } @@ -67,10 +62,12 @@ interface Props { 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. @@ -153,23 +150,24 @@ export function createConnectionManager$({ const epoch = connections.epoch; // Map the connections to list of {connection, participants}[] - const listOfConnectionsWithPublishingParticipants = - connections.value.map((connection) => { - return connection.remoteParticipantsWithTracks$.pipe( + const listOfConnectionsWithRemoteParticipants = connections.value.map( + (connection) => { + return connection.remoteParticipants$.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( diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts index 68a67546..91266dc5 100644 --- a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts +++ b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.test.ts @@ -14,7 +14,7 @@ import { BehaviorSubject, combineLatest, map, type Observable } from "rxjs"; import { type IConnectionManager } from "./ConnectionManager.ts"; import { - type MatrixLivekitMember, + type RemoteMatrixLivekitMember, createMatrixLivekitMembers$, } from "./MatrixLivekitMembers.ts"; import { @@ -102,10 +102,10 @@ test("should signal participant not yet connected to livekit", async () => { await flushPromises(); expect(matrixLivekitMember$.value.value).toSatisfy( - (data: MatrixLivekitMember[]) => { + (data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expect(data[0].membership$.value).toBe(bobMembership); - expect(data[0].participant$.value).toBe(null); + expect(data[0].participant.value$.value).toBe(null); expect(data[0].connection$.value).toBe(null); return true; }, @@ -171,10 +171,10 @@ test("should signal participant on a connection that is publishing", async () => await flushPromises(); expect(matrixLivekitMember$.value.value).toSatisfy( - (data: MatrixLivekitMember[]) => { + (data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expect(data[0].membership$.value).toBe(bobMembership); - expect(data[0].participant$.value).toSatisfy((participant) => { + expect(data[0].participant.value$.value).toSatisfy((participant) => { expect(participant).toBeDefined(); expect(participant!.identity).toEqual(bobParticipantId); return true; @@ -210,10 +210,10 @@ test("should signal participant on a connection that is not publishing", async ( }); await flushPromises(); expect(matrixLivekitMember$.value.value).toSatisfy( - (data: MatrixLivekitMember[]) => { + (data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(1); expect(data[0].membership$.value).toBe(bobMembership); - expect(data[0].participant$.value).toBe(null); + expect(data[0].participant.value$.value).toBe(null); expect(data[0].connection$.value).toBe(connection); return true; }, @@ -258,11 +258,11 @@ describe("Publication edge case", () => { }); await flushPromises(); expect(matrixLivekitMember$.value.value).toSatisfy( - (data: MatrixLivekitMember[]) => { + (data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(2); expect(data[0].membership$.value).toBe(bobMembership); expect(data[0].connection$.value).toBe(connectionA); - expect(data[0].participant$.value).toSatisfy((participant) => { + expect(data[0].participant.value$.value).toSatisfy((participant) => { expect(participant).toBeDefined(); expect(participant!.identity).toEqual(bobParticipantId); return true; @@ -317,11 +317,11 @@ test("bob is publishing in the wrong connection", async () => { await flushPromises(); expect(matrixLivekitMember$.value.value).toSatisfy( - (data: MatrixLivekitMember[]) => { + (data: RemoteMatrixLivekitMember[]) => { expect(data.length).toEqual(2); expect(data[0].membership$.value).toBe(bobMembership); expect(data[0].connection$.value).toBe(connectionA); - expect(data[0].participant$.value).toBe(null); + expect(data[0].participant.value$.value).toBe(null); return true; }, ); diff --git a/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts b/src/state/CallViewModel/remoteMembers/MatrixLivekitMembers.ts index 30abfc9b..0c61ba06 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, @@ -25,22 +22,44 @@ import { computeLivekitParticipantIdentity$ } from "./LivekitParticipantIdentity const logger = rootLogger.getChild("[MatrixLivekitMembers]"); -/** - * 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 { +interface LocalTaggedParticipant { + type: "local"; + value$: Behavior; +} +interface RemoteTaggedParticipant { + type: "remote"; + value$: Behavior; +} +export type TaggedParticipant = + | LocalTaggedParticipant + | RemoteTaggedParticipant; + +interface MatrixLivekitMember { membership$: Behavior; - participant$: Behavior< - LocalLivekitParticipant | RemoteLivekitParticipant | null - >; 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< @@ -62,7 +81,7 @@ export function createMatrixLivekitMembers$({ scope, membershipsWithTransport$, connectionManager, -}: Props): Behavior> { +}: Props): Behavior> { /** * This internal observable is used to compute the async sha256 hash of the user's identity. * a promise is treated like an observable. So we can switchMap on the promise from the identity computation. @@ -138,12 +157,14 @@ export function createMatrixLivekitMembers$({ logger.debug( `Generating member for livekitIdentity: ${identity}, userId:deviceId: ${userId}${deviceId}`, ); + 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 { identity, 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 3ad81fa8..202b3f56 100644 --- a/src/state/CallViewModel/remoteMembers/integration.test.ts +++ b/src/state/CallViewModel/remoteMembers/integration.test.ts @@ -30,7 +30,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"; @@ -138,7 +138,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]!; @@ -153,12 +153,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); @@ -167,7 +167,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, }); } @@ -178,7 +178,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", { @@ -195,7 +195,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); @@ -222,7 +222,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/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"); + }, +); diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 61fa2d8c..92868216 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,35 @@ 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); +}); + +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 74e64b93..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), + ), ), ); } @@ -268,7 +266,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, @@ -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. @@ -596,6 +594,21 @@ 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( + 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 private __speaking$: Behavior; public get speaking$(): Behavior { 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, 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..2f750c50 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, @@ -148,7 +150,7 @@ const UserMediaTile: FC = ({ const tile = ( = ({ 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/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[]], 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$, diff --git a/src/utils/test.ts b/src/utils/test.ts index 28fc8546..7d251640 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -321,12 +321,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", @@ -361,23 +361,26 @@ export function mockRemoteParticipant( } export function createRemoteMedia( - localRtcMember: CallMembership, + rtcMember: CallMembership, roomMember: Partial, - participant: Partial, + participant: RemoteParticipant | null, + livekitRoom: LivekitRoom | undefined = mockLivekitRoom( + {}, + { + remoteParticipants$: of(participant ? [participant] : []), + }, + ), ): 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]) }), - ), + constant(livekitRoom), constant("https://rtc-example.org"), constant(false), constant(member.rawDisplayName ?? "nodisplayname"),