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

Give CallFeed the capability to emit on volume changes #1865

Merged
merged 5 commits into from
Aug 25, 2021

Conversation

SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Aug 19, 2021

Type: enhancement
For matrix-org/matrix-react-sdk#6639


To be transparent, I have no idea what I am doing but it seems to work 😅


Here's what your changelog entry will look like:

✨ Features

  • Give CallFeed the capability to emit on volume changes (#1865). Contributed by SimonBrandner.

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner added T-Enhancement and removed T-Task Tasks for the team like planning labels Aug 19, 2021
@SimonBrandner SimonBrandner marked this pull request as ready for review August 19, 2021 10:20
@SimonBrandner SimonBrandner requested a review from a team as a code owner August 19, 2021 10:20
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Nice! It might be nice to actually shift more of this towards the js-sdk side and have it emit active-speaker events so the js-sdk handles the threshold etc rather than just throwing volume events at the app. Also unsure if we'll want to highlight all people who are actively speaking or just decide which person is the active speaker (like jitsi does). In a way I sort of like highlighting all the people we think are speaking, but probably a product call.

@SimonBrandner
Copy link
Contributor Author

It might be nice to actually shift more of this towards the js-sdk side and have it emit active-speaker events so the js-sdk handles the threshold etc rather than just throwing volume events at the app.

It was intentional in case some other app building on the js-sdk can access the volume directly but we can emit both volume_changed and speaking speaking and solve the problem that way

Also unsure if we'll want to highlight all people who are actively speaking or just decide which person is the active speaker (like jitsi does). In a way I sort of like highlighting all the people we think are speaking, but probably a product call.

Agreed, I would also prefer highlighting all speaking users. Guessing the main speaker feels a bit useless now and it doesn't feel like an important thing to have. We can always add it in the future. Figma also seems to agree: When a user speaks, its videofeed should be highlighted in Accent colour

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@dbkr dbkr merged commit 4f21b95 into matrix-org:develop Aug 25, 2021
@SimonBrandner SimonBrandner deleted the feature/voice-activity branch August 25, 2021 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants