Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Group Calls (WIP) #6848

Closed
wants to merge 100 commits into from
Closed

Group Calls (WIP) #6848

wants to merge 100 commits into from

Conversation

robertlong
Copy link

@robertlong robertlong commented Sep 21, 2021

Starting to implement group calls in Element. Will update these PR notes soon. This is following the work on Group Calls in matrix-js-sdk matrix-org/matrix-js-sdk#1902


This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://6173034b387f630f9c057210--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@robertlong robertlong requested a review from a team as a code owner September 21, 2021 18:49
@SimonBrandner SimonBrandner mentioned this pull request Sep 21, 2021
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one outstanding issue I see with this atm. The use of the GroupCallParticipants to display video elements is quite concerning to me. I find this solution quite ugly and not very scalable. I would very much prefer to stick to the current model of using CallFeeds and VideoFeed and AudioFeed components.

The are multiple reasons for this:

  • Using GroupCallParticipants feels like saying each participant is only sending one feed which isn't true
  • Using CallFeeds matches the current way of doing things and allows us to reuse already existing code
  • CallFeeds are screen-sharing friendly
  • CallFeeds are SFU friendly - with an SFU one GroupCallParticipant might be sending a bunch of streams, each one represented by a CallFeed (and corresponding components)

To be clear, I am not saying GroupCallParticipant should be removed or anything along those lines, I just think CallFeeds are better suited for being consumed by components for displaying video and playing audio

@robertlong robertlong marked this pull request as draft September 21, 2021 18:53
@SimonBrandner SimonBrandner removed the request for review from a team September 21, 2021 18:54
@robertlong
Copy link
Author

I've moved these changes into element-call itself now. This PR is no longer necessary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants