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

Auto switch simulcast layer if REMB is too low #2735

Closed

Conversation

sjkummer
Copy link
Contributor

Prevent congestion by automatically switchting a subscribers simulcast layer if its REMB (Receiver Estimated Maximum Bitrate) is too low.

Can be configured with min_rec_bitrate_simulcast setting in the videoroom plugin (default=256Kb/s).

Based on abs-send-time branch: #2721

Might need to be further improved and become more dynamic (?) @lminiero

@@ -54,6 +54,13 @@ general: {
# By default, integers are used as a unique ID for both rooms and participants.
# In case you want to use strings instead (e.g., a UUID), set string_ids to true.
#string_ids = true

#min_rec_bitrate_simulcast=0 # The minimum bitrate required to subscribe higher
Copy link
Contributor

Choose a reason for hiding this comment

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

don't you need two values here, one as the minimum bitrate for the low spatial layer and a second one for the mid one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also see at least two but i would say all three values might even make more sense.
If we do not reach the lowest bitrate janus could also drop the fps (but i would make that in a second task)
Having the bitrates on hand alows further improvement in the future.

e.g.
min_required_bitrate_vp8_sl_0: 150000
min_required_bitrate_vp8_sl_1: 500000
min_required_bitrate_vp8_sl_2: 1200000

Copy link
Member

Choose a reason for hiding this comment

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

This sounds overly specific: I don't want anything codec specific in there. Besides, I don't think it's a good idea to have generic room settings here, especially if the idea is to match the simulcast targets of publishers, who may be configured differently even when in the same room.

Copy link
Contributor

Choose a reason for hiding this comment

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

The question is how do you want janus to select the proper layer on the subscriber side if the plugin handle is not aware about the publisher configured bitrates for the layers? You don´t have them in janus as the publisher client has set them on the client side. (I also don´t see them in the general settings, i see them in an api call on the subscriber side)

Copy link
Member

Choose a reason for hiding this comment

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

What sounds more interesting is tracking the bitrate for the different substreams for all publishers, and use those as triggers, which would make it more dynamic as well (even though more complex, as we'd have to make sure substreams that are just starting don't confuse the logic, e.g., because the high substream temporarily results with a lower bitrate than the medium/low one).

Copy link
Member

Choose a reason for hiding this comment

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

You don´t have them in janus as the publisher client has set them on the client side

We never needed to know them. Having the publisher optionally pass the settings they used via API sounds viable too, even though not necessarily reliable, since those are targets and not the actual bitrate in use.

Copy link
Contributor

@JanFellner JanFellner Jul 13, 2021

Choose a reason for hiding this comment

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

We never needed to know them.

For the publisher side yeah, currently the browser is handling the layer sending according to available bitrates so no need to know them inside janus. For the subscriber side i think its the easist to start with an api that allows to set them dynamic and see if it works in real live (but yes you are totally right - having them in sync with the publisher side would make the most sense but is from the current perspective way more effort)
We will sync tomorrow @fippo - @sjkummer and me (you are always invited :D)

@lminiero
Copy link
Member

Please set the abs-send-time branch as the basis for the PR, and not master: as it is, it's showing the changes from that other PR too.

@sjkummer sjkummer changed the base branch from master to abs-send-time July 13, 2021 18:51
@sjkummer sjkummer marked this pull request as draft July 15, 2021 05:36
@sjkummer
Copy link
Contributor Author

Seems like we all agree that getting bitrates from the publisher is preferred over static configuration at server side.

Marking this PR as draft for now.

@lminiero lminiero deleted the branch meetecho:abs-send-time July 28, 2021 10:31
@lminiero lminiero closed this Jul 28, 2021
@lminiero
Copy link
Member

Whoops sorry, didn't mean to automatically close this: it must have happened after I deleted the branch since I merged it. You can try rebasing it to master.

@sjkummer
Copy link
Contributor Author

@lminiero No worries, I‘ve prepared something which will use the bitrates from the publisher in another brach and will create a new PR after internal testing has completed.

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.

None yet

4 participants