Backport changes to congestion control from libwebrtc #922
Conversation
|
Amazing. Just to clarify, this PR:
Is it correct? |
|
That is 100% correct. This is very WIP, but first commits that compiles. I will continue working on this. |
|
Wondering if we should mark the PR as draft to avoid wrong expectations for other readers. Not critical at all. |
|
Feel free to modify as needed, my laptop already died. Dunno when i ll
return:(
…On Mon, Oct 10, 2022, 12:51 Iñaki Baz Castillo ***@***.***> wrote:
Wondering if we should mark the PR as draft to avoid wrong expectations
for other readers. Not critical at all.
—
Reply to this email directly, view it on GitHub
<#922 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABD342YXRD2F3UVBGU5HN3DWCPRJXANCNFSM6AAAAAARBFD2XA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Converted to |
- Fix calculation of feedback min_pending_time in goog_cc - Fixes #849 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/d65dc979b17cdc7cd359aada59e5bce8a6f1b8ce%5E%21/ - Fix signed-to-unsigned overflow in send_side_bandwidth_estimation.cc - Fixes #872 - Issue in libwebrtc: https://bugs.chromium.org/p/webrtc/issues/detail?id=14272 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/9804aa5f6ad26a45338d685da66497c3bbd88ca6%5E%21/ NOTE: Some changes are already present in ongoing PR #922 but it's not yet merged.
- Fix calculation of feedback min_pending_time in goog_cc - Fixes #849 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/d65dc979b17cdc7cd359aada59e5bce8a6f1b8ce%5E%21/ - Fix signed-to-unsigned overflow in send_side_bandwidth_estimation.cc - Fixes #872 - Issue in libwebrtc: https://bugs.chromium.org/p/webrtc/issues/detail?id=14272 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/9804aa5f6ad26a45338d685da66497c3bbd88ca6%5E%21/ NOTE: Some changes are already present in ongoing PR #922 but it's not yet merged.
This reverts commit f01bd51.
# Conflicts: # worker/deps/libwebrtc/libwebrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc
…ave no traffic due to low reported estimated bitrate. Change min bitrate constant.
…ddenly have no traffic due to low reported estimated bitrate. Change min bitrate constant." This reverts commit 676b92e.
This reverts commit f498894.
…erefore stuck at low bitrate.
…Pacer, and therfore we should allow bigger bursts.
…Pacer, and therfore we should allow bigger bursts.
…unable to recover. Fix instant loss calculation at high bitrates. Increase ObservationDurationLowerBound.
…imit and decrease lower bound a bit to be able to reach higher bitrates.
|
Is there a timeline for when this PR can be merged into the mainline code? For the past 3 weeks, not had much activity, Is anything that I can be of any help with? |
|
@umeshc That is github showing wrong info, i was committing code yesterday, and will commit today. We are very actively testing everything to pick right params, don't worry, it will be merged as soon as we will be confident in stability and satisfied with overall results. |
…ant loss. Extract rtx bitrate from available outgoing bitrate when distributing.
|
As of right now, this is blocked by #989 |
- Fix calculation of feedback min_pending_time in goog_cc - Fixes versatica#849 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/d65dc979b17cdc7cd359aada59e5bce8a6f1b8ce%5E%21/ - Fix signed-to-unsigned overflow in send_side_bandwidth_estimation.cc - Fixes versatica#872 - Issue in libwebrtc: https://bugs.chromium.org/p/webrtc/issues/detail?id=14272 - Commit in libwebrtc: https://webrtc.googlesource.com/src/+/9804aa5f6ad26a45338d685da66497c3bbd88ca6%5E%21/ NOTE: Some changes are already present in ongoing PR versatica#922 but it's not yet merged.
|
May want to see this #1031 |
|
Do you have any update/plan for this PR? Thanks! |
We are in an offsite this week, let's come to this next week after Monday. |
So current state is on hold. This PR indeee modernized libwebrtc but it also comes with too many custom changes due to libwebrtc assumptions and hardcoded values, making it harder to maintain if we merge it. So we need to discuss how to proceed with this. Will update as soon as there is news. |
|
@sarumjanuch FYC Given how awesome (and risky) is this change I think it would be great if there is a simple doc / readme with this info:
That info would be great for people like me to understand the implications of the change and maybe could help to move it forward. Just an idea |
@ggarber, @vpalmisano and anyone interested. Here the report for all the research and changes @sarumjanuch did on this PR. https://teamaround.notion.site/BWE-research-report-4e3a371d04fd4b7889c64c0321fa950c Let's leave this PR as a space for any question or comment about it. |
This MR will track progress of sync libwebrtc congestion control in mediasoup to catchup with all the changes for the last 2 years, and most importantly- loss based estimator v2