Skip to content

Conversation

@SimonBrandner
Copy link
Contributor

@SimonBrandner SimonBrandner commented Mar 29, 2021

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner changed the title MSC: Support for multi-stream VoIP MSC3077: Support for multi-stream VoIP Mar 29, 2021
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
@SimonBrandner SimonBrandner marked this pull request as ready for review March 29, 2021 18:03
@turt2live turt2live added kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal proposal-in-review labels Mar 29, 2021
SimonBrandner and others added 3 commits March 29, 2021 20:57
Co-authored-by: Tulir Asokan <tulir@maunium.net>
Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Copy link
Contributor

@ShadowJonathan ShadowJonathan left a comment

Choose a reason for hiding this comment

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

screenshare when 👀

Copy link
Contributor

@ShadowJonathan ShadowJonathan left a comment

Choose a reason for hiding this comment

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

Follow-up: I would suggest adding some more explanation to what m.usermedia and m.screenshare actually means in this (new?) context, to understand what this adds exactly, but otherwise, this looks good.

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
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.

otherwise this looks great - congrats on your first MSC :)

I would be tempted to skip the capability advertisement for this and just say that the absence of stream metadata means clients just take the first or whatever MSC2746 says. I can't think of anything the capability advertisement caters for specifically? - Dave

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>

+ `purpose` - a string indicating the purpose of the stream. For compatibility
between clients values `m.usermedia` and `m.screenshare` are defined.
`m.usermedia` is the stream that contains the webcam and microphone tracks.
Copy link
Member

Choose a reason for hiding this comment

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

continuing thread on whether we should identify front/rear facing cameras with this, for purposes like when the user rotates the camera and the same stream changes to a different camera (eg. FaceTime displays an animation on the remote side to show that the camera has changed). I feel like this is probably something for separate MSC, unless we want two separate purposes for front/rear camera.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, unless we want a separate purpose for that probably a separate MSC. If we wanted a separate purpose, would we have m.usermedia, m.usermedia.front and m.usermedia.rear or something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

eg. FaceTime displays an animation on the remote side to show that the camera has changed

This bit seems more like a stream replacement problem rather than a purpose problem. But... maybe we could allow sending both and set a main stream which would tell the client on the other side to do the animation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My suggestion would be adding something like camere_type to further distinguish the cameras - this way not all clients need to support this and we avoid having two purposes that are almost the same

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Jun 15, 2023
SimonBrandner and others added 5 commits June 17, 2023 18:28
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@mscbot
Copy link
Collaborator

mscbot commented Jun 20, 2023

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jun 20, 2023
@turt2live turt2live merged commit ddad150 into matrix-org:old_master Jun 20, 2023
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jun 20, 2023
@SimonBrandner SimonBrandner deleted the msc/sdp-metadata branch June 20, 2023 16:22
turt2live pushed a commit that referenced this pull request Jun 21, 2023
* Draft of multi-stream MSC

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Remove unnecessary article and don't use CamelCase

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Fix naming and MSC number

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Make it prefixy

Co-authored-by: Tulir Asokan <tulir@maunium.net>

* Be more descriptive about keys

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Add more info about usermedia and screenshare

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Simplifie backwards compatibility

I would be tempted to skip the capability advertisement for this and just say that the absence of stream metadata means clients just take the first or whatever MSC2746 says. I can't think of anything the capability advertisement caters for specifically? - Dave

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Reword parts of backwards compatibility section

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Improve explanation of backwards compatibility

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Add missing spaces to unstable perifix table

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Remove support for stream-replacement

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Apply suggestions from code review

Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>

* Fix concerns

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>

* Link to specific spec version

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Be more readable

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Clarify

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Don't ref non-existing thing

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Remove confusing words

---------

Signed-off-by: Šimon Brandner <simon.bra.ag@gmail.com>
Co-authored-by: Tulir Asokan <tulir@maunium.net>
Co-authored-by: David Baker <dbkr@users.noreply.github.com>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
dbkr added a commit to matrix-org/matrix-spec that referenced this pull request Jul 18, 2023
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1602

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Aug 1, 2023
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1735

@turt2live
Copy link
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:feature MSC for not-core and not-maintenance stuff merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal voip

Projects

Status: Merged
Status: Done for now

Development

Successfully merging this pull request may close these issues.

Support for multi-stream WebRTC calls (SPEC-252)