From 5f8809ce031fb19a50d2a73075130b6fc61eb4ff Mon Sep 17 00:00:00 2001 From: Timo Date: Wed, 30 Oct 2024 14:29:16 +0100 Subject: [PATCH 01/84] make tiles based on rtc member --- src/room/InCallView.tsx | 4 +- src/state/CallViewModel.ts | 270 ++++++++++++++++++++++-------------- src/state/MediaViewModel.ts | 100 ++++++++----- 3 files changed, 234 insertions(+), 140 deletions(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 9492b2f01..a5847f0ef 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -118,7 +118,7 @@ export const ActiveCall: FC = (props) => { useEffect(() => { if (livekitRoom !== undefined) { const vm = new CallViewModel( - props.rtcSession.room, + props.rtcSession, livekitRoom, props.e2eeSystem, connStateObservable, @@ -127,7 +127,7 @@ export const ActiveCall: FC = (props) => { return (): void => vm.destroy(); } }, [ - props.rtcSession.room, + props.rtcSession, livekitRoom, props.e2eeSystem, connStateObservable, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index db2833b85..734d24102 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -18,12 +18,9 @@ import { RemoteParticipant, Track, } from "livekit-client"; +import { Room as MatrixRoom, RoomMember } from "matrix-js-sdk/src/matrix"; import { - Room as MatrixRoom, - RoomMember, - RoomStateEvent, -} from "matrix-js-sdk/src/matrix"; -import { + BehaviorSubject, EMPTY, Observable, Subject, @@ -51,6 +48,10 @@ import { withLatestFrom, } from "rxjs"; import { logger } from "matrix-js-sdk/src/logger"; +import { + MatrixRTCSession, + MatrixRTCSessionEvent, +} from "matrix-js-sdk/src/matrixrtc"; import { ViewModel } from "./ViewModel"; import { @@ -164,28 +165,37 @@ enum SortingBin { class UserMedia { private readonly scope = new ObservableScope(); public readonly vm: UserMediaViewModel; + public participant: BehaviorSubject< + LocalParticipant | RemoteParticipant | undefined + >; + public readonly speaker: Observable; public readonly presenter: Observable; - public constructor( public readonly id: string, member: RoomMember | undefined, - participant: LocalParticipant | RemoteParticipant, + participant: LocalParticipant | RemoteParticipant | undefined, encryptionSystem: EncryptionSystem, ) { - this.vm = participant.isLocal - ? new LocalUserMediaViewModel( - id, - member, - participant as LocalParticipant, - encryptionSystem, - ) - : new RemoteUserMediaViewModel( - id, - member, - participant as RemoteParticipant, - encryptionSystem, - ); + this.participant = new BehaviorSubject(participant); + + if (participant && participant.isLocal) { + this.vm = new LocalUserMediaViewModel( + this.id, + member, + this.participant.asObservable() as Observable, + encryptionSystem, + ); + } else { + this.vm = new RemoteUserMediaViewModel( + id, + member, + this.participant.asObservable() as Observable< + RemoteParticipant | undefined + >, + encryptionSystem, + ); + } this.speaker = this.vm.speaking.pipe( // Require 1 s of continuous speaking to become a speaker, and 60 s of @@ -195,7 +205,7 @@ class UserMedia { timer(s ? 1000 : 60000), // 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)), + this.vm!.speaking.pipe(filter((s1) => s1 !== s)), ), ), startWith(false), @@ -205,13 +215,21 @@ class UserMedia { this.scope.state(), ); - this.presenter = observeParticipantEvents( - participant, - ParticipantEvent.TrackPublished, - ParticipantEvent.TrackUnpublished, - ParticipantEvent.LocalTrackPublished, - ParticipantEvent.LocalTrackUnpublished, - ).pipe(map((p) => p.isScreenShareEnabled)); + this.presenter = this.participant.pipe( + switchMap( + (p) => + (p && + observeParticipantEvents( + p, + ParticipantEvent.TrackPublished, + ParticipantEvent.TrackUnpublished, + ParticipantEvent.LocalTrackPublished, + ParticipantEvent.LocalTrackUnpublished, + ).pipe(map((p) => p.isScreenShareEnabled))) ?? + of(false), + ), + this.scope.state(), + ); } public destroy(): void { @@ -222,6 +240,7 @@ class UserMedia { class ScreenShare { public readonly vm: ScreenShareViewModel; + private participant: BehaviorSubject; public constructor( id: string, @@ -229,10 +248,12 @@ class ScreenShare { participant: LocalParticipant | RemoteParticipant, encryptionSystem: EncryptionSystem, ) { + this.participant = new BehaviorSubject(participant); + this.vm = new ScreenShareViewModel( id, member, - participant, + this.participant.asObservable(), encryptionSystem, ); } @@ -244,7 +265,7 @@ class ScreenShare { type MediaItem = UserMedia | ScreenShare; -function findMatrixMember( +function findMatrixRoomMember( room: MatrixRoom, id: string, ): RoomMember | undefined { @@ -344,8 +365,15 @@ export class CallViewModel extends ViewModel { this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), duplicateTiles.value, - // Also react to changes in the list of members - fromEvent(this.matrixRoom, RoomStateEvent.Update).pipe(startWith(null)), + // Also react to changes in the MatrixRTC session list: + fromEvent( + this.matrixRTCSession, + MatrixRTCSessionEvent.MembershipsChanged, + ).pipe(startWith(null)), + // fromEvent( + // this.matrixRTCSession, + // MatrixRTCSessionEvent.EncryptionKeyChanged, + // ).pipe(startWith(null)), ]).pipe( scan( ( @@ -354,42 +382,64 @@ export class CallViewModel extends ViewModel { ) => { const newItems = new Map( function* (this: CallViewModel): Iterable<[string, MediaItem]> { - for (const p of [localParticipant, ...remoteParticipants]) { - const id = p === localParticipant ? "local" : p.identity; - const member = findMatrixMember(this.matrixRoom, id); - if (member === undefined) - logger.warn( - `Ruh, roh! No matrix member found for SFU participant '${p.identity}': creating g-g-g-ghost!`, + for (const rtcMember of this.matrixRTCSession.memberships) { + const room = this.matrixRTCSession.room; + // WARN! This is not exactly the sender but the user defined in the state key. + // This will be available once we change to the new "member as object" format in the MatrixRTC object. + let mediaId = rtcMember.sender + ":" + rtcMember.deviceId; + let participant = undefined; + if ( + rtcMember.sender === room.client.getUserId()! && + rtcMember.deviceId === room.client.getDeviceId() + ) { + mediaId = "local"; + participant = localParticipant; + } else { + participant = remoteParticipants.find( + (p) => p.identity === mediaId, ); + } + + const member = findMatrixRoomMember(room, mediaId); - // Create as many tiles for this participant as called for by - // the duplicateTiles option for (let i = 0; i < 1 + duplicateTiles; i++) { - const userMediaId = `${id}:${i}`; + const indexedMediaId = `${mediaId}:${i}`; + const prevMedia = prevItems.get(indexedMediaId); + if (prevMedia && prevMedia instanceof UserMedia) { + if ( + prevMedia.participant.value === undefined && + participant !== undefined + ) { + // Update the BahviourSubject in the UserMedia. + prevMedia.participant.next(participant); + } + } yield [ - userMediaId, - prevItems.get(userMediaId) ?? + indexedMediaId, + // We create UserMedia with or without a participant. + // This will be the initial value of a BehaviourSubject. + // Once a participant appears we will update the BehaviourSubject. (see above) + prevMedia ?? new UserMedia( - userMediaId, + mediaId, member, - p, + participant, + this.encryptionSystem, + ), + ]; + } + if (participant && participant.isScreenShareEnabled) { + const screenShareId = `${mediaId}:screen-share`; + yield [ + screenShareId, + prevItems.get(screenShareId) ?? + new ScreenShare( + screenShareId, + member, + participant, this.encryptionSystem, ), ]; - - if (p.isScreenShareEnabled) { - const screenShareId = `${userMediaId}:screen-share`; - yield [ - screenShareId, - prevItems.get(screenShareId) ?? - new ScreenShare( - screenShareId, - member, - p, - this.encryptionSystem, - ), - ]; - } } } }.bind(this)(), @@ -432,42 +482,43 @@ export class CallViewModel extends ViewModel { distinctUntilChanged(), ); - private readonly spotlightSpeaker: Observable = - this.userMedia.pipe( - switchMap((mediaItems) => - mediaItems.length === 0 - ? of([]) - : combineLatest( - mediaItems.map((m) => - m.vm.speaking.pipe(map((s) => [m, s] as const)), - ), + private readonly spotlightSpeaker: Observable< + UserMediaViewModel | undefined + > = this.userMedia.pipe( + switchMap((mediaItems) => + mediaItems.length === 0 + ? of([]) + : combineLatest( + mediaItems.map((m) => + m.vm.speaking.pipe(map((s) => [m, s] as const)), ), - ), - scan<(readonly [UserMedia, boolean])[], UserMedia, null>( - (prev, mediaItems) => { - // Only remote users that are still in the call should be sticky - const [stickyMedia, stickySpeaking] = - (!prev?.vm.local && mediaItems.find(([m]) => m === prev)) || []; - // Decide who to spotlight: - // If the previous speaker is still speaking, stick with them rather - // than switching eagerly to someone else - return stickySpeaking - ? stickyMedia! - : // Otherwise, select any remote user who is speaking - (mediaItems.find(([m, s]) => !m.vm.local && s)?.[0] ?? - // Otherwise, stick with the person who was last speaking - stickyMedia ?? - // Otherwise, spotlight an arbitrary remote user - mediaItems.find(([m]) => !m.vm.local)?.[0] ?? - // Otherwise, spotlight the local user - mediaItems.find(([m]) => m.vm.local)![0]); - }, - null, - ), - map((speaker) => speaker.vm), - this.scope.state(), - throttleTime(1600, undefined, { leading: true, trailing: true }), - ); + ), + ), + scan<(readonly [UserMedia, boolean])[], UserMedia | undefined, null>( + (prev, mediaItems) => { + // Only remote users that are still in the call should be sticky + const [stickyMedia, stickySpeaking] = + (!prev?.vm.local && mediaItems.find(([m]) => m === prev)) || []; + // Decide who to spotlight: + // If the previous speaker is still speaking, stick with them rather + // than switching eagerly to someone else + return stickySpeaking + ? stickyMedia! + : // Otherwise, select any remote user who is speaking + (mediaItems.find(([m, s]) => !m.vm.local && s)?.[0] ?? + // Otherwise, stick with the person who was last speaking + stickyMedia ?? + // Otherwise, spotlight an arbitrary remote user + mediaItems.find(([m]) => !m.vm.local)?.[0] ?? + // Otherwise, spotlight the local user + mediaItems.find(([m]) => m.vm.local)?.[0]); + }, + null, + ), + map((speaker) => speaker?.vm), + this.scope.state(), + throttleTime(1600, undefined, { leading: true, trailing: true }), + ); private readonly grid: Observable = this.userMedia.pipe( switchMap((mediaItems) => { @@ -510,20 +561,29 @@ export class CallViewModel extends ViewModel { > = this.screenShares.pipe( map((screenShares) => screenShares.length > 0 - ? ([of(screenShares.map((m) => m.vm)), this.spotlightSpeaker] as const) + ? ([ + of(screenShares.map((m) => m.vm)), + this.spotlightSpeaker.pipe( + map((speaker) => (speaker && speaker) ?? null), + ), + ] as const) : ([ - this.spotlightSpeaker.pipe(map((speaker) => [speaker!])), + this.spotlightSpeaker.pipe( + map((speaker) => (speaker && [speaker]) ?? []), + ), this.spotlightSpeaker.pipe( switchMap((speaker) => - speaker.local - ? of(null) - : this.localUserMedia.pipe( - switchMap((vm) => - vm.alwaysShow.pipe( - map((alwaysShow) => (alwaysShow ? vm : null)), + speaker + ? speaker.local + ? of(null) + : this.localUserMedia.pipe( + switchMap((vm) => + vm.alwaysShow.pipe( + map((alwaysShow) => (alwaysShow ? vm : null)), + ), ), - ), - ), + ) + : of(null), ), ), ] as const), @@ -843,7 +903,7 @@ export class CallViewModel extends ViewModel { public constructor( // A call is permanently tied to a single Matrix room and LiveKit room - private readonly matrixRoom: MatrixRoom, + private readonly matrixRTCSession: MatrixRTCSession, private readonly livekitRoom: LivekitRoom, private readonly encryptionSystem: EncryptionSystem, private readonly connectionState: Observable, diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 51a821af1..32c092572 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -68,33 +68,50 @@ export function useDisplayName(vm: MediaViewModel): string { } export function observeTrackReference( - participant: Participant, + participant: Observable, source: Track.Source, -): Observable { - return observeParticipantMedia(participant).pipe( - map(() => ({ - participant, - publication: participant.getTrackPublication(source), - source, - })), - distinctUntilKeyChanged("publication"), +): Observable { + const obs = participant.pipe( + switchMap((p) => { + if (p) { + return observeParticipantMedia(p).pipe( + map(() => ({ + participant: p, + publication: p.getTrackPublication(source), + source, + })), + distinctUntilKeyChanged("publication"), + ); + } else { + return of(undefined); + } + }), ); + return obs; } abstract class BaseMediaViewModel extends ViewModel { /** * Whether the media belongs to the local user. */ - public readonly local = this.participant.isLocal; + public readonly local = this.participant.pipe( + // We can assume, that the user is not local if the participant is undefined + // We assume the local LK participant will always be available. + map((p) => p?.isLocal ?? false), + ); /** * The LiveKit video track for this media. */ - public readonly video: Observable; + public readonly video: Observable; /** * Whether there should be a warning that this media is unencrypted. */ public readonly unencryptedWarning: Observable; + public readonly isRTCParticipantAvailable = this.participant.pipe( + map((p) => !!p), + ); + public constructor( /** * An opaque identifier for this media. @@ -106,7 +123,12 @@ abstract class BaseMediaViewModel extends ViewModel { // TODO: Fully separate the data layer from the UI layer by keeping the // member object internal public readonly member: RoomMember | undefined, - protected readonly participant: LocalParticipant | RemoteParticipant, + // We dont necassarly have a participant if a user connects via MatrixRTC but not (not yet) through + // livekit. + protected readonly participant: Observable< + LocalParticipant | RemoteParticipant | undefined + >, + encryptionSystem: EncryptionSystem, audioSource: AudioSource, videoSource: VideoSource, @@ -122,8 +144,8 @@ abstract class BaseMediaViewModel extends ViewModel { [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), ).pipe(this.scope.state()); } } @@ -143,12 +165,20 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { /** * Whether the participant is speaking. */ - public readonly speaking = observeParticipantEvents( - this.participant, - ParticipantEvent.IsSpeakingChanged, - ).pipe( - map((p) => p.isSpeaking), - this.scope.state(), + public readonly speaking = this.participant.pipe( + switchMap((p) => { + if (p) { + return observeParticipantEvents( + p, + ParticipantEvent.IsSpeakingChanged, + ).pipe( + map((p) => p.isSpeaking), + this.scope.state(), + ); + } else { + return of(false); + } + }), ); /** @@ -169,7 +199,7 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { public constructor( id: string, member: RoomMember | undefined, - participant: LocalParticipant | RemoteParticipant, + participant: Observable, encryptionSystem: EncryptionSystem, ) { super( @@ -181,12 +211,17 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { Track.Source.Camera, ); - const media = observeParticipantMedia(participant).pipe(this.scope.state()); + // const media = observeParticipantMedia(participant).pipe(this.scope.state()); + + const media = participant.pipe( + switchMap((p) => (p && observeParticipantMedia(p)) ?? of(undefined)), + this.scope.state(), + ); this.audioEnabled = media.pipe( - map((m) => m.microphoneTrack?.isMuted === false), + map((m) => m?.microphoneTrack?.isMuted === false), ); this.videoEnabled = media.pipe( - map((m) => m.cameraTrack?.isMuted === false), + map((m) => m?.cameraTrack?.isMuted === false), ); } @@ -204,7 +239,7 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { */ public readonly mirror = this.video.pipe( switchMap((v) => { - const track = v.publication?.track; + const track = v?.publication?.track; if (!(track instanceof LocalTrack)) return of(false); // Watch for track restarts, because they indicate a camera switch return fromEvent(track, TrackEvent.Restarted).pipe( @@ -226,7 +261,7 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { public constructor( id: string, member: RoomMember | undefined, - participant: LocalParticipant, + participant: Observable, encryptionSystem: EncryptionSystem, ) { super(id, member, participant, encryptionSystem); @@ -286,17 +321,16 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { public constructor( id: string, member: RoomMember | undefined, - participant: RemoteParticipant, + participant: Observable, encryptionSystem: EncryptionSystem, ) { super(id, member, participant, encryptionSystem); // Sync the local volume with LiveKit - this.localVolume - .pipe(this.scope.bind()) - .subscribe((volume) => - (this.participant as RemoteParticipant).setVolume(volume), - ); + combineLatest([ + participant, + this.localVolume.pipe(this.scope.bind()), + ]).subscribe(([p, volume]) => p && p.setVolume(volume)); } public toggleLocallyMuted(): void { @@ -319,7 +353,7 @@ export class ScreenShareViewModel extends BaseMediaViewModel { public constructor( id: string, member: RoomMember | undefined, - participant: LocalParticipant | RemoteParticipant, + participant: Observable, encryptionSystem: EncryptionSystem, ) { super( From 3f233a65551106052ea2f10ee64f60383a0f1174 Mon Sep 17 00:00:00 2001 From: Timo Date: Wed, 30 Oct 2024 19:40:14 +0100 Subject: [PATCH 02/84] display missing lk participant + fix tile multiplier --- src/state/CallViewModel.ts | 7 ++----- src/tile/GridTile.tsx | 12 +++++++++++- src/tile/MediaView.tsx | 4 ++-- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 734d24102..786e51383 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -406,10 +406,7 @@ export class CallViewModel extends ViewModel { const indexedMediaId = `${mediaId}:${i}`; const prevMedia = prevItems.get(indexedMediaId); if (prevMedia && prevMedia instanceof UserMedia) { - if ( - prevMedia.participant.value === undefined && - participant !== undefined - ) { + if (prevMedia.participant.value !== participant) { // Update the BahviourSubject in the UserMedia. prevMedia.participant.next(participant); } @@ -421,7 +418,7 @@ export class CallViewModel extends ViewModel { // Once a participant appears we will update the BehaviourSubject. (see above) prevMedia ?? new UserMedia( - mediaId, + indexedMediaId, member, participant, this.encryptionSystem, diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 3675e9a7c..0cbe1d258 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -84,6 +84,9 @@ const UserMediaTile = forwardRef( const videoEnabled = useObservableEagerState(vm.videoEnabled); const speaking = useObservableEagerState(vm.speaking); const cropVideo = useObservableEagerState(vm.cropVideo); + const isRTCParticipantAvailable = useObservableEagerState( + vm.isRTCParticipantAvailable, + ); const onSelectFitContain = useCallback( (e: Event) => { e.preventDefault(); @@ -134,7 +137,10 @@ const UserMediaTile = forwardRef( className={styles.muteIcon} /> } - displayName={displayName} + displayName={ + displayName + + (isRTCParticipantAvailable ? "" : " missing Livekit Participant...") + } primaryButton={ { e.preventDefault(); @@ -248,6 +255,9 @@ const RemoteUserMediaTile = forwardRef< mirror={false} menuStart={ <> + {/* {isRTCParticipantAvailable + ? "is available" + : "Loading RTC participant"} */} { style?: ComponentProps["style"]; targetWidth: number; targetHeight: number; - video: TrackReferenceOrPlaceholder; + video: TrackReferenceOrPlaceholder | undefined; videoFit: "cover" | "contain"; mirror: boolean; member: RoomMember | undefined; @@ -83,7 +83,7 @@ export const MediaView = forwardRef( src={member?.getMxcAvatarUrl()} className={styles.avatar} /> - {video.publication !== undefined && ( + {video?.publication !== undefined && ( Date: Wed, 30 Oct 2024 20:10:06 +0100 Subject: [PATCH 03/84] add show_non_member_participants config option --- src/config/ConfigOptions.ts | 2 ++ src/state/CallViewModel.ts | 52 ++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index 65f04c958..6718fc34f 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -112,6 +112,7 @@ export interface ResolvedConfigOptions extends ConfigOptions { enable_video: boolean; }; app_prompt: boolean; + show_non_member_participants: boolean; } export const DEFAULT_CONFIG: ResolvedConfigOptions = { @@ -127,4 +128,5 @@ export const DEFAULT_CONFIG: ResolvedConfigOptions = { enable_video: true, }, app_prompt: true, + show_non_member_participants: false, }; diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 786e51383..125e473c7 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -72,6 +72,7 @@ import { duplicateTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { EncryptionSystem } from "../e2ee/sharedKeyManagement"; +import { Config } from "../config/Config"; // How long we wait after a focus switch before showing the real participant // list again @@ -442,7 +443,56 @@ export class CallViewModel extends ViewModel { }.bind(this)(), ); - for (const [id, t] of prevItems) if (!newItems.has(id)) t.destroy(); + // Generate non member items (items without a corresponding MatrixRTC member) + // Those items should not be rendered, they are participants in livekit that do not have a corresponding + // matrix rtc members. This cannot be any good: + // - A malicious user impersonates someone + // - Someone injects abusive content + // - The user cannot have encryption keys so it makes no sense to participate + // We can only trust users that have a matrixRTC member event. + // + // This is still available as a debug option. This can be useful + // - If one wants to test scalability using the livekit cli. + // - If an experimental project does not yet do the matrixRTC bits. + // - If someone wants to debug if the LK connection works but matrixRTC room state failed to arrive. + const debugShowNonMember = Config.get().show_non_member_participants; + const newNonMemberItems = debugShowNonMember + ? new Map( + function* (this: CallViewModel): Iterable<[string, MediaItem]> { + for (let p = 0; p < remoteParticipants.length; p++) { + for (let i = 0; i < 1 + duplicateTiles; i++) { + const participant = remoteParticipants[p]; + const maybeNoMemberParticipantId = + participant.identity + ":" + i; + if (!newItems.has(maybeNoMemberParticipantId)) { + yield [ + maybeNoMemberParticipantId, + // We create UserMedia with or without a participant. + // This will be the initial value of a BehaviourSubject. + // Once a participant appears we will update the BehaviourSubject. (see above) + prevItems.get(maybeNoMemberParticipantId) ?? + new UserMedia( + maybeNoMemberParticipantId, + undefined, + participant, + this.encrypted, + ), + ]; + } + } + } + }.bind(this)(), + ) + : new Map(); + if (newNonMemberItems.size > 0) { + logger.debug("Added NonMember items: ", newNonMemberItems); + } + const combinedNew = new Map([ + ...newNonMemberItems.entries(), + ...newItems.entries(), + ]); + + for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy(); return newItems; }, new Map(), From e1e202d7c8d2b93df5d5a5a7dae1bf625761ef67 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 4 Nov 2024 12:23:51 +0100 Subject: [PATCH 04/84] per member tiles --- public/locales/en-GB/app.json | 1 + src/App.tsx | 9 +++++++ src/Header.tsx | 5 +++- src/config/ConfigOptions.ts | 4 ++-- src/room/InCallView.tsx | 2 ++ src/settings/SettingsModal.tsx | 17 +++++++++++++ src/settings/settings.ts | 2 ++ src/state/CallViewModel.ts | 44 ++++++++++++++++++++++------------ src/state/MediaViewModel.ts | 29 +++++++++++++++++++--- src/tile/GridTile.tsx | 2 ++ src/tile/MediaView.tsx | 9 +++++++ 11 files changed, 103 insertions(+), 21 deletions(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 02dd77401..62274c216 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -146,6 +146,7 @@ "feedback_tab_thank_you": "Thanks, we received your feedback!", "feedback_tab_title": "Feedback", "more_tab_title": "More", + "non_member_tiles": "Show non member tiles", "opt_in_description": "<0><1>You may withdraw consent by unchecking this box. If you are currently in a call, this setting will take effect at the end of the call.", "preferences_tab_body": "Here you can configure extra options for an improved experience", "preferences_tab_h4": "Preferences", diff --git a/src/App.tsx b/src/App.tsx index 8d841dba7..1bc23be83 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -28,6 +28,8 @@ import { Initializer } from "./initializer"; import { MediaDevicesProvider } from "./livekit/MediaDevicesContext"; import { widget } from "./widget"; import { useTheme } from "./useTheme"; +import { nonMemberTiles } from "./settings/settings"; +import { Config } from "./config/Config"; const SentryRoute = Sentry.withSentryRouting(Route); @@ -71,6 +73,13 @@ export const App: FC = ({ history }) => { .catch(logger.error); }); + // Update settings to use the non member tile information from the config if set + useEffect(() => { + if (loaded && Config.get().show_non_member_tiles) { + nonMemberTiles.setValue(true); + } + }); + const errorPage = ; return ( diff --git a/src/Header.tsx b/src/Header.tsx index 69e77935c..c17a0288a 100644 --- a/src/Header.tsx +++ b/src/Header.tsx @@ -117,6 +117,7 @@ interface RoomHeaderInfoProps { avatarUrl: string | null; encrypted: boolean; participantCount: number | null; + nonMemberItemCount: number | null; } export const RoomHeaderInfo: FC = ({ @@ -125,6 +126,7 @@ export const RoomHeaderInfo: FC = ({ avatarUrl, encrypted, participantCount, + nonMemberItemCount, }) => { const { t } = useTranslation(); const size = useMediaQuery("(max-width: 550px)") ? "sm" : "lg"; @@ -157,7 +159,8 @@ export const RoomHeaderInfo: FC = ({ aria-label={t("header_participants_label")} /> - {t("participant_count", { count: participantCount ?? 0 })} + {t("participant_count", { count: participantCount ?? 0 })}{" "} + {(nonMemberItemCount ?? 0) > 0 && <>(+ {nonMemberItemCount})} )} diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index 6718fc34f..c8f6d3e9c 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -112,7 +112,7 @@ export interface ResolvedConfigOptions extends ConfigOptions { enable_video: boolean; }; app_prompt: boolean; - show_non_member_participants: boolean; + show_non_member_tiles: boolean; } export const DEFAULT_CONFIG: ResolvedConfigOptions = { @@ -128,5 +128,5 @@ export const DEFAULT_CONFIG: ResolvedConfigOptions = { enable_video: true, }, app_prompt: true, - show_non_member_participants: false, + show_non_member_tiles: false, }; diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index a5847f0ef..66ef263d6 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -194,6 +194,7 @@ export const InCallView: FC = ({ } }, [connState, onLeave]); + const nonMemberItemCount = useObservableEagerState(vm.nonMemberItemCount); const containerRef1 = useRef(null); const [containerRef2, bounds] = useMeasure(); const boundsValid = bounds.height > 0; @@ -633,6 +634,7 @@ export const InCallView: FC = ({ avatarUrl={matrixInfo.roomAvatar} encrypted={matrixInfo.e2eeSystem.kind !== E2eeType.NONE} participantCount={participantCount} + nonMemberItemCount={nonMemberItemCount} /> diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index db702ef8f..6ef40c890 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -27,6 +27,7 @@ import { useSetting, developerSettingsTab as developerSettingsTabSetting, duplicateTiles as duplicateTilesSetting, + nonMemberTiles as nonMemberTilesSetting, useOptInAnalytics, } from "./settings"; import { isFirefox } from "../Platform"; @@ -68,6 +69,8 @@ export const SettingsModal: FC = ({ ); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); + const [nonMemberTiles, setNonMemberTiles] = useSetting(nonMemberTilesSetting); + // Generate a `SelectInput` with a list of devices for a given device kind. const generateDeviceSelection = ( devices: MediaDevice, @@ -236,6 +239,20 @@ export const SettingsModal: FC = ({ )} /> + + ): void => { + setNonMemberTiles(event.target.checked); + }, + [setNonMemberTiles], + )} + /> + ), }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 109a882b2..293d5d590 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -72,6 +72,8 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); +export const nonMemberTiles = new Setting("non-member-tiles", true); + export const audioInput = new Setting( "audio-input", undefined, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 125e473c7..cc5f05444 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -68,7 +68,7 @@ import { } from "./MediaViewModel"; import { accumulate, finalizeValue } from "../utils/observable"; import { ObservableScope } from "./ObservableScope"; -import { duplicateTiles } from "../settings/settings"; +import { duplicateTiles, nonMemberTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { EncryptionSystem } from "../e2ee/sharedKeyManagement"; @@ -177,6 +177,7 @@ class UserMedia { member: RoomMember | undefined, participant: LocalParticipant | RemoteParticipant | undefined, encryptionSystem: EncryptionSystem, + rtcSession: MatrixRTCSession, ) { this.participant = new BehaviorSubject(participant); @@ -186,6 +187,7 @@ class UserMedia { member, this.participant.asObservable() as Observable, encryptionSystem, + rtcSession, ); } else { this.vm = new RemoteUserMediaViewModel( @@ -195,6 +197,7 @@ class UserMedia { RemoteParticipant | undefined >, encryptionSystem, + rtcSession, ); } @@ -362,6 +365,7 @@ export class CallViewModel extends ViewModel { }, ); + public readonly nonMemberItemCount = new BehaviorSubject(0); private readonly mediaItems: Observable = combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -371,15 +375,18 @@ export class CallViewModel extends ViewModel { this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, ).pipe(startWith(null)), - // fromEvent( - // this.matrixRTCSession, - // MatrixRTCSessionEvent.EncryptionKeyChanged, - // ).pipe(startWith(null)), + nonMemberTiles.value, ]).pipe( scan( ( prevItems, - [remoteParticipants, { participant: localParticipant }, duplicateTiles], + [ + remoteParticipants, + { participant: localParticipant }, + duplicateTiles, + _participantChange, + nonMemberTiles, + ], ) => { const newItems = new Map( function* (this: CallViewModel): Iterable<[string, MediaItem]> { @@ -423,6 +430,7 @@ export class CallViewModel extends ViewModel { member, participant, this.encryptionSystem, + this.matrixRTCSession, ), ]; } @@ -455,27 +463,28 @@ export class CallViewModel extends ViewModel { // - If one wants to test scalability using the livekit cli. // - If an experimental project does not yet do the matrixRTC bits. // - If someone wants to debug if the LK connection works but matrixRTC room state failed to arrive. - const debugShowNonMember = Config.get().show_non_member_participants; + const debugShowNonMember = nonMemberTiles; //Config.get().show_non_member_tiles; const newNonMemberItems = debugShowNonMember ? new Map( function* (this: CallViewModel): Iterable<[string, MediaItem]> { - for (let p = 0; p < remoteParticipants.length; p++) { + for (const participant of remoteParticipants) { for (let i = 0; i < 1 + duplicateTiles; i++) { - const participant = remoteParticipants[p]; - const maybeNoMemberParticipantId = + const maybeNonMemberParticipantId = participant.identity + ":" + i; - if (!newItems.has(maybeNoMemberParticipantId)) { + if (!newItems.has(maybeNonMemberParticipantId)) { + const nonMemberId = maybeNonMemberParticipantId; yield [ - maybeNoMemberParticipantId, + nonMemberId, // We create UserMedia with or without a participant. // This will be the initial value of a BehaviourSubject. // Once a participant appears we will update the BehaviourSubject. (see above) - prevItems.get(maybeNoMemberParticipantId) ?? + prevItems.get(nonMemberId) ?? new UserMedia( - maybeNoMemberParticipantId, + nonMemberId, undefined, participant, this.encrypted, + this.matrixRTCSession, ), ]; } @@ -487,13 +496,18 @@ export class CallViewModel extends ViewModel { if (newNonMemberItems.size > 0) { logger.debug("Added NonMember items: ", newNonMemberItems); } + const newNonMemberItemCount = + newNonMemberItems.size / (1 + duplicateTiles); + if (this.nonMemberItemCount.value !== newNonMemberItemCount) + this.nonMemberItemCount.next(newNonMemberItemCount); + const combinedNew = new Map([ ...newNonMemberItems.entries(), ...newItems.entries(), ]); for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy(); - return newItems; + return combinedNew; }, new Map(), ), diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 32c092572..c83d6c08a 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -37,6 +37,11 @@ import { switchMap, } from "rxjs"; import { useEffect } from "react"; +import { + MatrixRTCSession, + MatrixRTCSessionEvent, +} from "matrix-js-sdk/src/matrixrtc"; +import { logger } from "matrix-js-sdk/src/logger"; import { ViewModel } from "./ViewModel"; import { useReactiveState } from "../useReactiveState"; @@ -196,11 +201,16 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { */ public readonly cropVideo: Observable = this._cropVideo; + public readonly keys = new BehaviorSubject( + [] as { index: number; key: Uint8Array }[], + ); + public constructor( id: string, member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, + rtcSession: MatrixRTCSession, ) { super( id, @@ -211,7 +221,18 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { Track.Source.Camera, ); - // const media = observeParticipantMedia(participant).pipe(this.scope.state()); + // rtcSession.on( + // MatrixRTCSessionEvent.EncryptionKeyChanged, + // (key, index, participantId) => { + // if (id.startsWith(participantId)) + // logger.info("got new keys: ", participant, { index, key }); + // logger.info("All keys for participant ", participant, " - ", [ + // ...this.keys.value, + // { index, key }, + // ]); + // this.keys.next([...this.keys.value, { index, key }]); + // }, + // ); const media = participant.pipe( switchMap((p) => (p && observeParticipantMedia(p)) ?? of(undefined)), @@ -263,8 +284,9 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, + rtcSession: MatrixRTCSession, ) { - super(id, member, participant, encryptionSystem); + super(id, member, participant, encryptionSystem, rtcSession); } } @@ -323,8 +345,9 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, + rtcSession: MatrixRTCSession, ) { - super(id, member, participant, encryptionSystem); + super(id, member, participant, encryptionSystem, rtcSession); // Sync the local volume with LiveKit combineLatest([ diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 0cbe1d258..980cb4f26 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -84,6 +84,7 @@ const UserMediaTile = forwardRef( const videoEnabled = useObservableEagerState(vm.videoEnabled); const speaking = useObservableEagerState(vm.speaking); const cropVideo = useObservableEagerState(vm.cropVideo); + const keys = useObservableEagerState(vm.keys); const isRTCParticipantAvailable = useObservableEagerState( vm.isRTCParticipantAvailable, ); @@ -121,6 +122,7 @@ const UserMediaTile = forwardRef( ref={ref} video={video} member={vm.member} + keys={keys} unencryptedWarning={unencryptedWarning} videoEnabled={videoEnabled && showVideo} videoFit={cropVideo ? "cover" : "contain"} diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 6cc9086f6..a8ec58c94 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -29,6 +29,7 @@ interface Props extends ComponentProps { videoFit: "cover" | "contain"; mirror: boolean; member: RoomMember | undefined; + keys: { index: number; key: Uint8Array }[]; videoEnabled: boolean; unencryptedWarning: boolean; nameTagLeadingIcon?: ReactNode; @@ -48,6 +49,7 @@ export const MediaView = forwardRef( videoFit, mirror, member, + keys, videoEnabled, unencryptedWarning, nameTagLeadingIcon, @@ -98,11 +100,18 @@ export const MediaView = forwardRef( minature={avatarSize < 96} showTimer={handRaiseTimerVisible} /> + {/* {keys && + keys.map(({ index, key }) => ( + + index:{index}, key:{key} + + ))} */}
{nameTagLeadingIcon} {displayName} + {unencryptedWarning && ( Date: Mon, 4 Nov 2024 15:38:17 +0100 Subject: [PATCH 05/84] merge fixes --- src/state/CallViewModel.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index cc5f05444..a3455af89 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -72,7 +72,6 @@ import { duplicateTiles, nonMemberTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { EncryptionSystem } from "../e2ee/sharedKeyManagement"; -import { Config } from "../config/Config"; // How long we wait after a focus switch before showing the real participant // list again @@ -295,11 +294,11 @@ function findMatrixRoomMember( export class CallViewModel extends ViewModel { public readonly localVideo: Observable = observeTrackReference( - this.livekitRoom.localParticipant, + of(this.livekitRoom.localParticipant), Track.Source.Camera, ).pipe( map((trackRef) => { - const track = trackRef.publication?.track; + const track = trackRef?.publication?.track; return track instanceof LocalVideoTrack ? track : null; }), ); @@ -366,6 +365,7 @@ export class CallViewModel extends ViewModel { ); public readonly nonMemberItemCount = new BehaviorSubject(0); + private readonly mediaItems: Observable = combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -409,15 +409,25 @@ export class CallViewModel extends ViewModel { } const member = findMatrixRoomMember(room, mediaId); - + if (!member) { + logger.error("Could not find member for media id: ", mediaId); + } for (let i = 0; i < 1 + duplicateTiles; i++) { const indexedMediaId = `${mediaId}:${i}`; - const prevMedia = prevItems.get(indexedMediaId); + let prevMedia = prevItems.get(indexedMediaId); if (prevMedia && prevMedia instanceof UserMedia) { if (prevMedia.participant.value !== participant) { // Update the BahviourSubject in the UserMedia. prevMedia.participant.next(participant); } + if (prevMedia.vm.member === undefined) { + // We have a previous media created because of the `debugShowNonMember` flag. + // In this case we actually replace the media item. + // This "hack" never occurs if we do not use the `debugShowNonMember` debugging + // option and if we always find a room member for each rtc member (which also + // only fails if we have a fundamental problem) + prevMedia = undefined; + } } yield [ indexedMediaId, @@ -483,7 +493,7 @@ export class CallViewModel extends ViewModel { nonMemberId, undefined, participant, - this.encrypted, + this.encryptionSystem, this.matrixRTCSession, ), ]; From 1ab4d50bab24d8c3c49126a4ab16ebe5eac673e2 Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 4 Nov 2024 16:29:19 +0100 Subject: [PATCH 06/84] linter --- src/state/MediaViewModel.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index c83d6c08a..329536511 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -37,11 +37,7 @@ import { switchMap, } from "rxjs"; import { useEffect } from "react"; -import { - MatrixRTCSession, - MatrixRTCSessionEvent, -} from "matrix-js-sdk/src/matrixrtc"; -import { logger } from "matrix-js-sdk/src/logger"; +import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; import { ViewModel } from "./ViewModel"; import { useReactiveState } from "../useReactiveState"; From 14919cae13aef9baa962473828a04e0252ea20ed Mon Sep 17 00:00:00 2001 From: Timo Date: Mon, 4 Nov 2024 16:36:13 +0100 Subject: [PATCH 07/84] linter and tests --- src/room/InCallView.tsx | 7 +------ src/utils/test.ts | 11 ++++++++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 66ef263d6..ab54f856b 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -126,12 +126,7 @@ export const ActiveCall: FC = (props) => { setVm(vm); return (): void => vm.destroy(); } - }, [ - props.rtcSession, - livekitRoom, - props.e2eeSystem, - connStateObservable, - ]); + }, [props.rtcSession, livekitRoom, props.e2eeSystem, connStateObservable]); if (livekitRoom === undefined || vm === null) return null; diff --git a/src/utils/test.ts b/src/utils/test.ts index 85e72d388..dbe3cb466 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -4,7 +4,7 @@ Copyright 2023, 2024 New Vector Ltd. SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { map } from "rxjs"; +import { map, of } from "rxjs"; import { RunHelpers, TestScheduler } from "rxjs/testing"; import { expect, vi } from "vitest"; import { RoomMember, Room as MatrixRoom } from "matrix-js-sdk/src/matrix"; @@ -15,6 +15,7 @@ import { RemoteTrackPublication, Room as LivekitRoom, } from "livekit-client"; +import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; import { LocalUserMediaViewModel, @@ -117,15 +118,17 @@ export function mockLocalParticipant( export async function withLocalMedia( member: Partial, + rtcSession: MatrixRTCSession, continuation: (vm: LocalUserMediaViewModel) => void | Promise, ): Promise { const vm = new LocalUserMediaViewModel( "local", mockMember(member), - mockLocalParticipant({}), + of(mockLocalParticipant({})), { kind: E2eeType.PER_PARTICIPANT, }, + rtcSession, ); try { await continuation(vm); @@ -150,15 +153,17 @@ export function mockRemoteParticipant( export async function withRemoteMedia( member: Partial, participant: Partial, + rtcSession: MatrixRTCSession, continuation: (vm: RemoteUserMediaViewModel) => void | Promise, ): Promise { const vm = new RemoteUserMediaViewModel( "remote", mockMember(member), - mockRemoteParticipant(participant), + of(mockRemoteParticipant(participant)), { kind: E2eeType.PER_PARTICIPANT, }, + rtcSession, ); try { await continuation(vm); From b5208ffeae1c409eb1b99a6da075b25776b54fea Mon Sep 17 00:00:00 2001 From: Timo Date: Tue, 5 Nov 2024 12:55:48 +0100 Subject: [PATCH 08/84] tests --- src/state/CallViewModel.ts | 5 ----- src/state/MediaViewModel.ts | 21 ++------------------- src/utils/test.ts | 5 ----- 3 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a3455af89..1d709df50 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -176,7 +176,6 @@ class UserMedia { member: RoomMember | undefined, participant: LocalParticipant | RemoteParticipant | undefined, encryptionSystem: EncryptionSystem, - rtcSession: MatrixRTCSession, ) { this.participant = new BehaviorSubject(participant); @@ -186,7 +185,6 @@ class UserMedia { member, this.participant.asObservable() as Observable, encryptionSystem, - rtcSession, ); } else { this.vm = new RemoteUserMediaViewModel( @@ -196,7 +194,6 @@ class UserMedia { RemoteParticipant | undefined >, encryptionSystem, - rtcSession, ); } @@ -440,7 +437,6 @@ export class CallViewModel extends ViewModel { member, participant, this.encryptionSystem, - this.matrixRTCSession, ), ]; } @@ -494,7 +490,6 @@ export class CallViewModel extends ViewModel { undefined, participant, this.encryptionSystem, - this.matrixRTCSession, ), ]; } diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 329536511..31d0dcc78 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -37,7 +37,6 @@ import { switchMap, } from "rxjs"; import { useEffect } from "react"; -import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; import { ViewModel } from "./ViewModel"; import { useReactiveState } from "../useReactiveState"; @@ -206,7 +205,6 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, - rtcSession: MatrixRTCSession, ) { super( id, @@ -217,19 +215,6 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { Track.Source.Camera, ); - // rtcSession.on( - // MatrixRTCSessionEvent.EncryptionKeyChanged, - // (key, index, participantId) => { - // if (id.startsWith(participantId)) - // logger.info("got new keys: ", participant, { index, key }); - // logger.info("All keys for participant ", participant, " - ", [ - // ...this.keys.value, - // { index, key }, - // ]); - // this.keys.next([...this.keys.value, { index, key }]); - // }, - // ); - const media = participant.pipe( switchMap((p) => (p && observeParticipantMedia(p)) ?? of(undefined)), this.scope.state(), @@ -280,9 +265,8 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, - rtcSession: MatrixRTCSession, ) { - super(id, member, participant, encryptionSystem, rtcSession); + super(id, member, participant, encryptionSystem); } } @@ -341,9 +325,8 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { member: RoomMember | undefined, participant: Observable, encryptionSystem: EncryptionSystem, - rtcSession: MatrixRTCSession, ) { - super(id, member, participant, encryptionSystem, rtcSession); + super(id, member, participant, encryptionSystem); // Sync the local volume with LiveKit combineLatest([ diff --git a/src/utils/test.ts b/src/utils/test.ts index dbe3cb466..d2014afb3 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -15,7 +15,6 @@ import { RemoteTrackPublication, Room as LivekitRoom, } from "livekit-client"; -import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; import { LocalUserMediaViewModel, @@ -118,7 +117,6 @@ export function mockLocalParticipant( export async function withLocalMedia( member: Partial, - rtcSession: MatrixRTCSession, continuation: (vm: LocalUserMediaViewModel) => void | Promise, ): Promise { const vm = new LocalUserMediaViewModel( @@ -128,7 +126,6 @@ export async function withLocalMedia( { kind: E2eeType.PER_PARTICIPANT, }, - rtcSession, ); try { await continuation(vm); @@ -153,7 +150,6 @@ export function mockRemoteParticipant( export async function withRemoteMedia( member: Partial, participant: Partial, - rtcSession: MatrixRTCSession, continuation: (vm: RemoteUserMediaViewModel) => void | Promise, ): Promise { const vm = new RemoteUserMediaViewModel( @@ -163,7 +159,6 @@ export async function withRemoteMedia( { kind: E2eeType.PER_PARTICIPANT, }, - rtcSession, ); try { await continuation(vm); From e64204d8976e3c041686ba91a0322eeef216fa21 Mon Sep 17 00:00:00 2001 From: Timo Date: Tue, 5 Nov 2024 17:15:50 +0100 Subject: [PATCH 09/84] adapt tests (wip) --- src/state/CallViewModel.test.ts | 61 ++++++++++++++++++++++++--------- src/useReactions.test.tsx | 9 +++-- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 142144076..4adfb291f 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -14,6 +14,11 @@ import { RemoteParticipant, } from "livekit-client"; import * as ComponentsCore from "@livekit/components-core"; +import { + CallMembership, + MatrixRTCSession, + MatrixRTCSessionEvent, +} from "matrix-js-sdk/src/matrixrtc"; import { CallViewModel, Layout } from "./CallViewModel"; import { @@ -30,25 +35,33 @@ import { ECConnectionState, } from "../livekit/useECConnectionState"; import { E2eeType } from "../e2ee/e2eeType"; +import { MockRoom, MockRTCSession } from "../useReactions.test"; vi.mock("@livekit/components-core"); -const aliceId = "@alice:example.org:AAAA"; -const bobId = "@bob:example.org:BBBB"; +const carolId = "@carol:example.org"; +const carolDev = "CCCC"; +const aliceId = "@alice:example.org"; +const aliceDev = "AAAA"; +const aliceRTCId = aliceId + ":" + aliceDev; + +const bobId = "@bob:example.org"; +const bobDev = "BBBB"; +const bobRTCId = bobId + ":" + bobDev; -const alice = mockMember({ userId: "@alice:example.org" }); -const bob = mockMember({ userId: "@bob:example.org" }); -const carol = mockMember({ userId: "@carol:example.org" }); +const alice = mockMember({ userId: aliceId }); +const bob = mockMember({ userId: bobId }); +const carol = mockMember({ userId: carolId }); const localParticipant = mockLocalParticipant({ identity: "" }); -const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); +const aliceParticipant = mockRemoteParticipant({ identity: aliceRTCId }); const aliceSharingScreen = mockRemoteParticipant({ - identity: aliceId, + identity: aliceRTCId, isScreenShareEnabled: true, }); -const bobParticipant = mockRemoteParticipant({ identity: bobId }); +const bobParticipant = mockRemoteParticipant({ identity: bobRTCId }); const bobSharingScreen = mockRemoteParticipant({ - identity: bobId, + identity: bobRTCId, isScreenShareEnabled: true, }); @@ -58,6 +71,9 @@ const members = new Map([ [carol.userId, carol], ]); +const rtcMemberAlice = { sender: aliceId, deviceId: aliceDev }; +const rtcMemberBob = { sender: bobId, deviceId: bobDev }; +const rtcMemberCarol = { sender: carolId, deviceId: carolDev }; export interface GridLayoutSummary { type: "grid"; spotlight?: string[]; @@ -132,9 +148,22 @@ function summarizeLayout(l: Layout): LayoutSummary { function withCallViewModel( { cold }: OurRunHelpers, remoteParticipants: Observable, + rtcMembers: Observable[]>, connectionState: Observable, continuation: (vm: CallViewModel) => void, ): void { + const room = mockMatrixRoom({ + client: { + getUserId: () => carolId, + getDeviceId: () => carolDev, + } as Partial as MatrixClient, + getMember: (userId) => members.get(userId) ?? null, + }); + const rtcSession = new MockRTCSession(room as unknown as MockRoom); + rtcMembers.subscribe((m) => { + rtcSession.memberships = m; + rtcSession.emit(MatrixRTCSessionEvent.MembershipsChanged); + }); const participantsSpy = vi .spyOn(ComponentsCore, "connectedParticipantsObserver") .mockReturnValue(remoteParticipants); @@ -152,12 +181,7 @@ function withCallViewModel( .mockImplementation((p) => cold("a", { a: p })); const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + rtcSession as unknown as MatrixRTCSession, mockLivekitRoom({ localParticipant }), { kind: E2eeType.PER_PARTICIPANT, @@ -180,6 +204,8 @@ test("participants are retained during a focus switch", () => { const { hot, expectObservable } = helpers; // Participants disappear on frame 2 and come back on frame 3 const partMarbles = "a-ba"; + // The RTC members never disappear + const rtcMemberMarbles = "a---"; // Start switching focus on frame 1 and reconnect on frame 3 const connMarbles = "ab-a"; // The visible participants should remain the same throughout the switch @@ -191,6 +217,9 @@ test("participants are retained during a focus switch", () => { a: [aliceParticipant, bobParticipant], b: [], }), + hot(rtcMemberMarbles, { + a: [rtcMemberAlice, rtcMemberBob, rtcMemberCarol], + }), hot(connMarbles, { a: ConnectionState.Connected, b: ECAddonConnectionState.ECSwitchingFocus, @@ -202,7 +231,7 @@ test("participants are retained during a focus switch", () => { a: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceRTCId}:0`, `${bobRTCId}:0`], }, }, ); diff --git a/src/useReactions.test.tsx b/src/useReactions.test.tsx index 79caeb0af..31962a953 100644 --- a/src/useReactions.test.tsx +++ b/src/useReactions.test.tsx @@ -23,6 +23,7 @@ import { } from "matrix-js-sdk/src/matrix"; import EventEmitter from "events"; import { randomUUID } from "crypto"; +import { CallMembership } from "matrix-js-sdk/src/matrixrtc"; import { ReactionsProvider, useReactions } from "./useReactions"; @@ -74,10 +75,12 @@ const TestComponentWrapper = ({ }; export class MockRTCSession extends EventEmitter { - public memberships = Object.entries(membership).map(([eventId, sender]) => ({ + public memberships: Partial[] = Object.entries( + membership, + ).map(([eventId, sender]) => ({ sender, eventId, - createdTs: (): Date => new Date(), + createdTs: (): number => Date.now(), })); public constructor(public readonly room: MockRoom) { @@ -93,7 +96,7 @@ export class MockRTCSession extends EventEmitter { this.memberships.push({ sender, eventId: `!fake-${randomUUID()}:event`, - createdTs: (): Date => new Date(), + createdTs: (): number => Date.now(), }); this.emit(MatrixRTCSessionEvent.MembershipsChanged); } From bb0febf16c12e3c5b5264b8a43099806241e2d54 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Tue, 5 Nov 2024 18:32:03 +0000 Subject: [PATCH 10/84] Remove unused keys --- src/state/MediaViewModel.ts | 4 ---- src/tile/GridTile.tsx | 2 -- src/tile/MediaView.tsx | 2 -- 3 files changed, 8 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 31d0dcc78..4edc9b8e2 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -196,10 +196,6 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { */ public readonly cropVideo: Observable = this._cropVideo; - public readonly keys = new BehaviorSubject( - [] as { index: number; key: Uint8Array }[], - ); - public constructor( id: string, member: RoomMember | undefined, diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 980cb4f26..0cbe1d258 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -84,7 +84,6 @@ const UserMediaTile = forwardRef( const videoEnabled = useObservableEagerState(vm.videoEnabled); const speaking = useObservableEagerState(vm.speaking); const cropVideo = useObservableEagerState(vm.cropVideo); - const keys = useObservableEagerState(vm.keys); const isRTCParticipantAvailable = useObservableEagerState( vm.isRTCParticipantAvailable, ); @@ -122,7 +121,6 @@ const UserMediaTile = forwardRef( ref={ref} video={video} member={vm.member} - keys={keys} unencryptedWarning={unencryptedWarning} videoEnabled={videoEnabled && showVideo} videoFit={cropVideo ? "cover" : "contain"} diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index a8ec58c94..85f54335d 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -29,7 +29,6 @@ interface Props extends ComponentProps { videoFit: "cover" | "contain"; mirror: boolean; member: RoomMember | undefined; - keys: { index: number; key: Uint8Array }[]; videoEnabled: boolean; unencryptedWarning: boolean; nameTagLeadingIcon?: ReactNode; @@ -49,7 +48,6 @@ export const MediaView = forwardRef( videoFit, mirror, member, - keys, videoEnabled, unencryptedWarning, nameTagLeadingIcon, From e7dbddb6512dfb90a17b92b1c22a10e2509ee793 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 11:15:18 +0000 Subject: [PATCH 11/84] Fix optionality of nonMemberItemCount --- src/Header.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Header.tsx b/src/Header.tsx index c17a0288a..419102b35 100644 --- a/src/Header.tsx +++ b/src/Header.tsx @@ -117,7 +117,7 @@ interface RoomHeaderInfoProps { avatarUrl: string | null; encrypted: boolean; participantCount: number | null; - nonMemberItemCount: number | null; + nonMemberItemCount?: number; } export const RoomHeaderInfo: FC = ({ From 0d1b54e071d3e240223b333df0d2ffa021484ee8 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 11:15:28 +0000 Subject: [PATCH 12/84] video is optional --- src/tile/SpotlightTile.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index a37d9cc22..5319c2306 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -48,7 +48,7 @@ interface SpotlightItemBaseProps { "data-id": string; targetWidth: number; targetHeight: number; - video: TrackReferenceOrPlaceholder; + video: TrackReferenceOrPlaceholder | undefined; member: RoomMember | undefined; unencryptedWarning: boolean; displayName: string; From fecd789d02a25c1d0de64be908be02f4d6e90983 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 11:22:29 +0000 Subject: [PATCH 13/84] Mock RTC members --- src/state/CallViewModel.test.ts | 19 +++++++++------- src/utils/test.ts | 39 +++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 4adfb291f..c5c472723 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details. */ import { test, vi, onTestFinished } from "vitest"; -import { map, Observable } from "rxjs"; +import { map, Observable, of } from "rxjs"; import { MatrixClient } from "matrix-js-sdk/src/matrix"; import { ConnectionState, @@ -25,10 +25,11 @@ import { mockLivekitRoom, mockLocalParticipant, mockMatrixRoom, - mockMember, + mockRoomMember, mockRemoteParticipant, OurRunHelpers, withTestScheduler, + mockMembership, } from "../utils/test"; import { ECAddonConnectionState, @@ -49,9 +50,9 @@ const bobId = "@bob:example.org"; const bobDev = "BBBB"; const bobRTCId = bobId + ":" + bobDev; -const alice = mockMember({ userId: aliceId }); -const bob = mockMember({ userId: bobId }); -const carol = mockMember({ userId: carolId }); +const alice = mockRoomMember({ userId: aliceId }); +const bob = mockRoomMember({ userId: bobId }); +const carol = mockRoomMember({ userId: carolId }); const localParticipant = mockLocalParticipant({ identity: "" }); const aliceParticipant = mockRemoteParticipant({ identity: aliceRTCId }); @@ -71,9 +72,10 @@ const members = new Map([ [carol.userId, carol], ]); -const rtcMemberAlice = { sender: aliceId, deviceId: aliceDev }; -const rtcMemberBob = { sender: bobId, deviceId: bobDev }; -const rtcMemberCarol = { sender: carolId, deviceId: carolDev }; +const rtcMemberAlice = mockMembership(aliceId, aliceDev); +const rtcMemberBob = mockMembership(bobId, bobDev); +const rtcMemberCarol = mockMembership(carolId, carolDev); + export interface GridLayoutSummary { type: "grid"; spotlight?: string[]; @@ -262,6 +264,7 @@ test("screen sharing activates spotlight layout", () => { c: [aliceSharingScreen, bobSharingScreen], d: [aliceParticipant, bobSharingScreen], }), + of([rtcMemberAlice, rtcMemberAlice]), hot("a", { a: ConnectionState.Connected }), (vm) => { schedule(modeMarbles, { diff --git a/src/utils/test.ts b/src/utils/test.ts index d2014afb3..1caef0ab8 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -7,7 +7,16 @@ Please see LICENSE in the repository root for full details. import { map, of } from "rxjs"; import { RunHelpers, TestScheduler } from "rxjs/testing"; import { expect, vi } from "vitest"; -import { RoomMember, Room as MatrixRoom } from "matrix-js-sdk/src/matrix"; +import { + RoomMember, + Room as MatrixRoom, + MatrixEvent, +} from "matrix-js-sdk/src/matrix"; +import { + CallMembership, + Focus, + SessionMembershipData, +} from "matrix-js-sdk/src/matrixrtc"; import { LocalParticipant, LocalTrackPublication, @@ -88,10 +97,32 @@ function mockEmitter(): EmitterMock { }; } +export function mockMembership( + userId: string, + deviceId: string, + callId = "", + fociPreferred: Focus[] = [], + focusActive: Focus = { type: "oldest_membership" }, + membership: Partial = {}, +): CallMembership { + const data: SessionMembershipData = { + application: "m.call", + call_id: callId, + device_id: deviceId, + foci_preferred: fociPreferred, + focus_active: focusActive, + ...membership, + }; + const event = new MatrixEvent({ + sender: userId, + }); + return new CallMembership(event, data); +} + // Maybe it'd be good to move this to matrix-js-sdk? Our testing needs are // rather simple, but if one util to mock a member is good enough for us, maybe // it's useful for matrix-js-sdk consumers in general. -export function mockMember(member: Partial): RoomMember { +export function mockRoomMember(member: Partial): RoomMember { return { ...mockEmitter(), ...member } as RoomMember; } @@ -121,7 +152,7 @@ export async function withLocalMedia( ): Promise { const vm = new LocalUserMediaViewModel( "local", - mockMember(member), + mockRoomMember(member), of(mockLocalParticipant({})), { kind: E2eeType.PER_PARTICIPANT, @@ -154,7 +185,7 @@ export async function withRemoteMedia( ): Promise { const vm = new RemoteUserMediaViewModel( "remote", - mockMember(member), + mockRoomMember(member), of(mockRemoteParticipant(participant)), { kind: E2eeType.PER_PARTICIPANT, From c829f2f59935d75e79c0ab389f932b317e5fc180 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 14:02:09 +0000 Subject: [PATCH 14/84] Lint --- src/state/CallViewModel.test.ts | 8 ++++++-- src/state/CallViewModel.ts | 11 ++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index eaf19866e..4ea5f8c20 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -24,7 +24,11 @@ import { } from "livekit-client"; import * as ComponentsCore from "@livekit/components-core"; import { isEqual } from "lodash"; -import { CallMembership, MatrixRTCSession, MatrixRTCSessionEvent } from "matrix-js-sdk/src/matrixrtc"; +import { + CallMembership, + MatrixRTCSession, + MatrixRTCSessionEvent, +} from "matrix-js-sdk/src/matrixrtc"; import { CallViewModel, Layout } from "./CallViewModel"; import { @@ -77,7 +81,7 @@ const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); const aliceRtcMember = mockMembership(aliceId, aliceDev); const bobRtcMember = mockMembership(bobId, bobDev); -const carolRtcMember = mockMembership(carolId, carolDev); +// const carolRtcMember = mockMembership(carolId, carolDev); const daveRtcMember = mockMembership(daveId, daveDev); export interface GridLayoutSummary { diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 4e9c4c5ec..1035e9ab3 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -497,7 +497,7 @@ export class CallViewModel extends ViewModel { member, participant, this.encryptionSystem, - this.livekitRoom + this.livekitRoom, ), ]; } @@ -552,6 +552,7 @@ export class CallViewModel extends ViewModel { undefined, participant, this.encryptionSystem, + this.livekitRoom, ), ]; } @@ -724,14 +725,6 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - private readonly hasRemoteScreenShares: Observable = - this.spotlight.pipe( - map((spotlight) => - spotlight.some((vm) => !vm.local && vm instanceof ScreenShareViewModel), - ), - distinctUntilChanged(), - ); - private readonly pip: Observable = this.spotlightAndPip.pipe(switchMap(([, pip]) => pip)); From 73107e413978dd1615b7bb8bdd8d988957c0d63e Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 14:02:14 +0000 Subject: [PATCH 15/84] Merge fixes --- src/state/MediaViewModel.ts | 127 ++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 62 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 4df82d3ff..47e674f6e 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -32,7 +32,6 @@ import { Observable, Subject, combineLatest, - distinctUntilChanged, distinctUntilKeyChanged, filter, fromEvent, @@ -40,7 +39,6 @@ import { map, merge, of, - shareReplay, startWith, switchMap, throttleTime, @@ -114,11 +112,11 @@ function observeRemoteTrackReceivingOkay( }; return combineLatest([ - observeTrackReference(participant, source), + observeTrackReference(of(participant), source), interval(1000).pipe(startWith(0)), ]).pipe( switchMap(async ([trackReference]) => { - const track = trackReference.publication?.track; + const track = trackReference?.publication?.track; if (!track || !(track instanceof RemoteTrack)) { return undefined; } @@ -267,64 +265,69 @@ abstract class BaseMediaViewModel extends ViewModel { encryptionSystem.kind !== E2eeType.NONE && (a?.publication?.isEncrypted === false || v?.publication?.isEncrypted === false), - ).pipe(this.scope.state()); - - if (participant.isLocal || encryptionSystem.kind === E2eeType.NONE) { - this.encryptionStatus = of(EncryptionStatus.Okay).pipe( - this.scope.state(), - ); - } else if (encryptionSystem.kind === E2eeType.PER_PARTICIPANT) { - this.encryptionStatus = combineLatest([ - encryptionErrorObservable( - livekitRoom, - participant, - encryptionSystem, - "MissingKey", - ), - encryptionErrorObservable( - livekitRoom, - participant, - encryptionSystem, - "InvalidKey", - ), - observeRemoteTrackReceivingOkay(participant, audioSource), - observeRemoteTrackReceivingOkay(participant, videoSource), - ]).pipe( - map(([keyMissing, keyInvalid, audioOkay, videoOkay]) => { - if (keyMissing) return EncryptionStatus.KeyMissing; - if (keyInvalid) return EncryptionStatus.KeyInvalid; - if (audioOkay || videoOkay) return EncryptionStatus.Okay; - return undefined; // no change - }), - filter((x) => !!x), - startWith(EncryptionStatus.Connecting), - this.scope.state(), - ); - } else { - this.encryptionStatus = combineLatest([ - encryptionErrorObservable( - livekitRoom, - participant, - encryptionSystem, - "InvalidKey", - ), - observeRemoteTrackReceivingOkay(participant, audioSource), - observeRemoteTrackReceivingOkay(participant, videoSource), - ]).pipe( - map( - ([keyInvalid, audioOkay, videoOkay]): - | EncryptionStatus - | undefined => { - if (keyInvalid) return EncryptionStatus.PasswordInvalid; - if (audioOkay || videoOkay) return EncryptionStatus.Okay; - return undefined; // no change - }, - ), - filter((x) => !!x), - startWith(EncryptionStatus.Connecting), - this.scope.state(), - ); - } + ).pipe(this.scope.state()); + + this.encryptionStatus = this.participant.pipe( + switchMap((participant): Observable => { + if ( + !participant || + participant.isLocal || + encryptionSystem.kind === E2eeType.NONE + ) { + return of(EncryptionStatus.Okay); + } else if (encryptionSystem.kind === E2eeType.PER_PARTICIPANT) { + return combineLatest([ + encryptionErrorObservable( + livekitRoom, + participant, + encryptionSystem, + "MissingKey", + ), + encryptionErrorObservable( + livekitRoom, + participant, + encryptionSystem, + "InvalidKey", + ), + observeRemoteTrackReceivingOkay(participant, audioSource), + observeRemoteTrackReceivingOkay(participant, videoSource), + ]).pipe( + map(([keyMissing, keyInvalid, audioOkay, videoOkay]) => { + if (keyMissing) return EncryptionStatus.KeyMissing; + if (keyInvalid) return EncryptionStatus.KeyInvalid; + if (audioOkay || videoOkay) return EncryptionStatus.Okay; + return undefined; // no change + }), + filter((x) => !!x), + startWith(EncryptionStatus.Connecting), + ); + } else { + return combineLatest([ + encryptionErrorObservable( + livekitRoom, + participant, + encryptionSystem, + "InvalidKey", + ), + observeRemoteTrackReceivingOkay(participant, audioSource), + observeRemoteTrackReceivingOkay(participant, videoSource), + ]).pipe( + map( + ([keyInvalid, audioOkay, videoOkay]): + | EncryptionStatus + | undefined => { + if (keyInvalid) return EncryptionStatus.PasswordInvalid; + if (audioOkay || videoOkay) return EncryptionStatus.Okay; + return undefined; // no change + }, + ), + filter((x) => !!x), + startWith(EncryptionStatus.Connecting), + ); + } + }), + this.scope.state(), + ); } } From bcbdb596a3d51f6a6f56c78848e37f0c59b669b1 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 14:12:32 +0000 Subject: [PATCH 16/84] Fix user id --- src/state/CallViewModel.test.ts | 8 ++++---- src/utils/test.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 4ea5f8c20..e738a2792 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -79,10 +79,10 @@ const daveParticipant = mockRemoteParticipant({ identity: daveId }); const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); -const aliceRtcMember = mockMembership(aliceId, aliceDev); -const bobRtcMember = mockMembership(bobId, bobDev); -// const carolRtcMember = mockMembership(carolId, carolDev); -const daveRtcMember = mockMembership(daveId, daveDev); +const aliceRtcMember = mockMembership(alice, aliceDev); +const bobRtcMember = mockMembership(bob, bobDev); +// const carolRtcMember = mockMembership(carol, carolDev); +const daveRtcMember = mockMembership(dave, daveDev); export interface GridLayoutSummary { type: "grid"; diff --git a/src/utils/test.ts b/src/utils/test.ts index 539623acb..b2df15622 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -98,7 +98,7 @@ function mockEmitter(): EmitterMock { } export function mockMembership( - userId: string, + user: string | RoomMember, deviceId: string, callId = "", fociPreferred: Focus[] = [], @@ -114,7 +114,7 @@ export function mockMembership( ...membership, }; const event = new MatrixEvent({ - sender: userId, + sender: typeof user === "string" ? user : user.userId, }); return new CallMembership(event, data); } From b0e0e0ed6b584ebf86cf0de06228f75ff9aa3bdb Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 14:13:00 +0000 Subject: [PATCH 17/84] Add explicit types for public fields --- src/state/MediaViewModel.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 47e674f6e..2a80fbe36 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -210,7 +210,7 @@ abstract class BaseMediaViewModel extends ViewModel { /** * Whether the media belongs to the local user. */ - public readonly local = this.participant.pipe( + public readonly local: Observable = this.participant.pipe( // We can assume, that the user is not local if the participant is undefined // We assume the local LK participant will always be available. map((p) => p?.isLocal ?? false), @@ -224,9 +224,8 @@ abstract class BaseMediaViewModel extends ViewModel { */ public readonly unencryptedWarning: Observable; - public readonly isRTCParticipantAvailable = this.participant.pipe( - map((p) => !!p), - ); + public readonly isRTCParticipantAvailable: Observable = + this.participant.pipe(map((p) => !!p)); public readonly encryptionStatus: Observable; From a1083f328486b26254730c6fb7a9741b05f2ba8d Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 14:29:09 +0000 Subject: [PATCH 18/84] isRTCParticipantAvailable => isLiveKitParticipantAvailable --- src/tile/GridTile.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index e7ac7a2d3..e493cba7b 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -86,8 +86,8 @@ const UserMediaTile = forwardRef( const videoEnabled = useObservableEagerState(vm.videoEnabled); const speaking = useObservableEagerState(vm.speaking); const cropVideo = useObservableEagerState(vm.cropVideo); - const isRTCParticipantAvailable = useObservableEagerState( - vm.isRTCParticipantAvailable, + const isLiveKitParticipantAvailable = useObservableEagerState( + vm.isLiveKitParticipantAvailable, ); const onSelectFitContain = useCallback( (e: Event) => { @@ -144,7 +144,9 @@ const UserMediaTile = forwardRef( } displayName={ displayName + - (isRTCParticipantAvailable ? "" : " missing Livekit Participant...") + (isLiveKitParticipantAvailable + ? "" + : " missing Livekit Participant...") } primaryButton={ - {/* {isRTCParticipantAvailable + {/* {isLiveKitParticipantAvailable ? "is available" : "Loading RTC participant"} */} Date: Wed, 6 Nov 2024 15:57:39 +0000 Subject: [PATCH 19/84] isLiveKitParticipantAvailable --- src/state/MediaViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 2a80fbe36..4fdf1131f 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -224,7 +224,7 @@ abstract class BaseMediaViewModel extends ViewModel { */ public readonly unencryptedWarning: Observable; - public readonly isRTCParticipantAvailable: Observable = + public readonly isLiveKitParticipantAvailable: Observable = this.participant.pipe(map((p) => !!p)); public readonly encryptionStatus: Observable; From d8d4e8942ce4b52c874b951e06e8d74a8e20f69c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 16:07:02 +0000 Subject: [PATCH 20/84] Readonly --- src/state/CallViewModel.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 1035e9ab3..10c56eea8 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -220,7 +220,7 @@ interface LayoutScanState { class UserMedia { private readonly scope = new ObservableScope(); public readonly vm: UserMediaViewModel; - public participant: BehaviorSubject< + public readonly participant: BehaviorSubject< LocalParticipant | RemoteParticipant | undefined >; @@ -298,7 +298,9 @@ class UserMedia { class ScreenShare { public readonly vm: ScreenShareViewModel; - private participant: BehaviorSubject; + private readonly participant: BehaviorSubject< + LocalParticipant | RemoteParticipant + >; public constructor( id: string, From efee27ad25b8224df2a47b03c9aa18616845665d Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 16:33:03 +0000 Subject: [PATCH 21/84] More keys removal --- src/tile/MediaView.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 741d25a05..5c672945f 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -122,12 +122,6 @@ export const MediaView = forwardRef( showTimer={handRaiseTimerVisible} onClick={raisedHandOnClick} /> - {/* {keys && - keys.map(({ index, key }) => ( - - index:{index}, key:{key} - - ))} */}
{nameTagLeadingIcon} From eec44702f743070cd5cbb24f84c38a92b83577ff Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 16:33:53 +0000 Subject: [PATCH 22/84] Make local field based on view model class not observable --- src/state/CallViewModel.ts | 1 + src/state/MediaViewModel.ts | 12 ++++-------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 10c56eea8..0052067e5 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -317,6 +317,7 @@ class ScreenShare { this.participant.asObservable(), encryptionSystem, liveKitRoom, + participant.isLocal, ); } diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 4fdf1131f..e3f244f92 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -207,14 +207,6 @@ export enum EncryptionStatus { } abstract class BaseMediaViewModel extends ViewModel { - /** - * Whether the media belongs to the local user. - */ - public readonly local: Observable = this.participant.pipe( - // We can assume, that the user is not local if the participant is undefined - // We assume the local LK participant will always be available. - map((p) => p?.isLocal ?? false), - ); /** * The LiveKit video track for this media. */ @@ -414,6 +406,7 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { * The local participant's user media. */ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { + public readonly local = true; /** * Whether the video should be mirrored. */ @@ -453,6 +446,8 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { * A remote participant's user media. */ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { + public readonly local = false; + private readonly locallyMutedToggle = new Subject(); private readonly localVolumeAdjustment = new Subject(); private readonly localVolumeCommit = new Subject(); @@ -538,6 +533,7 @@ export class ScreenShareViewModel extends BaseMediaViewModel { participant: Observable, encryptionSystem: EncryptionSystem, livekitRoom: LivekitRoom, + public readonly local: boolean, ) { super( id, From 462f0349d5e8ba7950834022fe40da79729b8cff Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 16:35:22 +0000 Subject: [PATCH 23/84] Wording --- public/locales/en-GB/app.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 1fae4194a..c04752500 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -152,7 +152,7 @@ "feedback_tab_thank_you": "Thanks, we received your feedback!", "feedback_tab_title": "Feedback", "more_tab_title": "More", - "non_member_tiles": "Show non member tiles", + "non_member_tiles": "Show tiles for non-member media", "opt_in_description": "<0><1>You may withdraw consent by unchecking this box. If you are currently in a call, this setting will take effect at the end of the call.", "preferences_tab_body": "Here you can configure extra options for an improved experience", "preferences_tab_h4": "Preferences", From efe9c9b8fed23aba052eaea1c81183d83ded1911 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 17:05:29 +0000 Subject: [PATCH 24/84] Fix RTC members in tes --- src/state/CallViewModel.test.ts | 58 ++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index e738a2792..bfc6e0c3b 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -49,21 +49,24 @@ import { MockRoom, MockRTCSession } from "../useReactions.test"; vi.mock("@livekit/components-core"); +const local = mockRoomMember({ userId: "@local:example.org" }); const alice = mockRoomMember({ userId: "@alice:example.org" }); const bob = mockRoomMember({ userId: "@bob:example.org" }); const carol = mockRoomMember({ userId: "@carol:example.org" }); const dave = mockRoomMember({ userId: "@dave:example.org" }); +const localDev = "LLLL"; const aliceDev = "AAAA"; const bobDev = "BBBB"; const carolDev = "CCCC"; const daveDev = "DDDD"; +const localId = `${local.userId}:${localDev}`; const aliceId = `${alice.userId}:${aliceDev}`; const bobId = `${bob.userId}:${bobDev}`; const carolId = `${carol.userId}:${carolDev}`; const daveId = `${dave.userId}:${daveDev}`; -const localParticipant = mockLocalParticipant({ identity: "" }); +const localParticipant = mockLocalParticipant({ identity: localId }); const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const aliceSharingScreen = mockRemoteParticipant({ @@ -79,6 +82,7 @@ const daveParticipant = mockRemoteParticipant({ identity: daveId }); const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); +const localRtcMember = mockMembership(local, localDev); const aliceRtcMember = mockMembership(alice, aliceDev); const bobRtcMember = mockMembership(bob, bobDev); // const carolRtcMember = mockMembership(carol, carolDev); @@ -268,7 +272,7 @@ test("participants are retained during a focus switch", () => { a: [aliceParticipant, bobParticipant], b: [], }), - of([aliceRtcMember, bobRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember]), cold(connectionMarbles, { c: ConnectionState.Connected, s: ECAddonConnectionState.ECSwitchingFocus, @@ -279,7 +283,7 @@ test("participants are retained during a focus switch", () => { a: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, }); }, @@ -307,7 +311,7 @@ test("screen sharing activates spotlight layout", () => { c: [aliceSharingScreen, bobSharingScreen], d: [aliceParticipant, bobSharingScreen], }), - of([aliceRtcMember, bobRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -320,37 +324,37 @@ test("screen sharing activates spotlight layout", () => { a: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, b: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`], - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, c: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`, `${bobId}:0:screen-share`], - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, d: { type: "spotlight-landscape", spotlight: [`${bobId}:0:screen-share`], - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, e: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: ["local:0", `${bobId}:0`], + grid: [`${localId}:0`, `${bobId}:0`], }, f: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`], - grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], + grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], }, g: { type: "grid", spotlight: undefined, - grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], + grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], }, }); expectObservable(vm.showSpeakingIndicators).toBe(showSpeakingMarbles, { @@ -377,7 +381,7 @@ test("participants stay in the same order unless to appear/disappear", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([aliceRtcMember, bobRtcMember, daveRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -402,17 +406,17 @@ test("participants stay in the same order unless to appear/disappear", () => { a: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], }, b: { type: "grid", spotlight: undefined, - grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], + grid: [`${localId}:0`, `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], }, c: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], }, }); }, @@ -436,7 +440,7 @@ test("spotlight speakers swap places", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([aliceRtcMember, bobRtcMember, daveRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -450,22 +454,22 @@ test("spotlight speakers swap places", () => { a: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: ["local:0", `${bobId}:0`, `${daveId}:0`], + grid: [`${localId}:0`, `${bobId}:0`, `${daveId}:0`], }, b: { type: "spotlight-landscape", spotlight: [`${bobId}:0`], - grid: ["local:0", `${aliceId}:0`, `${daveId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${daveId}:0`], }, c: { type: "spotlight-landscape", spotlight: [`${daveId}:0`], - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, d: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: ["local:0", `${daveId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${daveId}:0`, `${bobId}:0`], }, }); }, @@ -482,7 +486,7 @@ test("layout enters picture-in-picture mode when requested", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([aliceRtcMember, bobRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -495,7 +499,7 @@ test("layout enters picture-in-picture mode when requested", () => { a: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, b: { type: "pip", @@ -520,7 +524,7 @@ test("spotlight remembers whether it's expanded", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([aliceRtcMember, bobRtcMember]), + of([localRtcMember, aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -540,22 +544,22 @@ test("spotlight remembers whether it's expanded", () => { a: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: ["local:0", `${bobId}:0`], + grid: [`${localId}:0`, `${bobId}:0`], }, b: { type: "spotlight-expanded", spotlight: [`${aliceId}:0`], - pip: "local:0", + pip: `${localId}:0`, }, c: { type: "grid", spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], }, d: { type: "grid", spotlight: undefined, - grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], + grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], }, }); }, From 4f6b1b0e10540a151316b1922c8cdef0a0b92b5b Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 17:30:29 +0000 Subject: [PATCH 25/84] Tests again --- src/state/CallViewModel.test.ts | 62 +++++++++++++++------------------ 1 file changed, 29 insertions(+), 33 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index bfc6e0c3b..2cc4fc2f7 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -49,24 +49,21 @@ import { MockRoom, MockRTCSession } from "../useReactions.test"; vi.mock("@livekit/components-core"); -const local = mockRoomMember({ userId: "@local:example.org" }); const alice = mockRoomMember({ userId: "@alice:example.org" }); const bob = mockRoomMember({ userId: "@bob:example.org" }); const carol = mockRoomMember({ userId: "@carol:example.org" }); const dave = mockRoomMember({ userId: "@dave:example.org" }); -const localDev = "LLLL"; const aliceDev = "AAAA"; const bobDev = "BBBB"; const carolDev = "CCCC"; const daveDev = "DDDD"; -const localId = `${local.userId}:${localDev}`; const aliceId = `${alice.userId}:${aliceDev}`; const bobId = `${bob.userId}:${bobDev}`; const carolId = `${carol.userId}:${carolDev}`; const daveId = `${dave.userId}:${daveDev}`; -const localParticipant = mockLocalParticipant({ identity: localId }); +const localParticipant = mockLocalParticipant({ identity: carolId }); const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const aliceSharingScreen = mockRemoteParticipant({ @@ -82,10 +79,9 @@ const daveParticipant = mockRemoteParticipant({ identity: daveId }); const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); -const localRtcMember = mockMembership(local, localDev); const aliceRtcMember = mockMembership(alice, aliceDev); const bobRtcMember = mockMembership(bob, bobDev); -// const carolRtcMember = mockMembership(carol, carolDev); +const carolRtcMember = mockMembership(carol, carolDev); const daveRtcMember = mockMembership(dave, daveDev); export interface GridLayoutSummary { @@ -201,7 +197,7 @@ function withCallViewModel( ): void { const room = mockMatrixRoom({ client: { - getUserId: () => carolId, + getUserId: () => carol.userId, getDeviceId: () => carolDev, } as Partial as MatrixClient, getMember: (userId) => members.get(userId) ?? null, @@ -272,7 +268,7 @@ test("participants are retained during a focus switch", () => { a: [aliceParticipant, bobParticipant], b: [], }), - of([localRtcMember, aliceRtcMember, bobRtcMember]), + of([carolRtcMember, aliceRtcMember, bobRtcMember]), cold(connectionMarbles, { c: ConnectionState.Connected, s: ECAddonConnectionState.ECSwitchingFocus, @@ -283,7 +279,7 @@ test("participants are retained during a focus switch", () => { a: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: [`local:0`, `${aliceId}:0`, `${bobId}:0`], }, }); }, @@ -311,7 +307,7 @@ test("screen sharing activates spotlight layout", () => { c: [aliceSharingScreen, bobSharingScreen], d: [aliceParticipant, bobSharingScreen], }), - of([localRtcMember, aliceRtcMember, bobRtcMember]), + of([carolRtcMember, bobRtcMember, aliceRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -324,37 +320,37 @@ test("screen sharing activates spotlight layout", () => { a: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, b: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`], - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, c: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`, `${bobId}:0:screen-share`], - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, d: { type: "spotlight-landscape", spotlight: [`${bobId}:0:screen-share`], - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, e: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: [`${localId}:0`, `${bobId}:0`], + grid: ["local:0", `${bobId}:0`], }, f: { type: "spotlight-landscape", spotlight: [`${aliceId}:0:screen-share`], - grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], + grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], }, g: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], + grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], }, }); expectObservable(vm.showSpeakingIndicators).toBe(showSpeakingMarbles, { @@ -381,7 +377,7 @@ test("participants stay in the same order unless to appear/disappear", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([localRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), + of([carolRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -406,17 +402,17 @@ test("participants stay in the same order unless to appear/disappear", () => { a: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], }, b: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], + grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], }, c: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], }, }); }, @@ -440,7 +436,7 @@ test("spotlight speakers swap places", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([localRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), + of([carolRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -454,22 +450,22 @@ test("spotlight speakers swap places", () => { a: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: [`${localId}:0`, `${bobId}:0`, `${daveId}:0`], + grid: ["local:0", `${bobId}:0`, `${daveId}:0`], }, b: { type: "spotlight-landscape", spotlight: [`${bobId}:0`], - grid: [`${localId}:0`, `${aliceId}:0`, `${daveId}:0`], + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`], }, c: { type: "spotlight-landscape", spotlight: [`${daveId}:0`], - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, d: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: [`${localId}:0`, `${daveId}:0`, `${bobId}:0`], + grid: ["local:0", `${daveId}:0`, `${bobId}:0`], }, }); }, @@ -486,7 +482,7 @@ test("layout enters picture-in-picture mode when requested", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([localRtcMember, aliceRtcMember, bobRtcMember]), + of([carolRtcMember, aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -499,7 +495,7 @@ test("layout enters picture-in-picture mode when requested", () => { a: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, b: { type: "pip", @@ -524,7 +520,7 @@ test("spotlight remembers whether it's expanded", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([localRtcMember, aliceRtcMember, bobRtcMember]), + of([carolRtcMember, aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -544,22 +540,22 @@ test("spotlight remembers whether it's expanded", () => { a: { type: "spotlight-landscape", spotlight: [`${aliceId}:0`], - grid: [`${localId}:0`, `${bobId}:0`], + grid: ["local:0", `${bobId}:0`], }, b: { type: "spotlight-expanded", spotlight: [`${aliceId}:0`], - pip: `${localId}:0`, + pip: "local:0", }, c: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, d: { type: "grid", spotlight: undefined, - grid: [`${localId}:0`, `${bobId}:0`, `${aliceId}:0`], + grid: ["local:0", `${bobId}:0`, `${aliceId}:0`], }, }); }, From 59e49d5ff3ad1cb82c03d4c51951f99ae52c8489 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 17:31:06 +0000 Subject: [PATCH 26/84] Lint --- src/state/CallViewModel.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 2cc4fc2f7..7b773730a 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -279,7 +279,7 @@ test("participants are retained during a focus switch", () => { a: { type: "grid", spotlight: undefined, - grid: [`local:0`, `${aliceId}:0`, `${bobId}:0`], + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], }, }); }, From f9f8f378af6f4aab47de8be8e3739a68ff3418fc Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 6 Nov 2024 17:46:23 +0000 Subject: [PATCH 27/84] Disable showing non-member tiles by default --- src/settings/settings.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 293d5d590..d6358d32c 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -72,7 +72,7 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); -export const nonMemberTiles = new Setting("non-member-tiles", true); +export const nonMemberTiles = new Setting("non-member-tiles", false); export const audioInput = new Setting( "audio-input", From dfd7273ac6cb6952409035ebe745bea452a569b0 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 11:30:08 +0000 Subject: [PATCH 28/84] Duplicate screen sharing tiles like we used to --- src/state/CallViewModel.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 0052067e5..d1267cdff 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -503,20 +503,22 @@ export class CallViewModel extends ViewModel { this.livekitRoom, ), ]; - } - if (participant && participant.isScreenShareEnabled) { - const screenShareId = `${mediaId}:screen-share`; - yield [ - screenShareId, - prevItems.get(screenShareId) ?? - new ScreenShare( - screenShareId, - member, - participant, - this.encryptionSystem, - this.livekitRoom, - ), - ]; + + if (participant?.isScreenShareEnabled) { + const screenShareId = `${indexedMediaId}:screen-share`; + yield [ + screenShareId, + prevItems.get(screenShareId) ?? + new ScreenShare( + screenShareId, + member, + participant, + this.encryptionSystem, + this.livekitRoom, + ), + ]; + } + } } }.bind(this)(), From e51535934428d99825e7ca74185e565895ebfa5b Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 11:33:22 +0000 Subject: [PATCH 29/84] Lint --- src/state/CallViewModel.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index d1267cdff..8973bec52 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -518,7 +518,6 @@ export class CallViewModel extends ViewModel { ), ]; } - } } }.bind(this)(), From 9bfd2e371cd401024b1930c54d639096295470da Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 12:02:32 +0000 Subject: [PATCH 30/84] Revert function reordering --- src/state/CallViewModel.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 8973bec52..2e802b538 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -609,12 +609,6 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - private readonly hasRemoteScreenShares: Observable = - this.screenShares.pipe( - map((ms) => ms.some((m) => !m.vm.local)), - distinctUntilChanged(), - ); - private readonly spotlightSpeaker: Observable< UserMediaViewModel | undefined > = this.userMedia.pipe( @@ -729,6 +723,12 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); + private readonly hasRemoteScreenShares: Observable = + this.screenShares.pipe( + map((ms) => ms.some((m) => !m.vm.local)), + distinctUntilChanged(), + ); + private readonly pip: Observable = this.spotlightAndPip.pipe(switchMap(([, pip]) => pip)); From 0760796024e921295304747614ba422251c80d56 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 12:12:10 +0000 Subject: [PATCH 31/84] Remove throttleTime from bad merge --- src/state/CallViewModel.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 2e802b538..00541877c 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -43,7 +43,6 @@ import { switchMap, switchScan, take, - throttleTime, timer, withLatestFrom, } from "rxjs"; @@ -644,7 +643,6 @@ export class CallViewModel extends ViewModel { ), map((speaker) => speaker?.vm), this.scope.state(), - throttleTime(1600, undefined, { leading: true, trailing: true }), ); private readonly grid: Observable = this.userMedia.pipe( From 95effe085202f7f070c0f5eaad6494dbaf2f4e65 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 13:01:48 +0000 Subject: [PATCH 32/84] Cleanup --- src/state/CallViewModel.ts | 17 ++++++----------- src/state/MediaViewModel.ts | 5 ++--- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 00541877c..a83341bde 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -443,7 +443,7 @@ export class CallViewModel extends ViewModel { remoteParticipants, { participant: localParticipant }, duplicateTiles, - _participantChange, + _membershipsChanged, nonMemberTiles, ], ) => { @@ -682,16 +682,11 @@ export class CallViewModel extends ViewModel { ); private readonly spotlightAndPip: Observable< - [Observable, Observable] + [Observable, Observable] > = this.screenShares.pipe( map((screenShares) => screenShares.length > 0 - ? ([ - of(screenShares.map((m) => m.vm)), - this.spotlightSpeaker.pipe( - map((speaker) => (speaker && speaker) ?? null), - ), - ] as const) + ? ([of(screenShares.map((m) => m.vm)), this.spotlightSpeaker] as const) : ([ this.spotlightSpeaker.pipe( map((speaker) => (speaker && [speaker]) ?? []), @@ -700,15 +695,15 @@ export class CallViewModel extends ViewModel { switchMap((speaker) => speaker ? speaker.local - ? of(null) + ? of(undefined) : this.localUserMedia.pipe( switchMap((vm) => vm.alwaysShow.pipe( - map((alwaysShow) => (alwaysShow ? vm : null)), + map((alwaysShow) => (alwaysShow ? vm : undefined)), ), ), ) - : of(null), + : of(undefined), ), ), ] as const), diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index e3f244f92..e005fbc3f 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -78,7 +78,7 @@ export function observeTrackReference( participant: Observable, source: Track.Source, ): Observable { - const obs = participant.pipe( + return participant.pipe( switchMap((p) => { if (p) { return observeParticipantMedia(p).pipe( @@ -94,7 +94,6 @@ export function observeTrackReference( } }), ); - return obs; } function observeRemoteTrackReceivingOkay( @@ -232,7 +231,7 @@ abstract class BaseMediaViewModel extends ViewModel { // TODO: Fully separate the data layer from the UI layer by keeping the // member object internal public readonly member: RoomMember | undefined, - // We dont necassarly have a participant if a user connects via MatrixRTC but not (not yet) through + // We dont necessarily have a participant if a user connects via MatrixRTC but not (not yet) through // livekit. protected readonly participant: Observable< LocalParticipant | RemoteParticipant | undefined From 6bda8953abbb992659e74b48be6c8dcc70d80caf Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 14:02:25 +0000 Subject: [PATCH 33/84] Tidy config of show non-member settings --- public/locales/en-GB/app.json | 2 +- src/App.tsx | 4 ++-- src/config/Config.ts | 10 ++++++++++ src/settings/SettingsModal.tsx | 16 +++++++++------- src/settings/settings.ts | 5 ++++- src/state/CallViewModel.ts | 12 ++++++------ 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index c04752500..467824c33 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -152,12 +152,12 @@ "feedback_tab_thank_you": "Thanks, we received your feedback!", "feedback_tab_title": "Feedback", "more_tab_title": "More", - "non_member_tiles": "Show tiles for non-member media", "opt_in_description": "<0><1>You may withdraw consent by unchecking this box. If you are currently in a call, this setting will take effect at the end of the call.", "preferences_tab_body": "Here you can configure extra options for an improved experience", "preferences_tab_h4": "Preferences", "preferences_tab_show_hand_raised_timer_description": "Show a timer when a participant raises their hand", "preferences_tab_show_hand_raised_timer_label": "Show hand raise duration", + "show_non_member_tiles": "Show tiles for non-member media", "speaker_device_selection_label": "Speaker" }, "star_rating_input_label_one": "{{count}} stars", diff --git a/src/App.tsx b/src/App.tsx index 1bc23be83..0b6e32cf5 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -28,7 +28,7 @@ import { Initializer } from "./initializer"; import { MediaDevicesProvider } from "./livekit/MediaDevicesContext"; import { widget } from "./widget"; import { useTheme } from "./useTheme"; -import { nonMemberTiles } from "./settings/settings"; +import { showNonMemberTiles } from "./settings/settings"; import { Config } from "./config/Config"; const SentryRoute = Sentry.withSentryRouting(Route); @@ -76,7 +76,7 @@ export const App: FC = ({ history }) => { // Update settings to use the non member tile information from the config if set useEffect(() => { if (loaded && Config.get().show_non_member_tiles) { - nonMemberTiles.setValue(true); + showNonMemberTiles.setValue(true); } }); diff --git a/src/config/Config.ts b/src/config/Config.ts index 972c9e0ca..b230734c3 100644 --- a/src/config/Config.ts +++ b/src/config/Config.ts @@ -13,6 +13,7 @@ import { ConfigOptions, ResolvedConfigOptions, } from "./ConfigOptions"; +import { showNonMemberTiles } from "../settings/settings"; export class Config { private static internalInstance: Config | undefined; @@ -32,6 +33,7 @@ export class Config { "../config.json", ).then((config) => { internalInstance.config = merge({}, DEFAULT_CONFIG, config); + internalInstance.applyConfigToSettings(); }); } return Config.internalInstance.initPromise; @@ -66,6 +68,14 @@ export class Config { return Config.get().default_server_config?.["m.homeserver"].server_name; } + private applyConfigToSettings(): void { + if (!this.config) return; + // only the value from config if it hasn't been overridden + if (showNonMemberTiles.value === undefined) { + showNonMemberTiles.setValue(this.config.show_non_member_tiles); + } + } + public config?: ResolvedConfigOptions; private initPromise?: Promise; } diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 6ef40c890..eaf38be50 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -27,7 +27,7 @@ import { useSetting, developerSettingsTab as developerSettingsTabSetting, duplicateTiles as duplicateTilesSetting, - nonMemberTiles as nonMemberTilesSetting, + showNonMemberTiles as showNonMemberTilesSetting, useOptInAnalytics, } from "./settings"; import { isFirefox } from "../Platform"; @@ -69,7 +69,9 @@ export const SettingsModal: FC = ({ ); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); - const [nonMemberTiles, setNonMemberTiles] = useSetting(nonMemberTilesSetting); + const [showNonMemberTiles, setShowNonMemberTiles] = useSetting( + showNonMemberTilesSetting, + ); // Generate a `SelectInput` with a list of devices for a given device kind. const generateDeviceSelection = ( @@ -241,15 +243,15 @@ export const SettingsModal: FC = ({ ): void => { - setNonMemberTiles(event.target.checked); + setShowNonMemberTiles(event.target.checked); }, - [setNonMemberTiles], + [setShowNonMemberTiles], )} /> diff --git a/src/settings/settings.ts b/src/settings/settings.ts index d6358d32c..47b80d9b0 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -72,7 +72,10 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); -export const nonMemberTiles = new Setting("non-member-tiles", false); +export const showNonMemberTiles = new Setting( + "show-non-member-tiles", + undefined, +); export const audioInput = new Setting( "audio-input", diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a83341bde..b8559d246 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -67,7 +67,7 @@ import { } from "./MediaViewModel"; import { accumulate, finalizeValue } from "../utils/observable"; import { ObservableScope } from "./ObservableScope"; -import { duplicateTiles, nonMemberTiles } from "../settings/settings"; +import { duplicateTiles, showNonMemberTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { GridTileViewModel, SpotlightTileViewModel } from "./TileViewModel"; @@ -434,7 +434,7 @@ export class CallViewModel extends ViewModel { this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, ).pipe(startWith(null)), - nonMemberTiles.value, + showNonMemberTiles.value, ]).pipe( scan( ( @@ -444,7 +444,7 @@ export class CallViewModel extends ViewModel { { participant: localParticipant }, duplicateTiles, _membershipsChanged, - nonMemberTiles, + showNonMemberTiles, ], ) => { const newItems = new Map( @@ -534,7 +534,7 @@ export class CallViewModel extends ViewModel { // - If one wants to test scalability using the livekit cli. // - If an experimental project does not yet do the matrixRTC bits. // - If someone wants to debug if the LK connection works but matrixRTC room state failed to arrive. - const debugShowNonMember = nonMemberTiles; //Config.get().show_non_member_tiles; + const debugShowNonMember = showNonMemberTiles; //Config.get().show_non_member_tiles; const newNonMemberItems = debugShowNonMember ? new Map( function* (this: CallViewModel): Iterable<[string, MediaItem]> { @@ -689,7 +689,7 @@ export class CallViewModel extends ViewModel { ? ([of(screenShares.map((m) => m.vm)), this.spotlightSpeaker] as const) : ([ this.spotlightSpeaker.pipe( - map((speaker) => (speaker && [speaker]) ?? []), + map((speaker) => (speaker ? [speaker] : [])), ), this.spotlightSpeaker.pipe( switchMap((speaker) => @@ -722,7 +722,7 @@ export class CallViewModel extends ViewModel { distinctUntilChanged(), ); - private readonly pip: Observable = + private readonly pip: Observable = this.spotlightAndPip.pipe(switchMap(([, pip]) => pip)); private readonly pipEnabled: Observable = setPipEnabled.pipe( From 5e8a94778db1ab253f52cb82c60398cb1f03f841 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 14:16:55 +0000 Subject: [PATCH 34/84] tidy up handling of local rtc member in tests --- src/state/CallViewModel.test.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 7b773730a..5a20d3b81 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -81,7 +81,7 @@ const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); const aliceRtcMember = mockMembership(alice, aliceDev); const bobRtcMember = mockMembership(bob, bobDev); -const carolRtcMember = mockMembership(carol, carolDev); +const localRtcMember = mockMembership(carol, carolDev); const daveRtcMember = mockMembership(dave, daveDev); export interface GridLayoutSummary { @@ -204,7 +204,8 @@ function withCallViewModel( }); const rtcSession = new MockRTCSession(room as unknown as MockRoom); rtcMembers.subscribe((m) => { - rtcSession.memberships = m; + // always prepend the local participant + rtcSession.memberships = [localRtcMember, ...m]; rtcSession.emit(MatrixRTCSessionEvent.MembershipsChanged); }); const participantsSpy = vi @@ -268,7 +269,7 @@ test("participants are retained during a focus switch", () => { a: [aliceParticipant, bobParticipant], b: [], }), - of([carolRtcMember, aliceRtcMember, bobRtcMember]), + of([aliceRtcMember, bobRtcMember]), cold(connectionMarbles, { c: ConnectionState.Connected, s: ECAddonConnectionState.ECSwitchingFocus, @@ -307,7 +308,7 @@ test("screen sharing activates spotlight layout", () => { c: [aliceSharingScreen, bobSharingScreen], d: [aliceParticipant, bobSharingScreen], }), - of([carolRtcMember, bobRtcMember, aliceRtcMember]), + of([aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -364,10 +365,10 @@ test("screen sharing activates spotlight layout", () => { test("participants stay in the same order unless to appear/disappear", () => { withTestScheduler(({ cold, schedule, expectObservable }) => { - const modeMarbles = "a"; + const modeMarbles = " g"; // First Bob speaks, then Dave, then Alice const aSpeakingMarbles = "n- 1998ms - 1999ms y"; - const bSpeakingMarbles = "ny 1998ms n 1999ms "; + const bSpeakingMarbles = "ny 1998ms n 1999ms -"; const dSpeakingMarbles = "n- 1998ms y 1999ms n"; // Nothing should change when Bob speaks, because Bob is already on screen. // When Dave speaks he should switch with Alice because she's the one who @@ -377,7 +378,7 @@ test("participants stay in the same order unless to appear/disappear", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([carolRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), + of([aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -386,7 +387,7 @@ test("participants stay in the same order unless to appear/disappear", () => { ]), (vm) => { schedule(modeMarbles, { - a: () => { + g: () => { // We imagine that only three tiles (the first three) will be visible // on screen at a time vm.layout.subscribe((layout) => { @@ -436,7 +437,7 @@ test("spotlight speakers swap places", () => { withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), - of([carolRtcMember, aliceRtcMember, bobRtcMember, daveRtcMember]), + of([aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], @@ -482,7 +483,7 @@ test("layout enters picture-in-picture mode when requested", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([carolRtcMember, aliceRtcMember, bobRtcMember]), + of([aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { @@ -520,7 +521,7 @@ test("spotlight remembers whether it's expanded", () => { withCallViewModel( of([aliceParticipant, bobParticipant]), - of([carolRtcMember, aliceRtcMember, bobRtcMember]), + of([aliceRtcMember, bobRtcMember]), of(ConnectionState.Connected), new Map(), (vm) => { From bb56f4205b963e60712db4c12e248355a0bdfed0 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 14:24:20 +0000 Subject: [PATCH 35/84] tidy up test init --- src/state/CallViewModel.test.ts | 45 +++++++++++++++------------------ src/utils/test.ts | 11 ++++++-- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 5a20d3b81..9f2081123 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -49,21 +49,21 @@ import { MockRoom, MockRTCSession } from "../useReactions.test"; vi.mock("@livekit/components-core"); -const alice = mockRoomMember({ userId: "@alice:example.org" }); -const bob = mockRoomMember({ userId: "@bob:example.org" }); -const carol = mockRoomMember({ userId: "@carol:example.org" }); -const dave = mockRoomMember({ userId: "@dave:example.org" }); - -const aliceDev = "AAAA"; -const bobDev = "BBBB"; -const carolDev = "CCCC"; -const daveDev = "DDDD"; -const aliceId = `${alice.userId}:${aliceDev}`; -const bobId = `${bob.userId}:${bobDev}`; -const carolId = `${carol.userId}:${carolDev}`; -const daveId = `${dave.userId}:${daveDev}`; - -const localParticipant = mockLocalParticipant({ identity: carolId }); +const localRtcMember = mockMembership("@carol:example.org", "CCCC"); +const aliceRtcMember = mockMembership("@alice:example.org", "AAAA"); +const bobRtcMember = mockMembership("@bob:example.org", "BBBB"); +const daveRtcMember = mockMembership("@dave:example.org", "DDDD"); + +const alice = mockRoomMember(aliceRtcMember); +const bob = mockRoomMember(bobRtcMember); +const carol = mockRoomMember(localRtcMember); +const dave = mockRoomMember(daveRtcMember); + +const aliceId = `${alice.userId}:${aliceRtcMember.deviceId}`; +const bobId = `${bob.userId}:${bobRtcMember.deviceId}`; +const daveId = `${dave.userId}:${daveRtcMember.deviceId}`; + +const localParticipant = mockLocalParticipant({ identity: "" }); const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const aliceSharingScreen = mockRemoteParticipant({ @@ -77,12 +77,9 @@ const bobSharingScreen = mockRemoteParticipant({ }); const daveParticipant = mockRemoteParticipant({ identity: daveId }); -const members = new Map([alice, bob, carol, dave].map((p) => [p.userId, p])); - -const aliceRtcMember = mockMembership(alice, aliceDev); -const bobRtcMember = mockMembership(bob, bobDev); -const localRtcMember = mockMembership(carol, carolDev); -const daveRtcMember = mockMembership(dave, daveDev); +const roomMembers = new Map( + [alice, bob, carol, dave].map((p) => [p.userId, p]), +); export interface GridLayoutSummary { type: "grid"; @@ -197,10 +194,10 @@ function withCallViewModel( ): void { const room = mockMatrixRoom({ client: { - getUserId: () => carol.userId, - getDeviceId: () => carolDev, + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, + getMember: (userId) => roomMembers.get(userId) ?? null, }); const rtcSession = new MockRTCSession(room as unknown as MockRoom); rtcMembers.subscribe((m) => { diff --git a/src/utils/test.ts b/src/utils/test.ts index b2df15622..2965f991b 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -122,8 +122,15 @@ export function mockMembership( // Maybe it'd be good to move this to matrix-js-sdk? Our testing needs are // rather simple, but if one util to mock a member is good enough for us, maybe // it's useful for matrix-js-sdk consumers in general. -export function mockRoomMember(member: Partial): RoomMember { - return { ...mockEmitter(), ...member } as RoomMember; +export function mockRoomMember( + rtcMembership: CallMembership, + member: Partial = {}, +): RoomMember { + return { + ...mockEmitter(), + userId: rtcMembership.sender, + ...member, + } as RoomMember; } export function mockMatrixRoom(room: Partial): MatrixRoom { From 5da642b71d64438ad657fe4ea3a4e8939df2bff2 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 15:24:58 +0000 Subject: [PATCH 36/84] Fix mocks --- src/state/MediaViewModel.test.ts | 23 +++++++++++++++-------- src/tile/GridTile.test.tsx | 3 ++- src/tile/SpotlightTile.test.tsx | 4 +++- src/utils/test.ts | 10 ++++++---- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 5b5e59a76..0ff5b5e6d 100644 --- a/src/state/MediaViewModel.test.ts +++ b/src/state/MediaViewModel.test.ts @@ -8,14 +8,17 @@ Please see LICENSE in the repository root for full details. import { expect, test, vi } from "vitest"; import { + mockMembership, withLocalMedia, withRemoteMedia, withTestScheduler, } from "../utils/test"; +const rtcMembership = mockMembership("@alice:example.org", "AAAA"); + test("control a participant's volume", async () => { const setVolumeSpy = vi.fn(); - await withRemoteMedia({}, { setVolume: setVolumeSpy }, (vm) => + await withRemoteMedia(rtcMembership, {}, { setVolume: setVolumeSpy }, (vm) => withTestScheduler(({ expectObservable, schedule }) => { schedule("-ab---c---d|", { a() { @@ -60,7 +63,7 @@ test("control a participant's volume", async () => { }); test("toggle fit/contain for a participant's video", async () => { - await withRemoteMedia({}, {}, (vm) => + await withRemoteMedia(rtcMembership, {}, {}, (vm) => withTestScheduler(({ expectObservable, schedule }) => { schedule("-ab|", { a: () => vm.toggleFitContain(), @@ -76,17 +79,21 @@ test("toggle fit/contain for a participant's video", async () => { }); test("local media remembers whether it should always be shown", async () => { - await withLocalMedia({}, (vm) => + await withLocalMedia(rtcMembership, {}, (vm) => withTestScheduler(({ expectObservable, schedule }) => { schedule("-a|", { a: () => vm.setAlwaysShow(false) }); expectObservable(vm.alwaysShow).toBe("ab", { a: true, b: false }); }), ); // Next local media should start out *not* always shown - await withLocalMedia({}, (vm) => - withTestScheduler(({ expectObservable, schedule }) => { - schedule("-a|", { a: () => vm.setAlwaysShow(true) }); - expectObservable(vm.alwaysShow).toBe("ab", { a: false, b: true }); - }), + await withLocalMedia( + rtcMembership, + + {}, + (vm) => + withTestScheduler(({ expectObservable, schedule }) => { + schedule("-a|", { a: () => vm.setAlwaysShow(true) }); + expectObservable(vm.alwaysShow).toBe("ab", { a: false, b: true }); + }), ); }); diff --git a/src/tile/GridTile.test.tsx b/src/tile/GridTile.test.tsx index 81e501102..2f7a2a327 100644 --- a/src/tile/GridTile.test.tsx +++ b/src/tile/GridTile.test.tsx @@ -13,7 +13,7 @@ import { of } from "rxjs"; import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; import { GridTile } from "./GridTile"; -import { withRemoteMedia } from "../utils/test"; +import { mockMembership, withRemoteMedia } from "../utils/test"; import { GridTileViewModel } from "../state/TileViewModel"; import { ReactionsProvider } from "../useReactions"; @@ -25,6 +25,7 @@ global.IntersectionObserver = class MockIntersectionObserver { test("GridTile is accessible", async () => { await withRemoteMedia( + mockMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", diff --git a/src/tile/SpotlightTile.test.tsx b/src/tile/SpotlightTile.test.tsx index cedeea626..1b29cb9f0 100644 --- a/src/tile/SpotlightTile.test.tsx +++ b/src/tile/SpotlightTile.test.tsx @@ -12,7 +12,7 @@ import userEvent from "@testing-library/user-event"; import { of } from "rxjs"; import { SpotlightTile } from "./SpotlightTile"; -import { withLocalMedia, withRemoteMedia } from "../utils/test"; +import { mockMembership, withLocalMedia, withRemoteMedia } from "../utils/test"; import { SpotlightTileViewModel } from "../state/TileViewModel"; global.IntersectionObserver = class MockIntersectionObserver { @@ -22,6 +22,7 @@ global.IntersectionObserver = class MockIntersectionObserver { test("SpotlightTile is accessible", async () => { await withRemoteMedia( + mockMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", @@ -29,6 +30,7 @@ test("SpotlightTile is accessible", async () => { {}, async (vm1) => { await withLocalMedia( + mockMembership("@bob:example.org", "BBBB"), { rawDisplayName: "Bob", getMxcAvatarUrl: () => "mxc://dlskf", diff --git a/src/utils/test.ts b/src/utils/test.ts index 2965f991b..c6789a415 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -173,13 +173,14 @@ export function mockLocalParticipant( } export async function withLocalMedia( - member: Partial, + localRtcMember: CallMembership, + roomMember: Partial, continuation: (vm: LocalUserMediaViewModel) => void | Promise, ): Promise { const localParticipant = mockLocalParticipant({}); const vm = new LocalUserMediaViewModel( "local", - mockRoomMember(member), + mockRoomMember(localRtcMember, roomMember), of(localParticipant), { kind: E2eeType.PER_PARTICIPANT, @@ -207,14 +208,15 @@ export function mockRemoteParticipant( } export async function withRemoteMedia( - member: Partial, + localRtcMember: CallMembership, + roomMember: Partial, participant: Partial, continuation: (vm: RemoteUserMediaViewModel) => void | Promise, ): Promise { const remoteParticipant = mockRemoteParticipant(participant); const vm = new RemoteUserMediaViewModel( "remote", - mockRoomMember(member), + mockRoomMember(localRtcMember, roomMember), of(remoteParticipant), { kind: E2eeType.PER_PARTICIPANT, From bf41cfc005faf8e402deccf58e08a155a32cbc40 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 15:28:55 +0000 Subject: [PATCH 37/84] Cleanup --- src/state/CallViewModel.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index b8559d246..ee5eeda11 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -262,7 +262,7 @@ class UserMedia { timer(s ? 1000 : 60000), // 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)), + this.vm.speaking.pipe(filter((s1) => s1 !== s)), ), ), startWith(false), @@ -313,7 +313,7 @@ class ScreenShare { this.vm = new ScreenShareViewModel( id, member, - this.participant.asObservable(), + this.participant, encryptionSystem, liveKitRoom, participant.isLocal, From 0c4cddbb3290fbc9f030a5698dcac81cb3c248b4 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 16:23:40 +0000 Subject: [PATCH 38/84] Apply local override where participant not yet known --- src/state/CallViewModel.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index ee5eeda11..080adbf41 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -231,10 +231,11 @@ class UserMedia { participant: LocalParticipant | RemoteParticipant | undefined, encryptionSystem: EncryptionSystem, livekitRoom: LivekitRoom, + local: boolean, ) { this.participant = new BehaviorSubject(participant); - if (participant && participant.isLocal) { + if (participant?.isLocal || local) { this.vm = new LocalUserMediaViewModel( this.id, member, @@ -500,6 +501,7 @@ export class CallViewModel extends ViewModel { participant, this.encryptionSystem, this.livekitRoom, + mediaId === "local", ), ]; @@ -556,6 +558,7 @@ export class CallViewModel extends ViewModel { participant, this.encryptionSystem, this.livekitRoom, + false, ), ]; } From be250e2f2574618a91621ae2b2ae42d9b878ba4c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 16:34:09 +0000 Subject: [PATCH 39/84] Handle no visible media id --- src/tile/SpotlightTile.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index 470f028f5..33661c603 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -210,7 +210,9 @@ export const SpotlightTile = forwardRef( const ref = useMergedRefs(ourRef, theirRef); const maximised = useObservableEagerState(vm.maximised); const media = useObservableEagerState(vm.media); - const [visibleId, setVisibleId] = useState(media[0].id); + const [visibleId, setVisibleId] = useState( + media.length > 0 ? media[0].id : undefined, + ); const latestMedia = useLatest(media); const latestVisibleId = useLatest(visibleId); const visibleIndex = media.findIndex((vm) => vm.id === visibleId); From 5009f1fe1b4a08f5df361b6d465cd9653e0c67df Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 16:53:25 +0000 Subject: [PATCH 40/84] Assertions for one-on-one view --- src/state/CallViewModel.ts | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 080adbf41..0db03af15 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -802,6 +802,7 @@ export class CallViewModel extends ViewModel { // There might not be a remote tile if only the local user is in the call // and they're using the duplicate tiles option grid.some((vm) => !vm.local) && + grid.some((vm) => vm.local) && screenShares.length === 0, ); @@ -816,12 +817,14 @@ export class CallViewModel extends ViewModel { }), ); - private readonly spotlightLandscapeLayout: Observable = - combineLatest([this.grid, this.spotlight], (grid, spotlight) => ({ + private spotlightLandscapeLayout: Observable = combineLatest( + [this.grid, this.spotlight], + (grid, spotlight) => ({ type: "spotlight-landscape", spotlight, grid, - })); + }), + ); private readonly spotlightPortraitLayout: Observable = combineLatest([this.grid, this.spotlight], (grid, spotlight) => ({ @@ -839,11 +842,24 @@ export class CallViewModel extends ViewModel { private readonly oneOnOneLayout: Observable = this.mediaItems.pipe( - map((grid) => ({ - type: "one-on-one", - local: grid.find((vm) => vm.vm.local)!.vm as LocalUserMediaViewModel, - remote: grid.find((vm) => !vm.vm.local)!.vm as RemoteUserMediaViewModel, - })), + map((grid) => { + const local = grid.find((vm) => vm.vm.local)?.vm as + | LocalUserMediaViewModel + | undefined; + const remote = grid.find((vm) => !vm.vm.local)?.vm as + | RemoteUserMediaViewModel + | undefined; + if (!local || !remote) { + throw new Error( + "Invalid state: there should be local and remote media for one-on-one layout", + ); + } + return { + type: "one-on-one", + local, + remote, + }; + }), ); private readonly pipLayout: Observable = this.spotlight.pipe( From 238797aed862d4dfaa2f2bd69362a686d6cc5293 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 17:53:08 +0000 Subject: [PATCH 41/84] Remove isLiveKitParticipantAvailable and show via encryption status --- src/state/MediaViewModel.ts | 10 ++++------ src/tile/GridTile.tsx | 13 +------------ 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index e005fbc3f..31232b749 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -215,9 +215,6 @@ abstract class BaseMediaViewModel extends ViewModel { */ public readonly unencryptedWarning: Observable; - public readonly isLiveKitParticipantAvailable: Observable = - this.participant.pipe(map((p) => !!p)); - public readonly encryptionStatus: Observable; public constructor( @@ -231,7 +228,7 @@ abstract class BaseMediaViewModel extends ViewModel { // TODO: Fully separate the data layer from the UI layer by keeping the // member object internal public readonly member: RoomMember | undefined, - // We dont necessarily have a participant if a user connects via MatrixRTC but not (not yet) through + // We don't necessarily have a participant if a user connects via MatrixRTC but not (yet) through // livekit. protected readonly participant: Observable< LocalParticipant | RemoteParticipant | undefined @@ -259,8 +256,9 @@ abstract class BaseMediaViewModel extends ViewModel { this.encryptionStatus = this.participant.pipe( switchMap((participant): Observable => { - if ( - !participant || + if (!participant) { + return of(EncryptionStatus.Connecting); + } else if ( participant.isLocal || encryptionSystem.kind === E2eeType.NONE ) { diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index e493cba7b..bad9a613f 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -86,9 +86,6 @@ const UserMediaTile = forwardRef( const videoEnabled = useObservableEagerState(vm.videoEnabled); const speaking = useObservableEagerState(vm.speaking); const cropVideo = useObservableEagerState(vm.cropVideo); - const isLiveKitParticipantAvailable = useObservableEagerState( - vm.isLiveKitParticipantAvailable, - ); const onSelectFitContain = useCallback( (e: Event) => { e.preventDefault(); @@ -142,12 +139,7 @@ const UserMediaTile = forwardRef( className={styles.muteIcon} /> } - displayName={ - displayName + - (isLiveKitParticipantAvailable - ? "" - : " missing Livekit Participant...") - } + displayName={displayName} primaryButton={ - {/* {isLiveKitParticipantAvailable - ? "is available" - : "Loading RTC participant"} */} Date: Thu, 7 Nov 2024 17:53:44 +0000 Subject: [PATCH 42/84] Handle no local media (yet) --- src/state/CallViewModel.ts | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 0db03af15..0dff16386 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -598,11 +598,6 @@ export class CallViewModel extends ViewModel { ), ); - private readonly localUserMedia: Observable = - this.mediaItems.pipe( - map((ms) => ms.find((m) => m.vm.local)!.vm as LocalUserMediaViewModel), - ); - private readonly screenShares: Observable = this.mediaItems.pipe( map((mediaItems) => @@ -699,12 +694,19 @@ export class CallViewModel extends ViewModel { speaker ? speaker.local ? of(undefined) - : this.localUserMedia.pipe( - switchMap((vm) => - vm.alwaysShow.pipe( - map((alwaysShow) => (alwaysShow ? vm : undefined)), - ), - ), + : this.mediaItems.pipe( + switchMap((mediaItems) => { + const localUserMedia = mediaItems.find( + (m) => m.vm instanceof LocalUserMediaViewModel, + ) as LocalUserMediaViewModel | undefined; + return ( + localUserMedia?.alwaysShow?.pipe( + map((alwaysShow) => + alwaysShow ? localUserMedia : undefined, + ), + ) ?? of(undefined) + ); + }), ) : of(undefined), ), @@ -850,9 +852,17 @@ export class CallViewModel extends ViewModel { | RemoteUserMediaViewModel | undefined; if (!local || !remote) { - throw new Error( - "Invalid state: there should be local and remote media for one-on-one layout", + logger.warn( + `Falling back to grid layout for one-on-one layout with missing media: local=${!!local} remote=${!!remote}`, ); + return { + type: "grid", + grid: grid.filter( + (m) => + m instanceof LocalUserMediaViewModel || + m instanceof RemoteUserMediaViewModel, + ), + }; } return { type: "one-on-one", From 190ac9be3d72a52b8353853b37da19fc4f7ab21f Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 18:05:41 +0000 Subject: [PATCH 43/84] Remove unused effect for setting --- src/App.tsx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 0b6e32cf5..8d841dba7 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -28,8 +28,6 @@ import { Initializer } from "./initializer"; import { MediaDevicesProvider } from "./livekit/MediaDevicesContext"; import { widget } from "./widget"; import { useTheme } from "./useTheme"; -import { showNonMemberTiles } from "./settings/settings"; -import { Config } from "./config/Config"; const SentryRoute = Sentry.withSentryRouting(Route); @@ -73,13 +71,6 @@ export const App: FC = ({ history }) => { .catch(logger.error); }); - // Update settings to use the non member tile information from the config if set - useEffect(() => { - if (loaded && Config.get().show_non_member_tiles) { - showNonMemberTiles.setValue(true); - } - }); - const errorPage = ; return ( From cf3893bf521a73425a9b577a2ddc9a6492900e88 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 7 Nov 2024 18:24:13 +0000 Subject: [PATCH 44/84] Tidy settings --- src/config/Config.ts | 15 +++++++++++---- src/settings/SettingsModal.tsx | 2 +- src/settings/settings.ts | 6 ++++-- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/config/Config.ts b/src/config/Config.ts index b230734c3..cbe68e553 100644 --- a/src/config/Config.ts +++ b/src/config/Config.ts @@ -70,10 +70,17 @@ export class Config { private applyConfigToSettings(): void { if (!this.config) return; - // only the value from config if it hasn't been overridden - if (showNonMemberTiles.value === undefined) { - showNonMemberTiles.setValue(this.config.show_non_member_tiles); - } + // use the value from config if it hasn't been overridden + const showNonMemberTilesSubscription = showNonMemberTiles.value.subscribe( + (val) => { + if (val === undefined && this.config) { + // we don't persist the value to local storage so that it is set from the config + // file on every startup + showNonMemberTiles.setValue(this.config.show_non_member_tiles, false); + showNonMemberTilesSubscription.unsubscribe(); + } + }, + ); } public config?: ResolvedConfigOptions; diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index eaf38be50..d36a05d25 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -246,7 +246,7 @@ export const SettingsModal: FC = ({ id="showNonMemberTiles" type="checkbox" label={t("settings.show_non_member_tiles")} - checked={showNonMemberTiles} + checked={!!showNonMemberTiles} onChange={useCallback( (event: ChangeEvent): void => { setShowNonMemberTiles(event.target.checked); diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 47b80d9b0..b614f5db1 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -37,9 +37,11 @@ export class Setting { private readonly _value: BehaviorSubject; public readonly value: Observable; - public readonly setValue = (value: T): void => { + public readonly setValue = (value: T, persist = true): void => { this._value.next(value); - localStorage.setItem(this.key, JSON.stringify(value)); + if (persist) { + localStorage.setItem(this.key, JSON.stringify(value)); + } }; } From 886dc2c2feb8fed0d321a7ca61199971b2afddd6 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 8 Nov 2024 06:50:58 +0000 Subject: [PATCH 45/84] Avoid case of one-to-one layout with missing local or remote --- src/state/CallViewModel.ts | 71 ++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 42 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 0dff16386..431a099d2 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -797,16 +797,26 @@ export class CallViewModel extends ViewModel { this.gridModeUserSelection.next(value); } - private readonly oneOnOne: Observable = combineLatest( - [this.grid, this.screenShares], - (grid, screenShares) => - grid.length == 2 && - // There might not be a remote tile if only the local user is in the call - // and they're using the duplicate tiles option - grid.some((vm) => !vm.local) && - grid.some((vm) => vm.local) && - screenShares.length === 0, - ); + private readonly oneOnOne: Observable< + | { local: LocalUserMediaViewModel; remote: RemoteUserMediaViewModel } + | undefined + > = combineLatest([this.grid, this.screenShares], (grid, screenShares) => { + if (grid.length !== 2 || screenShares.length !== 0) { + return undefined; + } + + const local = grid.find((vm) => vm.local); + const remote = grid.find((vm) => !vm.local); + + if (!local || !remote) { + return undefined; + } + + return { + local, + remote, + }; + }); private readonly gridLayout: Observable = combineLatest( [this.grid, this.spotlight], @@ -842,36 +852,6 @@ export class CallViewModel extends ViewModel { pip: pip ?? undefined, })); - private readonly oneOnOneLayout: Observable = - this.mediaItems.pipe( - map((grid) => { - const local = grid.find((vm) => vm.vm.local)?.vm as - | LocalUserMediaViewModel - | undefined; - const remote = grid.find((vm) => !vm.vm.local)?.vm as - | RemoteUserMediaViewModel - | undefined; - if (!local || !remote) { - logger.warn( - `Falling back to grid layout for one-on-one layout with missing media: local=${!!local} remote=${!!remote}`, - ); - return { - type: "grid", - grid: grid.filter( - (m) => - m instanceof LocalUserMediaViewModel || - m instanceof RemoteUserMediaViewModel, - ), - }; - } - return { - type: "one-on-one", - local, - remote, - }; - }), - ); - private readonly pipLayout: Observable = this.spotlight.pipe( map((spotlight) => ({ type: "pip", spotlight })), ); @@ -888,8 +868,15 @@ export class CallViewModel extends ViewModel { switch (gridMode) { case "grid": return this.oneOnOne.pipe( - switchMap((oneOnOne) => - oneOnOne ? this.oneOnOneLayout : this.gridLayout, + switchMap( + (oneOnOne): Observable => + oneOnOne + ? of({ + type: "one-on-one", + local: oneOnOne.local, + remote: oneOnOne.remote, + }) + : this.gridLayout, ), ); case "spotlight": From 83514212cd008904223c8d8040ca376085b32c5d Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 13 Nov 2024 10:03:57 +0000 Subject: [PATCH 46/84] Iterate --- src/state/CallViewModel.test.ts | 19 +++++++------------ src/utils/test.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 9f2081123..5af59711c 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -24,11 +24,7 @@ import { } from "livekit-client"; import * as ComponentsCore from "@livekit/components-core"; import { isEqual } from "lodash"; -import { - CallMembership, - MatrixRTCSession, - MatrixRTCSessionEvent, -} from "matrix-js-sdk/src/matrixrtc"; +import { CallMembership, MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc"; import { CallViewModel, Layout } from "./CallViewModel"; import { @@ -39,13 +35,13 @@ import { mockRemoteParticipant, withTestScheduler, mockMembership, + MockRTCSession, } from "../utils/test"; import { ECAddonConnectionState, ECConnectionState, } from "../livekit/useECConnectionState"; import { E2eeType } from "../e2ee/e2eeType"; -import { MockRoom, MockRTCSession } from "../useReactions.test"; vi.mock("@livekit/components-core"); @@ -199,12 +195,11 @@ function withCallViewModel( } as Partial as MatrixClient, getMember: (userId) => roomMembers.get(userId) ?? null, }); - const rtcSession = new MockRTCSession(room as unknown as MockRoom); - rtcMembers.subscribe((m) => { - // always prepend the local participant - rtcSession.memberships = [localRtcMember, ...m]; - rtcSession.emit(MatrixRTCSessionEvent.MembershipsChanged); - }); + const rtcSession = new MockRTCSession( + room, + localRtcMember, + [], + ).withMemberships(rtcMembers); const participantsSpy = vi .spyOn(ComponentsCore, "connectedParticipantsObserver") .mockReturnValue(remoteParticipants); diff --git a/src/utils/test.ts b/src/utils/test.ts index c6789a415..68971bbef 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -11,10 +11,14 @@ import { RoomMember, Room as MatrixRoom, MatrixEvent, + Room, + TypedEventEmitter, } from "matrix-js-sdk/src/matrix"; import { CallMembership, Focus, + MatrixRTCSessionEvent, + MatrixRTCSessionEventHandlerMap, SessionMembershipData, } from "matrix-js-sdk/src/matrixrtc"; import { @@ -229,3 +233,30 @@ export async function withRemoteMedia( vm.destroy(); } } + +export class MockRTCSession extends TypedEventEmitter< + MatrixRTCSessionEvent, + MatrixRTCSessionEventHandlerMap +> { + public constructor( + public readonly room: Room, + private localMembership: CallMembership, + public memberships: CallMembership[] = [], + ) { + super(); + } + + public withMemberships( + rtcMembers: Observable[]>, + ): MockRTCSession { + rtcMembers.subscribe((m) => { + const old = this.memberships; + // always prepend the local participant + const updated = [this.localMembership, ...(m as CallMembership[])]; + this.memberships = updated; + this.emit(MatrixRTCSessionEvent.MembershipsChanged, old, updated); + }); + + return this; + } +} From 44935eeb401246a282b24f52824caae9de963177 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 13 Nov 2024 10:14:12 +0000 Subject: [PATCH 47/84] Remove option to show non-member tiles to simplify code review --- public/locales/en-GB/app.json | 1 - src/config/Config.ts | 17 -------- src/config/ConfigOptions.ts | 2 - src/settings/SettingsModal.tsx | 19 --------- src/settings/settings.ts | 5 --- src/state/CallViewModel.ts | 73 ++-------------------------------- 6 files changed, 3 insertions(+), 114 deletions(-) diff --git a/public/locales/en-GB/app.json b/public/locales/en-GB/app.json index 1b5e8a515..dc1b6010f 100644 --- a/public/locales/en-GB/app.json +++ b/public/locales/en-GB/app.json @@ -171,7 +171,6 @@ "preferences_tab_h4": "Preferences", "preferences_tab_show_hand_raised_timer_description": "Show a timer when a participant raises their hand", "preferences_tab_show_hand_raised_timer_label": "Show hand raise duration", - "show_non_member_tiles": "Show tiles for non-member media", "speaker_device_selection_label": "Speaker" }, "star_rating_input_label_one": "{{count}} stars", diff --git a/src/config/Config.ts b/src/config/Config.ts index cbe68e553..972c9e0ca 100644 --- a/src/config/Config.ts +++ b/src/config/Config.ts @@ -13,7 +13,6 @@ import { ConfigOptions, ResolvedConfigOptions, } from "./ConfigOptions"; -import { showNonMemberTiles } from "../settings/settings"; export class Config { private static internalInstance: Config | undefined; @@ -33,7 +32,6 @@ export class Config { "../config.json", ).then((config) => { internalInstance.config = merge({}, DEFAULT_CONFIG, config); - internalInstance.applyConfigToSettings(); }); } return Config.internalInstance.initPromise; @@ -68,21 +66,6 @@ export class Config { return Config.get().default_server_config?.["m.homeserver"].server_name; } - private applyConfigToSettings(): void { - if (!this.config) return; - // use the value from config if it hasn't been overridden - const showNonMemberTilesSubscription = showNonMemberTiles.value.subscribe( - (val) => { - if (val === undefined && this.config) { - // we don't persist the value to local storage so that it is set from the config - // file on every startup - showNonMemberTiles.setValue(this.config.show_non_member_tiles, false); - showNonMemberTilesSubscription.unsubscribe(); - } - }, - ); - } - public config?: ResolvedConfigOptions; private initPromise?: Promise; } diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index 1303ebe76..ed4d5bceb 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -136,7 +136,6 @@ export interface ResolvedConfigOptions extends ConfigOptions { enable_video: boolean; }; app_prompt: boolean; - show_non_member_tiles: boolean; } export const DEFAULT_CONFIG: ResolvedConfigOptions = { @@ -155,5 +154,4 @@ export const DEFAULT_CONFIG: ResolvedConfigOptions = { enable_video: true, }, app_prompt: true, - show_non_member_tiles: false, }; diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 98883359c..07ca57537 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -27,7 +27,6 @@ import { useSetting, developerSettingsTab as developerSettingsTabSetting, duplicateTiles as duplicateTilesSetting, - showNonMemberTiles as showNonMemberTilesSetting, useOptInAnalytics, soundEffectVolumeSetting, } from "./settings"; @@ -71,10 +70,6 @@ export const SettingsModal: FC = ({ ); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); - const [showNonMemberTiles, setShowNonMemberTiles] = useSetting( - showNonMemberTilesSetting, - ); - // Generate a `SelectInput` with a list of devices for a given device kind. const generateDeviceSelection = ( devices: MediaDevice, @@ -258,20 +253,6 @@ export const SettingsModal: FC = ({ )} /> - - ): void => { - setShowNonMemberTiles(event.target.checked); - }, - [setShowNonMemberTiles], - )} - /> - ), }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 71112bc26..4eb2bcaa7 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -77,11 +77,6 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); -export const showNonMemberTiles = new Setting( - "show-non-member-tiles", - undefined, -); - export const audioInput = new Setting( "audio-input", undefined, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index c408c3488..f3791013b 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -67,7 +67,7 @@ import { } from "./MediaViewModel"; import { accumulate, finalizeValue } from "../utils/observable"; import { ObservableScope } from "./ObservableScope"; -import { duplicateTiles, showNonMemberTiles } from "../settings/settings"; +import { duplicateTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { GridTileViewModel, SpotlightTileViewModel } from "./TileViewModel"; @@ -439,7 +439,6 @@ export class CallViewModel extends ViewModel { this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, ).pipe(startWith(null)), - showNonMemberTiles.value, ]).pipe( scan( ( @@ -449,7 +448,6 @@ export class CallViewModel extends ViewModel { { participant: localParticipant }, duplicateTiles, _membershipsChanged, - showNonMemberTiles, ], ) => { const newItems = new Map( @@ -478,20 +476,12 @@ export class CallViewModel extends ViewModel { } for (let i = 0; i < 1 + duplicateTiles; i++) { const indexedMediaId = `${mediaId}:${i}`; - let prevMedia = prevItems.get(indexedMediaId); + const prevMedia = prevItems.get(indexedMediaId); if (prevMedia && prevMedia instanceof UserMedia) { if (prevMedia.participant.value !== participant) { // Update the BahviourSubject in the UserMedia. prevMedia.participant.next(participant); } - if (prevMedia.vm.member === undefined) { - // We have a previous media created because of the `debugShowNonMember` flag. - // In this case we actually replace the media item. - // This "hack" never occurs if we do not use the `debugShowNonMember` debugging - // option and if we always find a room member for each rtc member (which also - // only fails if we have a fundamental problem) - prevMedia = undefined; - } } yield [ indexedMediaId, @@ -528,64 +518,7 @@ export class CallViewModel extends ViewModel { }.bind(this)(), ); - // Generate non member items (items without a corresponding MatrixRTC member) - // Those items should not be rendered, they are participants in livekit that do not have a corresponding - // matrix rtc members. This cannot be any good: - // - A malicious user impersonates someone - // - Someone injects abusive content - // - The user cannot have encryption keys so it makes no sense to participate - // We can only trust users that have a matrixRTC member event. - // - // This is still available as a debug option. This can be useful - // - If one wants to test scalability using the livekit cli. - // - If an experimental project does not yet do the matrixRTC bits. - // - If someone wants to debug if the LK connection works but matrixRTC room state failed to arrive. - const debugShowNonMember = showNonMemberTiles; //Config.get().show_non_member_tiles; - const newNonMemberItems = debugShowNonMember - ? new Map( - function* (this: CallViewModel): Iterable<[string, MediaItem]> { - for (const participant of remoteParticipants) { - for (let i = 0; i < 1 + duplicateTiles; i++) { - const maybeNonMemberParticipantId = - participant.identity + ":" + i; - if (!newItems.has(maybeNonMemberParticipantId)) { - const nonMemberId = maybeNonMemberParticipantId; - yield [ - nonMemberId, - // We create UserMedia with or without a participant. - // This will be the initial value of a BehaviourSubject. - // Once a participant appears we will update the BehaviourSubject. (see above) - prevItems.get(nonMemberId) ?? - new UserMedia( - nonMemberId, - undefined, - participant, - this.encryptionSystem, - this.livekitRoom, - false, - ), - ]; - } - } - } - }.bind(this)(), - ) - : new Map(); - if (newNonMemberItems.size > 0) { - logger.debug("Added NonMember items: ", newNonMemberItems); - } - const newNonMemberItemCount = - newNonMemberItems.size / (1 + duplicateTiles); - if (this.nonMemberItemCount.value !== newNonMemberItemCount) - this.nonMemberItemCount.next(newNonMemberItemCount); - - const combinedNew = new Map([ - ...newNonMemberItems.entries(), - ...newItems.entries(), - ]); - - for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy(); - return combinedNew; + return newItems; }, new Map(), ), From b3e725fcac99488923e041e129e43314035d7c63 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 13 Nov 2024 10:16:38 +0000 Subject: [PATCH 48/84] Remove unused code --- src/settings/settings.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 4eb2bcaa7..b2da16744 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -40,11 +40,9 @@ export class Setting { private readonly _value: BehaviorSubject; public readonly value: Observable; - public readonly setValue = (value: T, persist = true): void => { + public readonly setValue = (value: T): void => { this._value.next(value); - if (persist) { - localStorage.setItem(this.key, JSON.stringify(value)); - } + localStorage.setItem(this.key, JSON.stringify(value)); }; } From 87c3793f6958861b03b5c2c3fe15eba214606d52 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 14 Nov 2024 07:25:52 +0000 Subject: [PATCH 49/84] Remove more remnants of show-non-member-tiles --- src/Header.tsx | 5 +---- src/room/InCallView.tsx | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Header.tsx b/src/Header.tsx index 419102b35..69e77935c 100644 --- a/src/Header.tsx +++ b/src/Header.tsx @@ -117,7 +117,6 @@ interface RoomHeaderInfoProps { avatarUrl: string | null; encrypted: boolean; participantCount: number | null; - nonMemberItemCount?: number; } export const RoomHeaderInfo: FC = ({ @@ -126,7 +125,6 @@ export const RoomHeaderInfo: FC = ({ avatarUrl, encrypted, participantCount, - nonMemberItemCount, }) => { const { t } = useTranslation(); const size = useMediaQuery("(max-width: 550px)") ? "sm" : "lg"; @@ -159,8 +157,7 @@ export const RoomHeaderInfo: FC = ({ aria-label={t("header_participants_label")} /> - {t("participant_count", { count: participantCount ?? 0 })}{" "} - {(nonMemberItemCount ?? 0) > 0 && <>(+ {nonMemberItemCount})} + {t("participant_count", { count: participantCount ?? 0 })}
)} diff --git a/src/room/InCallView.tsx b/src/room/InCallView.tsx index 0fc0c5eda..0c3caf9aa 100644 --- a/src/room/InCallView.tsx +++ b/src/room/InCallView.tsx @@ -195,7 +195,6 @@ export const InCallView: FC = ({ } }, [connState, onLeave]); - const nonMemberItemCount = useObservableEagerState(vm.nonMemberItemCount); const containerRef1 = useRef(null); const [containerRef2, bounds] = useMeasure(); // Merge the refs so they can attach to the same element @@ -650,7 +649,6 @@ export const InCallView: FC = ({ avatarUrl={matrixInfo.roomAvatar} encrypted={matrixInfo.e2eeSystem.kind !== E2eeType.NONE} participantCount={participantCount} - nonMemberItemCount={nonMemberItemCount} /> From fa3c9fde14320b7f8558dbd2a5233a787323f36a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 14 Nov 2024 13:34:49 +0000 Subject: [PATCH 50/84] iterate --- src/state/CallViewModel.ts | 73 ++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index f3791013b..70486bb06 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -223,7 +223,7 @@ interface LayoutScanState { class UserMedia { private readonly scope = new ObservableScope(); public readonly vm: UserMediaViewModel; - public readonly participant: BehaviorSubject< + private readonly participant: BehaviorSubject< LocalParticipant | RemoteParticipant | undefined >; @@ -294,6 +294,15 @@ class UserMedia { ); } + public updateParticipant( + newParticipant: LocalParticipant | RemoteParticipant | undefined, + ): void { + if (this.participant.value !== newParticipant) { + // Update the BehaviourSubject in the UserMedia. + this.participant.next(newParticipant); + } + } + public destroy(): void { this.scope.end(); this.vm.destroy(); @@ -367,12 +376,17 @@ export class CallViewModel extends ViewModel { }), ); - private readonly rawRemoteParticipants = connectedParticipantsObserver( - this.livekitRoom, - ).pipe(this.scope.state()); + /** + * The raw list of RemoteParticipants as reported by LiveKit + */ - // Lists of participants to "hold" on display, even if LiveKit claims that - // they've left + private readonly rawRemoteParticipants: Observable = + connectedParticipantsObserver(this.livekitRoom).pipe(this.scope.state()); + + /** + * Lists of RemoteParticipants to "hold" on display, even if LiveKit claims that + * they've left + */ private readonly remoteParticipantHolds: Observable = this.connectionState.pipe( withLatestFrom(this.rawRemoteParticipants), @@ -407,6 +421,9 @@ export class CallViewModel extends ViewModel { ), ); + /** + * The RemoteParticipants including those that are being "held" on the screen + */ private readonly remoteParticipants: Observable = combineLatest( [this.rawRemoteParticipants, this.remoteParticipantHolds], @@ -428,8 +445,9 @@ export class CallViewModel extends ViewModel { }, ); - public readonly nonMemberItemCount = new BehaviorSubject(0); - + /** + * List of MediaItems that we want to display + */ private readonly mediaItems: Observable = combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -452,36 +470,42 @@ export class CallViewModel extends ViewModel { ) => { const newItems = new Map( function* (this: CallViewModel): Iterable<[string, MediaItem]> { + // m.rtc.members are the basis for calculating what is visible in the call for (const rtcMember of this.matrixRTCSession.memberships) { const room = this.matrixRTCSession.room; // WARN! This is not exactly the sender but the user defined in the state key. // This will be available once we change to the new "member as object" format in the MatrixRTC object. - let mediaId = rtcMember.sender + ":" + rtcMember.deviceId; - let participant = undefined; + let livekitParticipantId = + rtcMember.sender + ":" + rtcMember.deviceId; + + let participant: + | LocalParticipant + | RemoteParticipant + | undefined = undefined; if ( rtcMember.sender === room.client.getUserId()! && rtcMember.deviceId === room.client.getDeviceId() ) { - mediaId = "local"; + livekitParticipantId = "local"; participant = localParticipant; } else { participant = remoteParticipants.find( - (p) => p.identity === mediaId, + (p) => p.identity === livekitParticipantId, ); } - const member = findMatrixRoomMember(room, mediaId); + const member = findMatrixRoomMember(room, livekitParticipantId); if (!member) { - logger.error("Could not find member for media id: ", mediaId); + logger.error( + "Could not find member for media id: ", + livekitParticipantId, + ); } for (let i = 0; i < 1 + duplicateTiles; i++) { - const indexedMediaId = `${mediaId}:${i}`; + const indexedMediaId = `${livekitParticipantId}:${i}`; const prevMedia = prevItems.get(indexedMediaId); if (prevMedia && prevMedia instanceof UserMedia) { - if (prevMedia.participant.value !== participant) { - // Update the BahviourSubject in the UserMedia. - prevMedia.participant.next(participant); - } + prevMedia.updateParticipant(participant); } yield [ indexedMediaId, @@ -495,7 +519,7 @@ export class CallViewModel extends ViewModel { participant, this.encryptionSystem, this.livekitRoom, - mediaId === "local", + livekitParticipantId === "local", ), ]; @@ -529,12 +553,18 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); + /** + * List of MediaItems that we want to display, that are of type UserMedia + */ private readonly userMedia: Observable = this.mediaItems.pipe( map((mediaItems) => mediaItems.filter((m): m is UserMedia => m instanceof UserMedia), ), ); + /** + * List of MediaItems that we want to display, that are of type ScreenShare + */ private readonly screenShares: Observable = this.mediaItems.pipe( map((mediaItems) => @@ -853,7 +883,6 @@ export class CallViewModel extends ViewModel { }), this.scope.state(), ); - /** * The layout of tiles in the call interface. */ @@ -1006,7 +1035,7 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - public readonly showFooter = this.windowMode.pipe( + public readonly showFooter: Observable = this.windowMode.pipe( switchMap((mode) => { switch (mode) { case "pip": From f6a641b4786f2e5f17bb539461b0254cf6e6f3a5 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Thu, 14 Nov 2024 13:51:49 +0000 Subject: [PATCH 51/84] back --- src/state/CallViewModel.ts | 89 +++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index dcfe456aa..b1d64a7ce 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -379,7 +379,6 @@ export class CallViewModel extends ViewModel { /** * The raw list of RemoteParticipants as reported by LiveKit */ - private readonly rawRemoteParticipants: Observable = connectedParticipantsObserver(this.livekitRoom).pipe(this.scope.state()); @@ -573,42 +572,41 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - private readonly spotlightSpeaker: Observable< - UserMediaViewModel | undefined - > = this.userMedia.pipe( - switchMap((mediaItems) => - mediaItems.length === 0 - ? of([]) - : combineLatest( - mediaItems.map((m) => - m.vm.speaking.pipe(map((s) => [m, s] as const)), + private readonly spotlightSpeaker: Observable = + this.userMedia.pipe( + switchMap((mediaItems) => + mediaItems.length === 0 + ? of([]) + : combineLatest( + mediaItems.map((m) => + m.vm.speaking.pipe(map((s) => [m, s] as const)), + ), ), - ), - ), - scan<(readonly [UserMedia, boolean])[], UserMedia | undefined, null>( - (prev, mediaItems) => { - // Only remote users that are still in the call should be sticky - const [stickyMedia, stickySpeaking] = - (!prev?.vm.local && mediaItems.find(([m]) => m === prev)) || []; - // Decide who to spotlight: - // If the previous speaker is still speaking, stick with them rather - // than switching eagerly to someone else - return stickySpeaking - ? stickyMedia! - : // Otherwise, select any remote user who is speaking - (mediaItems.find(([m, s]) => !m.vm.local && s)?.[0] ?? - // Otherwise, stick with the person who was last speaking - stickyMedia ?? - // Otherwise, spotlight an arbitrary remote user - mediaItems.find(([m]) => !m.vm.local)?.[0] ?? - // Otherwise, spotlight the local user - mediaItems.find(([m]) => m.vm.local)?.[0]); - }, - null, - ), - map((speaker) => speaker?.vm), - this.scope.state(), - ); + ), + scan<(readonly [UserMedia, boolean])[], UserMedia | undefined, null>( + (prev, mediaItems) => { + // Only remote users that are still in the call should be sticky + const [stickyMedia, stickySpeaking] = + (!prev?.vm.local && mediaItems.find(([m]) => m === prev)) || []; + // Decide who to spotlight: + // If the previous speaker is still speaking, stick with them rather + // than switching eagerly to someone else + return stickySpeaking + ? stickyMedia! + : // Otherwise, select any remote user who is speaking + (mediaItems.find(([m, s]) => !m.vm.local && s)?.[0] ?? + // Otherwise, stick with the person who was last speaking + stickyMedia ?? + // Otherwise, spotlight an arbitrary remote user + mediaItems.find(([m]) => !m.vm.local)?.[0] ?? + // Otherwise, spotlight the local user + mediaItems.find(([m]) => m.vm.local)?.[0]); + }, + null, + ), + map((speaker) => speaker?.vm ?? null), + this.scope.state(), + ); private readonly grid: Observable = this.userMedia.pipe( switchMap((mediaItems) => { @@ -647,7 +645,7 @@ export class CallViewModel extends ViewModel { ); private readonly spotlightAndPip: Observable< - [Observable, Observable] + [Observable, Observable] > = this.screenShares.pipe( map((screenShares) => screenShares.length > 0 @@ -660,7 +658,7 @@ export class CallViewModel extends ViewModel { switchMap((speaker) => speaker ? speaker.local - ? of(undefined) + ? of(null) : this.mediaItems.pipe( switchMap((mediaItems) => { const localUserMedia = mediaItems.find( @@ -671,11 +669,11 @@ export class CallViewModel extends ViewModel { map((alwaysShow) => alwaysShow ? localUserMedia : undefined, ), - ) ?? of(undefined) + ) ?? of(null) ); }), ) - : of(undefined), + : of(null), ), ), ] as const), @@ -689,12 +687,14 @@ export class CallViewModel extends ViewModel { ); private readonly hasRemoteScreenShares: Observable = - this.screenShares.pipe( - map((ms) => ms.some((m) => !m.vm.local)), + this.spotlight.pipe( + map((spotlight) => + spotlight.some((vm) => !vm.local && vm instanceof ScreenShareViewModel), + ), distinctUntilChanged(), ); - private readonly pip: Observable = + private readonly pip: Observable = this.spotlightAndPip.pipe(switchMap(([, pip]) => pip)); private readonly pipEnabled: Observable = setPipEnabled.pipe( @@ -780,8 +780,7 @@ export class CallViewModel extends ViewModel { type: "spotlight-landscape", spotlight, grid, - }), - ); + })); private readonly spotlightPortraitLayoutMedia: Observable = combineLatest([this.grid, this.spotlight], (grid, spotlight) => ({ From 5cae84940e646bacfda93ede55b402e1135d4de5 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 18 Nov 2024 17:29:41 +0000 Subject: [PATCH 52/84] Fix unit test --- src/state/CallViewModel.test.ts | 55 +++++++++++++++++---------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 1fe2fa74d..968f87fe1 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -356,29 +356,29 @@ test("screen sharing activates spotlight layout", () => { }); test("participants stay in the same order unless to appear/disappear", () => { - withTestScheduler(({ cold, schedule, expectObservable }) => { - const modeMarbles = " g"; + withTestScheduler(({ hot, schedule, expectObservable }) => { + const inputModeMarbles = " g"; // First Bob speaks, then Dave, then Alice - const aSpeakingMarbles = "n- 1998ms - 1999ms y"; - const bSpeakingMarbles = "ny 1998ms n 1999ms -"; - const dSpeakingMarbles = "n- 1998ms y 1999ms n"; + const inputASpeakingMarbles = "n- 1998ms - 1999ms y"; + const inputBSpeakingMarbles = "ny 1998ms n 1999ms -"; + const inputDSpeakingMarbles = "n- 1998ms y 1999ms n"; // Nothing should change when Bob speaks, because Bob is already on screen. // When Dave speaks he should switch with Alice because she's the one who // hasn't spoken at all. Then when Alice speaks, she should return to her // place at the top. - const layoutMarbles = " a 1999ms b 1999ms a 57999ms c 1999ms a"; + const expectedLayoutMarbles = "a 1999ms b 1999ms a 57999ms c 1999ms a"; withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), of([aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ - [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], - [bobParticipant, cold(bSpeakingMarbles, { y: true, n: false })], - [daveParticipant, cold(dSpeakingMarbles, { y: true, n: false })], + [aliceParticipant, hot(inputASpeakingMarbles, { y: true, n: false })], + [bobParticipant, hot(inputBSpeakingMarbles, { y: true, n: false })], + [daveParticipant, hot(inputDSpeakingMarbles, { y: true, n: false })], ]), (vm) => { - schedule(modeMarbles, { + schedule(inputModeMarbles, { g: () => { // We imagine that only three tiles (the first three) will be visible // on screen at a time @@ -391,23 +391,26 @@ test("participants stay in the same order unless to appear/disappear", () => { }, }); - expectObservable(summarizeLayout(vm.layout)).toBe(layoutMarbles, { - a: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], + expectObservable(summarizeLayout(vm.layout)).toBe( + expectedLayoutMarbles, + { + a: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], + }, + b: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], + }, + c: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], + }, }, - b: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], - }, - c: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], - }, - }); + ); }, ); }); From 09de97637cb68820df61f06927395deea9b1ee1b Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 18 Nov 2024 17:50:59 +0000 Subject: [PATCH 53/84] Refactor --- src/state/CallViewModel.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 968f87fe1..804157dba 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -248,7 +248,7 @@ function withCallViewModel( } test("participants are retained during a focus switch", () => { - withTestScheduler(({ cold, expectObservable }) => { + withTestScheduler(({ hot, expectObservable }) => { // Participants disappear on frame 2 and come back on frame 3 const participantMarbles = "a-ba"; // Start switching focus on frame 1 and reconnect on frame 3 @@ -257,12 +257,12 @@ test("participants are retained during a focus switch", () => { const layoutMarbles = " a"; withCallViewModel( - cold(participantMarbles, { + hot(participantMarbles, { a: [aliceParticipant, bobParticipant], b: [], }), of([aliceRtcMember, bobRtcMember]), - cold(connectionMarbles, { + hot(connectionMarbles, { c: ConnectionState.Connected, s: ECAddonConnectionState.ECSwitchingFocus, }), @@ -281,7 +281,7 @@ test("participants are retained during a focus switch", () => { }); test("screen sharing activates spotlight layout", () => { - withTestScheduler(({ cold, schedule, expectObservable }) => { + withTestScheduler(({ hot, schedule, expectObservable }) => { // Start with no screen shares, then have Alice and Bob share their screens, // then return to no screen shares, then have just Alice share for a bit const participantMarbles = " abcda-ba"; @@ -294,7 +294,7 @@ test("screen sharing activates spotlight layout", () => { const layoutMarbles = " abcdaefeg"; const showSpeakingMarbles = "y----nyny"; withCallViewModel( - cold(participantMarbles, { + hot(participantMarbles, { a: [aliceParticipant, bobParticipant], b: [aliceSharingScreen, bobParticipant], c: [aliceSharingScreen, bobSharingScreen], @@ -417,7 +417,7 @@ test("participants stay in the same order unless to appear/disappear", () => { }); test("spotlight speakers swap places", () => { - withTestScheduler(({ cold, schedule, expectObservable }) => { + withTestScheduler(({ hot, schedule, expectObservable }) => { // Go immediately into spotlight mode for the test const modeMarbles = " s"; // First Bob speaks, then Dave, then Alice @@ -435,9 +435,9 @@ test("spotlight speakers swap places", () => { of([aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ - [aliceParticipant, cold(aSpeakingMarbles, { y: true, n: false })], - [bobParticipant, cold(bSpeakingMarbles, { y: true, n: false })], - [daveParticipant, cold(dSpeakingMarbles, { y: true, n: false })], + [aliceParticipant, hot(aSpeakingMarbles, { y: true, n: false })], + [bobParticipant, hot(bSpeakingMarbles, { y: true, n: false })], + [daveParticipant, hot(dSpeakingMarbles, { y: true, n: false })], ]), (vm) => { schedule(modeMarbles, { s: () => vm.setGridMode("spotlight") }); From 23579b2ab1ccf66b8c773bd9ba94d0460dacb244 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 18 Nov 2024 18:02:37 +0000 Subject: [PATCH 54/84] Expose TestScheduler as global --- src/utils/test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/utils/test.ts b/src/utils/test.ts index 68971bbef..25f60d14f 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -4,7 +4,7 @@ Copyright 2023, 2024 New Vector Ltd. SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { map, Observable, of } from "rxjs"; +import { map, Observable, of, SchedulerLike } from "rxjs"; import { RunHelpers, TestScheduler } from "rxjs/testing"; import { expect, vi } from "vitest"; import { @@ -52,15 +52,23 @@ export interface OurRunHelpers extends RunHelpers { schedule: (marbles: string, actions: Record void>) => void; } +interface TestRunnerGlobal { + rxjsTestScheduler?: SchedulerLike; +} + /** * Run Observables with a scheduler that virtualizes time, for testing purposes. */ export function withTestScheduler( continuation: (helpers: OurRunHelpers) => void, ): void { - new TestScheduler((actual, expected) => { + const scheduler = new TestScheduler((actual, expected) => { expect(actual).deep.equals(expected); - }).run((helpers) => + }); + // we set the test scheduler as a global so that you can watch it in a debugger + // and get the frame number. e.g. `rxjsTestScheduler?.now()` + (global as unknown as TestRunnerGlobal).rxjsTestScheduler = scheduler; + scheduler.run((helpers) => continuation({ ...helpers, schedule(marbles, actions) { From 8f59f087cc6e01905c807579b1044884b1ef481c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 10:09:27 +0000 Subject: [PATCH 55/84] Fix incorrect type assertion --- src/state/CallViewModel.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index b1d64a7ce..f8f440691 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -663,9 +663,11 @@ export class CallViewModel extends ViewModel { switchMap((mediaItems) => { const localUserMedia = mediaItems.find( (m) => m.vm instanceof LocalUserMediaViewModel, - ) as LocalUserMediaViewModel | undefined; + ) as UserMedia | undefined; return ( - localUserMedia?.alwaysShow?.pipe( + ( + localUserMedia?.vm as LocalUserMediaViewModel + ).alwaysShow.pipe( map((alwaysShow) => alwaysShow ? localUserMedia : undefined, ), From 256a65a119ce517e97dbf8d47b993fa0e6f819fd Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 10:09:56 +0000 Subject: [PATCH 56/84] Simplify speaking observer --- src/state/MediaViewModel.ts | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 31232b749..32ab77474 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -335,19 +335,14 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { * Whether the participant is speaking. */ public readonly speaking = this.participant.pipe( - switchMap((p) => { - if (p) { - return observeParticipantEvents( - p, - ParticipantEvent.IsSpeakingChanged, - ).pipe( - map((p) => p.isSpeaking), - this.scope.state(), - ); - } else { - return of(false); - } - }), + switchMap((p) => + p + ? observeParticipantEvents(p, ParticipantEvent.IsSpeakingChanged).pipe( + map((p) => p.isSpeaking), + ) + : of(false), + ), + this.scope.state(), ); /** From edb53ae0496a93022a37dbfb5a3a2aa0721e0937 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 10:24:38 +0000 Subject: [PATCH 57/84] Fix --- src/state/CallViewModel.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index f8f440691..a4c712b40 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -668,8 +668,11 @@ export class CallViewModel extends ViewModel { ( localUserMedia?.vm as LocalUserMediaViewModel ).alwaysShow.pipe( - map((alwaysShow) => - alwaysShow ? localUserMedia : undefined, + map( + (alwaysShow) => + (alwaysShow + ? localUserMedia?.vm + : undefined) ?? null, ), ) ?? of(null) ); From 3cb5eccd2c64a409d715e8a654cef57f09ab6171 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 10:26:19 +0000 Subject: [PATCH 58/84] Whitespace --- src/tile/GridTile.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 422753eb2..27695b656 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -250,7 +250,6 @@ const RemoteUserMediaTile = forwardRef< const { t } = useTranslation(); const locallyMuted = useObservableEagerState(vm.locallyMuted); const localVolume = useObservableEagerState(vm.localVolume); - const onSelectMute = useCallback( (e: Event) => { e.preventDefault(); From 7677514bf544d84c095175de96e97d71ccb67512 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 10:37:50 +0000 Subject: [PATCH 59/84] Make it clear that we are mocking MatrixRTC memberships --- src/state/CallViewModel.test.ts | 10 +++++----- src/state/MediaViewModel.test.ts | 4 ++-- src/tile/GridTile.test.tsx | 4 ++-- src/tile/SpotlightTile.test.tsx | 10 +++++++--- src/utils/test.ts | 2 +- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 804157dba..522951e55 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -34,7 +34,7 @@ import { mockRoomMember, mockRemoteParticipant, withTestScheduler, - mockMembership, + mockRtcMembership, MockRTCSession, } from "../utils/test"; import { @@ -45,10 +45,10 @@ import { E2eeType } from "../e2ee/e2eeType"; vi.mock("@livekit/components-core"); -const localRtcMember = mockMembership("@carol:example.org", "CCCC"); -const aliceRtcMember = mockMembership("@alice:example.org", "AAAA"); -const bobRtcMember = mockMembership("@bob:example.org", "BBBB"); -const daveRtcMember = mockMembership("@dave:example.org", "DDDD"); +const localRtcMember = mockRtcMembership("@carol:example.org", "CCCC"); +const aliceRtcMember = mockRtcMembership("@alice:example.org", "AAAA"); +const bobRtcMember = mockRtcMembership("@bob:example.org", "BBBB"); +const daveRtcMember = mockRtcMembership("@dave:example.org", "DDDD"); const alice = mockRoomMember(aliceRtcMember); const bob = mockRoomMember(bobRtcMember); diff --git a/src/state/MediaViewModel.test.ts b/src/state/MediaViewModel.test.ts index 0ff5b5e6d..c4e0bee63 100644 --- a/src/state/MediaViewModel.test.ts +++ b/src/state/MediaViewModel.test.ts @@ -8,13 +8,13 @@ Please see LICENSE in the repository root for full details. import { expect, test, vi } from "vitest"; import { - mockMembership, + mockRtcMembership, withLocalMedia, withRemoteMedia, withTestScheduler, } from "../utils/test"; -const rtcMembership = mockMembership("@alice:example.org", "AAAA"); +const rtcMembership = mockRtcMembership("@alice:example.org", "AAAA"); test("control a participant's volume", async () => { const setVolumeSpy = vi.fn(); diff --git a/src/tile/GridTile.test.tsx b/src/tile/GridTile.test.tsx index 5e668078b..c0cf9c480 100644 --- a/src/tile/GridTile.test.tsx +++ b/src/tile/GridTile.test.tsx @@ -13,7 +13,7 @@ import { of } from "rxjs"; import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; import { GridTile } from "./GridTile"; -import { mockMembership, withRemoteMedia } from "../utils/test"; +import { mockRtcMembership, withRemoteMedia } from "../utils/test"; import { GridTileViewModel } from "../state/TileViewModel"; import { ReactionsProvider } from "../useReactions"; @@ -25,7 +25,7 @@ global.IntersectionObserver = class MockIntersectionObserver { test("GridTile is accessible", async () => { await withRemoteMedia( - mockMembership("@alice:example.org", "AAAA"), + mockRtcMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", diff --git a/src/tile/SpotlightTile.test.tsx b/src/tile/SpotlightTile.test.tsx index 1b29cb9f0..29b574a29 100644 --- a/src/tile/SpotlightTile.test.tsx +++ b/src/tile/SpotlightTile.test.tsx @@ -12,7 +12,11 @@ import userEvent from "@testing-library/user-event"; import { of } from "rxjs"; import { SpotlightTile } from "./SpotlightTile"; -import { mockMembership, withLocalMedia, withRemoteMedia } from "../utils/test"; +import { + mockRtcMembership, + withLocalMedia, + withRemoteMedia, +} from "../utils/test"; import { SpotlightTileViewModel } from "../state/TileViewModel"; global.IntersectionObserver = class MockIntersectionObserver { @@ -22,7 +26,7 @@ global.IntersectionObserver = class MockIntersectionObserver { test("SpotlightTile is accessible", async () => { await withRemoteMedia( - mockMembership("@alice:example.org", "AAAA"), + mockRtcMembership("@alice:example.org", "AAAA"), { rawDisplayName: "Alice", getMxcAvatarUrl: () => "mxc://adfsg", @@ -30,7 +34,7 @@ test("SpotlightTile is accessible", async () => { {}, async (vm1) => { await withLocalMedia( - mockMembership("@bob:example.org", "BBBB"), + mockRtcMembership("@bob:example.org", "BBBB"), { rawDisplayName: "Bob", getMxcAvatarUrl: () => "mxc://dlskf", diff --git a/src/utils/test.ts b/src/utils/test.ts index 25f60d14f..cf1d9ec1f 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -109,7 +109,7 @@ function mockEmitter(): EmitterMock { }; } -export function mockMembership( +export function mockRtcMembership( user: string | RoomMember, deviceId: string, callId = "", From b7ecd396c55d12e37d0bf167532022addb8edfbe Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 11:16:13 +0000 Subject: [PATCH 60/84] Test case for only showing tiles for MatrixRTC session members --- src/state/CallViewModel.test.ts | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 522951e55..660ba3a58 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -558,3 +558,55 @@ test("spotlight remembers whether it's expanded", () => { ); }); }); + +test("participants must have a MatrixRTCSession to be visible", () => { + withTestScheduler(({ hot, expectObservable }) => { + // iterate through a number of combinations of participants and MatrixRTC memberships + // Bob never has an MatrixRTC membership + const scenarioInputMarbles = " abcdec"; + // Bob should never be visible + const expectedLayoutMarbles = "a-bc-b"; + + withCallViewModel( + hot(scenarioInputMarbles, { + a: [], + b: [bobParticipant], + c: [aliceParticipant, bobParticipant], + d: [aliceParticipant, daveParticipant, bobParticipant], + e: [aliceParticipant, daveParticipant, bobSharingScreen], + }), + hot(scenarioInputMarbles, { + a: [], + b: [], + c: [aliceRtcMember], + d: [aliceRtcMember, daveRtcMember], + e: [aliceRtcMember, daveRtcMember], + }), + of(ConnectionState.Connected), + new Map(), + (vm) => { + vm.setGridMode("grid"); + expectObservable(summarizeLayout(vm.layout)).toBe( + expectedLayoutMarbles, + { + a: { + type: "grid", + spotlight: undefined, + grid: ["local:0"], + }, + b: { + type: "one-on-one", + local: "local:0", + remote: `${aliceId}:0`, + }, + c: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`], + }, + }, + ); + }, + ); + }); +}); From 18e7ca567c07de472f435b4b7837377a3a1499f1 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 12:14:41 +0000 Subject: [PATCH 61/84] Simplify diff --- src/state/CallViewModel.test.ts | 55 ++++++++++++++++----------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 660ba3a58..dea33fcfc 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -357,29 +357,29 @@ test("screen sharing activates spotlight layout", () => { test("participants stay in the same order unless to appear/disappear", () => { withTestScheduler(({ hot, schedule, expectObservable }) => { - const inputModeMarbles = " g"; + const modeMarbles = " a"; // First Bob speaks, then Dave, then Alice - const inputASpeakingMarbles = "n- 1998ms - 1999ms y"; - const inputBSpeakingMarbles = "ny 1998ms n 1999ms -"; - const inputDSpeakingMarbles = "n- 1998ms y 1999ms n"; + const aSpeakingMarbles = "n- 1998ms - 1999ms y"; + const bSpeakingMarbles = "ny 1998ms n 1999ms -"; + const dSpeakingMarbles = "n- 1998ms y 1999ms n"; // Nothing should change when Bob speaks, because Bob is already on screen. // When Dave speaks he should switch with Alice because she's the one who // hasn't spoken at all. Then when Alice speaks, she should return to her // place at the top. - const expectedLayoutMarbles = "a 1999ms b 1999ms a 57999ms c 1999ms a"; + const layoutMarbles = " a 1999ms b 1999ms a 57999ms c 1999ms a"; withCallViewModel( of([aliceParticipant, bobParticipant, daveParticipant]), of([aliceRtcMember, bobRtcMember, daveRtcMember]), of(ConnectionState.Connected), new Map([ - [aliceParticipant, hot(inputASpeakingMarbles, { y: true, n: false })], - [bobParticipant, hot(inputBSpeakingMarbles, { y: true, n: false })], - [daveParticipant, hot(inputDSpeakingMarbles, { y: true, n: false })], + [aliceParticipant, hot(aSpeakingMarbles, { y: true, n: false })], + [bobParticipant, hot(bSpeakingMarbles, { y: true, n: false })], + [daveParticipant, hot(dSpeakingMarbles, { y: true, n: false })], ]), (vm) => { - schedule(inputModeMarbles, { - g: () => { + schedule(modeMarbles, { + a: () => { // We imagine that only three tiles (the first three) will be visible // on screen at a time vm.layout.subscribe((layout) => { @@ -391,26 +391,23 @@ test("participants stay in the same order unless to appear/disappear", () => { }, }); - expectObservable(summarizeLayout(vm.layout)).toBe( - expectedLayoutMarbles, - { - a: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], - }, - b: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], - }, - c: { - type: "grid", - spotlight: undefined, - grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], - }, + expectObservable(summarizeLayout(vm.layout)).toBe(layoutMarbles, { + a: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`, `${daveId}:0`], }, - ); + b: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${daveId}:0`, `${bobId}:0`, `${aliceId}:0`], + }, + c: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`, `${bobId}:0`], + }, + }); }, ); }); From a63d44af0c879c2f7d4d10857a2af4927d9cf9db Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 12:28:10 +0000 Subject: [PATCH 62/84] Simplify diff These changes are in https://github.com/element-hq/element-call/pull/2809 --- src/state/CallViewModel.ts | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 84e1592e1..a83eb1e8c 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -375,16 +375,12 @@ export class CallViewModel extends ViewModel { }), ); - /** - * The raw list of RemoteParticipants as reported by LiveKit - */ - private readonly rawRemoteParticipants: Observable = - connectedParticipantsObserver(this.livekitRoom).pipe(this.scope.state()); + private readonly rawRemoteParticipants = connectedParticipantsObserver( + this.livekitRoom, + ).pipe(this.scope.state()); - /** - * Lists of RemoteParticipants to "hold" on display, even if LiveKit claims that - * they've left - */ + // Lists of RemoteParticipants to "hold" on display, even if LiveKit claims that + // they've left private readonly remoteParticipantHolds: Observable = this.connectionState.pipe( withLatestFrom(this.rawRemoteParticipants), @@ -419,9 +415,6 @@ export class CallViewModel extends ViewModel { ), ); - /** - * The RemoteParticipants including those that are being "held" on the screen - */ private readonly remoteParticipants: Observable = combineLatest( [this.rawRemoteParticipants, this.remoteParticipantHolds], @@ -443,9 +436,6 @@ export class CallViewModel extends ViewModel { }, ); - /** - * List of MediaItems that we want to display - */ private readonly mediaItems: Observable = combineLatest([ this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), @@ -551,18 +541,12 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - /** - * List of MediaItems that we want to display, that are of type UserMedia - */ private readonly userMedia: Observable = this.mediaItems.pipe( map((mediaItems) => mediaItems.filter((m): m is UserMedia => m instanceof UserMedia), ), ); - /** - * List of MediaItems that we want to display, that are of type ScreenShare - */ private readonly screenShares: Observable = this.mediaItems.pipe( map((mediaItems) => From 1c04d22a5b26942b91012145ffea4cd0cd347b91 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 12:29:40 +0000 Subject: [PATCH 63/84] . --- src/state/CallViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index a83eb1e8c..4d83ee82f 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -1020,7 +1020,7 @@ export class CallViewModel extends ViewModel { this.scope.state(), ); - public readonly showFooter: Observable = this.windowMode.pipe( + public readonly showFooter = this.windowMode.pipe( switchMap((mode) => { switch (mode) { case "pip": From 3f11f51ff0c490c329226dcf076e1f31fc7ca83c Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 20 Nov 2024 12:34:20 +0000 Subject: [PATCH 64/84] Whitespaces --- src/state/CallViewModel.test.ts | 1 - src/state/CallViewModel.ts | 1 + src/tile/MediaView.tsx | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index dea33fcfc..af0cbeb73 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -60,7 +60,6 @@ const bobId = `${bob.userId}:${bobRtcMember.deviceId}`; const daveId = `${dave.userId}:${daveRtcMember.deviceId}`; const localParticipant = mockLocalParticipant({ identity: "" }); - const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const aliceSharingScreen = mockRemoteParticipant({ identity: aliceId, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 4d83ee82f..cd00e2520 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -868,6 +868,7 @@ export class CallViewModel extends ViewModel { }), this.scope.state(), ); + /** * The layout of tiles in the call interface. */ diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 563eccb6f..b88ee8784 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -136,7 +136,6 @@ export const MediaView = forwardRef( {displayName} - {unencryptedWarning && ( Date: Thu, 21 Nov 2024 11:32:03 +0000 Subject: [PATCH 65/84] Use asObservable when exposing subject --- src/state/CallViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index aaf7a5852..4f3952b54 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -326,7 +326,7 @@ class ScreenShare { this.vm = new ScreenShareViewModel( id, member, - this.participant, + this.participant.asObservable(), encryptionSystem, liveKitRoom, participant.isLocal, From 4f2591feae7138f547eb704e04b5c5e83aa0d210 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 25 Nov 2024 21:17:20 +0000 Subject: [PATCH 66/84] Show "waiting for media..." when no participant --- locales/en-GB/app.json | 3 ++- src/tile/MediaView.module.css | 14 ++++++++++---- src/tile/MediaView.test.tsx | 10 +++++++++- src/tile/MediaView.tsx | 11 ++++++++--- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/locales/en-GB/app.json b/locales/en-GB/app.json index 6340d1609..06b8e4171 100644 --- a/locales/en-GB/app.json +++ b/locales/en-GB/app.json @@ -191,6 +191,7 @@ "expand": "Expand", "mute_for_me": "Mute for me", "muted_for_me": "Muted for me", - "volume": "Volume" + "volume": "Volume", + "waiting_for_media": "Waiting for media..." } } diff --git a/src/tile/MediaView.module.css b/src/tile/MediaView.module.css index 4594c2841..dd7dfc50d 100644 --- a/src/tile/MediaView.module.css +++ b/src/tile/MediaView.module.css @@ -74,9 +74,9 @@ unconditionally select the container so we can use cqmin units */ calc(var(--media-view-border-radius) - var(--cpd-space-3x)) ); display: grid; - grid-template-columns: 1fr auto; + grid-template-columns: 30px 1fr 30px; grid-template-rows: 1fr auto; - grid-template-areas: "status status" "nameTag button"; + grid-template-areas: "reactions status ." "nameTag nameTag button"; gap: var(--cpd-space-1x); place-items: start; } @@ -101,8 +101,8 @@ unconditionally select the container so we can use cqmin units */ grid-area: status; justify-self: center; align-self: start; - padding: var(--cpd-space-1x); - padding-block: var(--cpd-space-1x); + padding: var(--cpd-space-2x); + padding-block: var(--cpd-space-2x); color: var(--cpd-color-text-primary); background-color: var(--cpd-color-bg-canvas-default); display: flex; @@ -116,6 +116,12 @@ unconditionally select the container so we can use cqmin units */ text-align: center; } +.reactions { + grid-area: reactions; + display: flex; + gap: var(--cpd-space-1x); +} + .nameTag > svg, .nameTag > span { flex-shrink: 0; diff --git a/src/tile/MediaView.test.tsx b/src/tile/MediaView.test.tsx index 238ffdd17..b5d7a6713 100644 --- a/src/tile/MediaView.test.tsx +++ b/src/tile/MediaView.test.tsx @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { describe, expect, test } from "vitest"; +import { describe, expect, it, test } from "vitest"; import { render, screen } from "@testing-library/react"; import { axe } from "vitest-axe"; import { TooltipProvider } from "@vector-im/compound-web"; @@ -59,6 +59,14 @@ describe("MediaView", () => { }); }); + describe("with no participant", () => { + it("shows avatar", () => { + render(); + expect(screen.getByRole("img", { name: "some name" })).toBeVisible(); + expect(screen.getByText("video_tile.waiting_for_media")).toBeVisible(); + }); + }); + describe("name tag", () => { test("is shown with name", () => { render(); diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 69c1ca912..51da4d226 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -90,7 +90,7 @@ export const MediaView = forwardRef( size={avatarSize} src={member?.getMxcAvatarUrl()} className={styles.avatar} - style={{ display: videoEnabled ? "none" : "initial" }} + style={{ display: video && videoEnabled ? "none" : "initial" }} /> {video?.publication !== undefined && ( ( // There's no reason for this to be focusable tabIndex={-1} disablePictureInPicture - style={{ display: videoEnabled ? "block" : "none" }} + style={{ display: video && videoEnabled ? "block" : "none" }} data-testid="video" /> )}
-
+
( /> )}
+ {!video && ( +
+ {t("video_tile.waiting_for_media")} +
+ )} {/* TODO: Bring this back once encryption status is less broken */} {/*encryptionStatus !== EncryptionStatus.Okay && (
From 16666b893393cc5bf4cebc0b1de5b55184e1e142 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 25 Nov 2024 21:23:18 +0000 Subject: [PATCH 67/84] Additional test case --- src/state/CallViewModel.test.ts | 51 ++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 033756a45..5dbfb1ca7 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only Please see LICENSE in the repository root for full details. */ -import { test, vi, onTestFinished } from "vitest"; +import { test, vi, onTestFinished, it } from "vitest"; import { combineLatest, debounceTime, @@ -635,3 +635,52 @@ test("participants must have a MatrixRTCSession to be visible", () => { ); }); }); + +it("should show at least one tile per MatrixRTCSession", () => { + withTestScheduler(({ hot, expectObservable }) => { + // iterate through some combinations of MatrixRTC memberships + const scenarioInputMarbles = " abcd"; + // There should always be one tile for each MatrixRTCSession + const expectedLayoutMarbles = "abcd"; + + withCallViewModel( + of([]), + hot(scenarioInputMarbles, { + a: [], + b: [aliceRtcMember], + c: [aliceRtcMember, daveRtcMember], + d: [daveRtcMember], + }), + of(ConnectionState.Connected), + new Map(), + (vm) => { + vm.setGridMode("grid"); + expectObservable(summarizeLayout(vm.layout)).toBe( + expectedLayoutMarbles, + { + a: { + type: "grid", + spotlight: undefined, + grid: ["local:0"], + }, + b: { + type: "one-on-one", + local: "local:0", + remote: `${aliceId}:0`, + }, + c: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${daveId}:0`], + }, + d: { + type: "one-on-one", + local: "local:0", + remote: `${daveId}:0`, + }, + }, + ); + }, + ); + }); +}); From 8f9bee79ae3ab2cc0cd5bf4b281ae210b3a32c30 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 25 Nov 2024 21:30:54 +0000 Subject: [PATCH 68/84] Don't show "waiting for media..." in case of local participant --- src/tile/GridTile.tsx | 1 + src/tile/MediaView.test.tsx | 16 ++++++++++++++-- src/tile/MediaView.tsx | 4 +++- src/tile/SpotlightTile.tsx | 2 ++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 27695b656..15f7c2954 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -175,6 +175,7 @@ const UserMediaTile = forwardRef( raisedHandTime={handRaised} currentReaction={currentReaction} raisedHandOnClick={raisedHandOnClick} + localParticipant={vm.local} {...props} /> ); diff --git a/src/tile/MediaView.test.tsx b/src/tile/MediaView.test.tsx index b5d7a6713..fea4303f9 100644 --- a/src/tile/MediaView.test.tsx +++ b/src/tile/MediaView.test.tsx @@ -42,6 +42,7 @@ describe("MediaView", () => { unencryptedWarning: false, video: trackReference, member: undefined, + localParticipant: false, }; test("is accessible", async () => { @@ -60,8 +61,19 @@ describe("MediaView", () => { }); describe("with no participant", () => { - it("shows avatar", () => { - render(); + it("shows avatar for local user", () => { + render( + , + ); + expect(screen.getByRole("img", { name: "some name" })).toBeVisible(); + expect(screen.queryAllByText("video_tile.waiting_for_media").length).toBe( + 0, + ); + }); + it("shows avatar and label for remote user", () => { + render( + , + ); expect(screen.getByRole("img", { name: "some name" })).toBeVisible(); expect(screen.getByText("video_tile.waiting_for_media")).toBeVisible(); }); diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 51da4d226..48871abdf 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -41,6 +41,7 @@ interface Props extends ComponentProps { raisedHandTime?: Date; currentReaction?: ReactionOption; raisedHandOnClick?: () => void; + localParticipant: boolean; } export const MediaView = forwardRef( @@ -63,6 +64,7 @@ export const MediaView = forwardRef( raisedHandTime, currentReaction, raisedHandOnClick, + localParticipant, ...props }, ref, @@ -118,7 +120,7 @@ export const MediaView = forwardRef( /> )}
- {!video && ( + {!video && !localParticipant && (
{t("video_tile.waiting_for_media")}
diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index 33661c603..7f701dede 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -55,6 +55,7 @@ interface SpotlightItemBaseProps { encryptionStatus: EncryptionStatus; displayName: string; "aria-hidden"?: boolean; + localParticipant: boolean; } interface SpotlightUserMediaItemBaseProps extends SpotlightItemBaseProps { @@ -163,6 +164,7 @@ const SpotlightItem = forwardRef( displayName, encryptionStatus, "aria-hidden": ariaHidden, + localParticipant: vm.local, }; return vm instanceof ScreenShareViewModel ? ( From 765f7b4f319b86839fcb4fde28d1e0604e542577 Mon Sep 17 00:00:00 2001 From: Timo Date: Tue, 26 Nov 2024 12:33:48 +0100 Subject: [PATCH 69/84] Make the loading state more subtle - instead of a label we show a animated gradient --- locales/en-GB/app.json | 3 +-- src/tile/GridTile.module.css | 34 +++++++++++++++++++++++++++++++++- src/tile/GridTile.tsx | 3 +++ src/tile/MediaView.test.tsx | 2 +- src/tile/MediaView.tsx | 5 ----- 5 files changed, 38 insertions(+), 9 deletions(-) diff --git a/locales/en-GB/app.json b/locales/en-GB/app.json index 06b8e4171..6340d1609 100644 --- a/locales/en-GB/app.json +++ b/locales/en-GB/app.json @@ -191,7 +191,6 @@ "expand": "Expand", "mute_for_me": "Mute for me", "muted_for_me": "Muted for me", - "volume": "Volume", - "waiting_for_media": "Waiting for media..." + "volume": "Volume" } } diff --git a/src/tile/GridTile.module.css b/src/tile/GridTile.module.css index bb0685120..0ec2b7e7a 100644 --- a/src/tile/GridTile.module.css +++ b/src/tile/GridTile.module.css @@ -18,7 +18,9 @@ borders don't support gradients */ position: absolute; z-index: -1; /* Put it below the outline */ opacity: 0; /* Hidden unless speaking */ - transition: opacity ease 0.15s; + /* this only animates in one direction. In the other direction, + the background will disappear and the opacity has no effect.*/ + transition: opacity ease 0.5s; inset: calc(-1 * var(--cpd-border-width-4)); border-radius: var(--cpd-space-5x); background-blend-mode: overlay, normal; @@ -48,6 +50,11 @@ borders don't support gradients */ outline: var(--cpd-border-width-2) solid var(--cpd-color-bg-canvas-default) !important; } +.tile.loading { + /* !important because loading border should take priority over hover */ + outline: var(--cpd-border-width-2) solid var(--cpd-color-bg-canvas-default) !important; +} + .tile.handRaised::before { background: linear-gradient( 119deg, @@ -62,6 +69,31 @@ borders don't support gradients */ opacity: 1; } +.tile.loading::before { + background: linear-gradient( + var(--angle), + var(--cpd-color-green-900) 0%, + var(--cpd-color-blue-200) 100% + ); + opacity: 1; + animation: rotate-gradient linear 2s infinite; +} + +@property --angle { + syntax: ""; + inherits: false; + initial-value: 0deg; +} + +@keyframes rotate-gradient { + from { + --angle: 0deg; + } + to { + --angle: 360deg; + } +} + @media (hover: hover) { .tile:hover { outline: var(--cpd-border-width-2) solid diff --git a/src/tile/GridTile.tsx b/src/tile/GridTile.tsx index 15f7c2954..11e9f04d1 100644 --- a/src/tile/GridTile.tsx +++ b/src/tile/GridTile.tsx @@ -42,6 +42,7 @@ import { useDisplayName, LocalUserMediaViewModel, RemoteUserMediaViewModel, + EncryptionStatus, } from "../state/MediaViewModel"; import { Slider } from "../Slider"; import { MediaView } from "./MediaView"; @@ -145,6 +146,8 @@ const UserMediaTile = forwardRef( className={classNames(className, styles.tile, { [styles.speaking]: showSpeaking, [styles.handRaised]: !showSpeaking && !!handRaised, + [styles.loading]: + encryptionStatus === EncryptionStatus.Connecting && !vm.local, })} nameTagLeadingIcon={ { , ); expect(screen.getByRole("img", { name: "some name" })).toBeVisible(); - expect(screen.getByText("video_tile.waiting_for_media")).toBeVisible(); + expect(screen.getByTestId("videoTile")).toBeVisible(); }); }); diff --git a/src/tile/MediaView.tsx b/src/tile/MediaView.tsx index 48871abdf..b94d4a163 100644 --- a/src/tile/MediaView.tsx +++ b/src/tile/MediaView.tsx @@ -120,11 +120,6 @@ export const MediaView = forwardRef( /> )}
- {!video && !localParticipant && ( -
- {t("video_tile.waiting_for_media")} -
- )} {/* TODO: Bring this back once encryption status is less broken */} {/*encryptionStatus !== EncryptionStatus.Okay && (
From 52cb3938efba6e2df524339ff353b4360b83a087 Mon Sep 17 00:00:00 2001 From: Timo <16718859+toger5@users.noreply.github.com> Date: Wed, 27 Nov 2024 15:44:12 +0100 Subject: [PATCH 70/84] Use correct key for matrix rtc foci in code comment. (#2838) --- src/config/ConfigOptions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/ConfigOptions.ts b/src/config/ConfigOptions.ts index ed4d5bceb..3947ba664 100644 --- a/src/config/ConfigOptions.ts +++ b/src/config/ConfigOptions.ts @@ -51,7 +51,7 @@ export interface ConfigOptions { // a livekit service url in the client well-known. // The well known needs to be formatted like so: // {"type":"livekit", "livekit_service_url":"https://livekit.example.com"} - // and stored under the key: "livekit_focus" + // and stored under the key: "org.matrix.msc4143.rtc_foci" livekit_service_url: string; }; From cc1f0d5d19aa5e4e608c212687dca7c9dbbdd2a4 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Nov 2024 14:46:14 +0000 Subject: [PATCH 71/84] Update src/tile/SpotlightTile.tsx Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> --- src/tile/SpotlightTile.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tile/SpotlightTile.tsx b/src/tile/SpotlightTile.tsx index 7f701dede..dce30d5ff 100644 --- a/src/tile/SpotlightTile.tsx +++ b/src/tile/SpotlightTile.tsx @@ -213,7 +213,7 @@ export const SpotlightTile = forwardRef( const maximised = useObservableEagerState(vm.maximised); const media = useObservableEagerState(vm.media); const [visibleId, setVisibleId] = useState( - media.length > 0 ? media[0].id : undefined, + media[0]?.id, ); const latestMedia = useLatest(media); const latestVisibleId = useLatest(visibleId); From 133dc26642e9e71c33634223f92fc1691d1ec15b Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Nov 2024 14:47:31 +0000 Subject: [PATCH 72/84] Update src/state/CallViewModel.ts Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> --- src/state/CallViewModel.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 721a9a2a4..b3e62e823 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -435,7 +435,9 @@ export class CallViewModel extends ViewModel { this.remoteParticipants, observeParticipantMedia(this.livekitRoom.localParticipant), duplicateTiles.value, - // Also react to changes in the MatrixRTC session list: + // Also react to changes in the MatrixRTC session list. + // The session list will also be update if a room membership changes. + // No additional RoomState event listener needs to be set up. fromEvent( this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, From 3d0bfaefe8743bd90c9b0267faf3d490219f6496 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 13:47:35 +0000 Subject: [PATCH 73/84] Make the purpose of BaseMediaViewModel.local explicit --- src/state/MediaViewModel.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 32ab77474..ceaca57cc 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -217,6 +217,11 @@ abstract class BaseMediaViewModel extends ViewModel { public readonly encryptionStatus: Observable; + /** + * Whether this media corresponds to the local participant. + */ + public abstract readonly local: boolean; + public constructor( /** * An opaque identifier for this media. @@ -392,13 +397,16 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel { public toggleFitContain(): void { this._cropVideo.next(!this._cropVideo.value); } + + public get local(): boolean { + return this instanceof LocalUserMediaViewModel; + } } /** * The local participant's user media. */ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { - public readonly local = true; /** * Whether the video should be mirrored. */ @@ -438,8 +446,6 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel { * A remote participant's user media. */ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { - public readonly local = false; - private readonly locallyMutedToggle = new Subject(); private readonly localVolumeAdjustment = new Subject(); private readonly localVolumeCommit = new Subject(); From 968de5eeebd28958780f4f982fac49b510019c43 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 14:00:12 +0000 Subject: [PATCH 74/84] Use named object instead of unnamed array for spotlightAndPip --- src/state/CallViewModel.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index b3e62e823..eb81304ab 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -630,17 +630,21 @@ export class CallViewModel extends ViewModel { }), ); - private readonly spotlightAndPip: Observable< - [Observable, Observable] - > = this.screenShares.pipe( + private readonly spotlightAndPip: Observable<{ + spotlight: Observable; + pip: Observable; + }> = this.screenShares.pipe( map((screenShares) => screenShares.length > 0 - ? ([of(screenShares.map((m) => m.vm)), this.spotlightSpeaker] as const) - : ([ - this.spotlightSpeaker.pipe( + ? { + spotlight: of(screenShares.map((m) => m.vm)), + pip: this.spotlightSpeaker, + } + : { + spotlight: this.spotlightSpeaker.pipe( map((speaker) => (speaker ? [speaker] : [])), ), - this.spotlightSpeaker.pipe( + pip: this.spotlightSpeaker.pipe( switchMap((speaker) => speaker ? speaker.local @@ -667,13 +671,13 @@ export class CallViewModel extends ViewModel { : of(null), ), ), - ] as const), + }, ), ); private readonly spotlight: Observable = this.spotlightAndPip.pipe( - switchMap(([spotlight]) => spotlight), + switchMap(({ spotlight }) => spotlight), this.scope.state(), ); @@ -686,7 +690,7 @@ export class CallViewModel extends ViewModel { ); private readonly pip: Observable = - this.spotlightAndPip.pipe(switchMap(([, pip]) => pip)); + this.spotlightAndPip.pipe(switchMap(({ pip }) => pip)); private readonly pipEnabled: Observable = setPipEnabled.pipe( startWith(false), From 33f398ddcf09f2b183abd43671a56a17098aca1b Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 14:10:31 +0000 Subject: [PATCH 75/84] Refactor spotlightAndPip into spotlight and pip --- src/state/CallViewModel.ts | 40 ++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index eb81304ab..89fa3950a 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -630,21 +630,24 @@ export class CallViewModel extends ViewModel { }), ); - private readonly spotlightAndPip: Observable<{ - spotlight: Observable; - pip: Observable; - }> = this.screenShares.pipe( - map((screenShares) => - screenShares.length > 0 - ? { - spotlight: of(screenShares.map((m) => m.vm)), - pip: this.spotlightSpeaker, - } - : { - spotlight: this.spotlightSpeaker.pipe( + private readonly spotlight: Observable = + this.screenShares.pipe( + switchMap((screenShares) => + screenShares.length > 0 + ? of(screenShares.map((m) => m.vm)) + : this.spotlightSpeaker.pipe( map((speaker) => (speaker ? [speaker] : [])), ), - pip: this.spotlightSpeaker.pipe( + ), + this.scope.state(), + ); + + private readonly pip: Observable = + this.screenShares.pipe( + switchMap((screenShares) => + screenShares.length > 0 + ? this.spotlightSpeaker + : this.spotlightSpeaker.pipe( switchMap((speaker) => speaker ? speaker.local @@ -671,13 +674,7 @@ export class CallViewModel extends ViewModel { : of(null), ), ), - }, - ), - ); - - private readonly spotlight: Observable = - this.spotlightAndPip.pipe( - switchMap(({ spotlight }) => spotlight), + ), this.scope.state(), ); @@ -689,9 +686,6 @@ export class CallViewModel extends ViewModel { distinctUntilChanged(), ); - private readonly pip: Observable = - this.spotlightAndPip.pipe(switchMap(({ pip }) => pip)); - private readonly pipEnabled: Observable = setPipEnabled.pipe( startWith(false), ); From 7fdcbb32922c62c1413b5db47666fe0ec1718f86 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 14:35:33 +0000 Subject: [PATCH 76/84] Use if statement instead of ternary for readability in spotlight and pip logic --- src/state/CallViewModel.ts | 86 +++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 89fa3950a..abd505f3f 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -632,49 +632,59 @@ export class CallViewModel extends ViewModel { private readonly spotlight: Observable = this.screenShares.pipe( - switchMap((screenShares) => - screenShares.length > 0 - ? of(screenShares.map((m) => m.vm)) - : this.spotlightSpeaker.pipe( - map((speaker) => (speaker ? [speaker] : [])), - ), - ), + switchMap((screenShares) => { + if (screenShares.length > 0) { + return of(screenShares.map((m) => m.vm)); + } + + return this.spotlightSpeaker.pipe( + map((speaker) => (speaker ? [speaker] : [])), + ); + }), this.scope.state(), ); private readonly pip: Observable = this.screenShares.pipe( - switchMap((screenShares) => - screenShares.length > 0 - ? this.spotlightSpeaker - : this.spotlightSpeaker.pipe( - switchMap((speaker) => - speaker - ? speaker.local - ? of(null) - : this.mediaItems.pipe( - switchMap((mediaItems) => { - const localUserMedia = mediaItems.find( - (m) => m.vm instanceof LocalUserMediaViewModel, - ) as UserMedia | undefined; - return ( - ( - localUserMedia?.vm as LocalUserMediaViewModel - ).alwaysShow.pipe( - map( - (alwaysShow) => - (alwaysShow - ? localUserMedia?.vm - : undefined) ?? null, - ), - ) ?? of(null) - ); - }), - ) - : of(null), - ), - ), - ), + switchMap((screenShares) => { + if (screenShares.length > 0) { + return this.spotlightSpeaker; + } + + return this.spotlightSpeaker.pipe( + switchMap((speaker) => { + if (!speaker || speaker.local) { + return of(null); + } + + return this.mediaItems.pipe( + switchMap((mediaItems) => { + const localUserMedia = mediaItems.find( + (m) => m.vm instanceof LocalUserMediaViewModel, + ) as UserMedia | undefined; + + const localUserMediaViewModel = localUserMedia?.vm as + | LocalUserMediaViewModel + | undefined; + + if (!localUserMediaViewModel) { + return of(null); + } + + return localUserMediaViewModel.alwaysShow.pipe( + map((alwaysShow) => { + if (alwaysShow) { + return localUserMediaViewModel; + } + + return null; + }), + ); + }), + ); + }), + ); + }), this.scope.state(), ); From 370d8bd5d599a42fcead3b038f6d25b111430047 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 14:45:20 +0000 Subject: [PATCH 77/84] Review feedback --- src/state/CallViewModel.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index abd505f3f..46106d1b6 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -235,11 +235,10 @@ class UserMedia { participant: LocalParticipant | RemoteParticipant | undefined, encryptionSystem: EncryptionSystem, livekitRoom: LivekitRoom, - local: boolean, ) { this.participant = new BehaviorSubject(participant); - if (participant?.isLocal || local) { + if (participant?.isLocal) { this.vm = new LocalUserMediaViewModel( this.id, member, @@ -504,7 +503,6 @@ export class CallViewModel extends ViewModel { participant, this.encryptionSystem, this.livekitRoom, - livekitParticipantId === "local", ), ]; From 70deaf0e573367a79da907880906c13f143bc024 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 18:30:04 +0000 Subject: [PATCH 78/84] Fix tests for CallEventAudioRenderer --- src/room/CallEventAudioRenderer.test.tsx | 182 ++++++++++++++++------- 1 file changed, 126 insertions(+), 56 deletions(-) diff --git a/src/room/CallEventAudioRenderer.test.tsx b/src/room/CallEventAudioRenderer.test.tsx index 9014e60b1..78d08f604 100644 --- a/src/room/CallEventAudioRenderer.test.tsx +++ b/src/room/CallEventAudioRenderer.test.tsx @@ -8,10 +8,14 @@ Please see LICENSE in the repository root for full details. import { render } from "@testing-library/react"; import { beforeEach, expect, test } from "vitest"; import { MatrixClient } from "matrix-js-sdk/src/client"; -import { ConnectionState, RemoteParticipant, Room } from "livekit-client"; -import { of } from "rxjs"; +import { ConnectionState, Room } from "livekit-client"; +import { BehaviorSubject, of } from "rxjs"; import { afterEach } from "node:test"; import { act } from "react"; +import { + CallMembership, + type MatrixRTCSession, +} from "matrix-js-sdk/src/matrixrtc"; import { soundEffectVolumeSetting } from "../settings/settings"; import { @@ -22,6 +26,8 @@ import { mockMatrixRoomMember, mockMediaPlay, mockRemoteParticipant, + mockRtcMembership, + MockRTCSession, } from "../utils/test"; import { E2eeType } from "../e2ee/e2eeType"; import { CallViewModel } from "../state/CallViewModel"; @@ -30,11 +36,15 @@ import { MAX_PARTICIPANT_COUNT_FOR_SOUND, } from "./CallEventAudioRenderer"; -const alice = mockMatrixRoomMember({ userId: "@alice:example.org" }); -const bob = mockMatrixRoomMember({ userId: "@bob:example.org" }); -const aliceId = `${alice.userId}:AAAA`; -const bobId = `${bob.userId}:BBBB`; +const localRtcMember = mockRtcMembership("@carol:example.org", "CCCC"); +const local = mockMatrixRoomMember(localRtcMember); +const aliceRtcMember = mockRtcMembership("@alice:example.org", "AAAA"); +const alice = mockMatrixRoomMember(aliceRtcMember); +const bobRtcMember = mockRtcMembership("@bob:example.org", "BBBB"); +const bob = mockMatrixRoomMember(bobRtcMember); const localParticipant = mockLocalParticipant({ identity: "" }); +const aliceId = `${alice.userId}:${aliceRtcMember.deviceId}`; +const bobId = `${bob.userId}:${bobRtcMember.deviceId}`; const aliceParticipant = mockRemoteParticipant({ identity: aliceId }); const bobParticipant = mockRemoteParticipant({ identity: bobId }); @@ -53,20 +63,28 @@ afterEach(() => { test("plays a sound when entering a call", () => { const audioIsPlaying: string[] = mockMediaPlay(); - const members = new Map([alice, bob].map((p) => [p.userId, p])); + const matrixRoomMembers = new Map( + [local, alice, bob].map((p) => [p.userId, p]), + ); const remoteParticipants = of([aliceParticipant]); const liveKitRoom = mockLivekitRoom( { localParticipant }, { remoteParticipants }, ); + const matrixRoom = mockMatrixRoom({ + client: { + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, + } as Partial as MatrixClient, + getMember: (userId) => matrixRoomMembers.get(userId) ?? null, + }); + + const session = new MockRTCSession(matrixRoom, localRtcMember, [ + aliceRtcMember, + ]) as unknown as MatrixRTCSession; const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + session, liveKitRoom, { kind: E2eeType.PER_PARTICIPANT, @@ -84,20 +102,29 @@ test("plays a sound when entering a call", () => { test("plays no sound when muted", () => { soundEffectVolumeSetting.setValue(0); const audioIsPlaying: string[] = mockMediaPlay(); - const members = new Map([alice, bob].map((p) => [p.userId, p])); + const matrixRoomMembers = new Map( + [local, alice, bob].map((p) => [p.userId, p]), + ); const remoteParticipants = of([aliceParticipant, bobParticipant]); const liveKitRoom = mockLivekitRoom( { localParticipant }, { remoteParticipants }, ); + const matrixRoom = mockMatrixRoom({ + client: { + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, + } as Partial as MatrixClient, + getMember: (userId) => matrixRoomMembers.get(userId) ?? null, + }); + + const session = new MockRTCSession(matrixRoom, localRtcMember, [ + aliceRtcMember, + ]) as unknown as MatrixRTCSession; + const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + session, liveKitRoom, { kind: E2eeType.PER_PARTICIPANT, @@ -112,7 +139,7 @@ test("plays no sound when muted", () => { test("plays a sound when a user joins", () => { const audioIsPlaying: string[] = mockMediaPlay(); - const members = new Map([alice].map((p) => [p.userId, p])); + const matrixRoomMembers = new Map([local, alice].map((p) => [p.userId, p])); const remoteParticipants = new Map( [aliceParticipant].map((p) => [p.identity, p]), ); @@ -121,13 +148,27 @@ test("plays a sound when a user joins", () => { remoteParticipants, }); + const matrixRoom = mockMatrixRoom({ + client: { + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, + } as Partial as MatrixClient, + getMember: (userId) => matrixRoomMembers.get(userId) ?? null, + }); + + const remoteRtcMemberships = new BehaviorSubject([ + aliceRtcMember, + ]); + // we give Bob an RTC session now, but no participant yet + const session = new MockRTCSession( + matrixRoom, + localRtcMember, + ).withMemberships( + remoteRtcMemberships.asObservable(), + ) as unknown as MatrixRTCSession; + const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + session, liveKitRoom as unknown as Room, { kind: E2eeType.PER_PARTICIPANT, @@ -137,20 +178,20 @@ test("plays a sound when a user joins", () => { render(); act(() => { - liveKitRoom.addParticipant(bobParticipant); + remoteRtcMemberships.next([aliceRtcMember, bobRtcMember]); }); // Play a sound when joining a call. expect(audioIsPlaying).toEqual([ // Joining the call enterSound, - // Bob leaves + // Bob joins enterSound, ]); }); test("plays a sound when a user leaves", () => { const audioIsPlaying: string[] = mockMediaPlay(); - const members = new Map([alice].map((p) => [p.userId, p])); + const matrixRoomMembers = new Map([local, alice].map((p) => [p.userId, p])); const remoteParticipants = new Map( [aliceParticipant].map((p) => [p.identity, p]), ); @@ -159,13 +200,25 @@ test("plays a sound when a user leaves", () => { remoteParticipants, }); + const matrixRoom = mockMatrixRoom({ + client: { + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, + } as Partial as MatrixClient, + getMember: (userId) => matrixRoomMembers.get(userId) ?? null, + }); + + const remoteRtcMemberships = new BehaviorSubject([ + aliceRtcMember, + ]); + + const session = new MockRTCSession( + matrixRoom, + localRtcMember, + ).withMemberships(remoteRtcMemberships) as unknown as MatrixRTCSession; + const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + session, liveKitRoom as unknown as Room, { kind: E2eeType.PER_PARTICIPANT, @@ -175,7 +228,7 @@ test("plays a sound when a user leaves", () => { render(); act(() => { - liveKitRoom.removeParticipant(aliceParticipant); + remoteRtcMemberships.next([]); }); expect(audioIsPlaying).toEqual([ // Joining the call @@ -185,30 +238,45 @@ test("plays a sound when a user leaves", () => { ]); }); -test("plays no sound when the participant list", () => { +test("plays no sound when the session member count is larger than the max, until decreased", () => { const audioIsPlaying: string[] = mockMediaPlay(); - const members = new Map([alice].map((p) => [p.userId, p])); - const remoteParticipants = new Map([ - [aliceParticipant.identity, aliceParticipant], - ...Array.from({ length: MAX_PARTICIPANT_COUNT_FOR_SOUND - 1 }).map< - [string, RemoteParticipant] - >((_, index) => { - const p = mockRemoteParticipant({ identity: `user${index}` }); - return [p.identity, p]; - }), - ]); + const matrixRoomMembers = new Map([local, alice].map((p) => [p.userId, p])); + const remoteParticipants = new Map( + [aliceParticipant].map((p) => [p.identity, p]), + ); + + const mockRtcMemberships: CallMembership[] = []; + + for (let i = 0; i < MAX_PARTICIPANT_COUNT_FOR_SOUND; i++) { + mockRtcMemberships.push( + mockRtcMembership(`@user${i}:example.org`, `DEVICE${i}`), + ); + } + + const remoteRtcMemberships = new BehaviorSubject( + mockRtcMemberships, + ); + const liveKitRoom = new EmittableMockLivekitRoom({ localParticipant, remoteParticipants, }); + const matrixRoom = mockMatrixRoom({ + client: { + getUserId: () => localRtcMember.sender, + getDeviceId: () => localRtcMember.deviceId, + } as Partial as MatrixClient, + getMember: (userId) => matrixRoomMembers.get(userId) ?? null, + }); + + const session = new MockRTCSession( + matrixRoom, + localRtcMember, + ).withMemberships(remoteRtcMemberships) as unknown as MatrixRTCSession; + const vm = new CallViewModel( - mockMatrixRoom({ - client: { - getUserId: () => "@carol:example.org", - } as Partial as MatrixClient, - getMember: (userId) => members.get(userId) ?? null, - }), + session, liveKitRoom as unknown as Room, { kind: E2eeType.PER_PARTICIPANT, @@ -217,9 +285,11 @@ test("plays no sound when the participant list", () => { ); render(); expect(audioIsPlaying).toEqual([]); - // When the count drops + // When the count drops to the max we should play the leave sound act(() => { - liveKitRoom.removeParticipant(aliceParticipant); + remoteRtcMemberships.next( + mockRtcMemberships.slice(0, MAX_PARTICIPANT_COUNT_FOR_SOUND - 1), + ); }); expect(audioIsPlaying).toEqual([leaveSound]); }); From e4087b2b45c95f006c7b4308f96fc36da0705de1 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 18:33:14 +0000 Subject: [PATCH 79/84] Lint --- src/utils/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/test.ts b/src/utils/test.ts index a4b5b292b..dca98825d 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -314,4 +314,4 @@ export class MockRTCSession extends TypedEventEmitter< return this; } -} \ No newline at end of file +} From 40526cb7c154c9fe7fd1b1facaf107bfa6f46161 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 13 Nov 2024 10:21:38 +0000 Subject: [PATCH 80/84] Developer setting to show non-member tiles This is based on top of https://github.com/element-hq/element-call/pull/2701 --- locales/en-GB/app.json | 3 +- src/settings/SettingsModal.tsx | 19 +++++++++ src/settings/settings.ts | 8 +++- src/state/CallViewModel.test.ts | 44 ++++++++++++++++++++ src/state/CallViewModel.ts | 74 +++++++++++++++++++++++++++++++-- 5 files changed, 142 insertions(+), 6 deletions(-) diff --git a/locales/en-GB/app.json b/locales/en-GB/app.json index bc37dd7bc..acc92741d 100644 --- a/locales/en-GB/app.json +++ b/locales/en-GB/app.json @@ -73,7 +73,8 @@ "device_id": "Device ID: {{id}}", "duplicate_tiles_label": "Number of additional tile copies per participant", "hostname": "Hostname: {{hostname}}", - "matrix_id": "Matrix ID: {{id}}" + "matrix_id": "Matrix ID: {{id}}", + "show_non_member_tiles": "Show tiles for non-member media" }, "disconnected_banner": "Connectivity to the server has been lost.", "full_screen_view_description": "<0>Submitting debug logs will help us track down the problem.", diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 4ffcecf5d..54c3fc604 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -26,6 +26,7 @@ import { useSetting, developerSettingsTab as developerSettingsTabSetting, duplicateTiles as duplicateTilesSetting, + showNonMemberTiles as showNonMemberTilesSetting, useOptInAnalytics, soundEffectVolumeSetting, } from "./settings"; @@ -70,6 +71,10 @@ export const SettingsModal: FC = ({ ); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); + const [showNonMemberTiles, setShowNonMemberTiles] = useSetting( + showNonMemberTilesSetting, + ); + const optInDescription = ( @@ -240,6 +245,20 @@ export const SettingsModal: FC = ({ )} /> + + ): void => { + setShowNonMemberTiles(event.target.checked); + }, + [setShowNonMemberTiles], + )} + /> + ), }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index b2da16744..8ced3b4fc 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -40,9 +40,11 @@ export class Setting { private readonly _value: BehaviorSubject; public readonly value: Observable; - public readonly setValue = (value: T): void => { + public readonly setValue = (value: T, persist = true): void => { this._value.next(value); - localStorage.setItem(this.key, JSON.stringify(value)); + if (persist) { + localStorage.setItem(this.key, JSON.stringify(value)); + } }; } @@ -75,6 +77,8 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); +export const showNonMemberTiles = new Setting("show-non-member-tiles", false); + export const audioInput = new Setting( "audio-input", undefined, diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 5dbfb1ca7..b10fc51d0 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -43,6 +43,7 @@ import { ECConnectionState, } from "../livekit/useECConnectionState"; import { E2eeType } from "../e2ee/e2eeType"; +import { showNonMemberTiles } from "../settings/settings"; vi.mock("@livekit/components-core"); @@ -636,6 +637,49 @@ test("participants must have a MatrixRTCSession to be visible", () => { }); }); +test("shows participants without MatrixRTCSession when enabled in settings", () => { + // enable the setting: + showNonMemberTiles.setValue(true); + withTestScheduler(({ hot, expectObservable }) => { + const scenarioInputMarbles = " abc"; + const expectedLayoutMarbles = "abc"; + + withCallViewModel( + hot(scenarioInputMarbles, { + a: [], + b: [aliceParticipant], + c: [aliceParticipant, bobParticipant], + }), + of([]), + of(ConnectionState.Connected), + new Map(), + (vm) => { + vm.setGridMode("grid"); + expectObservable(summarizeLayout(vm.layout)).toBe( + expectedLayoutMarbles, + { + a: { + type: "grid", + spotlight: undefined, + grid: ["local:0"], + }, + b: { + type: "one-on-one", + local: "local:0", + remote: `${aliceId}:0`, + }, + c: { + type: "grid", + spotlight: undefined, + grid: ["local:0", `${aliceId}:0`, `${bobId}:0`], + }, + }, + ); + }, + ); + }); +}); + it("should show at least one tile per MatrixRTCSession", () => { withTestScheduler(({ hot, expectObservable }) => { // iterate through some combinations of MatrixRTC memberships diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 210689b7b..393a5893c 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -66,7 +66,7 @@ import { } from "./MediaViewModel"; import { accumulate, finalizeValue } from "../utils/observable"; import { ObservableScope } from "./ObservableScope"; -import { duplicateTiles } from "../settings/settings"; +import { duplicateTiles, showNonMemberTiles } from "../settings/settings"; import { isFirefox } from "../Platform"; import { setPipEnabled } from "../controls"; import { GridTileViewModel, SpotlightTileViewModel } from "./TileViewModel"; @@ -427,6 +427,8 @@ export class CallViewModel extends ViewModel { }, ); + private readonly nonMemberItemCount = new BehaviorSubject(0); + /** * List of MediaItems that we want to display */ @@ -441,6 +443,7 @@ export class CallViewModel extends ViewModel { this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, ).pipe(startWith(null)), + showNonMemberTiles.value, ]).pipe( scan( ( @@ -450,6 +453,7 @@ export class CallViewModel extends ViewModel { { participant: localParticipant }, duplicateTiles, _membershipsChanged, + showNonMemberTiles, ], ) => { const newItems = new Map( @@ -487,9 +491,17 @@ export class CallViewModel extends ViewModel { } for (let i = 0; i < 1 + duplicateTiles; i++) { const indexedMediaId = `${livekitParticipantId}:${i}`; - const prevMedia = prevItems.get(indexedMediaId); + let prevMedia = prevItems.get(indexedMediaId); if (prevMedia && prevMedia instanceof UserMedia) { prevMedia.updateParticipant(participant); + if (prevMedia.vm.member === undefined) { + // We have a previous media created because of the `debugShowNonMember` flag. + // In this case we actually replace the media item. + // This "hack" never occurs if we do not use the `debugShowNonMember` debugging + // option and if we always find a room member for each rtc member (which also + // only fails if we have a fundamental problem) + prevMedia = undefined; + } } yield [ indexedMediaId, @@ -525,7 +537,63 @@ export class CallViewModel extends ViewModel { }.bind(this)(), ); - return newItems; + // Generate non member items (items without a corresponding MatrixRTC member) + // Those items should not be rendered, they are participants in livekit that do not have a corresponding + // matrix rtc members. This cannot be any good: + // - A malicious user impersonates someone + // - Someone injects abusive content + // - The user cannot have encryption keys so it makes no sense to participate + // We can only trust users that have a matrixRTC member event. + // + // This is still available as a debug option. This can be useful + // - If one wants to test scalability using the livekit cli. + // - If an experimental project does not yet do the matrixRTC bits. + // - If someone wants to debug if the LK connection works but matrixRTC room state failed to arrive. + const newNonMemberItems = showNonMemberTiles + ? new Map( + function* (this: CallViewModel): Iterable<[string, MediaItem]> { + for (const participant of remoteParticipants) { + for (let i = 0; i < 1 + duplicateTiles; i++) { + const maybeNonMemberParticipantId = + participant.identity + ":" + i; + if (!newItems.has(maybeNonMemberParticipantId)) { + const nonMemberId = maybeNonMemberParticipantId; + yield [ + nonMemberId, + // We create UserMedia with or without a participant. + // This will be the initial value of a BehaviourSubject. + // Once a participant appears we will update the BehaviourSubject. (see above) + prevItems.get(nonMemberId) ?? + new UserMedia( + nonMemberId, + undefined, + participant, + this.encryptionSystem, + this.livekitRoom, + ), + ]; + } + } + } + }.bind(this)(), + ) + : new Map(); + if (newNonMemberItems.size > 0) { + logger.debug("Added NonMember items: ", newNonMemberItems); + } + + const newNonMemberItemCount = + newNonMemberItems.size / (1 + duplicateTiles); + if (this.nonMemberItemCount.value !== newNonMemberItemCount) + this.nonMemberItemCount.next(newNonMemberItemCount); + + const combinedNew = new Map([ + ...newNonMemberItems.entries(), + ...newItems.entries(), + ]); + + for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy(); + return combinedNew; }, new Map(), ), From d1e90939b196380ebee91991ada0e5aaa6c63dbd Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Mon, 2 Dec 2024 18:50:50 +0000 Subject: [PATCH 81/84] Lint --- src/settings/settings.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 8ced3b4fc..c983c92f9 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -77,7 +77,10 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); -export const showNonMemberTiles = new Setting("show-non-member-tiles", false); +export const showNonMemberTiles = new Setting( + "show-non-member-tiles", + false, +); export const audioInput = new Setting( "audio-input", From 820ac7f28ab011250044544c603740c6861bd5eb Mon Sep 17 00:00:00 2001 From: Timo Date: Tue, 26 Nov 2024 12:33:48 +0100 Subject: [PATCH 82/84] Make the loading state more subtle - instead of a label we show a animated gradient From 5a1a1a7a4ea87301d0043b1923ff90f79b701122 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 6 Dec 2024 11:59:38 +0000 Subject: [PATCH 83/84] Remove changes that should be in https://github.com/element-hq/element-call/pull/2771 --- locales/en/app.json | 3 +-- src/settings/SettingsModal.tsx | 19 ------------------- src/settings/settings.ts | 5 ----- 3 files changed, 1 insertion(+), 26 deletions(-) diff --git a/locales/en/app.json b/locales/en/app.json index 94de72354..e500f66cd 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -73,8 +73,7 @@ "device_id": "Device ID: {{id}}", "duplicate_tiles_label": "Number of additional tile copies per participant", "hostname": "Hostname: {{hostname}}", - "matrix_id": "Matrix ID: {{id}}", - "show_non_member_tiles": "Show tiles for non-member media" + "matrix_id": "Matrix ID: {{id}}" }, "disconnected_banner": "Connectivity to the server has been lost.", "full_screen_view_description": "<0>Submitting debug logs will help us track down the problem.", diff --git a/src/settings/SettingsModal.tsx b/src/settings/SettingsModal.tsx index 54c3fc604..4ffcecf5d 100644 --- a/src/settings/SettingsModal.tsx +++ b/src/settings/SettingsModal.tsx @@ -26,7 +26,6 @@ import { useSetting, developerSettingsTab as developerSettingsTabSetting, duplicateTiles as duplicateTilesSetting, - showNonMemberTiles as showNonMemberTilesSetting, useOptInAnalytics, soundEffectVolumeSetting, } from "./settings"; @@ -71,10 +70,6 @@ export const SettingsModal: FC = ({ ); const [duplicateTiles, setDuplicateTiles] = useSetting(duplicateTilesSetting); - const [showNonMemberTiles, setShowNonMemberTiles] = useSetting( - showNonMemberTilesSetting, - ); - const optInDescription = ( @@ -245,20 +240,6 @@ export const SettingsModal: FC = ({ )} /> - - ): void => { - setShowNonMemberTiles(event.target.checked); - }, - [setShowNonMemberTiles], - )} - /> - ), }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index c983c92f9..4eb2bcaa7 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -77,11 +77,6 @@ export const developerSettingsTab = new Setting( export const duplicateTiles = new Setting("duplicate-tiles", 0); -export const showNonMemberTiles = new Setting( - "show-non-member-tiles", - false, -); - export const audioInput = new Setting( "audio-input", undefined, From 93710ff42a030882612b591fd0c04bc5477979f1 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 6 Dec 2024 12:00:16 +0000 Subject: [PATCH 84/84] Remove unused code --- src/settings/settings.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/settings/settings.ts b/src/settings/settings.ts index 4eb2bcaa7..b2da16744 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -40,11 +40,9 @@ export class Setting { private readonly _value: BehaviorSubject; public readonly value: Observable; - public readonly setValue = (value: T, persist = true): void => { + public readonly setValue = (value: T): void => { this._value.next(value); - if (persist) { - localStorage.setItem(this.key, JSON.stringify(value)); - } + localStorage.setItem(this.key, JSON.stringify(value)); }; }