Start probes only after network is connected.

webrtc/modules/congestion_controller/probe_controller.cc

Index: webrtc/modules/congestion_controller/probe_controller.cc
diff --git a/webrtc/modules/congestion_controller/probe_controller.cc b/webrtc/modules/congestion_controller/probe_controller.cc
index e7bf1fc9b3501d79c3c0c866556d3f366699b7d2..c539f96370553b2f7995a37602cdc196d277fd9c 100644
--- a/webrtc/modules/congestion_controller/probe_controller.cc
+++ b/webrtc/modules/congestion_controller/probe_controller.cc
@@ -14,6 +14,7 @@
 #include <initializer_list>
 
 #include "webrtc/base/logging.h"
+#include "webrtc/base/networkroute.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 
 namespace webrtc {
@@ -49,19 +50,17 @@ ProbeController::ProbeController(PacedSender* pacer, Clock* clock)
       min_bitrate_to_probe_further_bps_(kExponentialProbingDisabled),
       time_last_probing_initiated_ms_(0),
       estimated_bitrate_bps_(0),
+      start_bitrate_bps_(0),
       max_bitrate_bps_(0),
       last_alr_probing_time_(clock_->TimeInMilliseconds()) {}
 
-void ProbeController::SetBitrates(int min_bitrate_bps,
-                                  int start_bitrate_bps,
+void ProbeController::SetBitrates(int start_bitrate_bps,
+                                  int min_bitrate_bps,
                                   int max_bitrate_bps) {
   rtc::CritScope cs(&critsect_);
-  if (state_ == State::kInit) {
-    // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of
-    // 1.2 Mbps to continue probing.
-    InitiateProbing({3 * start_bitrate_bps, 6 * start_bitrate_bps},
-                    4 * start_bitrate_bps);
-  }
+
+  start_bitrate_bps_ =
+      start_bitrate_bps > 0 ? start_bitrate_bps : min_bitrate_bps;
 
   // Only do probing if:
   //   we are mid-call, which we consider to be if
@@ -69,7 +68,7 @@ void ProbeController::SetBitrates(int min_bitrate_bps,
   //     |estimated_bitrate_bps_| is valid (> 0) and
   //     the current bitrate is lower than the new |max_bitrate_bps|, and
   //     we actually want to increase the |max_bitrate_bps_|.
-  if (state_ != State::kWaitingForProbingResult &&
+  if (state_ == State::kProbingComplete &&

[stefan-webrtc, 2016/10/31 10:58:27]

Could you comment on this change?

[Sergey Ulanov, 2016/10/31 18:07:27]

We don't want to start probing here when the network is still offline and state_
= kInit. This logic supposed to apply mid-call, but previously it was applied in
kInit state, i.e. before transport is not connected, and this is not what we
want here.

[stefan-webrtc, 2016/11/02 13:27:17]

Acknowledged.

       estimated_bitrate_bps_ != 0 && estimated_bitrate_bps_ < max_bitrate_bps &&

[philipel, 2016/11/02 16:15:03]

In order for probing to start before we have a network it looks like we both
have to have an estimate (|estimated_bitrate_bps_| != 0) and we have to hand the
PacedSender a packet to send.

Do you know why that occurs?

[Sergey Ulanov, 2016/11/02 21:12:43]

Previously probing was started by InitiateProbing() we had above, which would
happen when SetBitrate() is called in kInit state.

This logic is to restart probing whenever max_bitrate is updated.

[philipel, 2016/11/08 09:48:18]

Ah, right :)

       max_bitrate_bps > max_bitrate_bps_) {
     InitiateProbing({max_bitrate_bps}, kExponentialProbingDisabled);
@@ -77,6 +76,16 @@ void ProbeController::SetBitrates(int min_bitrate_bps,
   max_bitrate_bps_ = max_bitrate_bps;
 }
 
+void ProbeController::OnNetworkRouteChanged(const rtc::NetworkRoute& route) {
+  rtc::CritScope cs(&critsect_);
+  if (route.connected && state_ == State::kInit) {

[philipel, 2016/11/02 16:15:03]

I think if we add a bool, something like |is_midcall_|, and set it to true here
we could replace the "estimated_bitrate_bps_ != 0" expression with |is_midcall_|
instead. I think it is a bit clearer and maybe make the mid-call probing a bit
more robust.

[Sergey Ulanov, 2016/11/02 21:12:43]

I'd like to work on mid-call probing later, in a separate CL. This CL just
improves behavior for the initial probing.

[philipel, 2016/11/08 09:48:18]

Acknowledged.

+    // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of
+    // 1.2 Mbps to continue probing.
+    InitiateProbing({3 * start_bitrate_bps_, 6 * start_bitrate_bps_},
+                    4 * start_bitrate_bps_);
+  }
+}
+
 void ProbeController::SetEstimatedBitrate(int bitrate_bps) {
   rtc::CritScope cs(&critsect_);
   if (state_ == State::kWaitingForProbingResult) {