security-union / videocall-rs

teleconference system written in rust
https://www.videocall.rs
MIT License
1.38k stars 119 forks source link

Dont send video optimization - proto side #162

Open mkamonMdt opened 2 months ago

mkamonMdt commented 2 months ago

The PR provides first part of implementation for #43 with an extension of .proto files to distinguish:

Following the above PR, the next steps will cover:

darioalessandro commented 2 months ago

@mkamonMdt why is audio a 'mandatory' media type?

also, would you kindly share a sequence diagram with how subscription should work?

mkamonMdt commented 2 months ago

@mkamonMdt why is audio a 'mandatory' media type?

So the Optionality comes from the use case when a user cannot follow more video/screen-share streams than the canvas limit (which can also be specified by a user e.g. changing layout to 2x2 or 5x2 etc) or wants to "Pin" (few) selected peers, So it is End User decision.

The audio (and maybe chat in the future) is kind of different nature. Usually a user cannot make decision which peers it wants to listen to. In some cases there is a special User(s) like host that have supernatural power of giving voice (or receiving all chat messages while other users will not all - e.g. gathering content for Q&A session during a meeting). In that regard, from the perspective of regular meeting participant the audio channel feels mandatory as it's Management is determined by a remote entity.

also, would you kindly share a sequence diagram with how subscription should work?

Sure, good idea. I will prepare a diagram for it :)

mkamonMdt commented 2 months ago

The basic idea behind the Optional Media Packet is presented on the following sequence diagram. In summary:

mkamonMdt commented 2 months ago

The subscription process in its first iteration will realize "AutoSubscribe" strategy (i.e. default mechanism that will be used until a User manually provides it's preferences -> clicks on "Pin to Peer" button), that is it will subscribe to any Peer whose ONGOING_STREAM notification it receives (within CANVAS_LIMIT).

autosubscribe_flow

mkamonMdt commented 2 months ago

Unsubscribe mechanism will start at removing a Peer from SubscribedPeerList. The communication to the Server will be triggered by reception of OptionalMedia Packet from a Peer that User is no longer subscribed to

unsubscribe_general_flow

darioalessandro commented 2 months ago

Hey @mkamonMdt:

First of all thank you for the comprehensive explanation and the clear diagrams, it really helps to understand the feature.

I do want to keep the media signaling as simple as possible, to this effect I propose the following:

  1. I would like to accept the addition of MEDIA_MANAGEMENT for peers to subscribe to peers media and to indicate that there's an ongoing stream, I think these are great ideas!!
  2. Regarding optional media, I do feel a bit anxious about it because they introduce a layer of complexity, but seems like we do need it.
mkamonMdt commented 2 months ago

Hey @mkamonMdt:

First of all thank you for the comprehensive explanation and the clear diagrams, it really helps to understand the feature.

I do want to keep the media signaling as simple as possible, to this effect I propose the following:

1. I would like to accept the addition of `MEDIA_MANAGEMENT` for peers to subscribe to peers media and to indicate that there's an ongoing stream, I think these are great ideas!!

Thanks :)

2. Regarding optional media, I do feel a bit anxious about it because they introduce a layer of complexity, but seems like we do need it.

The problem with MEDIA was that the server needs to distribute OPTIONAL packet according to "subscriptions" while letting MANDATORY packets like AUDIO or HEARTBEAT as it was. In general only thing that the server knows about the content of PacketWrapper is PacketType and source user as the data might be encrypted (and we would like to avoid parsing 'data' anyway I guess). Similar solution would be to lift up MediaType to PakcetType level.

Other solution could involve different Actix endpoint, for Packets of different kind.