Add publish track messages. #1134
Conversation
|
| @@ -50,6 +50,8 @@ message Signalv2ClientMessage { | |||
|
|
|||
| oneof message { | |||
| ConnectRequest connect_request = 2; | |||
| RPC_AddAudioTrackRequest rpc_add_audio_track_request = 3; | |||
| RPC_AddVideoTrackRequest rpc_add_video_track_request = 4; | |||
There was a problem hiding this comment.
RPC prefix is just syntactic sugar to identify RPC style messages.
| @@ -77,6 +104,20 @@ message ConnectRequest { | |||
| // will overwrite if the same key is in the token | |||
| // will not delete keys from token if there is a key collision and this sets that key to empty value | |||
| map<string, string> participant_attributes = 4; | |||
|
|
|||
| repeated AudioTrack audio_tracks = 5; | |||
| repeated VideoTrack video_tracks = 6; | |||
There was a problem hiding this comment.
These tracks are added as part of ConnectRequest. Ideally, these should always be added and never queued. But, this differs from sending AddTrackRequest later. How should we think about it?
| } | ||
|
|
||
| message RPC_AddAudioTrackResponse { | ||
| RPC_AddAudioTrackRequest rpc_add_audio_track_request = 1; // request reflected back |
There was a problem hiding this comment.
Reflecting the entire request for convenience on sender side. They have full context in the reply.
| repeated SimulcastCodec simulcast_codecs = 3; | ||
| BackupCodecPolicy backup_codec_policy = 4; | ||
| string sid = 5; // used when publishing a new codec to an existing track | ||
| } |
There was a problem hiding this comment.
Splitting audio/video into separate messages in an attempt to make things cleaner, i. e. to have only relevant fields.
| message RPC_AddVideoTrackResponse { | ||
| RPC_AddVideoTrackRequest rpc_add_video_track_request = 1; // request reflected back | ||
| AddTrackState add_track_state = 2; | ||
| TrackInfo track_info = 3; |
There was a problem hiding this comment.
Although on the request side, splitting message definition by track type, keeping TrackInfo in the updates. But, an option is to make AudioTrackInfo and VideoTrackInfo, but that will be a massive change on server side if we were to keep two different types as a lot of code relies on TrackInfo.
However to ease the implementation effort, can keep TrackInfo internally and convert to AudioTrackInfo and VideoTrackInfo when sending updates. If having separate types would really help, can dig into it more.
|
|
||
| message AudioTrack { | ||
| TrackSettings track_settings = 1; | ||
| repeated AudioTrackFeature audio_features = 5; |
There was a problem hiding this comment.
How about keeping the AudioTrackFeature enum but changing this field to an int32, using its bits to represent feature flags? This would significantly reduce message size, which is especially important for subsequent participant updates which can happen frequently.
There was a problem hiding this comment.
Interesting idea @ladvoc .
It would not make a big difference in this as this is request sent by clients when they want to publish a track. But, it would be in TrackInfo which would be in participant updates. So, we will have to add the extra field there and deprecate the feature field over time (or leaving it out based on client protocol version)
The enums will be varint encoded. So, each feature could be a byte.
Just trying to think if there are gotchas to adding a layer of translation to protbuf.
@davidzhao @lukasIO @paulwe thoughts on this suggestion?
No description provided.