Revert of Remove ViEEncoder::SetNetworkStatus (patchset #11 id:200001 of https://codereview.webrtc.org/1932683002/ )

Reason for revert:
Breaks Chrome FYI using H264.
Need to investigate.

https://build.chromium.org/p/chromium.webrtc.fyi/builders/Win7%20Tester/builds/4170

Original issue's description:
> Remove ViEEncoder::SetNetworkStatus
>
> This cl removed ViEEncoder::SetNetworkStatus. Instead the PacedSender will report that frames can not be sent when the network is down and the BitrateController will report an estimated available bandwidth of 0 bps.
>
> BUG=webrtc:5687
> NOTRY=True
>
> Committed: https://crrev.com/50b5c3be844ef571a28b2681c549443a26735d72
> Cr-Commit-Position: refs/heads/master@{#12699}

TBR=stefan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5687

Review-Url: https://codereview.webrtc.org/1978783002
Cr-Commit-Position: refs/heads/master@{#12715}

webrtc/modules/congestion_controller/congestion_controller.cc

@@ -151,10 +151,7 @@ CongestionController::CongestionController(
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
-      last_reported_bitrate_bps_(0),
-      last_reported_fraction_loss_(0),
-      last_reported_rtt_(0),
-      network_state_(kNetworkUp) {
+      send_queue_is_full_(false) {
   Init();
 }
 
@@ -173,10 +170,7 @@ CongestionController::CongestionController(
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
-      last_reported_bitrate_bps_(0),
-      last_reported_fraction_loss_(0),
-      last_reported_rtt_(0),
-      network_state_(kNetworkUp) {
+      send_queue_is_full_(false) {
   Init();
 }
 
@@ -198,10 +192,7 @@ CongestionController::CongestionController(
       remote_estimator_proxy_(clock_, packet_router_.get()),
       transport_feedback_adapter_(bitrate_controller_.get(), clock_),
       min_bitrate_bps_(RemoteBitrateEstimator::kDefaultMinBitrateBps),
-      last_reported_bitrate_bps_(0),
-      last_reported_fraction_loss_(0),
-      last_reported_rtt_(0),
-      network_state_(kNetworkUp) {
+      send_queue_is_full_(false) {
   Init();
 }
 
@@ -274,11 +265,6 @@ void CongestionController::SignalNetworkState(NetworkState state) {
   } else {
     pacer_->Pause();
   }
-  {
-    rtc::CritScope cs(&critsect_);
-    network_state_ = state;
-  }
-  MaybeTriggerOnNetworkChanged();
 }
 
 void CongestionController::OnSentPacket(const rtc::SentPacket& sent_packet) {
@@ -311,40 +297,24 @@ void CongestionController::MaybeTriggerOnNetworkChanged() {
   uint32_t bitrate_bps;
   uint8_t fraction_loss;
   int64_t rtt;
-  bool estimate_changed = bitrate_controller_->GetNetworkParameters(
+  bool network_changed = bitrate_controller_->GetNetworkParameters(
       &bitrate_bps, &fraction_loss, &rtt);
-  if (estimate_changed)
+  if (network_changed)
     pacer_->SetEstimatedBitrate(bitrate_bps);
-
-  bitrate_bps = IsNetworkDown() || IsSendQueueFull() ? 0 : bitrate_bps;
-
-  if (HasNetworkParametersToReportChanged(bitrate_bps, fraction_loss, rtt)) {
+  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) ||
+      UpdateSendQueueStatus(send_queue_is_full)) {
     observer_->OnNetworkChanged(bitrate_bps, fraction_loss, rtt);
   }
 }
 
-bool CongestionController::HasNetworkParametersToReportChanged(
-    uint32_t bitrate_bps,
-    uint8_t fraction_loss,
-    int64_t rtt) {
+bool CongestionController::UpdateSendQueueStatus(bool send_queue_is_full) {
   rtc::CritScope cs(&critsect_);
-  bool changed =
-      last_reported_bitrate_bps_ != bitrate_bps ||
-      (bitrate_bps > 0 && (last_reported_fraction_loss_ != fraction_loss ||
-                           last_reported_rtt_ != rtt));
-  last_reported_bitrate_bps_ = bitrate_bps;
-  last_reported_fraction_loss_ = fraction_loss;
-  last_reported_rtt_ = rtt;
-  return changed;
-}
-
-bool CongestionController::IsSendQueueFull() const {
-  return pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
-}
-
-bool CongestionController::IsNetworkDown() const {
-  rtc::CritScope cs(&critsect_);
-  return network_state_ == kNetworkDown;
+  bool result = send_queue_is_full_ != send_queue_is_full;
+  send_queue_is_full_ = send_queue_is_full;
+  return result;
 }
 
 }  // namespace webrtc

webrtc/modules/congestion_controller/congestion_controller_unittest.cc

@@ -113,45 +113,5 @@ TEST_F(CongestionControllerTest, OnSendQueueFullAndEstimateChange) {
   controller_->Process();
 }
 
-TEST_F(CongestionControllerTest, SignalNetworkState) {
-  EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
-  controller_->SignalNetworkState(kNetworkDown);
-
-  EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _));
-  controller_->SignalNetworkState(kNetworkUp);
-
-  EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
-  controller_->SignalNetworkState(kNetworkDown);
-}
-
-TEST_F(CongestionControllerTest,
-       SignalNetworkStateAndQueueIsFullAndEstimateChange) {
-  // Send queue is full
-  EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
-      .WillRepeatedly(Return(PacedSender::kMaxQueueLengthMs + 1));
-  EXPECT_CALL(observer_, OnNetworkChanged(0, _, _));
-  controller_->Process();
-
-  // Queue is full and network is down. Expect no bitrate change.
-  controller_->SignalNetworkState(kNetworkDown);
-  controller_->Process();
-
-  // Queue is full but network is up. Expect no bitrate change.
-  controller_->SignalNetworkState(kNetworkUp);
-  controller_->Process();
-
-  // Receive new estimate but let the queue still be full.
-  EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps * 2));
-  bandwidth_observer_->OnReceivedEstimatedBitrate(kInitialBitrateBps * 2);
-  clock_.AdvanceTimeMilliseconds(25);
-  controller_->Process();
-
-  // Let the pacer not be full next time the controller checks.
-  EXPECT_CALL(*pacer_, ExpectedQueueTimeMs())
-      .WillOnce(Return(PacedSender::kMaxQueueLengthMs - 1));
-  EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _));
-  controller_->Process();
-}
-
 }  // namespace test
 }  // namespace webrtc

webrtc/modules/congestion_controller/include/congestion_controller.h

@@ -95,12 +95,10 @@ class CongestionController : public CallStatsObserver, public Module {
  private:
   void Init();
   void MaybeTriggerOnNetworkChanged();
+  // Updates |send_queue_is_full_|. Returns true if |send_queue_is_full_|
+  // has changed.
+  bool UpdateSendQueueStatus(bool send_queue_is_full);
 
-  bool IsSendQueueFull() const;
-  bool IsNetworkDown() const;
-  bool HasNetworkParametersToReportChanged(uint32_t bitrate_bps,
-                                           uint8_t fraction_loss,
-                                           int64_t rtt);
   Clock* const clock_;
   Observer* const observer_;
   const std::unique_ptr<PacketRouter> packet_router_;
@@ -111,10 +109,7 @@ class CongestionController : public CallStatsObserver, public Module {
   TransportFeedbackAdapter transport_feedback_adapter_;
   int min_bitrate_bps_;
   rtc::CriticalSection critsect_;
-  uint32_t last_reported_bitrate_bps_ GUARDED_BY(critsect_);
-  uint8_t last_reported_fraction_loss_ GUARDED_BY(critsect_);
-  int64_t last_reported_rtt_ GUARDED_BY(critsect_);
-  NetworkState network_state_ GUARDED_BY(critsect_);
+  bool send_queue_is_full_ GUARDED_BY(critsect_);
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(CongestionController);
 };

webrtc/video/video_send_stream.cc

@@ -491,6 +491,21 @@ VideoSendStream::~VideoSendStream() {
   }
 }
 
+void VideoSendStream::SignalNetworkState(NetworkState state) {
+  // When network goes up, enable RTCP status before setting transmission state.
+  // When it goes down, disable RTCP afterwards. This ensures that any packets
+  // sent due to the network state changed will not be dropped.
+  if (state == kNetworkUp) {
+    for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
+      rtp_rtcp->SetRTCPStatus(config_.rtp.rtcp_mode);
+  }
+  vie_encoder_.SetNetworkTransmissionState(state == kNetworkUp);
+  if (state == kNetworkDown) {
+    for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
+      rtp_rtcp->SetRTCPStatus(RtcpMode::kOff);
+  }
+}
+
 bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
   for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_)
     rtp_rtcp->IncomingRtcpPacket(packet, length);
@@ -764,13 +779,6 @@ std::map<uint32_t, RtpState> VideoSendStream::GetRtpStates() const {
   return rtp_states;
 }
 
-void VideoSendStream::SignalNetworkState(NetworkState state) {
-  for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) {
-    rtp_rtcp->SetRTCPStatus(state == kNetworkUp ? config_.rtp.rtcp_mode
-                                                : RtcpMode::kOff);
-  }
-}
-
 int VideoSendStream::GetPaddingNeededBps() const {
   return vie_encoder_.GetPaddingNeededBps();
 }

webrtc/video/vie_encoder.cc

@@ -43,6 +43,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores,
       encoder_config_(),
       min_transmit_bitrate_bps_(0),
       last_observed_bitrate_bps_(0),
+      network_is_transmitting_(true),
       encoder_paused_(true),
       encoder_paused_and_dropped_frame_(false),
       module_process_thread_(module_process_thread),
@@ -63,6 +64,13 @@ ViEEncoder::~ViEEncoder() {
   module_process_thread_->DeRegisterModule(&video_sender_);
 }
 
+void ViEEncoder::SetNetworkTransmissionState(bool is_transmitting) {
+  {
+    rtc::CritScope lock(&data_cs_);
+    network_is_transmitting_ = is_transmitting;
+  }
+}
+
 void ViEEncoder::Pause() {
   rtc::CritScope lock(&data_cs_);
   encoder_paused_ = true;
@@ -191,9 +199,13 @@ int ViEEncoder::GetPaddingNeededBps() const {
 bool ViEEncoder::EncoderPaused() const {
   // Pause video if paused by caller or as long as the network is down or the
   // pacer queue has grown too large in buffered mode.
-  // If the pacer queue has grown to large or the network is down,
-  // last_observed_bitrate_bps_ will be 0.
-  return encoder_paused_ || video_suspended_ || last_observed_bitrate_bps_ == 0;
+  if (encoder_paused_) {
+    return true;
+  }
+  if (video_suspended_ || last_observed_bitrate_bps_ == 0) {
+    return true;
+  }
+  return !network_is_transmitting_;
 }
 
 void ViEEncoder::TraceFrameDropStart() {

webrtc/video/vie_encoder.h

@@ -63,6 +63,8 @@ class ViEEncoder : public VideoEncoderRateObserver,
 
   vcm::VideoSender* video_sender();
 
+  void SetNetworkTransmissionState(bool is_transmitting);
+
   // Returns the id of the owning channel.
   int Owner() const;
 
@@ -135,6 +137,7 @@ class ViEEncoder : public VideoEncoderRateObserver,
   VideoCodec encoder_config_ GUARDED_BY(data_cs_);
   int min_transmit_bitrate_bps_ GUARDED_BY(data_cs_);
   uint32_t last_observed_bitrate_bps_ GUARDED_BY(data_cs_);
+  bool network_is_transmitting_ GUARDED_BY(data_cs_);
   bool encoder_paused_ GUARDED_BY(data_cs_);
   bool encoder_paused_and_dropped_frame_ GUARDED_BY(data_cs_);