Re-enable ULPFEC end-to-end test.

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 558 matching lines @@
 
     uint32_t local_ssrc_;
     uint32_t remote_ssrc_;
     Transport* receive_transport_;
     rtc::Optional<uint16_t> sequence_number_to_retransmit_;
   } test;
 
   RunBaseTest(&test);
 }
 
-// Disabled due to flakyness, see
-// https://bugs.chromium.org/p/webrtc/issues/detail?id=7047.
-TEST_F(EndToEndTest, DISABLED_ReceivesUlpfec) {
+TEST_F(EndToEndTest, ReceivesUlpfec) {
   class UlpfecRenderObserver : public test::EndToEndTest,
                                public rtc::VideoSinkInterface<VideoFrame> {
    public:
     UlpfecRenderObserver()
         : EndToEndTest(kDefaultTimeoutMs),
+          encoder_(VP8Encoder::Create()),
           random_(0xcafef00d1),
           num_packets_sent_(0) {}
 
    private:
     Action OnSendRtp(const uint8_t* packet, size_t length) override {
       rtc::CritScope lock(&crit_);
       RTPHeader header;
       EXPECT_TRUE(parser_->Parse(packet, length, &header));
 
-      EXPECT_TRUE(header.payloadType == kFakeVideoSendPayloadType ||
+      EXPECT_TRUE(header.payloadType == kVideoSendPayloadType ||
                   header.payloadType == kRedPayloadType)
           << "Unknown payload type received.";
       EXPECT_EQ(kVideoSendSsrcs[0], header.ssrc) << "Unknown SSRC received.";
 
       // Parse RED header.
       int encapsulated_payload_type = -1;
       if (header.payloadType == kRedPayloadType) {
         encapsulated_payload_type =
             static_cast<int>(packet[header.headerLength]);
 
-        EXPECT_TRUE(encapsulated_payload_type == kFakeVideoSendPayloadType ||
+        EXPECT_TRUE(encapsulated_payload_type == kVideoSendPayloadType ||
                     encapsulated_payload_type == kUlpfecPayloadType)
             << "Unknown encapsulated payload type received.";
       }
 
-      // To reduce test flakiness, always let ULPFEC packets through.
+      // To minimize test flakiness, always let ULPFEC packets through.
       if (encapsulated_payload_type == kUlpfecPayloadType) {
         return SEND_PACKET;
       }
 
       // Simulate 5% video packet loss after rampup period. Record the
       // corresponding timestamps that were dropped.
       if (num_packets_sent_++ > 100 && random_.Rand(1, 100) <= 5) {
-        if (encapsulated_payload_type == kFakeVideoSendPayloadType) {
+        if (encapsulated_payload_type == kVideoSendPayloadType) {
           dropped_sequence_numbers_.insert(header.sequenceNumber);
           dropped_timestamps_.insert(header.timestamp);
         }
-
         return DROP_PACKET;
       }
 
       return SEND_PACKET;
     }
 
     void OnFrame(const VideoFrame& video_frame) override {
       rtc::CritScope lock(&crit_);
       // Rendering frame with timestamp of packet that was dropped -> FEC
       // protection worked.
       auto it = dropped_timestamps_.find(video_frame.timestamp());
       if (it != dropped_timestamps_.end()) {
         observation_complete_.Set();
       }
     }
 
     void ModifyVideoConfigs(
         VideoSendStream::Config* send_config,
         std::vector<VideoReceiveStream::Config>* receive_configs,
         VideoEncoderConfig* encoder_config) override {
-      send_config->rtp.extensions.push_back(
-          RtpExtension(RtpExtension::kTransportSequenceNumberUri,
-                       test::kTransportSequenceNumberExtensionId));
+      // Use VP8 instead of FAKE, since the latter does not have PictureID
+      // in the packetization headers.

[philipel, 2017/03/20 12:09:07]

I can't see why picture id matters in the case of FEC?

[brandtr_google, 2017/03/20 12:12:09]

I believe it has something to do with key frame requests from the JB, which seem
to be important for this particular test. When using the FAKE codec, a single
lost packet leads to a very long delay (~ seconds) until the next frame is
rendered. This doesn't seem to happen with VP8 packetization. The only
difference I can think of is that the FAKE encoder does not have PictureIDs. For
VP8, PictureID is enabled by default, right?

[brandtr_google, 2017/03/20 12:14:14]

I guess the main point is that switching to VP8 seems to make this test less
flaky. Since this is just a correctness test, I haven't studied the reason for
this in depth :)

+      send_config->encoder_settings.encoder = encoder_.get();
+      send_config->encoder_settings.payload_name = "VP8";
+      send_config->encoder_settings.payload_type = kVideoSendPayloadType;
+      VideoReceiveStream::Decoder decoder =
+          test::CreateMatchingDecoder(send_config->encoder_settings);
+      decoder_.reset(decoder.decoder);
+      (*receive_configs)[0].decoders.clear();
+      (*receive_configs)[0].decoders.push_back(decoder);
+
+      // Enable ULPFEC over RED.
       send_config->rtp.ulpfec.red_payload_type = kRedPayloadType;
       send_config->rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType;
-
-      (*receive_configs)[0].rtp.transport_cc = true;
       (*receive_configs)[0].rtp.ulpfec.red_payload_type = kRedPayloadType;
       (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType;
+
       (*receive_configs)[0].renderer = this;
     }
 
     void PerformTest() override {
       EXPECT_TRUE(Wait())
           << "Timed out waiting for dropped frames to be rendered.";
     }
 
     rtc::CriticalSection crit_;
+    std::unique_ptr<VideoEncoder> encoder_;
+    std::unique_ptr<VideoDecoder> decoder_;
     std::set<uint32_t> dropped_sequence_numbers_ GUARDED_BY(crit_);
     // Several packets can have the same timestamp.
     std::multiset<uint32_t> dropped_timestamps_ GUARDED_BY(crit_);
     Random random_;
-    int num_packets_sent_;
+    int num_packets_sent_ GUARDED_BY(crit_);
   } test;
 
   RunBaseTest(&test);
 }
 
 class FlexfecRenderObserver : public test::EndToEndTest,
                               public rtc::VideoSinkInterface<VideoFrame> {
  public:
   static constexpr uint32_t kVideoLocalSsrc = 123;
   static constexpr uint32_t kFlexfecLocalSsrc = 456;
@@ skipping 108 matching lines @@
       if (!expect_flexfec_rtcp_ || received_flexfec_rtcp_) {
         observation_complete_.Set();
       }
     }
   }
 
   void ModifyVideoConfigs(
       VideoSendStream::Config* send_config,
       std::vector<VideoReceiveStream::Config>* receive_configs,
       VideoEncoderConfig* encoder_config) override {
-    send_config->rtp.extensions.push_back(
-        RtpExtension(RtpExtension::kTransportSequenceNumberUri,
-                     test::kTransportSequenceNumberExtensionId));
-
     (*receive_configs)[0].rtp.local_ssrc = kVideoLocalSsrc;
-    (*receive_configs)[0].rtp.transport_cc = true;
     (*receive_configs)[0].renderer = this;
 
     if (enable_nack_) {
       send_config->rtp.nack.rtp_history_ms = test::CallTest::kNackRtpHistoryMs;
       send_config->rtp.ulpfec.red_rtx_payload_type =
           test::CallTest::kRtxRedPayloadType;
       send_config->rtp.rtx.ssrcs.push_back(test::CallTest::kSendRtxSsrcs[0]);
       send_config->rtp.rtx.payload_type = test::CallTest::kSendRtxPayloadType;
 
       (*receive_configs)[0].rtp.nack.rtp_history_ms =
@@ skipping 3368 matching lines @@
     std::unique_ptr<VideoEncoder> encoder_;
     std::unique_ptr<VideoDecoder> decoder_;
     rtc::CriticalSection crit_;
     int recorded_frames_ GUARDED_BY(crit_);
   } test(this);
 
   RunBaseTest(&test);
 }
 
 }  // namespace webrtc