Revert of Don't send FEC for H.264 with NACK enabled.

webrtc/video/video_send_stream_tests.cc

Index: webrtc/video/video_send_stream_tests.cc
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index 01267735e073a2e35677d4aef72686caa26f6cf1..00aa2a901815cb4b0be6296e55e7ade6dae09364 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -308,55 +308,48 @@
   StatisticianMap stats_map_;
 };
 
-class FecObserver : public test::EndToEndTest {
+class FecObserver : public test::SendTest {
  public:
-  FecObserver(bool header_extensions_enabled,
-              bool use_nack,
-              bool expect_red,
-              const std::string& codec)
-      : EndToEndTest(VideoSendStreamTest::kDefaultTimeoutMs),
-        payload_name_(codec),
-        use_nack_(use_nack),
-        expect_red_(expect_red),
+  explicit FecObserver(bool header_extensions_enabled)
+      : SendTest(VideoSendStreamTest::kDefaultTimeoutMs),
         send_count_(0),
         received_media_(false),
         received_fec_(false),
-        header_extensions_enabled_(header_extensions_enabled) {
-    if (codec == "H264") {
-      encoder_.reset(new test::FakeH264Encoder(Clock::GetRealTimeClock()));
-    } else if (codec == "VP8") {
-      encoder_.reset(VideoEncoder::Create(VideoEncoder::EncoderType::kVp8));
-    } else if (codec == "VP9") {
-      encoder_.reset(VideoEncoder::Create(VideoEncoder::EncoderType::kVp9));
-    } else {
-      RTC_NOTREACHED();
-    }
-  }
+        header_extensions_enabled_(header_extensions_enabled) {}
 
  private:
   Action OnSendRtp(const uint8_t* packet, size_t length) override {
     RTPHeader header;
     EXPECT_TRUE(parser_->Parse(packet, length, &header));
 
-    ++send_count_;
+    // Send lossy receive reports to trigger FEC enabling.
+    if (send_count_++ % 2 != 0) {
+      // Receive statistics reporting having lost 50% of the packets.
+      FakeReceiveStatistics lossy_receive_stats(
+          VideoSendStreamTest::kVideoSendSsrcs[0], header.sequenceNumber,
+          send_count_ / 2, 127);
+      RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(),
+                             &lossy_receive_stats, nullptr, nullptr,
+                             transport_adapter_.get());
+
+      rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
+      rtcp_sender.SetRemoteSSRC(VideoSendStreamTest::kVideoSendSsrcs[0]);
+
+      RTCPSender::FeedbackState feedback_state;
+
+      EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr));
+    }
+
     int encapsulated_payload_type = -1;
     if (header.payloadType == VideoSendStreamTest::kRedPayloadType) {
-      EXPECT_TRUE(expect_red_);
       encapsulated_payload_type = static_cast<int>(packet[header.headerLength]);
       if (encapsulated_payload_type !=
-          VideoSendStreamTest::kFakeVideoSendPayloadType) {
+          VideoSendStreamTest::kFakeVideoSendPayloadType)
         EXPECT_EQ(VideoSendStreamTest::kUlpfecPayloadType,
                   encapsulated_payload_type);
-      }
     } else {
       EXPECT_EQ(VideoSendStreamTest::kFakeVideoSendPayloadType,
                 header.payloadType);
-      if (static_cast<size_t>(header.headerLength + header.paddingLength) <
-          length) {
-        // Not padding-only, media received outside of RED.
-        EXPECT_FALSE(expect_red_);
-        received_media_ = true;
-      }
     }
 
     if (header_extensions_enabled_) {
@@ -386,25 +379,12 @@
       }
     }
 
-    if (send_count_ > 100 && received_media_) {
-      if (received_fec_ || !expect_red_)
-        observation_complete_.Set();
-    }
+    if (received_media_ && received_fec_ && send_count_ > 100)
+      observation_complete_.Set();
 
     prev_header_ = header;
 
     return SEND_PACKET;
-  }
-
-  test::PacketTransport* CreateSendTransport(Call* sender_call) override {
-    // At low RTT (< kLowRttNackMs) -> NACK only, no FEC.
-    // Configure some network delay.
-    const int kNetworkDelayMs = 100;
-    FakeNetworkPipe::Config config;
-    config.loss_percent = 50;
-    config.queue_delay_ms = kNetworkDelayMs;
-    return new test::PacketTransport(sender_call, this,
-                                     test::PacketTransport::kSender, config);
   }
 
   void ModifyVideoConfigs(
@@ -414,17 +394,10 @@
     transport_adapter_.reset(
         new internal::TransportAdapter(send_config->send_transport));
     transport_adapter_->Enable();
-    if (use_nack_) {
-      send_config->rtp.nack.rtp_history_ms =
-          (*receive_configs)[0].rtp.nack.rtp_history_ms =
-              VideoSendStreamTest::kNackRtpHistoryMs;
-    }
-    send_config->encoder_settings.encoder = encoder_.get();
-    send_config->encoder_settings.payload_name = payload_name_;
     send_config->rtp.fec.red_payload_type =
-            VideoSendStreamTest::kRedPayloadType;
+        VideoSendStreamTest::kRedPayloadType;
     send_config->rtp.fec.ulpfec_payload_type =
-            VideoSendStreamTest::kUlpfecPayloadType;
+        VideoSendStreamTest::kUlpfecPayloadType;
     if (header_extensions_enabled_) {
       send_config->rtp.extensions.push_back(RtpExtension(
           RtpExtension::kAbsSendTime, test::kAbsSendTimeExtensionId));
@@ -432,10 +405,6 @@
           RtpExtension(RtpExtension::kTransportSequenceNumber,
                        test::kTransportSequenceNumberExtensionId));
     }
-    (*receive_configs)[0].rtp.fec.red_payload_type =
-        send_config->rtp.fec.red_payload_type;
-    (*receive_configs)[0].rtp.fec.ulpfec_payload_type =
-        send_config->rtp.fec.ulpfec_payload_type;
   }
 
   void PerformTest() override {
@@ -443,10 +412,6 @@
   }
 
   rtc::scoped_ptr<internal::TransportAdapter> transport_adapter_;
-  rtc::scoped_ptr<VideoEncoder> encoder_;
-  const std::string payload_name_;
-  const bool use_nack_;
-  const bool expect_red_;
   int send_count_;
   bool received_media_;
   bool received_fec_;
@@ -455,37 +420,14 @@
 };
 
 TEST_F(VideoSendStreamTest, SupportsFecWithExtensions) {
-  FecObserver test(true, false, true, "VP8");
+  FecObserver test(true);
+
   RunBaseTest(&test);
 }
 
 TEST_F(VideoSendStreamTest, SupportsFecWithoutExtensions) {
-  FecObserver test(false, false, true, "VP8");
-  RunBaseTest(&test);
-}
-
-// The FEC scheme used is not efficient for H264, so we should not use RED/FEC
-// since we'll still have to re-request FEC packets, effectively wasting
-// bandwidth since the receiver has to wait for FEC retransmissions to determine
-// that the received state is actually decodable.
-TEST_F(VideoSendStreamTest, DoesNotUtilizeRedForH264WithNackEnabled) {
-  FecObserver test(false, true, false, "H264");
-  RunBaseTest(&test);
-}
-
-// Without retransmissions FEC for H264 is fine.
-TEST_F(VideoSendStreamTest, DoesUtilizeRedForH264WithoutNackEnabled) {
-  FecObserver test(false, false, true, "H264");
-  RunBaseTest(&test);
-}
-
-TEST_F(VideoSendStreamTest, DoesUtilizeRedForVp8WithNackEnabled) {
-  FecObserver test(false, true, true, "VP8");
-  RunBaseTest(&test);
-}
-
-TEST_F(VideoSendStreamTest, DoesUtilizeRedForVp9WithNackEnabled) {
-  FecObserver test(false, true, true, "VP9");
+  FecObserver test(false);
+
   RunBaseTest(&test);
 }