[MatrixRTC] Use relation based call membership to compute `create_ts()` #5031

toger5 · 2025-10-06T15:56:18Z

This PR changes the construction of the CallMembership object.
It will add an optional relatedEvent parameter which will store initial connect event for this call participation.

This is then used to fetch metadata (like application) and the origin_server_ts of the initial join event.

This requires that MatrixRTCSession.sessionMembershipsForSlot becomes async since it now also needs to fetch related events.

This change needs a bit of test adjustments and other places of the codebase need updating.

Checklist

Tests written for new code (and old code if feasible).
New or updated public/exported symbols have accurate TSDoc documentation.
Linter and other CI checks pass.
Sign-off given on the changes (see CONTRIBUTING.md).

Signed-off-by: Timo K <[email protected]>

- rename Focus -> Transport - add RtcMembershipData (next to `sessionMembershipData`) - make `new CallMembership` initializable with both - move oldest member calculation into CallMembership Signed-off-by: Timo K <[email protected]>

Signed-off-by: Timo K <[email protected]>

Co-authored-by: Robin <[email protected]>

Signed-off-by: Timo K <[email protected]>

toger5

What happens if two MatrixRTCSession.sessionMembershipsForSlot are running at the same time?
The second one could finish before the other.... Is this an issue?

robintown · 2025-10-08T04:54:57Z

@toger5 Is the diff meant to be as big as it is? I think I'm looking at a lot of lint fixes, or something.

toger5 · 2025-10-08T08:13:34Z

Is the diff meant to be as big as it is?
Yes the diff is meant to be this big. There is a very large amount of lines which add async and await

On top of this, the construct for callMembership has a signature change which requires a change in makeMockEvent.

before
new CallMembership(event, event.content, ....) now new CallMembership(event, relatedEvent,...)event.content will be aquired indirectly by reading event.getContent().relatedEvent` is new.

BillCarsonFr

The PR looks to be doind more than what the title suggests ^^
Nice to see all the tests

spec/unit/matrixrtc/CallMembership.spec.ts

BillCarsonFr · 2025-10-09T07:25:42Z

spec/unit/matrixrtc/CallMembership.spec.ts

            expect(membership.transports).toEqual([mockFocus]);
        });

        describe("getTransport", () => {


Looks like in all the tests foci_preferred is always an array of 1. Can the test clarify how this works?
I think it is a non multi-sfu thing to avoid switching focus, but for multi-sfu it is always one?
=> maybe a bug in the spec? the well-known also gives a list :/ should we pick one random?

I added a comment:

...membershipTemplate, // The list of foci_preferred provided by the homeserver. (in the test example we just provide one) // The oldest member logic will use the first item in this list. // The multi-sfu logic will (theoretically) also use all the items in the list at once // (currently the js-sdk sets it to only one item in multi-sfu mode). foci_preferred: [mockFocus], focus_active: { type: "livekit", focus_selection: "oldest_membership" },

I hope that clarifies things.

BillCarsonFr · 2025-10-09T07:29:06Z

spec/unit/matrixrtc/CallMembership.spec.ts

+                const membership = new CallMembership(makeMockEvent(0, membershipTemplate));

                // if we are the oldest member we use our focus.
                expect(membership.getTransport(membership)).toStrictEqual({ type: "livekit" });


I am a bit lost, the transport is only { type: livekit }? shouldn't there be a livekit_service_url and the alias?
Maybe we can keep "realistic" test data, and then use expect with expect.objectContaining({ .. }) to just test the field you want?

Update the tests to contain more realistic data.

BillCarsonFr · 2025-10-09T07:32:31Z

spec/unit/matrixrtc/CallMembership.spec.ts

                expect(membership.getTransport(oldestMembership)).toStrictEqual({ type: "livekit" });
            });
        });
        describe("correct values from computed fields", () => {


bonus point to have a test.each() that would try different configuration without reapeating code

test.each([ [1, 1, 2], [1, 2, 3], [2, 1, 3], ])('add(%i, %i) -> %i', (a, b, expected) => { expect(a + b).toBe(expected) })

you mean using different membership objects?
I am not sure what additional stability this would result in. The code just gets fields from different branches of the object but does not do things with the values. (except the expiration which we do in a dedicated test section)

BillCarsonFr · 2025-10-09T07:36:39Z

spec/unit/matrixrtc/MatrixRTCSession.spec.ts

        client.getDeviceId = jest.fn().mockReturnValue("AAAAAAA");
        client.sendEvent = jest.fn().mockResolvedValue({ event_id: "success" });
        client.decryptEventIfNeeded = jest.fn();
+        client.fetchRoomEvent = jest.fn().mockResolvedValue(undefined);


Not strictly this PR, but shouldn't we use getMockClientWithEventEmitter({}) insteaf of overriding the properties like that?
Currently this could be doing more thing than we think and polute the test?

This is definilty worth a discussion. I remember we discussed that it can also be benefitital to not use too many helpers so the tests are more accessible if devs are not familiar with the helper methods.

BillCarsonFr · 2025-10-09T07:47:40Z

spec/unit/matrixrtc/mocks.ts

-        getId: jest.fn().mockReturnValue(secureRandomString(8)),
-        isDecryptionFailure: jest.fn().mockReturnValue(false),
-    } as unknown as MatrixEvent;
+    return new MatrixEvent({


I think there is a makeMockEvent utility in mock.ts

this is just updating this method?
Or do you mean test-utils.ts

oh right, I was expecting to see a vi.mock()

src/matrixrtc/CallMembership.ts

BillCarsonFr · 2025-10-09T08:00:19Z

src/matrixrtc/IMembershipManager.ts

     * Start sending all necessary events to make this user participate in the RTC session.
     * @param fociPreferred the list of preferred foci to use in the joined RTC membership event.
-     * If multiSfuFocus is set, this is only needed if this client wants to publish to multiple transports simultaneously.
     * @param multiSfuFocus the active focus to use in the joined RTC membership event. Setting this implies the


I am still confused by that. My understanding was that there is a list of FOCI that my homeserver recommends. But if I join an existing call, there is already an active FOCI people are on, and we want to keep this one on top of my list in case I become the oldest member later, so that we don't switch focus.

So foci_active made sense to me, it was the existing Foci people are already using that I should connect on but also add on top of my list to avoid future jumps.

Now this signature do not make sense for me :/
Maybe if we change it to fociPreferred: Transport[], oldestMembership?: MembershipData, ..

Also I don't get why fociPreferred is an array

So foci_active made sense to me, it was the existing Foci people are already using that I should connect on but also add on top of my list to avoid future jumps.

focus_active was just the description what system a specific user was using to decide what transport to use.

It really is only used with m.call.member events (not with m.rtc.member).

So it is basically depreacted anyways.

Also I don't get why fociPreferred is an array

This was so multiple foci could be defined as fallbacks. All this is entirely depreacted due to multi-sfu.
In the new spec there is onlly rtc_transports that is the list of currently in use publishing transports.

BillCarsonFr · 2025-10-09T08:01:39Z

src/matrixrtc/MembershipManager.ts

                if (!this.state.delayId) {
                    // Delay id got reset. This action was used to check if the hs canceled the delayed event when the join state got sent.
-                    return createInsertActionUpdate(MembershipActionType.SendDelayedEvent);
+                    return Promise.resolve(createInsertActionUpdate(MembershipActionType.SendDelayedEvent));


is this needed? just returning the value is doing the same thing no?

the method is not async. I had issues with tests if this became an async method... It also is nto really async but just a wrapper to construct a promise.

t3chguy

Please fix the merge conflicts before review

…create-ts Signed-off-by: Timo K <[email protected]>

Signed-off-by: Timo K <[email protected]>

t3chguy

Could we please be a little more mindful to reviewers, there's little point in reviewing something with so many CI failures given it'll need re-reviewing after fixing.

toger5 · 2025-10-09T12:35:04Z

Could we please be a little more mindful to reviewers, there's little point in reviewing something with so many CI failures given it'll need re-reviewing after fixing.

Converted it to draft. As it should be until ci is sorted. Sorry for the confusion/overhead.

Signed-off-by: Timo K <[email protected]>

robintown and others added 17 commits August 27, 2025 11:19

WIP

41a2f47

temp

209eecd

Signed-off-by: Timo K <[email protected]>

Fix imports

6156d4c

Fix checkSessionsMembershipData thinking foci_preferred is required

b61e39a

Merge branch 'develop' into voip-team/multi-SFU

ca4a9c6

incorporate CallMembership changes

29879e8

- rename Focus -> Transport - add RtcMembershipData (next to `sessionMembershipData`) - make `new CallMembership` initializable with both - move oldest member calculation into CallMembership Signed-off-by: Timo K <[email protected]>

use correct event type

86f33f9

Signed-off-by: Timo K <[email protected]>

fix sonar cube conerns

bb7c23d

Signed-off-by: Timo K <[email protected]>

callMembership tests

8a5a8cd

Signed-off-by: Timo K <[email protected]>

make test correct

25f4d6f

Signed-off-by: Timo K <[email protected]>

make sonar cube happy (it does not know about the type constraints...)

84a3d56

Signed-off-by: Timo K <[email protected]>

remove created_ts from RtcMembership

5bc970c

Signed-off-by: Timo K <[email protected]>

fix imports

d94d02d

Signed-off-by: Timo K <[email protected]>

Update src/matrixrtc/IMembershipManager.ts

74b793c

Co-authored-by: Robin <[email protected]>

rename LivekitFocus.ts -> LivekitTransport.ts

e829a7b

Signed-off-by: Timo K <[email protected]>

add details to getTransport

f70cb14

Signed-off-by: Timo K <[email protected]>

Merge branch 'develop' into voip-team/multi-SFU

a343e8c

toger5 changed the title ~~Toger5/use relation based call membership create ts~~ [MatrixRTC] use relation based call membership create ts Oct 6, 2025

toger5 mentioned this pull request Oct 7, 2025

[MatrixRTC] Multi SFU support + m.rtc.member event type support #5022

Merged

4 tasks

toger5 added 6 commits October 7, 2025 14:58

review

11f610d

Signed-off-by: Timo K <[email protected]>

use DEFAULT_EXPIRE_DURATION in tests

66f202a

Signed-off-by: Timo K <[email protected]>

fix test does not provide focus if the selection method is unknown

4643844

Signed-off-by: Timo K <[email protected]>

Add the parent event to the CallMembership.

8a21ff6

Signed-off-by: Timo K <[email protected]>

fix lints

4bbb240

Signed-off-by: Timo K <[email protected]>

correct usage of methods that now became async

aa1cbe9

Signed-off-by: Timo K <[email protected]>

toger5 force-pushed the toger5/use-relation-based-CallMembership-create-ts branch from 9e977fb to aa1cbe9 Compare October 7, 2025 13:20

toger5 added the T-Enhancement label Oct 7, 2025

fix lints

65a3461

Signed-off-by: Timo K <[email protected]>

github-actions bot deployed to PR Documentation Preview October 7, 2025 15:12 View deployment

fix tests

e9dafb5

Signed-off-by: Timo K <[email protected]>

github-actions bot deployed to PR Documentation Preview October 7, 2025 18:11 View deployment

toger5 commented Oct 7, 2025

View reviewed changes

toger5 marked this pull request as ready for review October 7, 2025 18:25

toger5 requested a review from a team as a code owner October 7, 2025 18:25

toger5 requested review from BillCarsonFr and removed request for a team October 7, 2025 18:25

Base automatically changed from voip-team/multi-SFU to develop October 8, 2025 19:28

sonarqubecloud bot requested a review from a team as a code owner October 8, 2025 19:28

sonarqubecloud bot requested review from MidhunSureshR and t3chguy October 8, 2025 19:28

BillCarsonFr reviewed Oct 9, 2025

View reviewed changes

t3chguy requested changes Oct 9, 2025

View reviewed changes

toger5 added 3 commits October 9, 2025 10:42

Merge branch 'develop' into toger5/use-relation-based-CallMembership-…

fb23833

…create-ts Signed-off-by: Timo K <[email protected]>

introduce updateTs and connectedTs to fix expiration logic (+ fix tests)

b4abbfc

Signed-off-by: Timo K <[email protected]>

review valere

61b05a0

Signed-off-by: Timo K <[email protected]>

toger5 requested a review from t3chguy October 9, 2025 09:49

t3chguy requested changes Oct 9, 2025

View reviewed changes

toger5 marked this pull request as draft October 9, 2025 12:31

github-actions bot deployed to PR Documentation Preview October 9, 2025 12:41 View deployment

toger5 force-pushed the toger5/use-relation-based-CallMembership-create-ts branch from 61bfb76 to 6d18d73 Compare October 9, 2025 12:42

github-actions bot deployed to PR Documentation Preview October 9, 2025 12:44 View deployment

add sticky_key to makeMyMembership

06a46ac

Signed-off-by: Timo K <[email protected]>

toger5 force-pushed the toger5/use-relation-based-CallMembership-create-ts branch from 6d18d73 to 06a46ac Compare October 9, 2025 12:46

github-actions bot deployed to PR Documentation Preview October 9, 2025 12:48 View deployment

fix ts tests

4608506

Signed-off-by: Timo K <[email protected]>

github-actions bot deployed to PR Documentation Preview October 9, 2025 13:00 View deployment

toger5 marked this pull request as ready for review October 9, 2025 13:03

Uh oh!

[MatrixRTC] Use relation based call membership to compute create_ts() #5031

Are you sure you want to change the base?

[MatrixRTC] Use relation based call membership to compute create_ts() #5031

Uh oh!

Conversation

toger5 commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

toger5 left a comment

Choose a reason for hiding this comment

Uh oh!

robintown commented Oct 8, 2025

Uh oh!

toger5 commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BillCarsonFr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toger5 Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t3chguy left a comment

Choose a reason for hiding this comment

Uh oh!

t3chguy left a comment

Choose a reason for hiding this comment

Uh oh!

toger5 commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[MatrixRTC] Use relation based call membership to compute `create_ts()` #5031

[MatrixRTC] Use relation based call membership to compute `create_ts()` #5031

toger5 commented Oct 6, 2025 •

edited

Loading

toger5 commented Oct 8, 2025 •

edited

Loading

toger5 Oct 9, 2025 •

edited

Loading