Avoid cluster set up at max bitrate at start

webrtc/modules/congestion_controller/congestion_controller.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 152 matching lines @@
       packet_router_(new PacketRouter()),
       pacer_(new PacedSender(clock_, packet_router_.get())),
       remote_bitrate_estimator_(
           new WrappingBitrateEstimator(remote_bitrate_observer, clock_)),
       bitrate_controller_(
           BitrateController::CreateBitrateController(clock_, event_log)),
       retransmission_rate_limiter_(
           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),
       last_reported_fraction_loss_(0),
       last_reported_rtt_(0),
       network_state_(kNetworkUp) {
   Init();
 }
 
 CongestionController::CongestionController(
@@ skipping 38 matching lines @@
 }
 
 void CongestionController::SetBweBitrates(int min_bitrate_bps,
                                           int start_bitrate_bps,
                                           int max_bitrate_bps) {
   ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
   bitrate_controller_->SetBitrates(start_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);
     }
   }
   max_bitrate_bps_ = max_bitrate_bps;
 
   if (remote_bitrate_estimator_)
     remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps);
   min_bitrate_bps_ = min_bitrate_bps;
@@ skipping 138 matching lines @@
 bool CongestionController::IsSendQueueFull() const {
   return pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
 }
 
 bool CongestionController::IsNetworkDown() const {
   rtc::CritScope cs(&critsect_);
   return network_state_ == kNetworkDown;
 }
 
 }  // namespace webrtc