From a723f10d2cdb71d25725bb6d2d2ac628b4f2ead0 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 13 Dec 2024 14:53:08 +0000 Subject: [PATCH] Developer setting to show LiveKit participants that do not have MatrixRTC sessions a.k.a. non-member tiles (#2771) * make tiles based on rtc member * display missing lk participant + fix tile multiplier * add show_non_member_participants config option * per member tiles * merge fixes * linter * linter and tests * tests * adapt tests (wip) * Remove unused keys * Fix optionality of nonMemberItemCount * video is optional * Mock RTC members * Lint * Merge fixes * Fix user id * Add explicit types for public fields * isRTCParticipantAvailable => isLiveKitParticipantAvailable * isLiveKitParticipantAvailable * Readonly * More keys removal * Make local field based on view model class not observable * Wording * Fix RTC members in tes * Tests again * Lint * Disable showing non-member tiles by default * Duplicate screen sharing tiles like we used to * Lint * Revert function reordering * Remove throttleTime from bad merge * Cleanup * Tidy config of show non-member settings * tidy up handling of local rtc member in tests * tidy up test init * Fix mocks * Cleanup * Apply local override where participant not yet known * Handle no visible media id * Assertions for one-on-one view * Remove isLiveKitParticipantAvailable and show via encryption status * Handle no local media (yet) * Remove unused effect for setting * Tidy settings * Avoid case of one-to-one layout with missing local or remote * Iterate * Remove option to show non-member tiles to simplify code review * Remove unused code * Remove more remnants of show-non-member-tiles * iterate * back * Fix unit test * Refactor * Expose TestScheduler as global * Fix incorrect type assertion * Simplify speaking observer * Fix * Whitespace * Make it clear that we are mocking MatrixRTC memberships * Test case for only showing tiles for MatrixRTC session members * Simplify diff * Simplify diff These changes are in https://github.com/element-hq/element-call/pull/2809 * . * Whitespaces * Use asObservable when exposing subject * Show "waiting for media..." when no participant * Additional test case * Don't show "waiting for media..." in case of local participant * Make the loading state more subtle - instead of a label we show a animated gradient * Use correct key for matrix rtc foci in code comment. (#2838) * Update src/tile/SpotlightTile.tsx Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> * Update src/state/CallViewModel.ts Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> * Make the purpose of BaseMediaViewModel.local explicit * Use named object instead of unnamed array for spotlightAndPip * Refactor spotlightAndPip into spotlight and pip * Use if statement instead of ternary for readability in spotlight and pip logic * Review feedback * Fix tests for CallEventAudioRenderer * Lint * Developer setting to show non-member tiles This is based on top of https://github.com/element-hq/element-call/pull/2701 * Lint * Remove unused code * Remove changes that should be in https://github.com/element-hq/element-call/pull/2858 * Update src/state/CallViewModel.test.ts Co-authored-by: Robin * Merge branch 'livekit' into toger5/show-non-member-tiles * Remove unused nonMemberItemCount * Restore default showNonMemberTiles value after test * Update comments --------- Co-authored-by: Timo Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> Co-authored-by: Robin --- locales/en/app.json | 3 +- src/settings/DeveloperSettingsTab.tsx | 18 ++++++++ src/settings/settings.ts | 4 ++ src/state/CallViewModel.test.ts | 48 ++++++++++++++++++++ src/state/CallViewModel.ts | 64 +++++++++++++++++++++++++-- 5 files changed, 133 insertions(+), 4 deletions(-) diff --git a/locales/en/app.json b/locales/en/app.json index 84038c2b..33f8f921 100644 --- a/locales/en/app.json +++ b/locales/en/app.json @@ -74,7 +74,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/DeveloperSettingsTab.tsx b/src/settings/DeveloperSettingsTab.tsx index 209bc41e..057b0b0c 100644 --- a/src/settings/DeveloperSettingsTab.tsx +++ b/src/settings/DeveloperSettingsTab.tsx @@ -13,6 +13,7 @@ import { useSetting, duplicateTiles as duplicateTilesSetting, debugTileLayout as debugTileLayoutSetting, + showNonMemberTiles as showNonMemberTilesSetting, } from "./settings"; import type { MatrixClient } from "matrix-js-sdk/src/client"; @@ -26,6 +27,9 @@ export const DeveloperSettingsTab: FC = ({ client }) => { const [debugTileLayout, setDebugTileLayout] = useSetting( debugTileLayoutSetting, ); + const [showNonMemberTiles, setShowNonMemberTiles] = useSetting( + showNonMemberTilesSetting, + ); return ( <> @@ -85,6 +89,20 @@ export const DeveloperSettingsTab: FC = ({ client }) => { } /> + + ): void => { + setShowNonMemberTiles(event.target.checked); + }, + [setShowNonMemberTiles], + )} + /> + ); }; diff --git a/src/settings/settings.ts b/src/settings/settings.ts index e8dda379..a902f9ab 100644 --- a/src/settings/settings.ts +++ b/src/settings/settings.ts @@ -72,6 +72,10 @@ export const developerMode = new Setting("developer-settings-tab", false); export const duplicateTiles = new Setting("duplicate-tiles", 0); +export const showNonMemberTiles = new Setting( + "show-non-member-tiles", + false, +); export const debugTileLayout = new Setting("debug-tile-layout", false); export const audioInput = new Setting( diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index 0e45faa1..d5b84d49 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -46,6 +46,7 @@ import { type ECConnectionState, } from "../livekit/useECConnectionState"; import { E2eeType } from "../e2ee/e2eeType"; +import { showNonMemberTiles } from "../settings/settings"; vi.mock("@livekit/components-core"); @@ -686,6 +687,53 @@ test("participants must have a MatrixRTCSession to be visible", () => { }); }); +test("shows participants without MatrixRTCSession when enabled in settings", () => { + try { + // 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([]), // No one joins the MatrixRTC session + 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`], + }, + }, + ); + }, + ); + }); + } finally { + showNonMemberTiles.setValue(showNonMemberTiles.defaultValue); + } +}); + 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 108a44c9..c701519b 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -69,7 +69,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 { @@ -449,6 +449,7 @@ export class CallViewModel extends ViewModel { this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged, ).pipe(startWith(null)), + showNonMemberTiles.value, ]).pipe( scan( ( @@ -458,6 +459,7 @@ export class CallViewModel extends ViewModel { { participant: localParticipant }, duplicateTiles, _membershipsChanged, + showNonMemberTiles, ], ) => { const newItems = new Map( @@ -495,9 +497,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, @@ -533,7 +543,55 @@ 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 + // MatrixRTC 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 LiveKit 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, + 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 combinedNew = new Map([ + ...newNonMemberItems.entries(), + ...newItems.entries(), + ]); + + for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy(); + return combinedNew; }, new Map(), ),