Cleanup RemoteBitreateEstimatorSingleStream to use unit types

Use Timestamp,TimeDelta, and DataRate types instead of plain integer types.

Bug: webrtc:13756
Change-Id: I2a12f4abeeaa653dbd9534c297dbb72db63b012b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/314502
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40684}

modules/remote_bitrate_estimator/BUILD.gn

@@ -46,6 +46,7 @@ rtc_library("remote_bitrate_estimator") {
     "../../modules:module_api_public",
     "../../modules/congestion_controller/goog_cc:link_capacity_estimator",
     "../../modules/rtp_rtcp:rtp_rtcp_format",
+    "../../rtc_base:bitrate_tracker",
     "../../rtc_base:checks",
     "../../rtc_base:logging",
     "../../rtc_base:rate_statistics",

modules/remote_bitrate_estimator/aimd_rate_control.cc

@@ -152,7 +152,7 @@ DataRate AimdRateControl::Update(const RateControlInput& input,
   // TODO(bugs.webrtc.org/9379): The comment above doesn't match to the code.
   if (!bitrate_is_initialized_) {
     const TimeDelta kInitializationTime = TimeDelta::Seconds(5);
-    RTC_DCHECK_LE(kBitrateWindowMs, kInitializationTime.ms());
+    RTC_DCHECK_LE(kBitrateWindow, kInitializationTime);
     if (time_first_throughput_estimate_.IsInfinite()) {
       if (input.estimated_throughput)
         time_first_throughput_estimate_ = at_time;

modules/remote_bitrate_estimator/include/bwe_defines.h

@@ -16,12 +16,13 @@
 #include "absl/types/optional.h"
 #include "api/network_state_predictor.h"
 #include "api/units/data_rate.h"
+#include "api/units/time_delta.h"
 
 namespace webrtc {
 
-constexpr DataRate kCongestionControllerMinBitrate = DataRate::BitsPerSec(5000);
-
-static const int64_t kBitrateWindowMs = 1000;
+inline constexpr DataRate kCongestionControllerMinBitrate =
+    DataRate::BitsPerSec(5'000);
+inline constexpr TimeDelta kBitrateWindow = TimeDelta::Seconds(1);
 
 extern const char kBweTypeHistogram[];
 

modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h

@@ -55,8 +55,8 @@ class RemoteBitrateEstimator : public CallStatsObserver {
   virtual TimeDelta Process() = 0;
 
  protected:
-  static const int64_t kProcessIntervalMs = 500;
-  static const int64_t kStreamTimeOutMs = 2000;
+  static constexpr TimeDelta kProcessInterval = TimeDelta::Millis(500);
+  static constexpr TimeDelta kStreamTimeOut = TimeDelta::Seconds(2);
 };
 
 }  // namespace webrtc

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -345,7 +345,7 @@ TimeDelta RemoteBitrateEstimatorAbsSendTime::Process() {
 
 void RemoteBitrateEstimatorAbsSendTime::TimeoutStreams(Timestamp now) {
   for (auto it = ssrcs_.begin(); it != ssrcs_.end();) {
-    if (now - it->second > TimeDelta::Millis(kStreamTimeOutMs)) {
+    if (now - it->second > kStreamTimeOut) {
       ssrcs_.erase(it++);
     } else {
       ++it;

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h

@@ -104,7 +104,7 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
   std::unique_ptr<InterArrival> inter_arrival_;
   std::unique_ptr<OveruseEstimator> estimator_;
   OveruseDetector detector_;
-  RateStatistics incoming_bitrate_{kBitrateWindowMs, 8000};
+  RateStatistics incoming_bitrate_{kBitrateWindow.ms(), 8000};
   bool incoming_bitrate_initialized_ = false;
   std::list<Probe> probes_;
   size_t total_probes_received_ = 0;

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc

@@ -32,29 +32,21 @@ namespace {
 constexpr int kTimestampGroupLengthMs = 5;
 constexpr double kTimestampToMs = 1.0 / 90.0;
 
-absl::optional<DataRate> OptionalRateFromOptionalBps(
-    absl::optional<int> bitrate_bps) {
-  if (bitrate_bps) {
-    return DataRate::BitsPerSec(*bitrate_bps);
-  } else {
-    return absl::nullopt;
-  }
-}
 }  // namespace
 
 RemoteBitrateEstimatorSingleStream::Detector::Detector()
-    : inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {}
+    : last_packet_time(Timestamp::Zero()),
+      inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs) {}
 
 RemoteBitrateEstimatorSingleStream::RemoteBitrateEstimatorSingleStream(
     RemoteBitrateObserver* observer,
     Clock* clock)
     : clock_(clock),
-      incoming_bitrate_(kBitrateWindowMs, 8000),
-      last_valid_incoming_bitrate_(0),
+      incoming_bitrate_(kBitrateWindow),
+      last_valid_incoming_bitrate_(DataRate::Zero()),
       remote_rate_(field_trials_),
       observer_(observer),
-      last_process_time_(-1),
-      process_interval_ms_(kProcessIntervalMs),
+      process_interval_(kProcessInterval),
       uma_recorded_(false) {
   RTC_LOG(LS_INFO) << "RemoteBitrateEstimatorSingleStream: Instantiating.";
 }
@@ -76,28 +68,29 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
   uint32_t ssrc = rtp_packet.Ssrc();
   uint32_t rtp_timestamp =
       rtp_packet.Timestamp() + transmission_time_offset.value_or(0);
-  int64_t now_ms = clock_->TimeInMilliseconds();
+  Timestamp now = clock_->CurrentTime();
   Detector& estimator = overuse_detectors_[ssrc];
-  estimator.last_packet_time_ms = now_ms;
+  estimator.last_packet_time = now;
 
   // Check if incoming bitrate estimate is valid, and if it needs to be reset.
-  absl::optional<uint32_t> incoming_bitrate = incoming_bitrate_.Rate(now_ms);
+  absl::optional<DataRate> incoming_bitrate = incoming_bitrate_.Rate(now);
   if (incoming_bitrate) {
     last_valid_incoming_bitrate_ = *incoming_bitrate;
-  } else if (last_valid_incoming_bitrate_ > 0) {
+  } else if (last_valid_incoming_bitrate_ > DataRate::Zero()) {
     // Incoming bitrate had a previous valid value, but now not enough data
     // point are left within the current window. Reset incoming bitrate
     // estimator so that the window size will only contain new data points.
     incoming_bitrate_.Reset();
-    last_valid_incoming_bitrate_ = 0;
+    last_valid_incoming_bitrate_ = DataRate::Zero();
   }
   size_t payload_size = rtp_packet.payload_size() + rtp_packet.padding_size();
-  incoming_bitrate_.Update(payload_size, now_ms);
+  incoming_bitrate_.Update(payload_size, now);
 
   const BandwidthUsage prior_state = estimator.detector.State();
   uint32_t timestamp_delta = 0;
   int64_t time_delta = 0;
   int size_delta = 0;
+  int64_t now_ms = now.ms();
   if (estimator.inter_arrival.ComputeDeltas(
           rtp_timestamp, rtp_packet.arrival_time().ms(), now_ms, payload_size,
           &timestamp_delta, &time_delta, &size_delta)) {
@@ -108,42 +101,41 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
                               estimator.estimator.num_of_deltas(), now_ms);
   }
   if (estimator.detector.State() == BandwidthUsage::kBwOverusing) {
-    absl::optional<uint32_t> incoming_bitrate_bps =
-        incoming_bitrate_.Rate(now_ms);
-    if (incoming_bitrate_bps &&
+    absl::optional<DataRate> incoming_bitrate = incoming_bitrate_.Rate(now);
+    if (incoming_bitrate.has_value() &&
         (prior_state != BandwidthUsage::kBwOverusing ||
-         remote_rate_.TimeToReduceFurther(
-             Timestamp::Millis(now_ms),
-             DataRate::BitsPerSec(*incoming_bitrate_bps)))) {
+         remote_rate_.TimeToReduceFurther(now, *incoming_bitrate))) {
       // The first overuse should immediately trigger a new estimate.
       // We also have to update the estimate immediately if we are overusing
       // and the target bitrate is too high compared to what we are receiving.
-      UpdateEstimate(now_ms);
+      UpdateEstimate(now);
     }
   }
 }
 
 TimeDelta RemoteBitrateEstimatorSingleStream::Process() {
-  int64_t now_ms = clock_->TimeInMilliseconds();
-  int64_t next_process_time_ms = last_process_time_ + process_interval_ms_;
-  if (last_process_time_ == -1 || now_ms >= next_process_time_ms) {
-    UpdateEstimate(now_ms);
-    last_process_time_ = now_ms;
-    return TimeDelta::Millis(process_interval_ms_);
+  Timestamp now = clock_->CurrentTime();
+  Timestamp next_process_time = last_process_time_.has_value()
+                                    ? *last_process_time_ + process_interval_
+                                    : now;
+  // TODO(bugs.webrtc.org/13756): Removing rounding to milliseconds after
+  // investigating why tests fails without that rounding.
+  if (now.ms() >= next_process_time.ms()) {
+    UpdateEstimate(now);
+    last_process_time_ = now;
+    return process_interval_;
   }
 
-  return TimeDelta::Millis(next_process_time_ms - now_ms);
+  return next_process_time - now;
 }
 
-void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
+void RemoteBitrateEstimatorSingleStream::UpdateEstimate(Timestamp now) {
   BandwidthUsage bw_state = BandwidthUsage::kBwNormal;
   auto it = overuse_detectors_.begin();
   while (it != overuse_detectors_.end()) {
-    const int64_t time_of_last_received_packet = it->second.last_packet_time_ms;
-    if (time_of_last_received_packet >= 0 &&
-        now_ms - time_of_last_received_packet > kStreamTimeOutMs) {
-      // This over-use detector hasn't received packets for `kStreamTimeOutMs`
-      // milliseconds and is considered stale.
+    if (now - it->second.last_packet_time > kStreamTimeOut) {
+      // This over-use detector hasn't received packets for `kStreamTimeOut`
+      // and is considered stale.
       overuse_detectors_.erase(it++);
     } else {
       // Make sure that we trigger an over-use if any of the over-use detectors
@@ -159,13 +151,11 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
     return;
   }
 
-  const RateControlInput input(
-      bw_state, OptionalRateFromOptionalBps(incoming_bitrate_.Rate(now_ms)));
-  uint32_t target_bitrate =
-      remote_rate_.Update(input, Timestamp::Millis(now_ms)).bps<uint32_t>();
+  const RateControlInput input(bw_state, incoming_bitrate_.Rate(now));
+  uint32_t target_bitrate = remote_rate_.Update(input, now).bps<uint32_t>();
   if (remote_rate_.ValidEstimate()) {
-    process_interval_ms_ = remote_rate_.GetFeedbackInterval().ms();
-    RTC_DCHECK_GT(process_interval_ms_, 0);
+    process_interval_ = remote_rate_.GetFeedbackInterval();
+    RTC_DCHECK_GT(process_interval_, TimeDelta::Zero());
     if (observer_)
       observer_->OnReceiveBitrateChanged(GetSsrcs(), target_bitrate);
   }

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h

@@ -17,6 +17,7 @@
 #include <map>
 #include <vector>
 
+#include "absl/types/optional.h"
 #include "api/transport/field_trial_based_config.h"
 #include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
@@ -26,7 +27,7 @@
 #include "modules/remote_bitrate_estimator/inter_arrival.h"
 #include "modules/remote_bitrate_estimator/overuse_detector.h"
 #include "modules/remote_bitrate_estimator/overuse_estimator.h"
-#include "rtc_base/rate_statistics.h"
+#include "rtc_base/bitrate_tracker.h"
 
 namespace webrtc {
 
@@ -56,26 +57,26 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
   struct Detector {
     Detector();
 
-    int64_t last_packet_time_ms;
+    Timestamp last_packet_time;
     InterArrival inter_arrival;
     OveruseEstimator estimator;
     OveruseDetector detector;
   };
 
   // Triggers a new estimate calculation.
-  void UpdateEstimate(int64_t time_now);
+  void UpdateEstimate(Timestamp now);
 
   std::vector<uint32_t> GetSsrcs() const;
 
   Clock* const clock_;
   const FieldTrialBasedConfig field_trials_;
   std::map<uint32_t, Detector> overuse_detectors_;
-  RateStatistics incoming_bitrate_;
-  uint32_t last_valid_incoming_bitrate_;
+  BitrateTracker incoming_bitrate_;
+  DataRate last_valid_incoming_bitrate_;
   AimdRateControl remote_rate_;
   RemoteBitrateObserver* const observer_;
-  int64_t last_process_time_;
-  int64_t process_interval_ms_;
+  absl::optional<Timestamp> last_process_time_;
+  TimeDelta process_interval_;
   bool uma_recorded_;
 };