Fixes a bug where a video stream can get stuck in the suspended state.

webrtc/call/rampup_tests.cc

Index: webrtc/call/rampup_tests.cc
diff --git a/webrtc/call/rampup_tests.cc b/webrtc/call/rampup_tests.cc
index 46e97cd2a5352ad97b9f6901bb5e7c6bda92fb24..67e99bd2e28dead8a267aea89fdf9338745a77d7 100644
--- a/webrtc/call/rampup_tests.cc
+++ b/webrtc/call/rampup_tests.cc
@@ -11,6 +11,7 @@
 #include "webrtc/call/rampup_tests.h"
 
 #include "webrtc/base/checks.h"
+#include "webrtc/base/logging.h"
 #include "webrtc/base/platform_thread.h"
 #include "webrtc/test/encoder_settings.h"
 #include "webrtc/test/gtest.h"
@@ -20,7 +21,7 @@ namespace webrtc {
 namespace {
 
 static const int64_t kPollIntervalMs = 20;
-static const int kExpectedHighVideoBitrateBps = 60000;
+static const int kExpectedHighVideoBitrateBps = 80000;
 static const int kExpectedHighAudioBitrateBps = 30000;
 static const int kLowBandwidthLimitBps = 20000;
 static const int kExpectedLowBitrateBps = 20000;
@@ -35,6 +36,7 @@ std::vector<uint32_t> GenerateSsrcs(size_t num_streams, uint32_t ssrc_offset) {
 
 RampUpTester::RampUpTester(size_t num_video_streams,
                            size_t num_audio_streams,
+                           size_t num_flexfec_streams,
                            unsigned int start_bitrate_bps,
                            int64_t min_run_time_ms,
                            const std::string& extension_type,
@@ -46,6 +48,7 @@ RampUpTester::RampUpTester(size_t num_video_streams,
       clock_(Clock::GetRealTimeClock()),
       num_video_streams_(num_video_streams),
       num_audio_streams_(num_audio_streams),
+      num_flexfec_streams_(num_flexfec_streams),
       rtx_(rtx),
       red_(red),
       sender_call_(nullptr),
@@ -63,6 +66,8 @@ RampUpTester::RampUpTester(size_t num_video_streams,
       poller_thread_(&BitrateStatsPollingThread,
                      this,
                      "BitrateStatsPollingThread") {
+  if (red_)
+    EXPECT_EQ(0u, num_flexfec_streams_);
   EXPECT_LE(num_audio_streams_, 1u);
 }
 
@@ -100,6 +105,10 @@ size_t RampUpTester::GetNumAudioStreams() const {
   return num_audio_streams_;
 }
 
+size_t RampUpTester::GetNumFlexfecStreams() const {
+  return num_flexfec_streams_;
+}
+
 class RampUpTester::VideoStreamFactory
     : public VideoEncoderConfig::VideoStreamFactoryInterface {
  public:
@@ -209,6 +218,13 @@ void RampUpTester::ModifyVideoConfigs(
     }
     ++i;
   }
+
+  RTC_DCHECK_LE(num_flexfec_streams_, 1);
+  if (num_flexfec_streams_ == 1) {
+    send_config->rtp.flexfec.payload_type = test::CallTest::kFlexfecPayloadType;
+    send_config->rtp.flexfec.ssrc = test::CallTest::kFlexfecSendSsrc;
+    send_config->rtp.flexfec.protected_media_ssrcs = {video_ssrcs_[0]};
+  }
 }
 
 void RampUpTester::ModifyAudioConfigs(
@@ -219,6 +235,8 @@ void RampUpTester::ModifyAudioConfigs(
 
   EXPECT_NE(RtpExtension::kTimestampOffsetUri, extension_type_)
       << "Audio BWE not supported with toffset.";
+  EXPECT_NE(RtpExtension::kAbsSendTimeUri, extension_type_)
+      << "Audio BWE not supported with abs-send-time.";
 
   send_config->rtp.ssrc = audio_ssrcs_[0];
   send_config->rtp.extensions.clear();
@@ -227,11 +245,7 @@ void RampUpTester::ModifyAudioConfigs(
   send_config->max_bitrate_bps = 60000;
 
   bool transport_cc = false;
-  if (extension_type_ == RtpExtension::kAbsSendTimeUri) {
-    transport_cc = false;
-    send_config->rtp.extensions.push_back(
-        RtpExtension(extension_type_.c_str(), kAbsSendTimeExtensionId));
-  } else if (extension_type_ == RtpExtension::kTransportSequenceNumberUri) {
+  if (extension_type_ == RtpExtension::kTransportSequenceNumberUri) {
     transport_cc = true;
     send_config->rtp.extensions.push_back(RtpExtension(
         extension_type_.c_str(), kTransportSequenceNumberExtensionId));
@@ -244,6 +258,26 @@ void RampUpTester::ModifyAudioConfigs(
   }
 }
 
+void RampUpTester::ModifyFlexfecConfigs(
+    std::vector<FlexfecReceiveStream::Config>* receive_configs) {
+  if (num_flexfec_streams_ == 0)
+    return;
+  RTC_DCHECK_EQ(1, num_flexfec_streams_);
+  (*receive_configs)[0].payload_type = test::CallTest::kFlexfecPayloadType;
+  (*receive_configs)[0].remote_ssrc = test::CallTest::kFlexfecSendSsrc;
+  (*receive_configs)[0].protected_media_ssrcs = {video_ssrcs_[0]};
+  (*receive_configs)[0].local_ssrc = video_ssrcs_[0];
+  if (extension_type_ == RtpExtension::kAbsSendTimeUri) {
+    (*receive_configs)[0].transport_cc = false;
+    (*receive_configs)[0].rtp_header_extensions.push_back(
+        RtpExtension(extension_type_.c_str(), kAbsSendTimeExtensionId));
+  } else if (extension_type_ == RtpExtension::kTransportSequenceNumberUri) {
+    (*receive_configs)[0].transport_cc = true;
+    (*receive_configs)[0].rtp_header_extensions.push_back(RtpExtension(
+        extension_type_.c_str(), kTransportSequenceNumberExtensionId));
+  }
+}
+
 void RampUpTester::OnCallsCreated(Call* sender_call, Call* receiver_call) {
   sender_call_ = sender_call;
 }
@@ -351,12 +385,14 @@ void RampUpTester::PerformTest() {
 
 RampUpDownUpTester::RampUpDownUpTester(size_t num_video_streams,
                                        size_t num_audio_streams,
+                                       size_t num_flexfec_streams,
                                        unsigned int start_bitrate_bps,
                                        const std::string& extension_type,
                                        bool rtx,
                                        bool red)
     : RampUpTester(num_video_streams,
                    num_audio_streams,
+                   num_flexfec_streams,
                    start_bitrate_bps,
                    0,
                    extension_type,
@@ -438,18 +474,37 @@ void RampUpDownUpTester::EvolveTestState(int bitrate_bps, bool suspended) {
     case kFirstRampup: {
       EXPECT_FALSE(suspended);
       if (bitrate_bps >= GetExpectedHighBitrate()) {
+        webrtc::test::PrintResult("ramp_up_down_up", GetModifierString(),
+                                  "first_rampup", now - state_start_ms_, "ms",
+                                  false);
+        if (num_flexfec_streams_ > 0) {
+          // Increase loss rate and delay and wait for FEC to be produced.
+          forward_transport_config_.loss_percent = 25;
+          forward_transport_config_.queue_delay_ms = 100;
+        }
+        test_state_ = kWaitForFec;
+        send_transport_->SetConfig(forward_transport_config_);
+      }
+      break;
+    }
+    case kWaitForFec: {
+      size_t flex_fec_bytes = 0;
+      if (num_flexfec_streams_ > 0) {
+        webrtc::VideoSendStream::Stats stats = send_stream_->GetStats();
+        for (const auto& kv : stats.substreams)
+          flex_fec_bytes += kv.second.rtp_stats.fec.TotalBytes();
+      }
+      if (num_flexfec_streams_ == 0 || flex_fec_bytes > 0) {

[terelius, 2017/02/20 17:42:37]

How about functions SetLossRates(r1,r2,r3) and SetExpectedFecRates(r) that
allows configuring packet loss and FEC from each test? The tests would be
slightly more verbose (at least if many of them use FEC), but I think it would
increase flexibility and clarity. WDYT?

[stefan-webrtc, 2017/02/21 09:09:45]

I improved it quite a bit, but not exactly how you suggested.

         // The first ramp-up has reached the target bitrate. Change the
         // channel limit, and move to the next test state.
         forward_transport_config_.link_capacity_kbps =
             kLowBandwidthLimitBps / 1000;
+        forward_transport_config_.loss_percent = 0;
         send_transport_->SetConfig(forward_transport_config_);
-        test_state_ = kLowRate;
-        webrtc::test::PrintResult("ramp_up_down_up", GetModifierString(),
-                                  "first_rampup", now - state_start_ms_, "ms",
-                                  false);
         state_start_ms_ = now;
         interval_start_ms_ = now;
         sent_bytes_ = 0;
+        test_state_ = kLowRate;
       }
       break;
     }
@@ -499,70 +554,77 @@ class RampUpTest : public test::CallTest {
 static const uint32_t kStartBitrateBps = 60000;
 
 TEST_F(RampUpTest, UpDownUpAbsSendTimeSimulcastRedRtx) {
-  RampUpDownUpTester test(3, 0, kStartBitrateBps, RtpExtension::kAbsSendTimeUri,
-                          true, true);
+  RampUpDownUpTester test(3, 0, 0, kStartBitrateBps,
+                          RtpExtension::kAbsSendTimeUri, true, true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, UpDownUpTransportSequenceNumberRtx) {
-  RampUpDownUpTester test(3, 0, kStartBitrateBps,
+  RampUpDownUpTester test(1, 0, 0, kStartBitrateBps,

[terelius, 2017/02/20 17:42:37]

Why change the number of streams here? Is it because FlexFEC only protects one
stream and you want to compare results with the test below?

[stefan-webrtc, 2017/02/21 09:09:45]

Mistake. Changing back. :)

+                          RtpExtension::kTransportSequenceNumberUri, true,
+                          false);
+  RunBaseTest(&test);
+}
+
+TEST_F(RampUpTest, UpDownUpTransportSequenceNumberPacketLoss) {
+  RampUpDownUpTester test(1, 0, 1, kStartBitrateBps,
                           RtpExtension::kTransportSequenceNumberUri, true,
                           false);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, UpDownUpAudioVideoTransportSequenceNumberRtx) {
-  RampUpDownUpTester test(3, 1, kStartBitrateBps,
+  RampUpDownUpTester test(3, 1, 0, kStartBitrateBps,
                           RtpExtension::kTransportSequenceNumberUri, true,
                           false);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, UpDownUpAudioTransportSequenceNumberRtx) {
-  RampUpDownUpTester test(0, 1, kStartBitrateBps,
+  RampUpDownUpTester test(0, 1, 0, kStartBitrateBps,
                           RtpExtension::kTransportSequenceNumberUri, true,
                           false);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, TOffsetSimulcastRedRtx) {
-  RampUpTester test(3, 0, 0, 0, RtpExtension::kTimestampOffsetUri, true, true,
-                    true);
+  RampUpTester test(3, 0, 0, 0, 0, RtpExtension::kTimestampOffsetUri, true,
+                    true, true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, AbsSendTime) {
-  RampUpTester test(1, 0, 0, 0, RtpExtension::kAbsSendTimeUri, false, false,
+  RampUpTester test(1, 0, 0, 0, 0, RtpExtension::kAbsSendTimeUri, false, false,
                     true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, AbsSendTimeSimulcastRedRtx) {
-  RampUpTester test(3, 0, 0, 0, RtpExtension::kAbsSendTimeUri, true, true,
+  RampUpTester test(3, 0, 0, 0, 0, RtpExtension::kAbsSendTimeUri, true, true,
                     true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, TransportSequenceNumber) {
-  RampUpTester test(1, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
+  RampUpTester test(1, 0, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
                     false, false, true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, TransportSequenceNumberSimulcast) {
-  RampUpTester test(3, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
+  RampUpTester test(3, 0, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
                     false, false, true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, TransportSequenceNumberSimulcastRedRtx) {
-  RampUpTester test(3, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
+  RampUpTester test(3, 0, 0, 0, 0, RtpExtension::kTransportSequenceNumberUri,
                     true, true, true);
   RunBaseTest(&test);
 }
 
 TEST_F(RampUpTest, AudioTransportSequenceNumber) {
-  RampUpTester test(0, 1, 300000, 10000,
+  RampUpTester test(0, 1, 0, 300000, 10000,
                     RtpExtension::kTransportSequenceNumberUri, false, false,
                     false);
   RunBaseTest(&test);