Backport changes to congestion control from libwebrtc #922
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,11 @@ | |
| * be found in the AUTHORS file in the root of the source tree. | ||
| */ | ||
|
|
||
| #include "absl/strings/match.h" | ||
| #include "modules/bitrate_controller/loss_based_bandwidth_estimation.h" | ||
| #include "api/transport/webrtc_key_value_config.h" | ||
| #include "api/units/data_rate.h" | ||
| #include "api/units/time_delta.h" | ||
|
|
||
| #include <algorithm> | ||
| #include <string> | ||
|
|
@@ -21,6 +22,10 @@ namespace webrtc { | |
| namespace { | ||
| const char kBweLossBasedControl[] = "WebRTC-Bwe-LossBasedControl"; | ||
|
|
||
| // Expecting RTCP feedback to be sent with roughly 1s intervals, a 5s gap | ||
| // indicates a channel outage. | ||
| constexpr TimeDelta kMaxRtcpFeedbackInterval = TimeDelta::Millis<5000>(); | ||
|
|
||
| // Increase slower when RTT is high. | ||
| double GetIncreaseFactor(const LossBasedControlConfig& config, TimeDelta rtt) { | ||
| // Clamp the RTT | ||
|
|
@@ -70,10 +75,16 @@ double ExponentialUpdate(TimeDelta window, TimeDelta interval) { | |
| return 1.0f - exp(interval / window * -1.0); | ||
| } | ||
|
|
||
| bool IsEnabled(const WebRtcKeyValueConfig* key_value_config, | ||
| absl::string_view name) { | ||
| return key_value_config->Lookup(name).find("Enabled") == 0; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| LossBasedControlConfig::LossBasedControlConfig( | ||
| const WebRtcKeyValueConfig* key_value_config) | ||
| : enabled(IsEnabled(key_value_config, kBweLossBasedControl)), | ||
| min_increase_factor("min_incr", 1.02), | ||
| max_increase_factor("max_incr", 1.08), | ||
| increase_low_rtt("incr_low_rtt", TimeDelta::ms(200)), | ||
|
|
@@ -85,26 +96,27 @@ LossBasedControlConfig::LossBasedControlConfig() | |
| increase_offset("incr_offset", DataRate::bps(1000)), | ||
| loss_bandwidth_balance_increase("balance_incr", DataRate::kbps(0.5)), | ||
| loss_bandwidth_balance_decrease("balance_decr", DataRate::kbps(4)), | ||
| loss_bandwidth_balance_reset("balance_reset", DataRate::kbps(0.1)), | ||
| loss_bandwidth_balance_exponent("exponent", 0.5), | ||
| allow_resets("resets", false), | ||
| decrease_interval("decr_intvl", TimeDelta::ms(300)), | ||
| loss_report_timeout("timeout", TimeDelta::ms(6000)) { | ||
| ParseFieldTrial( | ||
| {&min_increase_factor, &max_increase_factor, &increase_low_rtt, | ||
| &increase_high_rtt, &decrease_factor, &loss_window, &loss_max_window, | ||
| &acknowledged_rate_max_window, &increase_offset, | ||
| &loss_bandwidth_balance_increase, &loss_bandwidth_balance_decrease, | ||
| &loss_bandwidth_balance_reset, &loss_bandwidth_balance_exponent, | ||
| &allow_resets, &decrease_interval, &loss_report_timeout}, | ||
| key_value_config->Lookup(kBweLossBasedControl)); | ||
| } | ||
| LossBasedControlConfig::LossBasedControlConfig(const LossBasedControlConfig&) = | ||
| default; | ||
| LossBasedControlConfig::~LossBasedControlConfig() = default; | ||
|
|
||
| LossBasedBandwidthEstimation::LossBasedBandwidthEstimation( | ||
| const WebRtcKeyValueConfig* key_value_config) | ||
| : config_(key_value_config), | ||
| average_loss_(0), | ||
| average_loss_max_(0), | ||
| loss_based_bitrate_(DataRate::Zero()), | ||
|
|
@@ -161,9 +173,14 @@ void LossBasedBandwidthEstimation::UpdateAcknowledgedBitrate( | |
| } | ||
| } | ||
|
|
||
| DataRate LossBasedBandwidthEstimation::Update(Timestamp at_time, | ||
| DataRate min_bitrate, | ||
| DataRate wanted_bitrate, | ||
| TimeDelta last_round_trip_time) { | ||
| if (loss_based_bitrate_.IsZero()) { | ||
| loss_based_bitrate_ = wanted_bitrate; | ||
| } | ||
|
|
||
| // Only increase if loss has been low for some time. | ||
| const double loss_estimate_for_increase = average_loss_max_; | ||
| // Avoid multiple decreases from averaging over one loss spike. | ||
|
|
@@ -173,8 +190,15 @@ void LossBasedBandwidthEstimation::Update(Timestamp at_time, | |
| !has_decreased_since_last_loss_report_ && | ||
| (at_time - time_last_decrease_ >= | ||
| last_round_trip_time + config_.decrease_interval); | ||
| // If packet lost reports are too old, dont increase bitrate. | ||
| const bool loss_report_valid = | ||
| at_time - last_loss_packet_report_ < 1.2 * kMaxRtcpFeedbackInterval; | ||
|
There was a problem hiding this comment. how the RTCP feedback calculated in mediaosup? If I remember RFC3550 calculate based on assumed bw. If we calculate based on that assumption, and the bw suddenly drop because of congestion collapse than the interval will increase massively which makes this loss_report_valid false. at this moment the interpretation collides with the behaviour of the rest of the code, so right now it does not cause any problem and only beauty suggestion is to add a debug log if loss_report_valid is false, so if we search something related to it in the future we might have a chance to find it easier.
Contributor
Author
There was a problem hiding this comment. This is old attempt of loss based BW estimate. It is disabled by default and is not used. Can be turned on by field trial. So we have:
There was a problem hiding this comment. Okay, so I am lost in the code a bit. Does this mean it won't have any effect because another estimator is used after this PR? In this case should this code marked somehow as deprecated? |
||
|
|
||
| if (loss_report_valid && config_.allow_resets && | ||
| loss_estimate_for_increase < loss_reset_threshold()) { | ||
| loss_based_bitrate_ = wanted_bitrate; | ||
| } else if (loss_report_valid && | ||
| loss_estimate_for_increase < loss_increase_threshold()) { | ||
| // Increase bitrate by RTT-adaptive ratio. | ||
| DataRate new_increased_bitrate = | ||
| min_bitrate * GetIncreaseFactor(config_, last_round_trip_time) + | ||
|
|
@@ -200,14 +224,21 @@ void LossBasedBandwidthEstimation::Update(Timestamp at_time, | |
| loss_based_bitrate_ = new_decreased_bitrate; | ||
| } | ||
| } | ||
| return loss_based_bitrate_; | ||
| } | ||
|
|
||
| void LossBasedBandwidthEstimation::Initialize(DataRate bitrate) { | ||
| loss_based_bitrate_ = bitrate; | ||
| average_loss_ = 0; | ||
| average_loss_max_ = 0; | ||
| } | ||
|
|
||
| double LossBasedBandwidthEstimation::loss_reset_threshold() const { | ||
| return LossFromBitrate(loss_based_bitrate_, | ||
| config_.loss_bandwidth_balance_reset, | ||
| config_.loss_bandwidth_balance_exponent); | ||
| } | ||
|
|
||
| double LossBasedBandwidthEstimation::loss_increase_threshold() const { | ||
| return LossFromBitrate(loss_based_bitrate_, | ||
| config_.loss_bandwidth_balance_increase, | ||
|
|
@@ -223,14 +254,4 @@ double LossBasedBandwidthEstimation::loss_decrease_threshold() const { | |
| DataRate LossBasedBandwidthEstimation::decreased_bitrate() const { | ||
| return config_.decrease_factor * acknowledged_bitrate_max_; | ||
| } | ||
| } // namespace webrtc | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.