Cleanup of target rates in GoogCC/SendSideBandwidthEstimation.
Removing the redundant last_estimated_bitrate_bps_ and renaming some
members to better reflect the contents. Also replacing the CurrentEstimate
method of SendSideBandwidthEstimation with value specific access methods.
Bug: webrtc:9883
Change-Id: I73cb08e09374adddf5991cb3793fa4a4fee20c85
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154351
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29304}
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
index 78b1236..850673e 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc
@@ -96,8 +96,8 @@
acknowledged_bitrate_estimator_(
std::make_unique<AcknowledgedBitrateEstimator>(key_value_config_)),
initial_config_(config),
- last_raw_target_rate_(*config.constraints.starting_rate),
- last_pushback_target_rate_(last_raw_target_rate_),
+ last_loss_based_target_rate_(*config.constraints.starting_rate),
+ last_pushback_target_rate_(last_loss_based_target_rate_),
pacing_factor_(config.stream_based_config.pacing_factor.value_or(
kDefaultPaceMultiplier)),
min_total_allocated_bitrate_(
@@ -132,12 +132,7 @@
if (!estimated_bitrate)
estimated_bitrate = acknowledged_bitrate_estimator_->PeekRate();
} else {
- int32_t target_bitrate_bps;
- uint8_t fraction_loss;
- int64_t rtt_ms;
- bandwidth_estimation_->CurrentEstimate(&target_bitrate_bps,
- &fraction_loss, &rtt_ms);
- estimated_bitrate = DataRate::bps(target_bitrate_bps);
+ estimated_bitrate = bandwidth_estimation_->target_rate();
}
if (estimated_bitrate) {
if (msg.constraints.starting_rate) {
@@ -392,7 +387,7 @@
time_window += time_since_last_packet;
}
- DataSize data_window = last_raw_target_rate_ * time_window;
+ DataSize data_window = last_loss_based_target_rate_ * time_window;
if (current_data_window_) {
data_window =
std::max(kMinCwnd, (data_window + current_data_window_.value()) / 2);
@@ -568,18 +563,18 @@
NetworkControlUpdate GoogCcNetworkController::GetNetworkState(
Timestamp at_time) const {
- TimeDelta rtt = TimeDelta::ms(last_estimated_rtt_ms_);
NetworkControlUpdate update;
update.target_rate = TargetTransferRate();
update.target_rate->network_estimate.at_time = at_time;
update.target_rate->network_estimate.loss_rate_ratio =
- last_estimated_fraction_loss_ / 255.0;
- update.target_rate->network_estimate.round_trip_time = rtt;
+ last_estimated_fraction_loss_.value_or(0) / 255.0;
+ update.target_rate->network_estimate.round_trip_time =
+ last_estimated_round_trip_time_;
update.target_rate->network_estimate.bwe_period =
delay_based_bwe_->GetExpectedBwePeriod();
update.target_rate->at_time = at_time;
- update.target_rate->target_rate = last_raw_target_rate_;
+ update.target_rate->target_rate = last_pushback_target_rate_;
update.target_rate->stable_target_rate =
bandwidth_estimation_->GetEstimatedLinkCapacity();
update.pacer_config = GetPacingRates(at_time);
@@ -590,64 +585,61 @@
void GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
NetworkControlUpdate* update,
Timestamp at_time) {
- int32_t estimated_bitrate_bps;
- uint8_t fraction_loss;
- int64_t rtt_ms;
- bandwidth_estimation_->CurrentEstimate(&estimated_bitrate_bps, &fraction_loss,
- &rtt_ms);
+ uint8_t fraction_loss = bandwidth_estimation_->fraction_loss();
+ TimeDelta round_trip_time = bandwidth_estimation_->round_trip_time();
+ DataRate loss_based_target_rate = bandwidth_estimation_->target_rate();
+ DataRate pushback_target_rate = loss_based_target_rate;
BWE_TEST_LOGGING_PLOT(1, "fraction_loss_%", at_time.ms(),
(fraction_loss * 100) / 256);
- BWE_TEST_LOGGING_PLOT(1, "rtt_ms", at_time.ms(), rtt_ms);
+ BWE_TEST_LOGGING_PLOT(1, "rtt_ms", at_time.ms(), round_trip_time.ms());
BWE_TEST_LOGGING_PLOT(1, "Target_bitrate_kbps", at_time.ms(),
- estimated_bitrate_bps / 1000);
+ loss_based_target_rate.kbps());
- DataRate target_rate = DataRate::bps(estimated_bitrate_bps);
if (congestion_window_pushback_controller_) {
int64_t pushback_rate =
congestion_window_pushback_controller_->UpdateTargetBitrate(
- target_rate.bps());
+ loss_based_target_rate.bps());
pushback_rate = std::max<int64_t>(bandwidth_estimation_->GetMinBitrate(),
pushback_rate);
- target_rate = DataRate::bps(pushback_rate);
+ pushback_target_rate = DataRate::bps(pushback_rate);
}
- if ((estimated_bitrate_bps != last_estimated_bitrate_bps_) ||
+ if ((loss_based_target_rate != last_loss_based_target_rate_) ||
(fraction_loss != last_estimated_fraction_loss_) ||
- (rtt_ms != last_estimated_rtt_ms_) ||
- (target_rate != last_pushback_target_rate_)) {
- last_pushback_target_rate_ = target_rate;
- last_estimated_bitrate_bps_ = estimated_bitrate_bps;
+ (round_trip_time != last_estimated_round_trip_time_) ||
+ (pushback_target_rate != last_pushback_target_rate_)) {
+ last_loss_based_target_rate_ = loss_based_target_rate;
+ last_pushback_target_rate_ = pushback_target_rate;
last_estimated_fraction_loss_ = fraction_loss;
- last_estimated_rtt_ms_ = rtt_ms;
+ last_estimated_round_trip_time_ = round_trip_time;
- alr_detector_->SetEstimatedBitrate(estimated_bitrate_bps);
-
- last_raw_target_rate_ = DataRate::bps(estimated_bitrate_bps);
+ alr_detector_->SetEstimatedBitrate(loss_based_target_rate.bps());
TimeDelta bwe_period = delay_based_bwe_->GetExpectedBwePeriod();
TargetTransferRate target_rate_msg;
target_rate_msg.at_time = at_time;
- target_rate_msg.target_rate = target_rate;
- target_rate_msg.stable_target_rate = std::min(
- bandwidth_estimation_->GetEstimatedLinkCapacity(), target_rate);
+ target_rate_msg.target_rate = pushback_target_rate;
+ target_rate_msg.stable_target_rate =
+ std::min(bandwidth_estimation_->GetEstimatedLinkCapacity(),
+ pushback_target_rate);
target_rate_msg.network_estimate.at_time = at_time;
- target_rate_msg.network_estimate.round_trip_time = TimeDelta::ms(rtt_ms);
+ target_rate_msg.network_estimate.round_trip_time = round_trip_time;
target_rate_msg.network_estimate.loss_rate_ratio = fraction_loss / 255.0f;
target_rate_msg.network_estimate.bwe_period = bwe_period;
update->target_rate = target_rate_msg;
auto probes = probe_controller_->SetEstimatedBitrate(
- last_raw_target_rate_.bps(), at_time.ms());
+ loss_based_target_rate.bps(), at_time.ms());
update->probe_cluster_configs.insert(update->probe_cluster_configs.end(),
probes.begin(), probes.end());
update->pacer_config = GetPacingRates(at_time);
RTC_LOG(LS_VERBOSE) << "bwe " << at_time.ms() << " pushback_target_bps="
<< last_pushback_target_rate_.bps()
- << " estimate_bps=" << last_raw_target_rate_.bps();
+ << " estimate_bps=" << loss_based_target_rate.bps();
}
}
@@ -655,7 +647,7 @@
// Pacing rate is based on target rate before congestion window pushback,
// because we don't want to build queues in the pacer when pushback occurs.
DataRate pacing_rate =
- std::max(min_total_allocated_bitrate_, last_raw_target_rate_) *
+ std::max(min_total_allocated_bitrate_, last_loss_based_target_rate_) *
pacing_factor_;
DataRate padding_rate =
std::min(max_padding_rate_, last_pushback_target_rate_);
diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
index d78ca17..bc7b66f 100644
--- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h
+++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h
@@ -118,12 +118,11 @@
std::deque<int64_t> feedback_max_rtts_;
- DataRate last_raw_target_rate_;
+ DataRate last_loss_based_target_rate_;
DataRate last_pushback_target_rate_;
- int32_t last_estimated_bitrate_bps_ = 0;
- uint8_t last_estimated_fraction_loss_ = 0;
- int64_t last_estimated_rtt_ms_ = 0;
+ absl::optional<uint8_t> last_estimated_fraction_loss_ = 0;
+ TimeDelta last_estimated_round_trip_time_ = TimeDelta::PlusInfinity();
Timestamp last_packet_received_time_ = Timestamp::MinusInfinity();
double pacing_factor_;
diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
index a310bc0..15480f1 100644
--- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
+++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc
@@ -307,12 +307,8 @@
return min_bitrate_configured_.bps<int>();
}
-void SendSideBandwidthEstimation::CurrentEstimate(int* bitrate,
- uint8_t* loss,
- int64_t* rtt) const {
- *bitrate = std::max<int32_t>(current_bitrate_.bps<int>(), GetMinBitrate());
- *loss = last_fraction_loss_;
- *rtt = last_round_trip_time_.ms<int64_t>();
+DataRate SendSideBandwidthEstimation::target_rate() const {
+ return std::max(min_bitrate_configured_, current_bitrate_);
}
DataRate SendSideBandwidthEstimation::GetEstimatedLinkCapacity() const {
diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h
index 6ae7df9..d2ab240 100644
--- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h
+++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h
@@ -75,7 +75,11 @@
~SendSideBandwidthEstimation();
void OnRouteChange();
- void CurrentEstimate(int* bitrate, uint8_t* loss, int64_t* rtt) const;
+
+ DataRate target_rate() const;
+ uint8_t fraction_loss() const { return last_fraction_loss_; }
+ TimeDelta round_trip_time() const { return last_round_trip_time_; }
+
DataRate GetEstimatedLinkCapacity() const;
// Call periodically to update estimate.
void UpdateEstimate(Timestamp at_time);
diff --git a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc
index 5c8366e..710c71f 100644
--- a/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc
+++ b/modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc
@@ -56,11 +56,7 @@
bwe.UpdateReceiverEstimate(Timestamp::ms(now_ms), DataRate::bps(kRembBps));
}
bwe.UpdateEstimate(Timestamp::ms(now_ms));
- int bitrate;
- uint8_t fraction_loss;
- int64_t rtt;
- bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt);
- EXPECT_EQ(kRembBps, bitrate);
+ EXPECT_EQ(kRembBps, bwe.target_rate().bps());
// Second REMB doesn't apply immediately.
now_ms += 2001;
@@ -72,9 +68,7 @@
DataRate::bps(kSecondRembBps));
}
bwe.UpdateEstimate(Timestamp::ms(now_ms));
- bitrate = 0;
- bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt);
- EXPECT_EQ(kRembBps, bitrate);
+ EXPECT_EQ(kRembBps, bwe.target_rate().bps());
}
TEST(SendSideBweTest, InitialRembWithProbing) {
@@ -103,13 +97,9 @@
static const int64_t kRttMs = 50;
now_ms += 10000;
- int bitrate_bps;
- uint8_t fraction_loss;
- int64_t rtt_ms;
- bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
- EXPECT_EQ(kInitialBitrateBps, bitrate_bps);
- EXPECT_EQ(0, fraction_loss);
- EXPECT_EQ(0, rtt_ms);
+ EXPECT_EQ(kInitialBitrateBps, bwe.target_rate().bps());
+ EXPECT_EQ(0, bwe.fraction_loss());
+ EXPECT_EQ(0, bwe.round_trip_time().ms());
// Signal heavy loss to go down in bitrate.
bwe.UpdatePacketsLost(/*packets_lost=*/50, /*number_of_packets=*/100,
@@ -119,30 +109,27 @@
// Trigger an update 2 seconds later to not be rate limited.
now_ms += 1000;
bwe.UpdateEstimate(Timestamp::ms(now_ms));
-
- bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
- EXPECT_LT(bitrate_bps, kInitialBitrateBps);
+ EXPECT_LT(bwe.target_rate().bps(), kInitialBitrateBps);
// Verify that the obtained bitrate isn't hitting the min bitrate, or this
// test doesn't make sense. If this ever happens, update the thresholds or
// loss rates so that it doesn't hit min bitrate after one bitrate update.
- EXPECT_GT(bitrate_bps, kMinBitrateBps);
- EXPECT_EQ(kFractionLoss, fraction_loss);
- EXPECT_EQ(kRttMs, rtt_ms);
+ EXPECT_GT(bwe.target_rate().bps(), kMinBitrateBps);
+ EXPECT_EQ(kFractionLoss, bwe.fraction_loss());
+ EXPECT_EQ(kRttMs, bwe.round_trip_time().ms());
// Triggering an update shouldn't apply further downgrade nor upgrade since
// there's no intermediate receiver block received indicating whether this is
// currently good or not.
- int last_bitrate_bps = bitrate_bps;
+ int last_bitrate_bps = bwe.target_rate().bps();
// Trigger an update 2 seconds later to not be rate limited (but it still
// shouldn't update).
now_ms += 1000;
bwe.UpdateEstimate(Timestamp::ms(now_ms));
- bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
- EXPECT_EQ(last_bitrate_bps, bitrate_bps);
+ EXPECT_EQ(last_bitrate_bps, bwe.target_rate().bps());
// The old loss rate should still be applied though.
- EXPECT_EQ(kFractionLoss, fraction_loss);
- EXPECT_EQ(kRttMs, rtt_ms);
+ EXPECT_EQ(kFractionLoss, bwe.fraction_loss());
+ EXPECT_EQ(kRttMs, bwe.round_trip_time().ms());
}
TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) {
@@ -155,9 +142,6 @@
static const int kForcedHighBitrate = 2500000;
int64_t now_ms = 0;
- int bitrate_bps;
- uint8_t fraction_loss;
- int64_t rtt_ms;
bwe.SetMinMaxBitrate(DataRate::bps(kMinBitrateBps),
DataRate::bps(kMaxBitrateBps));
@@ -166,13 +150,11 @@
bwe.UpdateDelayBasedEstimate(Timestamp::ms(now_ms),
DataRate::bps(kDelayBasedBitrateBps));
bwe.UpdateEstimate(Timestamp::ms(now_ms));
- bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
- EXPECT_GE(bitrate_bps, kInitialBitrateBps);
- EXPECT_LE(bitrate_bps, kDelayBasedBitrateBps);
+ EXPECT_GE(bwe.target_rate().bps(), kInitialBitrateBps);
+ EXPECT_LE(bwe.target_rate().bps(), kDelayBasedBitrateBps);
bwe.SetSendBitrate(DataRate::bps(kForcedHighBitrate), Timestamp::ms(now_ms));
- bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
- EXPECT_EQ(bitrate_bps, kForcedHighBitrate);
+ EXPECT_EQ(bwe.target_rate().bps(), kForcedHighBitrate);
}
} // namespace webrtc
