Avoid cluster set up at max bitrate at start

webrtc/modules/congestion_controller/congestion_controller.cc

Index: webrtc/modules/congestion_controller/congestion_controller.cc
diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc
index 68c306ce10077a93bbbf8e6c792b8dd20121970d..ee985ec043d8cebecb6ea38750c044b8d9c9b91a 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -170,6 +170,7 @@ CongestionController::CongestionController(
           new RateLimiter(clock, kRetransmitWindowSizeMs)),
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
+      start_bitrate_bps_(0),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
       max_bitrate_bps_(0),
       last_reported_bitrate_bps_(0),
@@ -228,14 +229,19 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps,
                                    min_bitrate_bps,
                                    max_bitrate_bps);
 
+  if (start_bitrate_bps > 0)
+    start_bitrate_bps_ = start_bitrate_bps;
   {
     // Only do probing if:
-    //   - we are mid-call, which we consider to be if
-    //     |last_reported_bitrate_bps_| != 0, and
-    //   - the current bitrate is lower than the new |max_bitrate_bps|, and
-    //   - we actually want to increase the |max_bitrate_bps_|.
+    //   we are mid-call, which we consider to be if
+    //     |last_reported_bitrate_bps_| is valid (> 0) and
+    //     |last_reported_bitrate_bps_| != |start_bitrate_bps_|, and
+    //     the current bitrate is lower than the new |max_bitrate_bps|, and
+    //     we actually want to increase the |max_bitrate_bps_|.
     rtc::CritScope cs(&critsect_);
-    if (last_reported_bitrate_bps_ != 0 &&
+    if (last_reported_bitrate_bps_ &&

[stefan-webrtc, 2016/09/02 13:49:15]

!= 0

[Irfan, 2016/09/06 20:32:12]

Done.

+        last_reported_bitrate_bps_ !=
+            static_cast<uint32_t>(start_bitrate_bps_) &&

[stefan-webrtc, 2016/09/02 13:49:15]

Seems a bit arbitrary to not allow probing if the last reported bitrate happens
to be equal to the start bitrate? That could happen mid call, right?

[Irfan, 2016/09/06 20:32:12]

Right, I mention this in the commit details.

The current way to determine whether we are in mid-call is to check that the
|last_reported_bitrate_bps| is not 0. This does not work as intended right now
since it starts at |start_bitrate_bps|. This fix is avoiding the unnecessary
probing at |max_bitrate_bps| (which happens to be 10 Mbps for remoting and yeti)
in the beginning of a session and basically improving over the current state.

The real fix should be to determine mid-call correctly (through an explicit flag
perhaps).

[stefan-webrtc, 2016/09/07 13:42:01]

So shouldn't this be triggered if initial_probing_triggered_ is false then? I
would really prefer if we did this in a nicer way...

         last_reported_bitrate_bps_ < static_cast<uint32_t>(max_bitrate_bps) &&
         max_bitrate_bps > max_bitrate_bps_) {
       pacer_->CreateProbeCluster(max_bitrate_bps, 5);