Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

js-sdk voip architecture: MatrixRTCSessionManager #1432

Open
dbkr opened this issue Sep 12, 2023 · 0 comments
Open

js-sdk voip architecture: MatrixRTCSessionManager #1432

dbkr opened this issue Sep 12, 2023 · 0 comments
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks

Comments

@dbkr
Copy link
Member

dbkr commented Sep 12, 2023

Your use case

matrix-org/matrix-js-sdk#3663 introduces the MatrixRTCSessionManager. Daniel asks: does this class need to exist or could it be split between Room and MatrixcRTCSession?

Full comment:

I'm not confident that we really need the rtc session manager at this point, I'd rather invested more on the rtc session itself. IMHO currently it's fairly complicated (for the amount of code that it contains it has a high accidental complexity) and feels somewhat brittle (my gut feeling is that there will be bugs and edge-cases with the changes introduced in the PR and the solutions for them won't be particularly elegant - e.g. the solution for the async/await function looks quite complicated atm and hard to understand and it's generally not a very idiomatic way of solving state access/synchronisation problems). I also think that the API surface of the matrix RTC session could resemble LiveKit's Room a bit more (conceptually it's the same thing and it would also help unifying/merging them once we work on a deeper integration to introduce cascading). I'm particularly picky on such topics because I think that the matrix RTC session is a foundation upon which the rest of the things are going to be built in the future. That being said, if the changes are considered temporary and we don't plan to build much stuff on top of that, it might be fine to leave it like this.

We'd like to revisit this architecture later and see if the design could be better without this class (eg. maybe when we introduce support for user sessions or when implementing MatrixRTC in the rust SDK.

Have you considered any alternatives?

No response

Additional context

No response

@dbkr dbkr added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
No open projects
Development

No branches or pull requests

1 participant