Detach RemoteBitrateEstimator interface from Module

Bug: webrtc:7219
Change-Id: I8302c5044582d73b0918013a0df89b9390788728
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267140
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Auto-Submit: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37393}

modules/congestion_controller/BUILD.gn

@@ -27,7 +27,6 @@ rtc_library("congestion_controller") {
   ]
 
   deps = [
-    "..:module_api",
     "../../api/transport:field_trial_based_config",
     "../../api/transport:network_control",
     "../../api/units:data_rate",

modules/congestion_controller/include/receive_side_congestion_controller.h

@@ -17,8 +17,8 @@
 #include "api/transport/field_trial_based_config.h"
 #include "api/transport/network_control.h"
 #include "api/units/data_rate.h"
+#include "api/units/time_delta.h"
 #include "modules/congestion_controller/remb_throttler.h"
-#include "modules/include/module.h"
 #include "modules/pacing/packet_router.h"
 #include "modules/remote_bitrate_estimator/remote_estimator_proxy.h"
 #include "rtc_base/synchronization/mutex.h"
@@ -68,15 +68,12 @@ class ReceiveSideCongestionController : public CallStatsObserver {
   // Noop if receive side bwe is not used or stream doesn't participate in it.
   void RemoveStream(uint32_t ssrc);
 
-  [[deprecated]] int64_t TimeUntilNextProcess();
-  [[deprecated]] void Process();
-
   // Runs periodic tasks if it is time to run them, returns time until next
   // call to `MaybeProcess` should be non idle.
   TimeDelta MaybeProcess();
 
  private:
-  class WrappingBitrateEstimator : public RemoteBitrateEstimator {
+  class WrappingBitrateEstimator {
    public:
     WrappingBitrateEstimator(RemoteBitrateObserver* observer, Clock* clock);
 
@@ -85,24 +82,22 @@ class ReceiveSideCongestionController : public CallStatsObserver {
     WrappingBitrateEstimator& operator=(const WrappingBitrateEstimator&) =
         delete;
 
-    ~WrappingBitrateEstimator() override;
+    ~WrappingBitrateEstimator();
 
     void IncomingPacket(int64_t arrival_time_ms,
                         size_t payload_size,
-                        const RTPHeader& header) override;
+                        const RTPHeader& header);
 
-    void Process() override;
+    TimeDelta Process();
 
-    int64_t TimeUntilNextProcess() override;
+    void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms);
 
-    void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
+    void RemoveStream(unsigned int ssrc);
-
-    void RemoveStream(unsigned int ssrc) override;
 
     bool LatestEstimate(std::vector<unsigned int>* ssrcs,
-                        unsigned int* bitrate_bps) const override;
+                        unsigned int* bitrate_bps) const;
 
-    void SetMinBitrate(int min_bitrate_bps) override;
+    void SetMinBitrate(int min_bitrate_bps);
 
    private:
     void PickEstimatorFromHeader(const RTPHeader& header)

modules/congestion_controller/receive_side_congestion_controller.cc

@@ -44,15 +44,9 @@ void ReceiveSideCongestionController::WrappingBitrateEstimator::IncomingPacket(
   rbe_->IncomingPacket(arrival_time_ms, payload_size, header);
 }
 
-void ReceiveSideCongestionController::WrappingBitrateEstimator::Process() {
+TimeDelta ReceiveSideCongestionController::WrappingBitrateEstimator::Process() {
   MutexLock lock(&mutex_);
-  rbe_->Process();
+  return rbe_->Process();
-}
-
-int64_t ReceiveSideCongestionController::WrappingBitrateEstimator::
-    TimeUntilNextProcess() {
-  MutexLock lock(&mutex_);
-  return rbe_->TimeUntilNextProcess();
 }
 
 void ReceiveSideCongestionController::WrappingBitrateEstimator::OnRttUpdate(
@@ -171,22 +165,9 @@ void ReceiveSideCongestionController::OnBitrateChanged(int bitrate_bps) {
   remote_estimator_proxy_.OnBitrateChanged(bitrate_bps);
 }
 
-int64_t ReceiveSideCongestionController::TimeUntilNextProcess() {
-  return remote_bitrate_estimator_.TimeUntilNextProcess();
-}
-
-void ReceiveSideCongestionController::Process() {
-  remote_bitrate_estimator_.Process();
-}
-
 TimeDelta ReceiveSideCongestionController::MaybeProcess() {
   Timestamp now = clock_.CurrentTime();
-  int64_t time_until_rbe_ms = remote_bitrate_estimator_.TimeUntilNextProcess();
+  TimeDelta time_until_rbe = remote_bitrate_estimator_.Process();
-  if (time_until_rbe_ms <= 0) {
-    remote_bitrate_estimator_.Process();
-    time_until_rbe_ms = remote_bitrate_estimator_.TimeUntilNextProcess();
-  }
-  TimeDelta time_until_rbe = TimeDelta::Millis(time_until_rbe_ms);
   TimeDelta time_until_rep = remote_estimator_proxy_.Process(now);
   TimeDelta time_until = std::min(time_until_rbe, time_until_rep);
   return std::max(time_until, TimeDelta::Zero());

modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h

@@ -17,7 +17,7 @@
 #include <memory>
 #include <vector>
 
-#include "modules/include/module.h"
+#include "api/units/time_delta.h"
 #include "modules/include/module_common_types.h"
 #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 #include "modules/rtp_rtcp/source/rtcp_packet.h"
@@ -38,7 +38,7 @@ class RemoteBitrateObserver {
   virtual ~RemoteBitrateObserver() {}
 };
 
-class RemoteBitrateEstimator : public CallStatsObserver, public Module {
+class RemoteBitrateEstimator : public CallStatsObserver {
  public:
   ~RemoteBitrateEstimator() override {}
 
@@ -62,6 +62,8 @@ class RemoteBitrateEstimator : public CallStatsObserver, public Module {
 
   virtual void SetMinBitrate(int min_bitrate_bps) = 0;
 
+  virtual TimeDelta Process() = 0;
+
  protected:
   static const int64_t kProcessIntervalMs = 500;
   static const int64_t kStreamTimeOutMs = 2000;

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.cc

@@ -353,11 +353,8 @@ void RemoteBitrateEstimatorAbsSendTime::IncomingPacketInfo(
   }
 }
 
-void RemoteBitrateEstimatorAbsSendTime::Process() {}
+TimeDelta RemoteBitrateEstimatorAbsSendTime::Process() {
-
+  return TimeDelta::PlusInfinity();
-int64_t RemoteBitrateEstimatorAbsSendTime::TimeUntilNextProcess() {
-  const int64_t kDisabledModuleTime = 1000;
-  return kDisabledModuleTime;
 }
 
 void RemoteBitrateEstimatorAbsSendTime::TimeoutStreams(Timestamp now) {

modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h

@@ -55,12 +55,7 @@ class RemoteBitrateEstimatorAbsSendTime : public RemoteBitrateEstimator {
   void IncomingPacket(int64_t arrival_time_ms,
                       size_t payload_size,
                       const RTPHeader& header) override;
-  // This class relies on Process() being called periodically (at least once
+  TimeDelta Process() override;
-  // every other second) for streams to be timed out properly. Therefore it
-  // shouldn't be detached from the ProcessThread except if it's about to be
-  // deleted.
-  void Process() override;
-  int64_t TimeUntilNextProcess() override;
   void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
   void RemoveStream(uint32_t ssrc) override;
   bool LatestEstimate(std::vector<uint32_t>* ssrcs,

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.cc

@@ -155,22 +155,17 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
   }
 }
 
-void RemoteBitrateEstimatorSingleStream::Process() {
+TimeDelta RemoteBitrateEstimatorSingleStream::Process() {
-  {
   MutexLock lock(&mutex_);
-    UpdateEstimate(clock_->TimeInMilliseconds());
+  int64_t now_ms = clock_->TimeInMilliseconds();
-  }
+  int64_t next_process_time_ms = last_process_time_ + process_interval_ms_;
-  last_process_time_ = clock_->TimeInMilliseconds();
+  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_);
   }
 
-int64_t RemoteBitrateEstimatorSingleStream::TimeUntilNextProcess() {
+  return TimeDelta::Millis(next_process_time_ms - now_ms);
-  if (last_process_time_ < 0) {
-    return 0;
-  }
-  MutexLock lock_(&mutex_);
-  RTC_DCHECK_GT(process_interval_ms_, 0);
-  return last_process_time_ + process_interval_ms_ -
-         clock_->TimeInMilliseconds();
 }
 
 void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {

modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h

@@ -19,6 +19,8 @@
 #include <vector>
 
 #include "api/transport/field_trial_based_config.h"
+#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
 #include "modules/remote_bitrate_estimator/aimd_rate_control.h"
 #include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "rtc_base/rate_statistics.h"
@@ -46,8 +48,7 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
   void IncomingPacket(int64_t arrival_time_ms,
                       size_t payload_size,
                       const RTPHeader& header) override;
-  void Process() override;
+  TimeDelta Process() override;
-  int64_t TimeUntilNextProcess() override;
   void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override;
   void RemoveStream(uint32_t ssrc) override;
   bool LatestEstimate(std::vector<uint32_t>* ssrcs,

modules/remote_bitrate_estimator/remote_bitrate_estimator_unittest_helper.cc

@@ -271,7 +271,6 @@ bool RemoteBitrateEstimatorTest::GenerateAndProcessFrame(uint32_t ssrc,
     delete packet;
     packets.pop_front();
   }
-  if (bitrate_estimator_->TimeUntilNextProcess() <= 0)
   bitrate_estimator_->Process();
   clock_.AdvanceTimeMicroseconds(next_time_us - clock_.TimeInMicroseconds());
   return overuse;