-
Notifications
You must be signed in to change notification settings - Fork 423
MSC3291: Muting in VoIP calls #3291
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
78bfa25
MSC: Muting in VoIP calls
SimonBrandner a3d1abc
Update MSC number
SimonBrandner 57a6330
Missing :
SimonBrandner 7658c4a
Fix MSC number in prefix
SimonBrandner 2130df9
Fix unstable prefix table
SimonBrandner 3a9701d
Reword pottential issues
SimonBrandner d809c3b
Add an Alternatives section
SimonBrandner 67168b3
Remove trailing comma
SimonBrandner ec24d95
Fix a typo
SimonBrandner 0c01908
Fix missing word
SimonBrandner cc8b73a
Link to MSC for holding
SimonBrandner f1ac61f
Update unstable prefixes
SimonBrandner 29e3c6d
Simplify things
SimonBrandner 5372b7f
Be explicit about deps
SimonBrandner 00da3d3
Be clearer about how things work
SimonBrandner fb8f957
Update proposals/3291-muting.md
SimonBrandner c97f650
`disabled` -> `enabled`
SimonBrandner 1aa2cd4
Fix client mention
SimonBrandner dec84a8
Improve explaination
SimonBrandner 56d3541
Be more precise
SimonBrandner 8905378
Fix negation
SimonBrandner 4da6f86
Don't refer to something that doesn't exist
SimonBrandner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Link to MSC for holding
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
- Loading branch information
commit cc8b73ae846f590d4a3f09ed1586397e9ea39d0a
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure we don't want to go for optimal bandwidth usage? I personally often turn off video to have more bandwidth available for audio when the quality is terrible. Feels unfortunate to exclude this. Also, this doesn't feel much harder to implement.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this further, why not remove the track entirely and send a
m.call.negotiateevent with the metadata update rather than just set the direction of the transceiver? It's kind of annoying that the webcam light doesn't turn off when you mute your video.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be the choice of the client. In general not sending the video when video is off makes sense. The extra latency of starting up the stream is usually quite small and the bandwidth savings is significant which can make a huge difference for someone on a connection that struggle to send audio and video but can support audio reliably. Turning off the camera is less often used because actually getting a new stream from the camera can have very high and unpredictable latency depending on the hardware or the OS. Probably a good client will have a variety of heuristics to decide how far to shut down the camera but the key thing from the spec point of view that completely turning off the camera is possible if desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I stand against going with both options at the same time - let the client choose. @dbkr, would you agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, we do this now: matrix-org/matrix-js-sdk#3028
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I believe Enrico recently discovered with his experiments that browsers (at least some of them) now send nothing when a video track is disabled, so the bandwidth argument is even less relevant. Turning cameras off is orthogonal and can be done whether or not you remove the video track. The negotiate mechanism still exists either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this documented anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the MSC need to be updated given the conclusions drawn here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to assume not and resolve this thread. Please re-open if you believe otherwise.