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

Dont send video optimization - proto side #162

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

mkamonMdt
Copy link
Contributor

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

  • Media type split to Optional (video, screen share), Mandatory (Audio), Management (new type that will be used for notifications and subscription control)
  • Dedicated Packet for MediaManagement that allow to communicate subscription commands from user to server and notification about ongoing stream that a user can subscribe to.

Following the above PR, the next steps will cover:

  • Client's side "AutoSubscribe" handling: in order not to break functionality, Clients will auto subscribe to each new peer
  • Server's side subscription handling: full implementation of "on-demand" media transfer and subscription handling and "ONGOING_STREAM" notifications
  • Client's side handling of "ONGOING_STREAM" notification.
  • Client's side "Pin Peer" capability: enabling User decision to select Peers to be followed

mkamonMdt and others added 3 commits July 9, 2024 22:36
The commit splits PacketType::MEDIA into following three types:
- MEDIA_MANDATORY: all media that will always be send to all connected
    peers in a room
- MEDIA_OPTIONAL: the type that the Server will use to perform
    subscribtion-based Recipeint selection
- MEDIA_MANAGEMENT: the type that will indicate to the Server that the
    Server is target Recipient of the message
In order to provide a mechanism for managing subscribtions there is
a need an user and the server to exchange following set of messages:
- SUBSCRIBE: request of optional peer data, send from an user to
the server
- UNSUBSCRIBE: information that a users is no longer interested in
specific peer
- ONGOING_STREAM: information from the server to an unsubscirbed
user that there is a peer that streams video or shares screen.
@darioalessandro
Copy link
Member

darioalessandro commented Jul 11, 2024

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

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

@mkamonMdt
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

  • A Producer sends a Optional media content (VIDEO or SCREEN_SHARE) that server forwards only to users that subscribed for the Producer.
  • Since unsubscribed users cannot know that there is ongoing Media stream, the Producer must provide lightweight notifications as long as the stream continuous.
  • The notification can be sent in lesser frequency than Optional Media Packets themself.
    optional_media_general_flow

@mkamonMdt
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

darioalessandro commented Jul 26, 2024

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
Copy link
Contributor Author

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.

@darioalessandro
Copy link
Member

@mkamonMdt thank you again for your awesome diagrams!

I do feel strongly that the protobuf contract shouldn't change.

How can we meet halfway?

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

Successfully merging this pull request may close these issues.

2 participants