diff --git a/package.json b/package.json index af08ce12..1df79c26 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "livekit-client": "^2.13.0", "lodash-es": "^4.17.21", "loglevel": "^1.9.1", - "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#head=toger5/membership-manager-likely-disconnected", + "matrix-js-sdk": "github:matrix-org/matrix-js-sdk#head=develop", "matrix-widget-api": "^1.13.0", "normalize.css": "^8.0.1", "observable-hooks": "^4.2.3", diff --git a/src/room/CallEventAudioRenderer.test.tsx b/src/room/CallEventAudioRenderer.test.tsx index ff9fa6d4..1c515175 100644 --- a/src/room/CallEventAudioRenderer.test.tsx +++ b/src/room/CallEventAudioRenderer.test.tsx @@ -122,6 +122,8 @@ test("plays no sound when the participant list is more than the maximum size", ( render(); expect(playSound).not.toBeCalled(); + // Remove the last membership in the array to test the leaving sound + // (The array has length MAX_PARTICIPANT_COUNT_FOR_SOUND + 1) act(() => { rtcMemberships$.next( mockRtcMemberships.slice(0, MAX_PARTICIPANT_COUNT_FOR_SOUND), diff --git a/src/state/CallViewModel.test.ts b/src/state/CallViewModel.test.ts index eec471b6..d45a5c03 100644 --- a/src/state/CallViewModel.test.ts +++ b/src/state/CallViewModel.test.ts @@ -17,7 +17,7 @@ import { of, switchMap, } from "rxjs"; -import { type MatrixClient } from "matrix-js-sdk"; +import { SyncState, type MatrixClient } from "matrix-js-sdk"; import { ConnectionState, type LocalParticipant, @@ -48,6 +48,7 @@ import { mockRtcMembership, MockRTCSession, mockMediaDevices, + mockEmitter, } from "../utils/test"; import { ECAddonConnectionState, @@ -240,6 +241,7 @@ function withCallViewModel( mediaDevices: MediaDevices, continuation: ( vm: CallViewModel, + rtcSession: MockRTCSession, subjects: { raisedHands$: BehaviorSubject> }, ) => void, options: CallViewModelOptions = { @@ -249,8 +251,10 @@ function withCallViewModel( ): void { const room = mockMatrixRoom({ client: { + ...mockEmitter(), getUserId: () => localRtcMember.sender, getDeviceId: () => localRtcMember.deviceId, + getSyncState: () => SyncState.Syncing, } as Partial as MatrixClient, getMember: (userId) => roomMembers.get(userId) ?? null, }); @@ -307,7 +311,7 @@ function withCallViewModel( roomEventSelectorSpy!.mockRestore(); }); - continuation(vm, { raisedHands$: raisedHands$ }); + continuation(vm, rtcSession, { raisedHands$: raisedHands$ }); } test("participants are retained during a focus switch", () => { @@ -976,7 +980,7 @@ it("should rank raised hands above video feeds and below speakers and presenters of(ConnectionState.Connected), new Map(), mockMediaDevices({}), - (vm, { raisedHands$ }) => { + (vm, _rtcSession, { raisedHands$ }) => { schedule("ab", { a: () => { // We imagine that only two tiles (the first two) will be visible on screen at a time @@ -1240,7 +1244,7 @@ test("audio output changes when toggling earpiece mode", () => { }); test("media tracks are paused while reconnecting to MatrixRTC", () => { - withTestScheduler(({ behavior, schedule, expectObservable }) => { + withTestScheduler(({ schedule, expectObservable }) => { const trackRunning$ = new BehaviorSubject(true); const originalPublications = localParticipant.trackPublications; localParticipant.trackPublications = new Map([ @@ -1267,17 +1271,26 @@ test("media tracks are paused while reconnecting to MatrixRTC", () => { localParticipant.trackPublications = originalPublications; }); - const rtcMemberMarbles = " aba"; + // TODO: Add marbles for sync state and membership status as well + const connectedMarbles = " yny"; const expectedReconnectingMarbles = "nyn"; const expectedTrackRunningMarbles = "yny"; withCallViewModel( constant([]), - behavior(rtcMemberMarbles, { a: [localRtcMember], b: [] }), + constant([localRtcMember]), of(ConnectionState.Connected), new Map(), mockMediaDevices({}), - (vm) => { + (vm, rtcSession) => { + schedule(connectedMarbles, { + y: () => { + rtcSession.probablyLeft = false; + }, + n: () => { + rtcSession.probablyLeft = true; + }, + }); expectObservable(vm.reconnecting$).toBe( expectedReconnectingMarbles, yesNo, diff --git a/src/state/CallViewModel.ts b/src/state/CallViewModel.ts index 42a67eee..3b0a933d 100644 --- a/src/state/CallViewModel.ts +++ b/src/state/CallViewModel.ts @@ -56,6 +56,7 @@ import { type MatrixRTCSession, MatrixRTCSessionEvent, MembershipManagerEvent, + Status, } from "matrix-js-sdk/lib/matrixrtc"; import { ViewModel } from "./ViewModel"; @@ -407,17 +408,6 @@ function getRoomMemberFromRtcMember( // TODO: Move wayyyy more business logic from the call and lobby views into here export class CallViewModel extends ViewModel { private readonly userId = this.matrixRoom.client.getUserId(); - private readonly deviceId = this.matrixRoom.client.getDeviceId(); - - private readonly memberships$: Observable = merge( - // Handle call membership changes. - fromEvent(this.matrixRTCSession, MatrixRTCSessionEvent.MembershipsChanged), - // Handle room membership changes (and displayname updates) - fromEvent(this.matrixRoom, RoomStateEvent.Members), - ).pipe( - startWith(null), - map(() => this.matrixRTCSession.memberships), - ); private readonly matrixConnected$ = this.scope.behavior( // To consider ourselves connected to MatrixRTC, we check the following: @@ -431,26 +421,24 @@ export class CallViewModel extends ViewModel { startWith([this.matrixRoom.client.getSyncState()]), map(([state]) => state === SyncState.Syncing), ), - // We can see our own call membership - this.memberships$.pipe( - map((ms) => - ms.some( - (m) => m.sender === this.userId && m.deviceId === this.deviceId, - ), - ), + // Room state observed by session says we're connected + fromEvent( + this.matrixRTCSession, + MembershipManagerEvent.StatusChanged, + ).pipe( + startWith(null), + map(() => this.matrixRTCSession.membershipStatus === Status.Connected), ), // Also watch out for warnings that we've likely hit a timeout and our // delayed leave event is being sent (this condition is here because it // provides an earlier warning than the sync loop timeout, and we wouldn't // see the actual leave event until we reconnect to the sync loop) - ( - fromEvent( - this.matrixRTCSession, - MembershipManagerEvent.ProbablyLeft, - ) as Observable<[SyncState]> + fromEvent( + this.matrixRTCSession, + MembershipManagerEvent.ProbablyLeft, ).pipe( - startWith([false]), - map(([probablyLeft]) => !probablyLeft), + startWith(null), + map(() => this.matrixRTCSession.probablyLeft !== true), ), ), ); @@ -576,8 +564,18 @@ export class CallViewModel extends ViewModel { // than on Chrome/Firefox). This means it is important that we multicast the result so that we // don't do this work more times than we need to. This is achieved by converting to a behavior: public readonly memberDisplaynames$ = this.scope.behavior( - this.memberships$.pipe( - map((memberships) => { + merge( + // Handle call membership changes. + fromEvent( + this.matrixRTCSession, + MatrixRTCSessionEvent.MembershipsChanged, + ), + // Handle room membership changes (and displayname updates) + fromEvent(this.matrixRoom, RoomStateEvent.Members), + ).pipe( + startWith(null), + map(() => { + const memberships = this.matrixRTCSession.memberships; const displaynameMap = new Map(); const room = this.matrixRoom; @@ -1592,9 +1590,12 @@ export class CallViewModel extends ViewModel { ) { super(); - // Pause all media tracks when we're disconnected from MatrixRTC, because it - // can be an unpleasant surprise for the app to say 'reconnecting' and yet - // still be transmitting your media to others. + // Pause upstream of all local media tracks when we're disconnected from + // MatrixRTC, because it can be an unpleasant surprise for the app to say + // 'reconnecting' and yet still be transmitting your media to others. + // We use matrixConnected$ rather than reconnecting$ because we want to + // pause tracks during the initial joining sequence too until we're sure + // that our own media is displayed on screen. this.matrixConnected$.pipe(this.scope.bind()).subscribe((connected) => { const publications = this.livekitRoom.localParticipant.trackPublications.values(); @@ -1602,7 +1603,9 @@ export class CallViewModel extends ViewModel { for (const p of publications) { if (p.track?.isUpstreamPaused === true) { const kind = p.track.kind; - logger.log(`Reconnected to MatrixRTC; resuming ${kind} track`); + logger.log( + `Resumming ${kind} track (MatrixRTC connection present)`, + ); p.track .resumeUpstream() .catch((e) => @@ -1617,7 +1620,7 @@ export class CallViewModel extends ViewModel { for (const p of publications) { if (p.track?.isUpstreamPaused === false) { const kind = p.track.kind; - logger.log(`Lost connection to MatrixRTC; pausing ${kind} track`); + logger.log(`Pausing ${kind} track (no MatrixRTC connection)`); p.track .pauseUpstream() .catch((e) => diff --git a/src/state/MediaViewModel.ts b/src/state/MediaViewModel.ts index 48fbb733..dc2c135a 100644 --- a/src/state/MediaViewModel.ts +++ b/src/state/MediaViewModel.ts @@ -626,17 +626,6 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { ), ); - /** - * The local volume, taking into account whether we're supposed to pretend - * that the audio stream is disconnected (since we don't necessarily want that - * to modify the UI state). - */ - private readonly actualLocalVolume$ = this.scope.behavior( - this.pretendToBeDisconnected$.pipe( - switchMap((disconnected) => (disconnected ? of(0) : this.localVolume$)), - ), - ); - // This private field is used to override the value from the superclass private __videoEnabled$: Behavior; public get videoEnabled$(): Behavior { @@ -691,7 +680,13 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel { // Sync the local volume with LiveKit combineLatest([ participant$, - this.actualLocalVolume$.pipe(this.scope.bind()), + // The local volume, taking into account whether we're supposed to pretend + // that the audio stream is disconnected (since we don't necessarily want + // that to modify the UI state). + this.pretendToBeDisconnected$.pipe( + switchMap((disconnected) => (disconnected ? of(0) : this.localVolume$)), + this.scope.bind(), + ), ]).subscribe(([p, volume]) => p?.setVolume(volume)); } diff --git a/src/utils/observable.ts b/src/utils/observable.ts index 4ffcff46..74acfaf2 100644 --- a/src/utils/observable.ts +++ b/src/utils/observable.ts @@ -21,7 +21,8 @@ import { tap, withLatestFrom, } from "rxjs"; -import { Behavior } from "../state/Behavior"; + +import { type Behavior } from "../state/Behavior"; const nothing = Symbol("nothing"); diff --git a/src/utils/test-viewmodel.ts b/src/utils/test-viewmodel.ts index ce7082c1..463b3557 100644 --- a/src/utils/test-viewmodel.ts +++ b/src/utils/test-viewmodel.ts @@ -15,7 +15,12 @@ import { vitest } from "vitest"; import { type RelationsContainer } from "matrix-js-sdk/lib/models/relations-container"; import EventEmitter from "events"; -import type { RoomMember, MatrixClient, Room } from "matrix-js-sdk"; +import { + type RoomMember, + type MatrixClient, + type Room, + SyncState, +} from "matrix-js-sdk"; import { E2eeType } from "../e2ee/e2eeType"; import { CallViewModel } from "../state/CallViewModel"; import { @@ -52,6 +57,7 @@ export function getBasicRTCSession( client: { getUserId: () => localRtcMember.sender, getDeviceId: () => localRtcMember.deviceId, + getSyncState: () => SyncState.Syncing, sendEvent: vitest.fn().mockResolvedValue({ event_id: "$fake:event" }), redactEvent: vitest.fn().mockResolvedValue({ event_id: "$fake:event" }), decryptEventIfNeeded: vitest.fn().mockResolvedValue(undefined), diff --git a/src/utils/test.ts b/src/utils/test.ts index a7a9e489..c9976c51 100644 --- a/src/utils/test.ts +++ b/src/utils/test.ts @@ -19,8 +19,11 @@ import { type Focus, MatrixRTCSessionEvent, type MatrixRTCSessionEventHandlerMap, + MembershipManagerEvent, type SessionMembershipData, + Status, } from "matrix-js-sdk/lib/matrixrtc"; +import { type MembershipManagerEventHandlerMap } from "matrix-js-sdk/lib/matrixrtc/IMembershipManager"; import { type LocalParticipant, type LocalTrackPublication, @@ -318,8 +321,10 @@ export function mockConfig(config: Partial = {}): void { } export class MockRTCSession extends TypedEventEmitter< - MatrixRTCSessionEvent | RoomAndToDeviceEvents, - MatrixRTCSessionEventHandlerMap & RoomAndToDeviceEventsHandlerMap + MatrixRTCSessionEvent | RoomAndToDeviceEvents | MembershipManagerEvent, + MatrixRTCSessionEventHandlerMap & + RoomAndToDeviceEventsHandlerMap & + MembershipManagerEventHandlerMap > { public readonly statistics = { counters: {}, @@ -354,6 +359,17 @@ export class MockRTCSession extends TypedEventEmitter< return this; } + + public readonly membershipStatus = Status.Connected; + + private _probablyLeft = false; + public get probablyLeft(): boolean { + return this._probablyLeft; + } + public set probablyLeft(value: boolean) { + this._probablyLeft = value; + this.emit(MembershipManagerEvent.ProbablyLeft, value); + } } export const mockTrack = (identity: string): TrackReference => diff --git a/yarn.lock b/yarn.lock index 747dfd1d..0d2cf54a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7537,7 +7537,7 @@ __metadata: livekit-client: "npm:^2.13.0" lodash-es: "npm:^4.17.21" loglevel: "npm:^1.9.1" - matrix-js-sdk: "github:matrix-org/matrix-js-sdk#head=toger5/membership-manager-likely-disconnected" + matrix-js-sdk: "github:matrix-org/matrix-js-sdk#head=develop" matrix-widget-api: "npm:^1.13.0" normalize.css: "npm:^8.0.1" observable-hooks: "npm:^4.2.3" @@ -10278,9 +10278,9 @@ __metadata: languageName: node linkType: hard -"matrix-js-sdk@github:matrix-org/matrix-js-sdk#head=toger5/membership-manager-likely-disconnected": +"matrix-js-sdk@github:matrix-org/matrix-js-sdk#head=develop": version: 37.13.0 - resolution: "matrix-js-sdk@https://github.com/matrix-org/matrix-js-sdk.git#commit=3f71ea8547c7765ea94710406f34834e7d6df82a" + resolution: "matrix-js-sdk@https://github.com/matrix-org/matrix-js-sdk.git#commit=3a33c658bbcb8ce8791ec066db899f2571f5c52f" dependencies: "@babel/runtime": "npm:^7.12.5" "@matrix-org/matrix-sdk-crypto-wasm": "npm:^15.1.0" @@ -10296,7 +10296,7 @@ __metadata: sdp-transform: "npm:^2.14.1" unhomoglyph: "npm:^1.0.6" uuid: "npm:11" - checksum: 10c0/90238daaa5cad519b799a8ec53f768cbe295c6f5f4f7b82018dfee1f6311d6adbbc6fe71e86d4153fa3d2f711b67281f8202ad27dc0b1052c5a63621e96e05a2 + checksum: 10c0/1db0d39cfbe4f1c69c8acda0ea7580a4819fc47a7d4bff057382e33e72d9a610f8c03043a6c00bc647dfdc2815aa643c69d25022fb759342a92b77e1841524f1 languageName: node linkType: hard