From 6cfc2066c9e5f53b871c3849d1dad0cc3882be8a Mon Sep 17 00:00:00 2001 From: Robin Date: Sat, 20 Jan 2024 20:39:12 -0500 Subject: [PATCH 1/4] Add data models for the new call layouts This is a start at implementing the call layouts from the new designs. I've added data types to model the contents of each possible layout, and begun implementing the business logic to produce these layouts in the call view model. --- src/state/CallViewModel.ts | 360 +++++++++++++++++- .../{TileViewModel.ts => MediaViewModel.ts} | 25 +- src/state/ObservableScope.ts | 40 ++ src/state/ViewModel.ts | 9 +- src/video-grid/VideoTile.tsx | 16 +- 5 files changed, 407 insertions(+), 43 deletions(-) rename src/state/{TileViewModel.ts => MediaViewModel.ts} (90%) create mode 100644 src/state/ObservableScope.ts diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index b630a2b4..d20598df 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -1,5 +1,5 @@ /* -Copyright 2023 New Vector Ltd +Copyright 2023-2024 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -16,25 +16,41 @@ limitations under the License. import { connectedParticipantsObserver, + observeParticipantEvents, observeParticipantMedia, } from "@livekit/components-core"; -import { Room as LivekitRoom, RemoteParticipant } from "livekit-client"; +import { + Room as LivekitRoom, + LocalParticipant, + ParticipantEvent, + RemoteParticipant, +} from "livekit-client"; import { Room as MatrixRoom, RoomMember } from "matrix-js-sdk/src/matrix"; import { useEffect, useRef } from "react"; import { + BehaviorSubject, EMPTY, Observable, + audit, combineLatest, concat, + distinctUntilChanged, + filter, + map, + merge, mergeAll, of, sample, scan, + shareReplay, startWith, - takeUntil, + switchAll, + switchMap, + throttleTime, + timer, zip, } from "rxjs"; -import { state } from "@react-rxjs/core"; +import { StateObservable, state } from "@react-rxjs/core"; import { logger } from "matrix-js-sdk/src/logger"; import { ViewModel } from "./ViewModel"; @@ -45,14 +61,21 @@ import { } from "../livekit/useECConnectionState"; import { usePrevious } from "../usePrevious"; import { - TileViewModel, - UserMediaTileViewModel, - ScreenShareTileViewModel, -} from "./TileViewModel"; + MediaViewModel, + UserMediaViewModel, + ScreenShareViewModel, +} from "./MediaViewModel"; import { finalizeValue } from "../observable-utils"; +import { ObservableScope } from "./ObservableScope"; + +// How long we wait after a focus switch before showing the real participant +// list again +const POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS = 3000; // Represents something that should get a tile on the layout, // ie. a user's video feed or a screen share feed. +// TODO: This exposes too much information to the view layer, let's keep this +// information internal to the view model and switch to using Tile instead export interface TileDescriptor { id: string; focused: boolean; @@ -65,9 +88,128 @@ export interface TileDescriptor { data: T; } -// How long we wait after a focus switch before showing the real participant -// list again -const POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS = 3000; +/** + * A media tile within the call interface. + */ +export interface Tile { + id: string; + data: T; +} + +export interface GridLayout { + type: "grid"; + spotlight?: Tile; + grid: Tile[]; +} + +export interface SpotlightLayout { + type: "spotlight"; + spotlight: Tile; + grid: Tile[]; +} + +export interface FullScreenLayout { + type: "full screen"; + spotlight: Tile; + pip?: Tile; +} + +export interface PipLayout { + type: "pip"; + spotlight: Tile; +} + +/** + * A layout defining the media tiles present on screen and their visual + * arrangement. + */ +export type Layout = + | GridLayout + | SpotlightLayout + | FullScreenLayout + | PipLayout; + +export type GridMode = "grid" | "spotlight"; + +export type WindowMode = "normal" | "full screen" | "pip"; + +/** + * Sorting bins defining the order in which media tiles appear in the grid. + */ +enum GridBin { + SelfStart, + Presenters, + Speakers, + VideoAndAudio, + Video, + Audio, + NoMedia, + SelfEnd, +} + +class UserMedia { + private readonly scope = new ObservableScope(); + public readonly vm: UserMediaViewModel; + public readonly speaker: Observable; + public readonly presenter: Observable; + + public constructor( + public readonly id: string, + member: RoomMember | undefined, + participant: LocalParticipant | RemoteParticipant, + callEncrypted: boolean, + ) { + this.vm = new UserMediaViewModel(id, member, participant, callEncrypted); + + this.speaker = this.vm.speaking.pipeState( + // Require 1 s of continuous speaking to become a speaker, and 10 s of + // continuous silence to stop being considered a speaker + audit((s) => + merge( + timer(s ? 1000 : 10000), + this.vm.speaking.pipe(filter((s1) => s1 !== s)), + ), + ), + distinctUntilChanged(), + this.scope.bind(), + // Make this Observable hot so that the timers don't reset when you + // resubscribe + shareReplay(1), + ); + + this.presenter = observeParticipantEvents( + participant, + ParticipantEvent.TrackPublished, + ParticipantEvent.TrackUnpublished, + ParticipantEvent.LocalTrackPublished, + ParticipantEvent.LocalTrackUnpublished, + ).pipe(map((p) => p.isScreenShareEnabled)); + } + + public destroy(): void { + this.scope.end(); + this.vm.destroy(); + } +} + +class ScreenShare { + public readonly vm: ScreenShareViewModel; + + public constructor( + id: string, + member: RoomMember | undefined, + participant: LocalParticipant | RemoteParticipant, + callEncrypted: boolean, + ) { + this.vm = new ScreenShareViewModel(id, member, participant, callEncrypted); + } + + public destroy(): void { + this.vm.destroy(); + } +} + +type MediaItem = UserMedia | ScreenShare; function findMatrixMember( room: MatrixRoom, @@ -116,7 +258,7 @@ export class CallViewModel extends ViewModel { ), new Promise((resolve) => { const subscription = this.connectionState - .pipe(takeUntil(this.destroyed)) + .pipe(this.scope.bind()) .subscribe((s) => { if (s !== ECAddonConnectionState.ECSwitchingFocus) { resolve(); @@ -165,9 +307,193 @@ export class CallViewModel extends ViewModel { }, ); + private readonly mediaItems = state( + combineLatest([ + this.remoteParticipants, + observeParticipantMedia(this.livekitRoom.localParticipant), + ]).pipe( + scan( + ( + prevItems, + [remoteParticipants, { participant: localParticipant }], + ) => { + let allGhosts = true; + + const newItems = new Map( + function* (this: CallViewModel): Iterable<[string, MediaItem]> { + for (const p of [localParticipant, ...remoteParticipants]) { + const member = findMatrixMember(this.matrixRoom, p.identity); + allGhosts &&= member === undefined; + // We always start with a local participant with the empty string as + // their ID before we're connected, this is fine and we'll be in + // "all ghosts" mode. + if (p.identity !== "" && member === undefined) { + logger.warn( + `Ruh, roh! No matrix member found for SFU participant '${p.identity}': creating g-g-g-ghost!`, + ); + } + + const userMediaId = p.identity; + yield [ + userMediaId, + prevItems.get(userMediaId) ?? + new UserMedia(userMediaId, member, p, this.encrypted), + ]; + + if (p.isScreenShareEnabled) { + const screenShareId = `${userMediaId}:screen-share`; + yield [ + screenShareId, + prevItems.get(screenShareId) ?? + new ScreenShare(screenShareId, member, p, this.encrypted), + ]; + } + } + }.bind(this)(), + ); + + for (const [id, t] of prevItems) if (!newItems.has(id)) t.destroy(); + + // If every item is a ghost, that probably means we're still connecting + // and shouldn't bother showing anything yet + return allGhosts ? new Map() : newItems; + }, + new Map(), + ), + map((ms) => [...ms.values()]), + finalizeValue((ts) => { + for (const t of ts) t.destroy(); + }), + ), + ); + + private readonly userMedia = this.mediaItems.pipe( + map((ms) => ms.filter((m): m is UserMedia => m instanceof UserMedia)), + ); + + private readonly screenShares = this.mediaItems.pipe( + map((ms) => ms.filter((m): m is ScreenShare => m instanceof ScreenShare)), + ); + + private readonly spotlightSpeaker = this.userMedia.pipe( + switchMap((ms) => + combineLatest( + ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), + ), + ), + scan<(readonly [UserMedia, boolean])[], UserMedia | null, null>( + (prev, ms) => + // Decide who to spotlight: + // If the previous speaker is still speaking, stick with them rather + // than switching eagerly to someone else + ms.find(([m, s]) => m === prev && s)?.[0] ?? + // Otherwise, select anyone who is speaking + ms.find(([, s]) => s)?.[0] ?? + // Otherwise, stick with the person who was last speaking + prev ?? + // Otherwise, spotlight the local user + ms.find(([m]) => m.vm.local)?.[0] ?? + null, + null, + ), + distinctUntilChanged(), + throttleTime(800, undefined, { leading: true, trailing: true }), + ); + + private readonly grid = this.userMedia.pipe( + switchMap((ms) => { + const bins = ms.map((m) => + combineLatest( + [m.speaker, m.presenter, m.vm.audioEnabled, m.vm.videoEnabled], + (speaker, presenter, audio, video) => + [ + m, + m.vm.local + ? GridBin.SelfStart + : presenter + ? GridBin.Presenters + : speaker + ? GridBin.Speakers + : video + ? audio + ? GridBin.VideoAndAudio + : GridBin.Video + : audio + ? GridBin.Audio + : GridBin.NoMedia, + ] as const, + ), + ); + // Sort the media by bin order and generate a tile for each one + return combineLatest(bins, (...bins) => + bins + .sort(([, bin1], [, bin2]) => bin1 - bin2) + .map(([m]) => ({ id: m.id, data: m.vm })), + ); + }), + ); + + private readonly spotlight = combineLatest( + [this.screenShares, this.spotlightSpeaker], + (screenShares, spotlightSpeaker): Tile => ({ + id: "spotlight", + data: + screenShares.length > 0 + ? screenShares.map((m) => m.vm) + : spotlightSpeaker === null + ? [] + : [spotlightSpeaker.vm], + }), + ); + + private readonly gridMode = new BehaviorSubject("grid"); + + public setGridMode(value: GridMode): void { + this.gridMode.next(value); + } + + // TODO: Make this react to changes in window dimensions and screen + // orientation + private readonly windowMode = of("normal"); + + public readonly layout: StateObservable = state( + combineLatest([this.gridMode, this.windowMode], (gridMode, windowMode) => { + switch (windowMode) { + case "full screen": + throw new Error("unimplemented"); + case "pip": + throw new Error("unimplemented"); + case "normal": { + switch (gridMode) { + case "grid": + return combineLatest( + [this.grid, this.spotlight, this.screenShares], + (grid, spotlight, screenShares): Layout => ({ + type: "grid", + spotlight: screenShares.length > 0 ? spotlight : undefined, + grid, + }), + ); + case "spotlight": + return combineLatest( + [this.grid, this.spotlight], + (grid, spotlight): Layout => ({ + type: "spotlight", + spotlight, + grid, + }), + ); + } + } + } + }).pipe(switchAll()), + ); + /** * The media tiles to be displayed in the call view. */ + // TODO: Get rid of this field, replacing it with the 'layout' field above + // which keeps more details of the layout order internal to the view model public readonly tiles = state( combineLatest([ this.remoteParticipants, @@ -197,10 +523,10 @@ export class CallViewModel extends ViewModel { const userMediaVm = tilesById.get(userMediaId)?.data ?? - new UserMediaTileViewModel(userMediaId, member, p, this.encrypted); + new UserMediaViewModel(userMediaId, member, p, this.encrypted); tilesById.delete(userMediaId); - const userMediaTile: TileDescriptor = { + const userMediaTile: TileDescriptor = { id: userMediaId, focused: false, isPresenter: p.isScreenShareEnabled, @@ -215,7 +541,7 @@ export class CallViewModel extends ViewModel { const screenShareId = `${userMediaId}:screen-share`; const screenShareVm = tilesById.get(screenShareId)?.data ?? - new ScreenShareTileViewModel( + new ScreenShareViewModel( screenShareId, member, p, @@ -223,7 +549,7 @@ export class CallViewModel extends ViewModel { ); tilesById.delete(screenShareId); - const screenShareTile: TileDescriptor = { + const screenShareTile: TileDescriptor = { id: screenShareId, focused: true, isPresenter: false, @@ -246,7 +572,7 @@ export class CallViewModel extends ViewModel { // If every item is a ghost, that probably means we're still connecting // and shouldn't bother showing anything yet return allGhosts ? [] : newTiles; - }, [] as TileDescriptor[]), + }, [] as TileDescriptor[]), finalizeValue((ts) => { for (const t of ts) t.data.destroy(); }), diff --git a/src/state/TileViewModel.ts b/src/state/MediaViewModel.ts similarity index 90% rename from src/state/TileViewModel.ts rename to src/state/MediaViewModel.ts index b059c42c..0327ce7e 100644 --- a/src/state/TileViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -1,5 +1,5 @@ /* -Copyright 2023 New Vector Ltd +Copyright 2023-2024 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -43,7 +43,6 @@ import { of, startWith, switchMap, - takeUntil, } from "rxjs"; import { ViewModel } from "./ViewModel"; @@ -64,13 +63,13 @@ function observeTrackReference( ); } -abstract class BaseTileViewModel extends ViewModel { +abstract class BaseMediaViewModel extends ViewModel { /** - * Whether the tile belongs to the local user. + * Whether the media belongs to the local user. */ public readonly local = this.participant.isLocal; /** - * The LiveKit video track to be shown on this tile. + * The LiveKit video track for this media. */ public readonly video: StateObservable; /** @@ -83,7 +82,7 @@ abstract class BaseTileViewModel extends ViewModel { // soon as that code is moved into the view models public readonly id: string, /** - * The Matrix room member to which this tile belongs. + * The Matrix room member to which this media belongs. */ // TODO: Fully separate the data layer from the UI layer by keeping the // member object internal @@ -109,14 +108,14 @@ abstract class BaseTileViewModel extends ViewModel { } /** - * A tile displaying some media. + * Some participant's media. */ -export type TileViewModel = UserMediaTileViewModel | ScreenShareTileViewModel; +export type MediaViewModel = UserMediaViewModel | ScreenShareViewModel; /** - * A tile displaying some participant's user media. + * Some participant's user media. */ -export class UserMediaTileViewModel extends BaseTileViewModel { +export class UserMediaViewModel extends BaseMediaViewModel { /** * Whether the video should be mirrored. */ @@ -201,7 +200,7 @@ export class UserMediaTileViewModel extends BaseTileViewModel { combineLatest([this._locallyMuted, this._localVolume], (muted, volume) => muted ? 0 : volume, ) - .pipe(takeUntil(this.destroyed)) + .pipe(this.scope.bind()) .subscribe((volume) => { (this.participant as RemoteParticipant).setVolume(volume); }); @@ -221,9 +220,9 @@ export class UserMediaTileViewModel extends BaseTileViewModel { } /** - * A tile displaying some participant's screen share. + * Some participant's screen share media. */ -export class ScreenShareTileViewModel extends BaseTileViewModel { +export class ScreenShareViewModel extends BaseMediaViewModel { public constructor( id: string, member: RoomMember | undefined, diff --git a/src/state/ObservableScope.ts b/src/state/ObservableScope.ts new file mode 100644 index 00000000..cb7cbd17 --- /dev/null +++ b/src/state/ObservableScope.ts @@ -0,0 +1,40 @@ +/* +Copyright 2024 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { MonoTypeOperatorFunction, Subject, takeUntil } from "rxjs"; + +/** + * A scope which limits the execution lifetime of its bound Observables. + */ +export class ObservableScope { + private readonly ended = new Subject(); + + /** + * Binds an Observable to this scope, so that it completes when the scope + * ends. + */ + public bind(): MonoTypeOperatorFunction { + return takeUntil(this.ended); + } + + /** + * Ends the scope, causing any bound Observables to complete. + */ + public end(): void { + this.ended.next(); + this.ended.complete(); + } +} diff --git a/src/state/ViewModel.ts b/src/state/ViewModel.ts index d10afad1..dd7d422c 100644 --- a/src/state/ViewModel.ts +++ b/src/state/ViewModel.ts @@ -1,5 +1,5 @@ /* -Copyright 2023 New Vector Ltd +Copyright 2023-2024 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,20 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { Subject } from "rxjs"; +import { ObservableScope } from "./ObservableScope"; /** * An MVVM view model. */ export abstract class ViewModel { - protected readonly destroyed = new Subject(); + protected readonly scope = new ObservableScope(); /** * Instructs the ViewModel to clean up its resources. If you forget to call * this, there may be memory leaks! */ public destroy(): void { - this.destroyed.next(); - this.destroyed.complete(); + this.scope.end(); } } diff --git a/src/video-grid/VideoTile.tsx b/src/video-grid/VideoTile.tsx index c239c2f8..d4a7442e 100644 --- a/src/video-grid/VideoTile.tsx +++ b/src/video-grid/VideoTile.tsx @@ -58,10 +58,10 @@ import { Avatar } from "../Avatar"; import styles from "./VideoTile.module.css"; import { useReactiveState } from "../useReactiveState"; import { - ScreenShareTileViewModel, - TileViewModel, - UserMediaTileViewModel, -} from "../state/TileViewModel"; + ScreenShareViewModel, + MediaViewModel, + UserMediaViewModel, +} from "../state/MediaViewModel"; import { subscribe } from "../state/subscribe"; import { useMergedRefs } from "../useMergedRefs"; import { Slider } from "../Slider"; @@ -170,7 +170,7 @@ const Tile = forwardRef( Tile.displayName = "Tile"; interface UserMediaTileProps { - vm: UserMediaTileViewModel; + vm: UserMediaViewModel; className?: string; style?: ComponentProps["style"]; targetWidth: number; @@ -329,7 +329,7 @@ const UserMediaTile = subscribe( UserMediaTile.displayName = "UserMediaTile"; interface ScreenShareTileProps { - vm: ScreenShareTileViewModel; + vm: ScreenShareViewModel; className?: string; style?: ComponentProps["style"]; targetWidth: number; @@ -403,7 +403,7 @@ const ScreenShareTile = subscribe( ScreenShareTile.displayName = "ScreenShareTile"; interface Props { - vm: TileViewModel; + vm: MediaViewModel; maximised: boolean; fullscreen: boolean; onToggleFullscreen: (itemId: string) => void; @@ -455,7 +455,7 @@ export const VideoTile = forwardRef( ? t("video_tile.sfu_participant_local") : displayName; - if (vm instanceof UserMediaTileViewModel) { + if (vm instanceof UserMediaViewModel) { return ( Date: Thu, 2 May 2024 15:59:40 -0400 Subject: [PATCH 2/4] Fix speaking data taking 10 seconds to be available --- src/state/CallViewModel.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index d20598df..0cfee12b 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -167,9 +167,12 @@ class UserMedia { audit((s) => merge( timer(s ? 1000 : 10000), + // If the speaking flag resets to its original value during this time, + // end the silencing window to stick with that original value this.vm.speaking.pipe(filter((s1) => s1 !== s)), ), ), + startWith(false), distinctUntilChanged(), this.scope.bind(), // Make this Observable hot so that the timers don't reset when you From dcb4d10afb376744f4acf7d78bc0b6aa1251e1c6 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 2 May 2024 16:00:05 -0400 Subject: [PATCH 3/4] Remove unnecessary Tile indirection --- src/state/CallViewModel.ts | 65 ++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 0cfee12b..222712b3 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -88,35 +88,27 @@ export interface TileDescriptor { data: T; } -/** - * A media tile within the call interface. - */ -export interface Tile { - id: string; - data: T; -} - export interface GridLayout { type: "grid"; - spotlight?: Tile; - grid: Tile[]; + spotlight?: MediaViewModel[]; + grid: UserMediaViewModel[]; } export interface SpotlightLayout { type: "spotlight"; - spotlight: Tile; - grid: Tile[]; + spotlight: MediaViewModel[]; + grid: UserMediaViewModel[]; } export interface FullScreenLayout { type: "full screen"; - spotlight: Tile; - pip?: Tile; + spotlight: MediaViewModel[]; + pip?: UserMediaViewModel; } export interface PipLayout { type: "pip"; - spotlight: Tile; + spotlight: MediaViewModel[]; } /** @@ -380,9 +372,11 @@ export class CallViewModel extends ViewModel { private readonly spotlightSpeaker = this.userMedia.pipe( switchMap((ms) => - combineLatest( - ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), - ), + ms.length === 0 + ? of([]) + : combineLatest( + ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), + ), ), scan<(readonly [UserMedia, boolean])[], UserMedia | null, null>( (prev, ms) => @@ -428,31 +422,32 @@ export class CallViewModel extends ViewModel { ), ); // Sort the media by bin order and generate a tile for each one - return combineLatest(bins, (...bins) => - bins - .sort(([, bin1], [, bin2]) => bin1 - bin2) - .map(([m]) => ({ id: m.id, data: m.vm })), - ); + return bins.length === 0 + ? of([]) + : combineLatest(bins, (...bins) => + bins.sort(([, bin1], [, bin2]) => bin1 - bin2).map(([m]) => m.vm), + ); }), ); private readonly spotlight = combineLatest( [this.screenShares, this.spotlightSpeaker], - (screenShares, spotlightSpeaker): Tile => ({ - id: "spotlight", - data: - screenShares.length > 0 - ? screenShares.map((m) => m.vm) - : spotlightSpeaker === null - ? [] - : [spotlightSpeaker.vm], - }), + (screenShares, spotlightSpeaker): MediaViewModel[] => + screenShares.length > 0 + ? screenShares.map((m) => m.vm) + : spotlightSpeaker === null + ? [] + : [spotlightSpeaker.vm], ); - private readonly gridMode = new BehaviorSubject("grid"); + private readonly _gridMode = new BehaviorSubject("grid"); + /** + * The layout mode of the media tile grid. + */ + public readonly gridMode = state(this._gridMode); public setGridMode(value: GridMode): void { - this.gridMode.next(value); + this._gridMode.next(value); } // TODO: Make this react to changes in window dimensions and screen @@ -460,7 +455,7 @@ export class CallViewModel extends ViewModel { private readonly windowMode = of("normal"); public readonly layout: StateObservable = state( - combineLatest([this.gridMode, this.windowMode], (gridMode, windowMode) => { + combineLatest([this._gridMode, this.windowMode], (gridMode, windowMode) => { switch (windowMode) { case "full screen": throw new Error("unimplemented"); From e9c98a02f0418edb7f263ae9029050ff55cb2e6f Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 2 May 2024 16:32:48 -0400 Subject: [PATCH 4/4] Respond to suggestions from code review --- src/state/CallViewModel.ts | 383 ++++++++++++++++++------------------- 1 file changed, 191 insertions(+), 192 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 222712b3..4ad2f024 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -126,9 +126,9 @@ export type GridMode = "grid" | "spotlight"; export type WindowMode = "normal" | "full screen" | "pip"; /** - * Sorting bins defining the order in which media tiles appear in the grid. + * Sorting bins defining the order in which media tiles appear in the layout. */ -enum GridBin { +enum SortingBin { SelfStart, Presenters, Speakers, @@ -235,74 +235,76 @@ export class CallViewModel extends ViewModel { // Lists of participants to "hold" on display, even if LiveKit claims that // they've left - private readonly remoteParticipantHolds = zip( - this.connectionState, - this.rawRemoteParticipants.pipe(sample(this.connectionState)), - (s, ps) => { - // Whenever we switch focuses, we should retain all the previous - // participants for at least POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS ms to - // give their clients time to switch over and avoid jarring layout shifts - if (s === ECAddonConnectionState.ECSwitchingFocus) { - return concat( - // Hold these participants - of({ hold: ps }), - // Wait for time to pass and the connection state to have changed - Promise.all([ - new Promise((resolve) => - setTimeout(resolve, POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS), - ), - new Promise((resolve) => { - const subscription = this.connectionState - .pipe(this.scope.bind()) - .subscribe((s) => { - if (s !== ECAddonConnectionState.ECSwitchingFocus) { - resolve(); - subscription.unsubscribe(); - } - }); - }), - // Then unhold them - ]).then(() => Promise.resolve({ unhold: ps })), - ); - } else { - return EMPTY; - } - }, - ).pipe( - mergeAll(), - // Aggregate the hold instructions into a single list showing which - // participants are being held - scan( - (holds, instruction) => - "hold" in instruction - ? [instruction.hold, ...holds] - : holds.filter((h) => h !== instruction.unhold), - [] as RemoteParticipant[][], - ), - startWith([]), - ); + private readonly remoteParticipantHolds: Observable = + zip( + this.connectionState, + this.rawRemoteParticipants.pipe(sample(this.connectionState)), + (s, ps) => { + // Whenever we switch focuses, we should retain all the previous + // participants for at least POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS ms to + // give their clients time to switch over and avoid jarring layout shifts + if (s === ECAddonConnectionState.ECSwitchingFocus) { + return concat( + // Hold these participants + of({ hold: ps }), + // Wait for time to pass and the connection state to have changed + Promise.all([ + new Promise((resolve) => + setTimeout(resolve, POST_FOCUS_PARTICIPANT_UPDATE_DELAY_MS), + ), + new Promise((resolve) => { + const subscription = this.connectionState + .pipe(this.scope.bind()) + .subscribe((s) => { + if (s !== ECAddonConnectionState.ECSwitchingFocus) { + resolve(); + subscription.unsubscribe(); + } + }); + }), + // Then unhold them + ]).then(() => Promise.resolve({ unhold: ps })), + ); + } else { + return EMPTY; + } + }, + ).pipe( + mergeAll(), + // Aggregate the hold instructions into a single list showing which + // participants are being held + scan( + (holds, instruction) => + "hold" in instruction + ? [instruction.hold, ...holds] + : holds.filter((h) => h !== instruction.unhold), + [] as RemoteParticipant[][], + ), + startWith([]), + ); - private readonly remoteParticipants = combineLatest( - [this.rawRemoteParticipants, this.remoteParticipantHolds], - (raw, holds) => { - const result = [...raw]; - const resultIds = new Set(result.map((p) => p.identity)); + private readonly remoteParticipants: Observable = + combineLatest( + [this.rawRemoteParticipants, this.remoteParticipantHolds], + (raw, holds) => { + const result = [...raw]; + const resultIds = new Set(result.map((p) => p.identity)); - // Incorporate the held participants into the list - for (const hold of holds) { - for (const p of hold) { - if (!resultIds.has(p.identity)) { - result.push(p); - resultIds.add(p.identity); + // Incorporate the held participants into the list + for (const hold of holds) { + for (const p of hold) { + if (!resultIds.has(p.identity)) { + result.push(p); + resultIds.add(p.identity); + } } } - } - return result; - }, - ); + return result; + }, + ); - private readonly mediaItems = state( + private readonly mediaItems: StateObservable = state( combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -362,63 +364,59 @@ export class CallViewModel extends ViewModel { ), ); - private readonly userMedia = this.mediaItems.pipe( + private readonly userMedia: Observable = this.mediaItems.pipe( map((ms) => ms.filter((m): m is UserMedia => m instanceof UserMedia)), ); - private readonly screenShares = this.mediaItems.pipe( - map((ms) => ms.filter((m): m is ScreenShare => m instanceof ScreenShare)), - ); + private readonly screenShares: Observable = + this.mediaItems.pipe( + map((ms) => ms.filter((m): m is ScreenShare => m instanceof ScreenShare)), + ); - private readonly spotlightSpeaker = this.userMedia.pipe( - switchMap((ms) => - ms.length === 0 - ? of([]) - : combineLatest( - ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), - ), - ), - scan<(readonly [UserMedia, boolean])[], UserMedia | null, null>( - (prev, ms) => - // Decide who to spotlight: - // If the previous speaker is still speaking, stick with them rather - // than switching eagerly to someone else - ms.find(([m, s]) => m === prev && s)?.[0] ?? - // Otherwise, select anyone who is speaking - ms.find(([, s]) => s)?.[0] ?? - // Otherwise, stick with the person who was last speaking - prev ?? - // Otherwise, spotlight the local user - ms.find(([m]) => m.vm.local)?.[0] ?? + private readonly spotlightSpeaker: Observable = + this.userMedia.pipe( + switchMap((ms) => + ms.length === 0 + ? of([]) + : combineLatest( + ms.map((m) => m.vm.speaking.pipe(map((s) => [m, s] as const))), + ), + ), + scan<(readonly [UserMedia, boolean])[], UserMedia | null, null>( + (prev, ms) => + // Decide who to spotlight: + // If the previous speaker is still speaking, stick with them rather + // than switching eagerly to someone else + ms.find(([m, s]) => m === prev && s)?.[0] ?? + // Otherwise, select anyone who is speaking + ms.find(([, s]) => s)?.[0] ?? + // Otherwise, stick with the person who was last speaking + prev ?? + // Otherwise, spotlight the local user + ms.find(([m]) => m.vm.local)?.[0] ?? + null, null, - null, - ), - distinctUntilChanged(), - throttleTime(800, undefined, { leading: true, trailing: true }), - ); + ), + distinctUntilChanged(), + throttleTime(800, undefined, { leading: true, trailing: true }), + ); - private readonly grid = this.userMedia.pipe( + private readonly grid: Observable = this.userMedia.pipe( switchMap((ms) => { const bins = ms.map((m) => combineLatest( [m.speaker, m.presenter, m.vm.audioEnabled, m.vm.videoEnabled], - (speaker, presenter, audio, video) => - [ - m, - m.vm.local - ? GridBin.SelfStart - : presenter - ? GridBin.Presenters - : speaker - ? GridBin.Speakers - : video - ? audio - ? GridBin.VideoAndAudio - : GridBin.Video - : audio - ? GridBin.Audio - : GridBin.NoMedia, - ] as const, + (speaker, presenter, audio, video) => { + let bin: SortingBin; + if (m.vm.local) bin = SortingBin.SelfStart; + else if (presenter) bin = SortingBin.Presenters; + else if (speaker) bin = SortingBin.Speakers; + else if (video) + bin = audio ? SortingBin.VideoAndAudio : SortingBin.Video; + else bin = audio ? SortingBin.Audio : SortingBin.NoMedia; + + return [m, bin] as const; + }, ), ); // Sort the media by bin order and generate a tile for each one @@ -430,7 +428,7 @@ export class CallViewModel extends ViewModel { }), ); - private readonly spotlight = combineLatest( + private readonly spotlight: Observable = combineLatest( [this.screenShares, this.spotlightSpeaker], (screenShares, spotlightSpeaker): MediaViewModel[] => screenShares.length > 0 @@ -440,6 +438,10 @@ export class CallViewModel extends ViewModel { : [spotlightSpeaker.vm], ); + // TODO: Make this react to changes in window dimensions and screen + // orientation + private readonly windowMode = of("normal"); + private readonly _gridMode = new BehaviorSubject("grid"); /** * The layout mode of the media tile grid. @@ -450,10 +452,6 @@ export class CallViewModel extends ViewModel { this._gridMode.next(value); } - // TODO: Make this react to changes in window dimensions and screen - // orientation - private readonly windowMode = of("normal"); - public readonly layout: StateObservable = state( combineLatest([this._gridMode, this.windowMode], (gridMode, windowMode) => { switch (windowMode) { @@ -492,90 +490,91 @@ export class CallViewModel extends ViewModel { */ // TODO: Get rid of this field, replacing it with the 'layout' field above // which keeps more details of the layout order internal to the view model - public readonly tiles = state( - combineLatest([ - this.remoteParticipants, - observeParticipantMedia(this.livekitRoom.localParticipant), - ]).pipe( - scan((ts, [remoteParticipants, { participant: localParticipant }]) => { - const ps = [localParticipant, ...remoteParticipants]; - const tilesById = new Map(ts.map((t) => [t.id, t])); - const now = Date.now(); - let allGhosts = true; + public readonly tiles: StateObservable[]> = + state( + combineLatest([ + this.remoteParticipants, + observeParticipantMedia(this.livekitRoom.localParticipant), + ]).pipe( + scan((ts, [remoteParticipants, { participant: localParticipant }]) => { + const ps = [localParticipant, ...remoteParticipants]; + const tilesById = new Map(ts.map((t) => [t.id, t])); + const now = Date.now(); + let allGhosts = true; - const newTiles = ps.flatMap((p) => { - const userMediaId = p.identity; - const member = findMatrixMember(this.matrixRoom, userMediaId); - allGhosts &&= member === undefined; - const spokeRecently = - p.lastSpokeAt !== undefined && now - +p.lastSpokeAt <= 10000; + const newTiles = ps.flatMap((p) => { + const userMediaId = p.identity; + const member = findMatrixMember(this.matrixRoom, userMediaId); + allGhosts &&= member === undefined; + const spokeRecently = + p.lastSpokeAt !== undefined && now - +p.lastSpokeAt <= 10000; - // We always start with a local participant with the empty string as - // their ID before we're connected, this is fine and we'll be in - // "all ghosts" mode. - if (userMediaId !== "" && member === undefined) { - logger.warn( - `Ruh, roh! No matrix member found for SFU participant '${userMediaId}': creating g-g-g-ghost!`, - ); - } - - const userMediaVm = - tilesById.get(userMediaId)?.data ?? - new UserMediaViewModel(userMediaId, member, p, this.encrypted); - tilesById.delete(userMediaId); - - const userMediaTile: TileDescriptor = { - id: userMediaId, - focused: false, - isPresenter: p.isScreenShareEnabled, - isSpeaker: (p.isSpeaking || spokeRecently) && !p.isLocal, - hasVideo: p.isCameraEnabled, - local: p.isLocal, - largeBaseSize: false, - data: userMediaVm, - }; - - if (p.isScreenShareEnabled) { - const screenShareId = `${userMediaId}:screen-share`; - const screenShareVm = - tilesById.get(screenShareId)?.data ?? - new ScreenShareViewModel( - screenShareId, - member, - p, - this.encrypted, + // We always start with a local participant with the empty string as + // their ID before we're connected, this is fine and we'll be in + // "all ghosts" mode. + if (userMediaId !== "" && member === undefined) { + logger.warn( + `Ruh, roh! No matrix member found for SFU participant '${userMediaId}': creating g-g-g-ghost!`, ); - tilesById.delete(screenShareId); + } - const screenShareTile: TileDescriptor = { - id: screenShareId, - focused: true, - isPresenter: false, - isSpeaker: false, - hasVideo: true, + const userMediaVm = + tilesById.get(userMediaId)?.data ?? + new UserMediaViewModel(userMediaId, member, p, this.encrypted); + tilesById.delete(userMediaId); + + const userMediaTile: TileDescriptor = { + id: userMediaId, + focused: false, + isPresenter: p.isScreenShareEnabled, + isSpeaker: (p.isSpeaking || spokeRecently) && !p.isLocal, + hasVideo: p.isCameraEnabled, local: p.isLocal, - largeBaseSize: true, - placeNear: userMediaId, - data: screenShareVm, + largeBaseSize: false, + data: userMediaVm, }; - return [userMediaTile, screenShareTile]; - } else { - return [userMediaTile]; - } - }); - // Any tiles left in the map are unused and should be destroyed - for (const t of tilesById.values()) t.data.destroy(); + if (p.isScreenShareEnabled) { + const screenShareId = `${userMediaId}:screen-share`; + const screenShareVm = + tilesById.get(screenShareId)?.data ?? + new ScreenShareViewModel( + screenShareId, + member, + p, + this.encrypted, + ); + tilesById.delete(screenShareId); - // If every item is a ghost, that probably means we're still connecting - // and shouldn't bother showing anything yet - return allGhosts ? [] : newTiles; - }, [] as TileDescriptor[]), - finalizeValue((ts) => { - for (const t of ts) t.data.destroy(); - }), - ), - ); + const screenShareTile: TileDescriptor = { + id: screenShareId, + focused: true, + isPresenter: false, + isSpeaker: false, + hasVideo: true, + local: p.isLocal, + largeBaseSize: true, + placeNear: userMediaId, + data: screenShareVm, + }; + return [userMediaTile, screenShareTile]; + } else { + return [userMediaTile]; + } + }); + + // Any tiles left in the map are unused and should be destroyed + for (const t of tilesById.values()) t.data.destroy(); + + // If every item is a ghost, that probably means we're still connecting + // and shouldn't bother showing anything yet + return allGhosts ? [] : newTiles; + }, [] as TileDescriptor[]), + finalizeValue((ts) => { + for (const t of ts) t.data.destroy(); + }), + ), + ); public constructor( // A call is permanently tied to a single Matrix room and LiveKit room