Upgrade trendline estimator to improve low bandwidth conditions #1055
Conversation
|
I have to review this yet but something that worries me is that we have an ongoing branch that updates libwebrtc (plus many tweaks) and this effort in current super old libwebrtc version goes in the other direction. Should these changes also apply to the other ongoing PR? |
Probably yes, @ggarber used the latest version of trendline_estimator.cc |
Let me rephrase my question since it was wrong: Should these changes also BE APPLIED into the other ongoing PR or are those already included in modern libwebrtc version? |
If they are not already included in that other branch then yes, they should imo. The way I see it is, eventually that other branch will have enough testing from many ppl in different network conditions and be stable enough but it is risky and potentially can take time until we have confidence in it. In the mean time there are issues that look easy and important enough so imo we should do them in parallel. Also all the testing infrastructure we have now can be reused to test the newer version in the future so this effort is also providing that value. |
|
Sure. I'm just worried about introducing changes in this old version that later forget to apply to the newer one (in case those changes are not built in). |
Some of the changes applied to the libwebrtc code are related to the logging macros. Probably we could use some shim macros/functions in order to keep those macros and limit the changes made to the original code. |
The existing bwe implementation tends to generate a too high estimation in networks with bandwidth limited. This fact creates congestion and packet loss that decrease the bandwidth estimation. That process is repeated constantly creating a saw shaped bandwidth estimation with instable quality and packets lost.
After some debugging we narrowed the issue to the trendline estimator and discovered that with the latest version from libwebrtc (where there are some parameters changed and also some more advanced calculations) the results are MUUUUUCH better.
This is the result of one of the tests with bandwidth limited to 500kbps and 3 simulcast consumers. The first half of the graphs is the result with current algorithm and the second half is the result with this PR:
Kudos to @vpalmisano for the help debugging and testing and generating those amazing graphs.
For the PR I have disabled the Field Trials support in the trendline_estimator class because in the new version they use new functions not available in the mediasoup version of libwebrtc. With some effort it could be ported if somebody needs them.
PD We are also able to reproduce these issues and the improvements consistently with some unit tests using a new network emulation framework but that is not ready to be shared yet.