Reenable EndToEndTest.ReceivedFecPacketsNotNacked

webrtc/video/end_to_end_tests.cc

 /*
  *  Copyright (c) 2013 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.
  */
 #include <algorithm>
@@ skipping 566 matching lines @@
     }
 
     rtc::CriticalSection crit_;
     std::set<uint32_t> protected_sequence_numbers_ GUARDED_BY(crit_);
     std::set<uint32_t> protected_timestamps_ GUARDED_BY(crit_);
   } test;
 
   RunBaseTest(&test);
 }
 
-// Flacky on all platforms. See webrtc:4328.
-TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
+TEST_F(EndToEndTest, ReceivedFecPacketsNotNacked) {
   class FecNackObserver : public test::EndToEndTest {
    public:
     FecNackObserver()
         : EndToEndTest(kDefaultTimeoutMs),
           state_(kFirstPacket),
           fec_sequence_number_(0),
           has_last_sequence_number_(false),
-          last_sequence_number_(0) {}
+          last_sequence_number_(0),
+          encoder_(VideoEncoder::Create(VideoEncoder::EncoderType::kVp8)),
+          decoder_(VP8Decoder::Create()) {}
 
    private:
     Action OnSendRtp(const uint8_t* packet, size_t length) override {
       rtc::CritScope lock_(&crit_);
       RTPHeader header;
       EXPECT_TRUE(parser_->Parse(packet, length, &header));
 
       int encapsulated_payload_type = -1;
       if (header.payloadType == kRedPayloadType) {
         encapsulated_payload_type =
@@ skipping 22 matching lines @@
           if (fec_packet) {
             state_ = kDropAllMediaPacketsUntilFec;
           } else if (header.sequenceNumber % 2 == 0) {
             return DROP_PACKET;
           }
           break;
         case kDropAllMediaPacketsUntilFec:
           if (!fec_packet)
             return DROP_PACKET;
           fec_sequence_number_ = header.sequenceNumber;
+          state_ = kDropOneMediaPacket;
+          break;
+        case kDropOneMediaPacket:
+          if (fec_packet)
+            return DROP_PACKET;
+          state_ = kPassOneMediaPacket;
+          return DROP_PACKET;
+          break;
+        case kPassOneMediaPacket:
+          if (fec_packet)
+            return DROP_PACKET;
+          // Pass one media packet after dropped packet after last fec,

[pbos-webrtc, 2016/05/27 11:53:25]

FEC

[danilchap, 2016/05/27 12:11:57]

Done.

+          // otherwise receiver might never see a seq_no after
+          // |fec_sequence_number_|
           state_ = kVerifyFecPacketNotInNackList;
           break;
         case kVerifyFecPacketNotInNackList:
           // Continue to drop packets. Make sure no frame can be decoded.
           if (fec_packet || header.sequenceNumber % 2 == 0)
             return DROP_PACKET;
           break;
       }
       return SEND_PACKET;
     }
 
     Action OnReceiveRtcp(const uint8_t* packet, size_t length) override {
       rtc::CritScope lock_(&crit_);
       if (state_ == kVerifyFecPacketNotInNackList) {
         test::RtcpPacketParser rtcp_parser;
         rtcp_parser.Parse(packet, length);
         std::vector<uint16_t> nacks = rtcp_parser.nack_item()->last_nack_list();
+        EXPECT_TRUE(std::find(
+            nacks.begin(), nacks.end(), fec_sequence_number_) == nacks.end());

[pbos-webrtc, 2016/05/27 11:53:25]

Add a << "error message" to this explaining what went wrong.

[danilchap, 2016/05/27 12:11:57]

Done.

         if (!nacks.empty() &&
             IsNewerSequenceNumber(nacks.back(), fec_sequence_number_)) {
-          EXPECT_TRUE(std::find(
-              nacks.begin(), nacks.end(), fec_sequence_number_) == nacks.end());
           observation_complete_.Set();
         }
       }
       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 = 50;
@@ skipping 13 matching lines @@
     }
 
     void ModifyVideoConfigs(
         VideoSendStream::Config* send_config,
         std::vector<VideoReceiveStream::Config>* receive_configs,
         VideoEncoderConfig* encoder_config) override {
       // Configure hybrid NACK/FEC.
       send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
       send_config->rtp.fec.red_payload_type = kRedPayloadType;
       send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
+      // Set codec to VP8, otherwise NACK/FEC hybrid will be disabled.
+      send_config->encoder_settings.encoder = encoder_.get();
+      send_config->encoder_settings.payload_name = "VP8";

[pbos-webrtc, 2016/05/27 11:53:25]

For the future (if this goes flaky on DrMemory or something) we could consider
adding a fake VP8 encoder that'll produce valid headers.

+      send_config->encoder_settings.payload_type = kFakeVideoSendPayloadType;
+      encoder_config->streams[0].min_bitrate_bps = 50000;
+      encoder_config->streams[0].max_bitrate_bps =
+          encoder_config->streams[0].target_bitrate_bps = 2000000;
+
       (*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
       (*receive_configs)[0].rtp.fec.red_payload_type = kRedPayloadType;
       (*receive_configs)[0].rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
+
+      (*receive_configs)[0].decoders.resize(1);
+      (*receive_configs)[0].decoders[0].payload_type =
+          send_config->encoder_settings.payload_type;
+      (*receive_configs)[0].decoders[0].payload_name =
+          send_config->encoder_settings.payload_name;
+      (*receive_configs)[0].decoders[0].decoder = decoder_.get();
     }
 
     void PerformTest() override {
       EXPECT_TRUE(Wait())
           << "Timed out while waiting for FEC packets to be received.";
     }
 
     enum {
       kFirstPacket,
       kDropEveryOtherPacketUntilFec,
       kDropAllMediaPacketsUntilFec,
+      kDropOneMediaPacket,
+      kPassOneMediaPacket,
       kVerifyFecPacketNotInNackList,
     } state_;
 
     rtc::CriticalSection crit_;
     uint16_t fec_sequence_number_ GUARDED_BY(&crit_);
     bool has_last_sequence_number_;
     uint16_t last_sequence_number_;
+    std::unique_ptr<webrtc::VideoEncoder> encoder_;
+    std::unique_ptr<webrtc::VideoDecoder> decoder_;
   } test;
 
   RunBaseTest(&test);
 }
 
 // This test drops second RTP packet with a marker bit set, makes sure it's
 // retransmitted and renders. Retransmission SSRCs are also checked.
 void EndToEndTest::DecodesRetransmittedFrame(bool enable_rtx, bool enable_red) {
   static const int kDroppedFrameNumber = 10;
   class RetransmissionObserver : public test::EndToEndTest,
@@ skipping 2774 matching lines @@
    private:
     bool video_observed_;
     bool audio_observed_;
     SequenceNumberUnwrapper unwrapper_;
     std::set<int64_t> received_packet_ids_;
   } test;
 
   RunBaseTest(&test);
 }
 }  // namespace webrtc