From c57b0ee11c438a0c9c11b5b452527a3a74e79604 Mon Sep 17 00:00:00 2001
From: Sebastian Jansson <srte@webrtc.org>
Date: Wed, 26 Jun 2019 16:22:39 +0200
Subject: [PATCH] Fix for NACK retransmission in Scenario tests.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes a bug where NACK mode was not properly enabled
due to missing send side configuration.

Bug: webrtc:9510
Change-Id: I318fdf44f17e57d30589115a452f6a64f81ee973
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/143781
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28391}
---
 test/scenario/video_stream.cc          | 17 +++++++++++------
 test/scenario/video_stream_unittest.cc | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc
index 94a2438718..6b0b4f0aa9 100644
--- a/test/scenario/video_stream.cc
+++ b/test/scenario/video_stream.cc
@@ -124,6 +124,8 @@ VideoSendStream::Config CreateVideoSendStreamConfig(VideoStreamConfig config,
   VideoSendStream::Config send_config(send_transport);
   send_config.rtp.payload_name = CodecTypeToPayloadString(config.encoder.codec);
   send_config.rtp.payload_type = CodecTypeToPayloadType(config.encoder.codec);
+  send_config.rtp.nack.rtp_history_ms =
+      config.stream.nack_history_time.ms<int>();
 
   send_config.rtp.ssrcs = ssrcs;
   send_config.rtp.extensions = GetVideoRtpExtensions(config);
@@ -359,20 +361,23 @@ SendVideoStream::SendVideoStream(CallClient* sender,
   using Codec = VideoStreamConfig::Encoder::Codec;
   switch (config.encoder.implementation) {
     case Encoder::Implementation::kFake:
-      if (config.encoder.codec == Codec::kVideoCodecGeneric) {
         encoder_factory_ =
             absl::make_unique<FunctionVideoEncoderFactory>([this]() {
               rtc::CritScope cs(&crit_);
-              auto encoder =
-                  absl::make_unique<test::FakeEncoder>(sender_->clock_);
+              std::unique_ptr<FakeEncoder> encoder;
+              if (config_.encoder.codec == Codec::kVideoCodecVP8) {
+                encoder =
+                    absl::make_unique<test::FakeVP8Encoder>(sender_->clock_);
+              } else if (config_.encoder.codec == Codec::kVideoCodecGeneric) {
+                encoder = absl::make_unique<test::FakeEncoder>(sender_->clock_);
+              } else {
+                RTC_NOTREACHED();
+              }
               fake_encoders_.push_back(encoder.get());
               if (config_.encoder.fake.max_rate.IsFinite())
                 encoder->SetMaxBitrate(config_.encoder.fake.max_rate.kbps());
               return encoder;
             });
-      } else {
-        RTC_NOTREACHED();
-      }
       break;
     case VideoStreamConfig::Encoder::Implementation::kSoftware:
       encoder_factory_.reset(new InternalEncoderFactory());
diff --git a/test/scenario/video_stream_unittest.cc b/test/scenario/video_stream_unittest.cc
index ebd9d623e8..74dc9a2f51 100644
--- a/test/scenario/video_stream_unittest.cc
+++ b/test/scenario/video_stream_unittest.cc
@@ -114,6 +114,22 @@ TEST(VideoStreamTest, RecievesVp8SimulcastFrames) {
   EXPECT_GE(frame_counts[2], kExpectedCount);
 }
 
+TEST(VideoStreamTest, SendsNacksOnLoss) {
+  Scenario s;
+  auto route =
+      s.CreateRoutes(s.CreateClient("caller", CallClientConfig()),
+                     {s.CreateSimulationNode([](NetworkSimulationConfig* c) {
+                       c->loss_rate = 0.2;
+                     })},
+                     s.CreateClient("callee", CallClientConfig()),
+                     {s.CreateSimulationNode(NetworkSimulationConfig())});
+  // NACK retransmissions are enabled by default.
+  auto video = s.CreateVideoStream(route->forward(), VideoStreamConfig());
+  s.RunFor(TimeDelta::seconds(1));
+  auto stream_stats = video->send()->GetStats().substreams.begin()->second;
+  EXPECT_GT(stream_stats.rtp_stats.retransmitted.packets, 0u);
+}
+
 TEST(VideoStreamTest, SendsFecWithUlpFec) {
   Scenario s;
   auto route =
@@ -124,6 +140,8 @@ TEST(VideoStreamTest, SendsFecWithUlpFec) {
                      s.CreateClient("callee", CallClientConfig()),
                      {s.CreateSimulationNode(NetworkSimulationConfig())});
   auto video = s.CreateVideoStream(route->forward(), [&](VideoStreamConfig* c) {
+    // We do not allow NACK+ULPFEC for generic codec, using VP8.
+    c->encoder.codec = VideoStreamConfig::Encoder::Codec::kVideoCodecVP8;
     c->stream.use_ulpfec = true;
   });
   s.RunFor(TimeDelta::seconds(5));