Fix connection quality stats not reset when setting a new peer connection #5751
Conversation
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
2. developing
bug
feature: frontend 🖌️
|
/backport to stable21.1 |
|
/backport to stable21 |
|
/backport to stable20 |
PVince81
left a comment
PVince81
left a comment
There was a problem hiding this comment.
Code makes sense. What's missing ?
The stats needs to be reset when a peer connection starts. The stats from the previous peer connection are unrelated to the new one, and that will cause a wrong analysis until the stats are filled again with values from the new peer connection. Similarly, the stats should be reset too when a peer connection is restarted. In this case the reported stats continue from the last values. However, during the reconnection the stats will not be updated, so the timestamps will suddenly increase once the connection is ready again, which could cause a wrong analysis. Due to all that now the stats are reset before the analysis is started, as the analysis is started only when the peer connection is started or restarted. Note that right now the sender connections with the HPB are never restarted; a new peer connection is always established. Nevertheless, this approach should be more "future proof". Signed-off-by: Daniel Calviño Sánchez <[email protected]>
This is just for tidyness, as stats for screen streams only contain data for video, and thus in practice this change makes no difference. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
fix-connection-quality-stats-not-reset-when-setting-a-new-peer-connection
branch
from
e427df4 to
df63caa
Compare
As stated in the pull request description I needed to check if there was a better place to reset the stats (there was) and check if it also worked as expected when switching devices (it will be addressed in a different pull request if needed). Now it is ready for review ;-) |
My internal task parser doesn't work on plain text 😅 |
PVince81
deleted the
fix-connection-quality-stats-not-reset-when-setting-a-new-peer-connection
branch
|
/backport to stable20.1 |
Still in draft; I need to check if there is a better place to reset the stats (for example, when the ICE connection state changes), and check if it also works as expected when switching devices.- It is better to reset the stats just before the analysis is started.Regarding switching devices I found a different issue related to stats being reset when they should not (when it is sent to other participants whether audio and video is enabled or disabled), so this needs to be fixed first before being able to check switching devices. Anyway, this will be done in another pull request.
How to test
Result with this pull request
The connection quality warning will not be shown.
Result without this pull request
The connection quality warning will be shown.