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

Modify SimulcastConsumer to keep using layers without good scores #804

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

ggarber
Copy link
Contributor

@ggarber ggarber commented Mar 30, 2022

We have cases were some simulcast consumers get all the video layers disabled very often when the producers have a high amount of packet loss. In those cases we see up to 5-6 transitions per minute from "no layers" <-> "first layer" and we end up with frozen video or black video depending on when it happens.

I think mediasoup doesn't need to have the logic to estimate the quality of the producer streams because of these reasons:

  • In many use cases it can be better to have bad quality video that no video at all.
  • The current logic is creating many transitions (5-6 constantly per minute in some of our user reports) that makes the experience not very stable.
  • With this change an app listening for the 'layerschange' event from the consumer will know that if layers have changed is because of the bandwidth estimation available instead of for a combination of multiple factors (bandwidth estimation and score of the producer). So it is easier to present that information in the UI.
  • The current logic can be implemented by the app checking the producers scores if it is important.
  • This way the logic is simpler.
  • I haven't seen this logic in other SFUs like Janus/Signal/Jitsi.... (I could be wrong and it is not a strong reason per se, but just fyc).

If it is better to open this as an issue or forum post instead of a PR let me know. Thx for taking a look.

Copy link
Member

@jmillan jmillan left a comment

Choose a reason for hiding this comment

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

LGTM.

Makes sense to me.

@ibc
Copy link
Member

ibc commented Apr 21, 2022

I assume this should be also changed in SvcConsumer, right?

@ggarber
Copy link
Contributor Author

ggarber commented Apr 26, 2022

@ibc This logic was only available in SimulcastConsumer and not SvcConsumer. Probably another reason apart from the ones mentioned in the description to remove it from SimulcastConsumer.

@ibc
Copy link
Member

ibc commented Apr 26, 2022

I think this is ok. Will merge and release today.

@ibc ibc merged commit ef0e2b7 into versatica:v3 Apr 26, 2022
@ibc
Copy link
Member

ibc commented Apr 27, 2022

Published in 3.9.11

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.

3 participants