Cleanup construction parameters for remote bitrate estimator helpers

Values of these parameters are always the same and thus can be hardcoded

Bug: None
Change-Id: Ie19a1c6305d503ad2c92af503006a72b7981e178
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298622
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39637}

modules/remote_bitrate_estimator/inter_arrival.cc

@@ -19,13 +19,11 @@ static const int kBurstDeltaThresholdMs = 5;
 static const int kMaxBurstDurationMs = 100;
 
 InterArrival::InterArrival(uint32_t timestamp_group_length_ticks,
-                           double timestamp_to_ms_coeff,
+                           double timestamp_to_ms_coeff)
-                           bool enable_burst_grouping)
     : kTimestampGroupLengthTicks(timestamp_group_length_ticks),
       current_timestamp_group_(),
       prev_timestamp_group_(),
       timestamp_to_ms_coeff_(timestamp_to_ms_coeff),
-      burst_grouping_(enable_burst_grouping),
       num_consecutive_reordered_packets_(0) {}
 
 bool InterArrival::ComputeDeltas(uint32_t timestamp,
@@ -136,9 +134,6 @@ bool InterArrival::NewTimestampGroup(int64_t arrival_time_ms,
 
 bool InterArrival::BelongsToBurst(int64_t arrival_time_ms,
                                   uint32_t timestamp) const {
-  if (!burst_grouping_) {
-    return false;
-  }
   RTC_DCHECK_GE(current_timestamp_group_.complete_time_ms, 0);
   int64_t arrival_time_delta_ms =
       arrival_time_ms - current_timestamp_group_.complete_time_ms;

modules/remote_bitrate_estimator/inter_arrival.h

@@ -30,8 +30,7 @@ class InterArrival {
   // most timestamp_group_length_ticks older than the first timestamp in that
   // group.
   InterArrival(uint32_t timestamp_group_length_ticks,
-               double timestamp_to_ms_coeff,
+               double timestamp_to_ms_coeff);
-               bool enable_burst_grouping);
 
   InterArrival() = delete;
   InterArrival(const InterArrival&) = delete;
@@ -87,7 +86,6 @@ class InterArrival {
   TimestampGroup current_timestamp_group_;
   TimestampGroup prev_timestamp_group_;
   double timestamp_to_ms_coeff_;
-  bool burst_grouping_;
   int num_consecutive_reordered_packets_;
 };
 }  // namespace webrtc

modules/remote_bitrate_estimator/inter_arrival_unittest.cc

@@ -33,12 +33,11 @@ const double kAstToMs = 1000.0 / static_cast<double>(1 << kInterArrivalShift);
 class InterArrivalTest : public ::testing::Test {
  protected:
   virtual void SetUp() {
-    inter_arrival_.reset(
+    inter_arrival_.reset(new InterArrival(kTimestampGroupLengthUs / 1000, 1.0));
-        new InterArrival(kTimestampGroupLengthUs / 1000, 1.0, true));
     inter_arrival_rtp_.reset(new InterArrival(
-        MakeRtpTimestamp(kTimestampGroupLengthUs), kRtpTimestampToMs, true));
+        MakeRtpTimestamp(kTimestampGroupLengthUs), kRtpTimestampToMs));
-    inter_arrival_ast_.reset(new InterArrival(
+    inter_arrival_ast_.reset(
-        MakeAbsSendTime(kTimestampGroupLengthUs), kAstToMs, true));
+        new InterArrival(MakeAbsSendTime(kTimestampGroupLengthUs), kAstToMs));
   }
 
   // Test that neither inter_arrival instance complete the timestamp group from

modules/remote_bitrate_estimator/overuse_detector_unittest.cc

@@ -35,8 +35,7 @@ class OveruseDetectorTest : public ::testing::Test {
         receive_time_ms_(0),
         rtp_timestamp_(10 * 90),
         overuse_detector_(),
-        overuse_estimator_(new OveruseEstimator(options_)),
+        inter_arrival_(5 * 90, kRtpTimestampToMs),
-        inter_arrival_(new InterArrival(5 * 90, kRtpTimestampToMs, true)),
         random_(123456789) {}
 
  protected:
@@ -99,15 +98,15 @@ class OveruseDetectorTest : public ::testing::Test {
     uint32_t timestamp_delta;
     int64_t time_delta;
     int size_delta;
-    if (inter_arrival_->ComputeDeltas(
+    if (inter_arrival_.ComputeDeltas(
             rtp_timestamp, receive_time_ms, receive_time_ms, packet_size,
             &timestamp_delta, &time_delta, &size_delta)) {
       double timestamp_delta_ms = timestamp_delta / 90.0;
-      overuse_estimator_->Update(time_delta, timestamp_delta_ms, size_delta,
+      overuse_estimator_.Update(time_delta, timestamp_delta_ms, size_delta,
                                 overuse_detector_->State(), receive_time_ms);
-      overuse_detector_->Detect(
+      overuse_detector_->Detect(overuse_estimator_.offset(), timestamp_delta_ms,
-          overuse_estimator_->offset(), timestamp_delta_ms,
+                                overuse_estimator_.num_of_deltas(),
-          overuse_estimator_->num_of_deltas(), receive_time_ms);
+                                receive_time_ms);
     }
   }
 
@@ -115,10 +114,9 @@ class OveruseDetectorTest : public ::testing::Test {
   int64_t now_ms_;
   int64_t receive_time_ms_;
   uint32_t rtp_timestamp_;
-  OverUseDetectorOptions options_;
   std::unique_ptr<OveruseDetector> overuse_detector_;
-  std::unique_ptr<OveruseEstimator> overuse_estimator_;
+  OveruseEstimator overuse_estimator_;
-  std::unique_ptr<InterArrival> inter_arrival_;
+  InterArrival inter_arrival_;
   Random random_;
 };
 

modules/remote_bitrate_estimator/overuse_estimator.cc

@@ -20,29 +20,14 @@
 #include "rtc_base/logging.h"
 
 namespace webrtc {
+namespace {
 
-enum { kMinFramePeriodHistoryLength = 60 };
+constexpr int kMinFramePeriodHistoryLength = 60;
-enum { kDeltaCounterMax = 1000 };
+constexpr int kDeltaCounterMax = 1000;
 
-OveruseEstimator::OveruseEstimator(const OverUseDetectorOptions& options)
+}  // namespace
-    : options_(options),
-      num_of_deltas_(0),
-      slope_(options_.initial_slope),
-      offset_(options_.initial_offset),
-      prev_offset_(options_.initial_offset),
-      E_(),
-      process_noise_(),
-      avg_noise_(options_.initial_avg_noise),
-      var_noise_(options_.initial_var_noise),
-      ts_delta_hist_() {
-  memcpy(E_, options_.initial_e, sizeof(E_));
-  memcpy(process_noise_, options_.initial_process_noise,
-         sizeof(process_noise_));
-}
 
-OveruseEstimator::~OveruseEstimator() {
+OveruseEstimator::OveruseEstimator() = default;
-  ts_delta_hist_.clear();
-}
 
 void OveruseEstimator::Update(int64_t t_delta,
                               double ts_delta,

modules/remote_bitrate_estimator/overuse_estimator.h

@@ -18,29 +18,15 @@
 
 namespace webrtc {
 
-// Bandwidth over-use detector options.  These are used to drive
-// experimentation with bandwidth estimation parameters.
-// TODO(terelius): This is only used in overuse_estimator.cc, and only in the
-// default constructed state. Can we move the relevant variables into that
-// class and delete this?
-struct OverUseDetectorOptions {
-  OverUseDetectorOptions() = default;
-  double initial_slope = 8.0 / 512.0;
-  double initial_offset = 0;
-  double initial_e[2][2] = {{100.0, 0.0}, {0.0, 1e-1}};
-  double initial_process_noise[2] = {1e-13, 1e-3};
-  double initial_avg_noise = 0.0;
-  double initial_var_noise = 50.0;
-};
-
 class OveruseEstimator {
  public:
-  explicit OveruseEstimator(const OverUseDetectorOptions& options);
+  OveruseEstimator();
-  ~OveruseEstimator();
 
   OveruseEstimator(const OveruseEstimator&) = delete;
   OveruseEstimator& operator=(const OveruseEstimator&) = delete;
 
+  ~OveruseEstimator() = default;
+
   // Update the estimator with a new sample. The deltas should represent deltas
   // between timestamp groups as defined by the InterArrival class.
   // `current_hypothesis` should be the hypothesis of the over-use detector at
@@ -59,23 +45,20 @@ class OveruseEstimator {
 
   // Returns the number of deltas which the current over-use estimator state is
   // based on.
-  unsigned int num_of_deltas() const { return num_of_deltas_; }
+  int num_of_deltas() const { return num_of_deltas_; }
 
  private:
   double UpdateMinFramePeriod(double ts_delta);
   void UpdateNoiseEstimate(double residual, double ts_delta, bool stable_state);
 
-  // Must be first member variable. Cannot be const because we need to be
+  int num_of_deltas_ = 0;
-  // copyable.
+  double slope_ = 8.0 / 512.0;
-  OverUseDetectorOptions options_;
+  double offset_ = 0;
-  uint16_t num_of_deltas_;
+  double prev_offset_ = 0;
-  double slope_;
+  double E_[2][2] = {{100.0, 0.0}, {0.0, 1e-1}};
-  double offset_;
+  double process_noise_[2] = {1e-13, 1e-3};
-  double prev_offset_;
+  double avg_noise_ = 0.0;
-  double E_[2][2];
+  double var_noise_ = 50.0;
-  double process_noise_[2];
-  double avg_noise_;
-  double var_noise_;
   std::deque<double> ts_delta_hist_;
 };
 }  // namespace webrtc

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -362,9 +362,8 @@ void RemoteBitrateEstimatorAbsSendTime::TimeoutStreams(Timestamp now) {
   if (ssrcs_.empty()) {
     // We can't update the estimate if we don't have any active streams.
     inter_arrival_ = std::make_unique<InterArrival>(
-        (kTimestampGroupLengthMs << kInterArrivalShift) / 1000, kTimestampToMs,
+        (kTimestampGroupLengthMs << kInterArrivalShift) / 1000, kTimestampToMs);
-        true);
+    estimator_ = std::make_unique<OveruseEstimator>();
-    estimator_ = std::make_unique<OveruseEstimator>(OverUseDetectorOptions());
     // We deliberately don't reset the first_packet_time_ms_ here for now since
     // we only probe for bandwidth in the beginning of a call right now.
   }

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc

@@ -42,15 +42,11 @@ static const double kTimestampToMs = 1.0 / 90.0;
 
 struct RemoteBitrateEstimatorSingleStream::Detector {
   explicit Detector(int64_t last_packet_time_ms,
-                    const OverUseDetectorOptions& options,
-                    bool enable_burst_grouping,
                     const FieldTrialsView* key_value_config)
       : last_packet_time_ms(last_packet_time_ms),
-        inter_arrival(90 * kTimestampGroupLengthMs,
+        inter_arrival(90 * kTimestampGroupLengthMs, kTimestampToMs),
-                      kTimestampToMs,
-                      enable_burst_grouping),
-        estimator(options),
         detector(key_value_config) {}
+
   int64_t last_packet_time_ms;
   InterArrival inter_arrival;
   OveruseEstimator estimator;
@@ -104,8 +100,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
     // group.
     std::pair<SsrcOveruseEstimatorMap::iterator, bool> insert_result =
         overuse_detectors_.insert(
-            std::make_pair(ssrc, new Detector(now_ms, OverUseDetectorOptions(),
+            std::make_pair(ssrc, new Detector(now_ms, &field_trials_)));
-                                              true, &field_trials_)));
     it = insert_result.first;
   }
   Detector* estimator = it->second;