Move delay_based_bwe_ into CongestionController

webrtc/modules/congestion_controller/include/congestion_controller.h

Index: webrtc/modules/congestion_controller/include/congestion_controller.h
diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h
index 500966b7c1ab0fc7a0fc475a9c98f8a959d3df86..017152592be2cbce58dc125c51e0dcf50f08e498 100644
--- a/webrtc/modules/congestion_controller/include/congestion_controller.h
+++ b/webrtc/modules/congestion_controller/include/congestion_controller.h
@@ -17,6 +17,7 @@
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/criticalsection.h"
 #include "webrtc/common_types.h"
+#include "webrtc/modules/congestion_controller/delay_based_bwe.h"
 #include "webrtc/modules/congestion_controller/transport_feedback_adapter.h"
 #include "webrtc/modules/include/module.h"
 #include "webrtc/modules/include/module_common_types.h"
@@ -37,9 +38,10 @@ class RateLimiter;
 class RemoteBitrateEstimator;
 class RemoteBitrateObserver;
 class RtcEventLog;
-class TransportFeedbackObserver;
 
-class CongestionController : public CallStatsObserver, public Module {
+class CongestionController : public CallStatsObserver,
+                             public Module,
+                             public TransportFeedbackObserver {
  public:
   // Observer class for bitrate changes announced due to change in bandwidth
   // estimate or due to that the send pacer is full. Fraction loss and rtt is
@@ -66,7 +68,8 @@ class CongestionController : public CallStatsObserver, public Module {
                        RemoteBitrateObserver* remote_bitrate_observer,
                        RtcEventLog* event_log,
                        PacketRouter* packet_router,
-                       std::unique_ptr<PacedSender> pacer);
+                       std::unique_ptr<PacedSender> pacer,
+                       std::unique_ptr<BitrateController> bitrate_controller);
   virtual ~CongestionController();
 
   virtual void OnReceivedPacket(int64_t arrival_time_ms,
@@ -91,7 +94,6 @@ class CongestionController : public CallStatsObserver, public Module {
   // TODO(nisse): Delete this accessor function. The pacer should be
   // internal to the congestion controller.
   virtual PacedSender* pacer() { return pacer_.get(); }
-  virtual TransportFeedbackObserver* GetTransportFeedbackObserver();
   RateLimiter* GetRetransmissionRateLimiter();
   void EnablePeriodicAlrProbing(bool enable);
 
@@ -117,6 +119,13 @@ class CongestionController : public CallStatsObserver, public Module {
   int64_t TimeUntilNextProcess() override;
   void Process() override;
 
+  // Implements TransportFeedbackObserver.
+  void AddPacket(uint16_t sequence_number,
+                 size_t length,
+                 const PacedPacketInfo& pacing_info) override;
+  void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override;
+  std::vector<PacketFeedback> GetTransportFeedbackVector() const override;
+
  private:
   class WrappingBitrateEstimator : public RemoteBitrateEstimator {
    public:
@@ -165,6 +174,7 @@ class CongestionController : public CallStatsObserver, public Module {
                                            int64_t rtt);
   Clock* const clock_;

[minyue-webrtc, 2017/03/02 19:53:58]

if you can be nice, add "const"

[elad.alon_webrtc.org, 2017/03/08 18:25:09]

This one is a bit less straight-forward than you think, because it's propagated
to many other classes, whose constructors, etc., expect a |Clock*|, not a |const
Clock*|. I'll do that in a separate CL, so as not to overwhelm this CL with
unrelated changes.

[elad.alon_webrtc.org, 2017/03/08 19:03:55]

Done separately in the following CL: https://codereview.webrtc.org/2735423002

   Observer* const observer_;

[minyue-webrtc, 2017/03/02 19:53:58]

here too

[elad.alon_webrtc.org, 2017/03/08 18:25:10]

Done.

+  RtcEventLog* const event_log_;

[minyue-webrtc, 2017/03/02 19:53:58]

const RtcEventLog* const

[elad.alon_webrtc.org, 2017/03/08 18:25:09]

Can't, because RtcEventLog::LogDelayBasedBweUpdate() isn't const.

   PacketRouter* const packet_router_;

[minyue-webrtc, 2017/03/02 19:53:58]

here too

[elad.alon_webrtc.org, 2017/03/08 18:25:10]

Similar issue (due to PacketRouter::SendFeedback() this time).

   const std::unique_ptr<PacedSender> pacer_;
   const std::unique_ptr<BitrateController> bitrate_controller_;
@@ -180,6 +190,11 @@ class CongestionController : public CallStatsObserver, public Module {
   uint8_t last_reported_fraction_loss_ GUARDED_BY(critsect_);
   int64_t last_reported_rtt_ GUARDED_BY(critsect_);
   NetworkState network_state_ GUARDED_BY(critsect_);
+  rtc::CriticalSection bwe_lock_;
+  std::unique_ptr<DelayBasedBwe> delay_based_bwe_ GUARDED_BY(&bwe_lock_);

[stefan-webrtc, 2017/03/09 09:41:02]

Why is &bwe_lock_ needed here but not &critsect_ above?

[elad.alon_webrtc.org, 2017/03/09 12:03:53]

It works with either. I happened to have the ampersand because I copy/pasted
(typing is annoying...) from a different file. Ampersand removed.

+
+  // TODO(elad.alon): Subsequent CL re-introduces the thread-checking that
+  // was inside of TransportFeedbackAdapter before.
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(CongestionController);
 };