Skip to content

Commit a723f10

Browse files
hughnstoger5robintown
authored
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 #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 #2701 * Lint * Remove unused code * Remove changes that should be in #2858 * Update src/state/CallViewModel.test.ts Co-authored-by: Robin <robin@robin.town> * Merge branch 'livekit' into toger5/show-non-member-tiles * Remove unused nonMemberItemCount * Restore default showNonMemberTiles value after test * Update comments --------- Co-authored-by: Timo <toger5@hotmail.de> Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> Co-authored-by: Robin <robin@robin.town>
1 parent 0c66ec5 commit a723f10

File tree

5 files changed

+133
-4
lines changed

5 files changed

+133
-4
lines changed

locales/en/app.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@
7474
"device_id": "Device ID: {{id}}",
7575
"duplicate_tiles_label": "Number of additional tile copies per participant",
7676
"hostname": "Hostname: {{hostname}}",
77-
"matrix_id": "Matrix ID: {{id}}"
77+
"matrix_id": "Matrix ID: {{id}}",
78+
"show_non_member_tiles": "Show tiles for non-member media"
7879
},
7980
"disconnected_banner": "Connectivity to the server has been lost.",
8081
"full_screen_view_description": "<0>Submitting debug logs will help us track down the problem.</0>",

src/settings/DeveloperSettingsTab.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
useSetting,
1414
duplicateTiles as duplicateTilesSetting,
1515
debugTileLayout as debugTileLayoutSetting,
16+
showNonMemberTiles as showNonMemberTilesSetting,
1617
} from "./settings";
1718
import type { MatrixClient } from "matrix-js-sdk/src/client";
1819

@@ -26,6 +27,9 @@ export const DeveloperSettingsTab: FC<Props> = ({ client }) => {
2627
const [debugTileLayout, setDebugTileLayout] = useSetting(
2728
debugTileLayoutSetting,
2829
);
30+
const [showNonMemberTiles, setShowNonMemberTiles] = useSetting(
31+
showNonMemberTilesSetting,
32+
);
2933

3034
return (
3135
<>
@@ -85,6 +89,20 @@ export const DeveloperSettingsTab: FC<Props> = ({ client }) => {
8589
}
8690
/>
8791
</FieldRow>
92+
<FieldRow>
93+
<InputField
94+
id="showNonMemberTiles"
95+
type="checkbox"
96+
label={t("developer_mode.show_non_member_tiles")}
97+
checked={!!showNonMemberTiles}
98+
onChange={useCallback(
99+
(event: ChangeEvent<HTMLInputElement>): void => {
100+
setShowNonMemberTiles(event.target.checked);
101+
},
102+
[setShowNonMemberTiles],
103+
)}
104+
/>
105+
</FieldRow>
88106
</>
89107
);
90108
};

src/settings/settings.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ export const developerMode = new Setting("developer-settings-tab", false);
7272

7373
export const duplicateTiles = new Setting("duplicate-tiles", 0);
7474

75+
export const showNonMemberTiles = new Setting<boolean>(
76+
"show-non-member-tiles",
77+
false,
78+
);
7579
export const debugTileLayout = new Setting("debug-tile-layout", false);
7680

7781
export const audioInput = new Setting<string | undefined>(

src/state/CallViewModel.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
type ECConnectionState,
4747
} from "../livekit/useECConnectionState";
4848
import { E2eeType } from "../e2ee/e2eeType";
49+
import { showNonMemberTiles } from "../settings/settings";
4950

5051
vi.mock("@livekit/components-core");
5152

@@ -686,6 +687,53 @@ test("participants must have a MatrixRTCSession to be visible", () => {
686687
});
687688
});
688689

690+
test("shows participants without MatrixRTCSession when enabled in settings", () => {
691+
try {
692+
// enable the setting:
693+
showNonMemberTiles.setValue(true);
694+
withTestScheduler(({ hot, expectObservable }) => {
695+
const scenarioInputMarbles = " abc";
696+
const expectedLayoutMarbles = "abc";
697+
698+
withCallViewModel(
699+
hot(scenarioInputMarbles, {
700+
a: [],
701+
b: [aliceParticipant],
702+
c: [aliceParticipant, bobParticipant],
703+
}),
704+
of([]), // No one joins the MatrixRTC session
705+
of(ConnectionState.Connected),
706+
new Map(),
707+
(vm) => {
708+
vm.setGridMode("grid");
709+
expectObservable(summarizeLayout(vm.layout)).toBe(
710+
expectedLayoutMarbles,
711+
{
712+
a: {
713+
type: "grid",
714+
spotlight: undefined,
715+
grid: ["local:0"],
716+
},
717+
b: {
718+
type: "one-on-one",
719+
local: "local:0",
720+
remote: `${aliceId}:0`,
721+
},
722+
c: {
723+
type: "grid",
724+
spotlight: undefined,
725+
grid: ["local:0", `${aliceId}:0`, `${bobId}:0`],
726+
},
727+
},
728+
);
729+
},
730+
);
731+
});
732+
} finally {
733+
showNonMemberTiles.setValue(showNonMemberTiles.defaultValue);
734+
}
735+
});
736+
689737
it("should show at least one tile per MatrixRTCSession", () => {
690738
withTestScheduler(({ hot, expectObservable }) => {
691739
// iterate through some combinations of MatrixRTC memberships

src/state/CallViewModel.ts

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import {
6969
} from "./MediaViewModel";
7070
import { accumulate, finalizeValue } from "../utils/observable";
7171
import { ObservableScope } from "./ObservableScope";
72-
import { duplicateTiles } from "../settings/settings";
72+
import { duplicateTiles, showNonMemberTiles } from "../settings/settings";
7373
import { isFirefox } from "../Platform";
7474
import { setPipEnabled } from "../controls";
7575
import {
@@ -449,6 +449,7 @@ export class CallViewModel extends ViewModel {
449449
this.matrixRTCSession,
450450
MatrixRTCSessionEvent.MembershipsChanged,
451451
).pipe(startWith(null)),
452+
showNonMemberTiles.value,
452453
]).pipe(
453454
scan(
454455
(
@@ -458,6 +459,7 @@ export class CallViewModel extends ViewModel {
458459
{ participant: localParticipant },
459460
duplicateTiles,
460461
_membershipsChanged,
462+
showNonMemberTiles,
461463
],
462464
) => {
463465
const newItems = new Map(
@@ -495,9 +497,17 @@ export class CallViewModel extends ViewModel {
495497
}
496498
for (let i = 0; i < 1 + duplicateTiles; i++) {
497499
const indexedMediaId = `${livekitParticipantId}:${i}`;
498-
const prevMedia = prevItems.get(indexedMediaId);
500+
let prevMedia = prevItems.get(indexedMediaId);
499501
if (prevMedia && prevMedia instanceof UserMedia) {
500502
prevMedia.updateParticipant(participant);
503+
if (prevMedia.vm.member === undefined) {
504+
// We have a previous media created because of the `debugShowNonMember` flag.
505+
// In this case we actually replace the media item.
506+
// This "hack" never occurs if we do not use the `debugShowNonMember` debugging
507+
// option and if we always find a room member for each rtc member (which also
508+
// only fails if we have a fundamental problem)
509+
prevMedia = undefined;
510+
}
501511
}
502512
yield [
503513
indexedMediaId,
@@ -533,7 +543,55 @@ export class CallViewModel extends ViewModel {
533543
}.bind(this)(),
534544
);
535545

536-
return newItems;
546+
// Generate non member items (items without a corresponding MatrixRTC member)
547+
// Those items should not be rendered, they are participants in LiveKit that do not have a corresponding
548+
// MatrixRTC members. This cannot be any good:
549+
// - A malicious user impersonates someone
550+
// - Someone injects abusive content
551+
// - The user cannot have encryption keys so it makes no sense to participate
552+
// We can only trust users that have a MatrixRTC member event.
553+
//
554+
// This is still available as a debug option. This can be useful
555+
// - If one wants to test scalability using the LiveKit CLI.
556+
// - If an experimental project does not yet do the MatrixRTC bits.
557+
// - If someone wants to debug if the LiveKit connection works but MatrixRTC room state failed to arrive.
558+
const newNonMemberItems = showNonMemberTiles
559+
? new Map(
560+
function* (this: CallViewModel): Iterable<[string, MediaItem]> {
561+
for (const participant of remoteParticipants) {
562+
for (let i = 0; i < 1 + duplicateTiles; i++) {
563+
const maybeNonMemberParticipantId =
564+
participant.identity + ":" + i;
565+
if (!newItems.has(maybeNonMemberParticipantId)) {
566+
const nonMemberId = maybeNonMemberParticipantId;
567+
yield [
568+
nonMemberId,
569+
prevItems.get(nonMemberId) ??
570+
new UserMedia(
571+
nonMemberId,
572+
undefined,
573+
participant,
574+
this.encryptionSystem,
575+
this.livekitRoom,
576+
),
577+
];
578+
}
579+
}
580+
}
581+
}.bind(this)(),
582+
)
583+
: new Map();
584+
if (newNonMemberItems.size > 0) {
585+
logger.debug("Added NonMember items: ", newNonMemberItems);
586+
}
587+
588+
const combinedNew = new Map([
589+
...newNonMemberItems.entries(),
590+
...newItems.entries(),
591+
]);
592+
593+
for (const [id, t] of prevItems) if (!combinedNew.has(id)) t.destroy();
594+
return combinedNew;
537595
},
538596
new Map<string, MediaItem>(),
539597
),

0 commit comments

Comments
 (0)