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}

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -96,8 +96,8 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config,
       acknowledged_bitrate_estimator_(
           std::make_unique<AcknowledgedBitrateEstimator>(key_value_config_)),
       initial_config_(config),
-      last_raw_target_rate_(*config.constraints.starting_rate),
+      last_loss_based_target_rate_(*config.constraints.starting_rate),
-      last_pushback_target_rate_(last_raw_target_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 @@ NetworkControlUpdate GoogCcNetworkController::OnNetworkRouteChange(
       if (!estimated_bitrate)
         estimated_bitrate = acknowledged_bitrate_estimator_->PeekRate();
     } else {
-      int32_t target_bitrate_bps;
+      estimated_bitrate = bandwidth_estimation_->target_rate();
-      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);
     }
     if (estimated_bitrate) {
       if (msg.constraints.starting_rate) {
@@ -392,7 +387,7 @@ void GoogCcNetworkController::UpdateCongestionWindowSize(
     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::OnNetworkStateEstimate(
 
 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;
+      last_estimated_fraction_loss_.value_or(0) / 255.0;
-  update.target_rate->network_estimate.round_trip_time = rtt;
+  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 @@ NetworkControlUpdate GoogCcNetworkController::GetNetworkState(
 void GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
     NetworkControlUpdate* update,
     Timestamp at_time) {
-  int32_t estimated_bitrate_bps;
+  uint8_t fraction_loss = bandwidth_estimation_->fraction_loss();
-  uint8_t fraction_loss;
+  TimeDelta round_trip_time = bandwidth_estimation_->round_trip_time();
-  int64_t rtt_ms;
+  DataRate loss_based_target_rate = bandwidth_estimation_->target_rate();
-  bandwidth_estimation_->CurrentEstimate(&estimated_bitrate_bps, &fraction_loss,
+  DataRate pushback_target_rate = loss_based_target_rate;
-                                         &rtt_ms);
 
   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_) ||
+      (round_trip_time != last_estimated_round_trip_time_) ||
-      (target_rate != last_pushback_target_rate_)) {
+      (pushback_target_rate != last_pushback_target_rate_)) {
-    last_pushback_target_rate_ = target_rate;
+    last_loss_based_target_rate_ = loss_based_target_rate;
-    last_estimated_bitrate_bps_ = estimated_bitrate_bps;
+    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);
+    alr_detector_->SetEstimatedBitrate(loss_based_target_rate.bps());
-
-    last_raw_target_rate_ = DataRate::bps(estimated_bitrate_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.target_rate = pushback_target_rate;
-    target_rate_msg.stable_target_rate = std::min(
+    target_rate_msg.stable_target_rate =
-        bandwidth_estimation_->GetEstimatedLinkCapacity(), 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 @@ PacerConfig GoogCcNetworkController::GetPacingRates(Timestamp at_time) const {
   // 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_);

modules/congestion_controller/goog_cc/goog_cc_network_control.h

@@ -118,12 +118,11 @@ class GoogCcNetworkController : public NetworkControllerInterface {
 
   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;
+  absl::optional<uint8_t> last_estimated_fraction_loss_ = 0;
-  uint8_t last_estimated_fraction_loss_ = 0;
+  TimeDelta last_estimated_round_trip_time_ = TimeDelta::PlusInfinity();
-  int64_t last_estimated_rtt_ms_ = 0;
   Timestamp last_packet_received_time_ = Timestamp::MinusInfinity();
 
   double pacing_factor_;

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.cc

@@ -307,12 +307,8 @@ int SendSideBandwidthEstimation::GetMinBitrate() const {
   return min_bitrate_configured_.bps<int>();
 }
 
-void SendSideBandwidthEstimation::CurrentEstimate(int* bitrate,
+DataRate SendSideBandwidthEstimation::target_rate() const {
-                                                  uint8_t* loss,
+  return std::max(min_bitrate_configured_, current_bitrate_);
-                                                  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::GetEstimatedLinkCapacity() const {

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation.h

@@ -75,7 +75,11 @@ class SendSideBandwidthEstimation {
   ~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);

modules/congestion_controller/goog_cc/send_side_bandwidth_estimation_unittest.cc

@@ -56,11 +56,7 @@ void TestProbing(bool use_delay_based) {
     bwe.UpdateReceiverEstimate(Timestamp::ms(now_ms), DataRate::bps(kRembBps));
   }
   bwe.UpdateEstimate(Timestamp::ms(now_ms));
-  int bitrate;
+  EXPECT_EQ(kRembBps, bwe.target_rate().bps());
-  uint8_t fraction_loss;
-  int64_t rtt;
-  bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt);
-  EXPECT_EQ(kRembBps, bitrate);
 
   // Second REMB doesn't apply immediately.
   now_ms += 2001;
@@ -72,9 +68,7 @@ void TestProbing(bool use_delay_based) {
                                DataRate::bps(kSecondRembBps));
   }
   bwe.UpdateEstimate(Timestamp::ms(now_ms));
-  bitrate = 0;
+  EXPECT_EQ(kRembBps, bwe.target_rate().bps());
-  bwe.CurrentEstimate(&bitrate, &fraction_loss, &rtt);
-  EXPECT_EQ(kRembBps, bitrate);
 }
 
 TEST(SendSideBweTest, InitialRembWithProbing) {
@@ -103,13 +97,9 @@ TEST(SendSideBweTest, DoesntReapplyBitrateDecreaseWithoutFollowingRemb) {
   static const int64_t kRttMs = 50;
   now_ms += 10000;
 
-  int bitrate_bps;
+  EXPECT_EQ(kInitialBitrateBps, bwe.target_rate().bps());
-  uint8_t fraction_loss;
+  EXPECT_EQ(0, bwe.fraction_loss());
-  int64_t rtt_ms;
+  EXPECT_EQ(0, bwe.round_trip_time().ms());
-  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
-  EXPECT_EQ(kInitialBitrateBps, bitrate_bps);
-  EXPECT_EQ(0, fraction_loss);
-  EXPECT_EQ(0, rtt_ms);
 
   // Signal heavy loss to go down in bitrate.
   bwe.UpdatePacketsLost(/*packets_lost=*/50, /*number_of_packets=*/100,
@@ -119,30 +109,27 @@ TEST(SendSideBweTest, DoesntReapplyBitrateDecreaseWithoutFollowingRemb) {
   // Trigger an update 2 seconds later to not be rate limited.
   now_ms += 1000;
   bwe.UpdateEstimate(Timestamp::ms(now_ms));
-
+  EXPECT_LT(bwe.target_rate().bps(), kInitialBitrateBps);
-  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
-  EXPECT_LT(bitrate_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_GT(bwe.target_rate().bps(), kMinBitrateBps);
-  EXPECT_EQ(kFractionLoss, fraction_loss);
+  EXPECT_EQ(kFractionLoss, bwe.fraction_loss());
-  EXPECT_EQ(kRttMs, rtt_ms);
+  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(kFractionLoss, bwe.fraction_loss());
-  EXPECT_EQ(kRttMs, rtt_ms);
+  EXPECT_EQ(kRttMs, bwe.round_trip_time().ms());
 }
 
 TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) {
@@ -155,9 +142,6 @@ TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) {
   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 @@ TEST(SendSideBweTest, SettingSendBitrateOverridesDelayBasedEstimate) {
   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(bwe.target_rate().bps(), kInitialBitrateBps);
-  EXPECT_GE(bitrate_bps, kInitialBitrateBps);
+  EXPECT_LE(bwe.target_rate().bps(), kDelayBasedBitrateBps);
-  EXPECT_LE(bitrate_bps, kDelayBasedBitrateBps);
 
   bwe.SetSendBitrate(DataRate::bps(kForcedHighBitrate), Timestamp::ms(now_ms));
-  bwe.CurrentEstimate(&bitrate_bps, &fraction_loss, &rtt_ms);
+  EXPECT_EQ(bwe.target_rate().bps(), kForcedHighBitrate);
-  EXPECT_EQ(bitrate_bps, kForcedHighBitrate);
 }
 
 }  // namespace webrtc