-
Notifications
You must be signed in to change notification settings - Fork 508
[stable21] Fix connection quality warning shown due to stalled stats #5924
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
Merged
nickvergessen
merged 7 commits into
stable21
from
backport/5884-5885/stable21-fix-connection-quality-warning-shown-due-to-stalled-stats
Jul 2, 2021
Merged
[stable21] Fix connection quality warning shown due to stalled stats #5924
nickvergessen
merged 7 commits into
stable21
from
backport/5884-5885/stable21-fix-connection-quality-warning-shown-due-to-stalled-stats
Jul 2, 2021
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The logs are printed when the PeerConnectionAnalyzer changes to a state that causes a connection quality warning to be shown (very bad quality or not transmitted data). Currently the connection quality is analyzed only when the HPB is used and only for the sender participant, so there will be at most a single PeerConnectionAnalyzer. Due to this, for simplicity, for now the stats are logged without any participant identifier. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The stats were reset whenever "setAnalysisEnabledXXX(true)" was called. Due to this, if the analysis for audio or video was already enabled and the method was called again the stats were wrongly reset. The stats should be reset only when the analysis is really started again. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
This makes the attribute consistent with the stats, and will also make
possible to directly get the quality value given a string identifying
its kind ("audio" or "video").
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When no packets are transmitted the packet lost ratio is set to a value higher than 1 to move towards a "no transmitted data" state faster, although not immediately. Both the "no transmitted data" and "very bad quality" states are based on the packet lost ratio, so this causes the "very bad quality" state to be triggered as soon as no packets are transmitted. However, the stats reported by the browser can sometimes stall for a second (it is unclear whether the browser does not update the stats or Janus does not send them, though). When that happens the stats are still reported, but with the same values as in the previous report. As there were no transmitted packets the "very bad quality" state was (wrongly) triggered. To solve this now if there were no transmitted packets in the last report the analysis is kept on hold. If in the next report the number of packets has changed then the previous report is considered to have stalled and the new values are distributed between the previous and current report. If the number of packets is still zero then the previous report is considered to have been legit and the previous and current values are added as normal. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
PVince81
approved these changes
Jul 2, 2021
Member
PVince81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #5884 and #5885
Tested and works 👍