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

Properly handle ksvc bandwidth allocation #1036

Merged

Conversation

vpalmisano
Copy link
Contributor

When we use VP9 with K-SVC configuration, the spatial layers are independent; this means that the bandwidth allocator, when it tries to increase the allocated layer, should not take into account the lower spatial levels bitrates (same behaviour of the Simulcast case).

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

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

Are you sure we don't need similar logic in ApplyBitrate() method?

@vpalmisano
Copy link
Contributor Author

Are you sure we don't need similar logic in ApplyBitrate() method?

Do you mean taking into account that logic when we pass maxBitrate to the congestion controller?

@ibc
Copy link
Member

ibc commented Mar 28, 2023

Nope, I mean another thing but maybe we changed it long ago and what I said no longer makes sense. Will recheck tomorrow.

@ibc
Copy link
Member

ibc commented Mar 29, 2023

Checking this.

@ibc
Copy link
Member

ibc commented Mar 29, 2023

Checking it later today.

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

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

Ok @vpalmisano, I found the missing thing:

  • See SimulcastConsumer::GetDesiredBitrate(). It returns the highest bitrate of all its streams (or this->rtpParameters.encodings[0].maxBitrate if given).
  • However SvcConsumer::GetDesiredBitrate() returns the total bitrate of the whole SVC or K-SVC stream. If K-SVC we should just return the bitrate of the spatial layer that produces the most bitrate, right?

NOTE: Consumer::GetDesiredBitrate() is used by Transport::ComputeOutgoingDesiredBitrate() method.

@vpalmisano
Copy link
Contributor Author

  • If K-SVC we should just return the bitrate of the spatial layer that produces more bitrate, right?

Correct, I'm taking a look.

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

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

Will review tomorrow. Thanks.

worker/src/RTC/SvcConsumer.cpp Outdated Show resolved Hide resolved
worker/src/RTC/SvcConsumer.cpp Show resolved Hide resolved
@ibc ibc merged commit eafca69 into versatica:v3 Mar 30, 2023
@ibc
Copy link
Member

ibc commented Mar 30, 2023

Releasing in 3.11.19 right now.

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

Successfully merging this pull request may close these issues.

2 participants