Remove ViEEncoder::SetNetworkStatus

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 f3a8432310cb257ddc33bd978b623a7c2756422b..2c6e3c94b73e7d230050ad87c6868aa8533c9ccf 100644
--- a/webrtc/modules/congestion_controller/congestion_controller.cc
+++ b/webrtc/modules/congestion_controller/congestion_controller.cc
@@ -151,7 +151,9 @@ CongestionController::CongestionController(
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
-      send_queue_is_full_(false) {
+      send_queue_is_full_(false),
+      network_state_(kNetworkUp),
+      network_state_changed_(false) {
   Init();
 }
 
@@ -173,7 +175,9 @@ CongestionController::CongestionController(
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
-      send_queue_is_full_(false) {
+      send_queue_is_full_(false),
+      network_state_(kNetworkUp),
+      network_state_changed_(false) {
   Init();
 }
 
@@ -251,6 +255,9 @@ void CongestionController::SignalNetworkState(NetworkState state) {
   } else {
     pacer_->Pause();
   }
+  rtc::CritScope cs(&critsect_);
+  network_state_ = state;
+  network_state_changed_ = true;
 }
 
 void CongestionController::OnSentPacket(const rtc::SentPacket& sent_packet) {
@@ -278,14 +285,19 @@ void CongestionController::MaybeTriggerOnNetworkChanged() {
   uint32_t bitrate_bps;
   uint8_t fraction_loss;
   int64_t rtt;
-  bool network_changed = bitrate_controller_->GetNetworkParameters(
+  bool estimate_changed = bitrate_controller_->GetNetworkParameters(
       &bitrate_bps, &fraction_loss, &rtt);
-  if (network_changed)
+  if (estimate_changed)
     pacer_->SetEstimatedBitrate(bitrate_bps);
   bool send_queue_is_full =
       pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
-  bitrate_bps = send_queue_is_full ? 0 : bitrate_bps;
-  if ((network_changed && !send_queue_is_full) ||
+
+  bool network_state_changed = false;
+  bitrate_bps = IsNetworkDown(&network_state_changed) || send_queue_is_full
+                    ? 0
+                    : bitrate_bps;
+
+  if ((estimate_changed && !send_queue_is_full) || network_state_changed ||
       UpdateSendQueueStatus(send_queue_is_full)) {

[stefan-webrtc, 2016/05/04 07:28:58]

I don't like this if statement. Can we do something to make it easier to read?
Maybe move out UpdateSendQueueStatus()? It's not even clear to me what that
method would return based on just reading the if statement.

[perkj_webrtc, 2016/05/04 10:00:15]

I agree - and its not even 100% correct. PTAL

     observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt);
   }
@@ -298,4 +310,11 @@ bool CongestionController::UpdateSendQueueStatus(bool send_queue_is_full) {
   return result;
 }
 
+bool CongestionController::IsNetworkDown(bool* changed) {
+  rtc::CritScope cs(&critsect_);
+  *changed = network_state_changed_;
+  network_state_changed_ = false;
+  return network_state_ == kNetworkDown;
+}
+
 }  // namespace webrtc