Revert of Turn off error resilience for VP9 if no spatial or temporal layers are configured and NACK is enabl… (patchset #1 id:40001 of https://codereview.webrtc.org/2532053002/ )

Reason for revert:
Increase in encode time larger than expected.

Original issue's description:
> Turn off error resilience for VP9 if no spatial or temporal layers are configured and NACK is enabled.
>
> Error resilience is currently always enabled for VP9 which reduces quality.
>
> BUG=webrtc:6783
>
> Committed: https://crrev.com/4eb03c76fa2320534d669fda2aabf800e7a6f579
> Cr-Commit-Position: refs/heads/master@{#15390}

TBR=stefan@webrtc.org,marpan@webrtc.org,mflodman@webrtc.org,marpan@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=webrtc:6783

Review-Url: https://codereview.webrtc.org/2554403006
Cr-Commit-Position: refs/heads/master@{#15501}

webrtc/common_types.h

@@ -537,7 +537,7 @@ struct VideoCodecVP8 {
 // VP9 specific.
 struct VideoCodecVP9 {
   VideoCodecComplexity complexity;
-  bool resilienceOn;
+  int resilience;
   unsigned char numberOfTemporalLayers;
   bool denoisingOn;
   bool frameDroppingOn;

webrtc/modules/video_coding/codec_database.cc

@@ -46,7 +46,7 @@ VideoCodecVP9 VideoEncoder::GetDefaultVp9Settings() {
   VideoCodecVP9 vp9_settings;
   memset(&vp9_settings, 0, sizeof(vp9_settings));
 
-  vp9_settings.resilienceOn = true;
+  vp9_settings.resilience = 1;
   vp9_settings.numberOfTemporalLayers = 1;
   vp9_settings.denoisingOn = false;
   vp9_settings.frameDroppingOn = true;

webrtc/modules/video_coding/codecs/vp9/vp9_impl.cc

@@ -298,7 +298,7 @@ int VP9EncoderImpl::InitEncode(const VideoCodec* inst,
   config_->g_w = codec_.width;
   config_->g_h = codec_.height;
   config_->rc_target_bitrate = inst->startBitrate;  // in kbit/s
-  config_->g_error_resilient = inst->VP9().resilienceOn ? 1 : 0;
+  config_->g_error_resilient = 1;
   // Setting the time base of the codec.
   config_->g_timebase.num = 1;
   config_->g_timebase.den = 90000;

webrtc/modules/video_coding/video_codec_initializer.cc

@@ -21,15 +21,6 @@
 #include "webrtc/system_wrappers/include/clock.h"
 
 namespace webrtc {
-namespace {
-bool TemporalLayersConfigured(const std::vector<VideoStream>& streams) {
-  for (const VideoStream& stream : streams) {
-    if (stream.temporal_layer_thresholds_bps.size() > 0)
-      return true;
-  }
-  return false;
-}
-}  // namespace
 
 bool VideoCodecInitializer::SetupCodec(
     const VideoEncoderConfig& config,
@@ -128,8 +119,12 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
         *video_codec.VP8() = VideoEncoder::GetDefaultVp8Settings();
       video_codec.VP8()->numberOfTemporalLayers = static_cast<unsigned char>(
           streams.back().temporal_layer_thresholds_bps.size() + 1);
-
+      bool temporal_layers_configured = false;
-      if (nack_enabled && !TemporalLayersConfigured(streams)) {
+      for (const VideoStream& stream : streams) {
+        if (stream.temporal_layer_thresholds_bps.size() > 0)
+          temporal_layers_configured = true;
+      }
+      if (nack_enabled && !temporal_layers_configured) {
         LOG(LS_INFO) << "No temporal layers and nack enabled -> resilience off";
         video_codec.VP8()->resilience = kResilienceOff;
       }
@@ -147,13 +142,6 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec(
       }
       video_codec.VP9()->numberOfTemporalLayers = static_cast<unsigned char>(
           streams.back().temporal_layer_thresholds_bps.size() + 1);
-
-      if (nack_enabled && !TemporalLayersConfigured(streams) &&
-          video_codec.VP9()->numberOfSpatialLayers == 1) {
-        LOG(LS_INFO) << "No temporal or spatial layers and nack enabled -> "
-                     << "resilience off";
-        video_codec.VP9()->resilienceOn = false;
-      }
       break;
     }
     case kVideoCodecH264: {

webrtc/video/vie_encoder_unittest.cc

@@ -28,7 +28,6 @@ using ScaleReason = ScalingObserverInterface::ScaleReason;
 namespace {
 const size_t kMaxPayloadLength = 1440;
 const int kTargetBitrateBps = 100000;
-const unsigned char kNumSlDummy = 0;
 
 class TestBuffer : public webrtc::I420Buffer {
  public:
@@ -142,7 +141,6 @@ class ViEEncoderTest : public ::testing::Test {
   void ResetEncoder(const std::string& payload_name,
                     size_t num_streams,
                     size_t num_temporal_layers,
-                    unsigned char num_spatial_layers,
                     bool nack_enabled) {
     video_send_config_.encoder_settings.payload_name = payload_name;
 
@@ -151,13 +149,6 @@ class ViEEncoderTest : public ::testing::Test {
     video_encoder_config.max_bitrate_bps = 1000000;
     video_encoder_config.video_stream_factory =
         new rtc::RefCountedObject<VideoStreamFactory>(num_temporal_layers);
-    if (payload_name == "VP9") {
-      VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings();
-      vp9_settings.numberOfSpatialLayers = num_spatial_layers;
-      video_encoder_config.encoder_specific_settings =
-          new rtc::RefCountedObject<
-              VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings);
-    }
     ConfigureEncoder(std::move(video_encoder_config), nack_enabled);
   }
 
@@ -442,7 +433,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOffFor1S1TLWithNackEnabled) {
   const bool kNackEnabled = true;
   const size_t kNumStreams = 1;
   const size_t kNumTl = 1;
-  ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+  ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
   vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
 
   // Capture a frame and wait for it to synchronize with the encoder thread.
@@ -462,7 +453,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOffFor2S1TlWithNackEnabled) {
   const bool kNackEnabled = true;
   const size_t kNumStreams = 2;
   const size_t kNumTl = 1;
-  ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+  ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
   vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
 
   // Capture a frame and wait for it to synchronize with the encoder thread.
@@ -482,7 +473,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S1TLWithNackDisabled) {
   const bool kNackEnabled = false;
   const size_t kNumStreams = 1;
   const size_t kNumTl = 1;
-  ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+  ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
   vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
 
   // Capture a frame and wait for it to synchronize with the encoder thread.
@@ -502,7 +493,7 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S2TlWithNackEnabled) {
   const bool kNackEnabled = true;
   const size_t kNumStreams = 1;
   const size_t kNumTl = 2;
-  ResetEncoder("VP8", kNumStreams, kNumTl, kNumSlDummy, kNackEnabled);
+  ResetEncoder("VP8", kNumStreams, kNumTl, kNackEnabled);
   vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
 
   // Capture a frame and wait for it to synchronize with the encoder thread.
@@ -518,94 +509,6 @@ TEST_F(ViEEncoderTest, Vp8ResilienceIsOnFor1S2TlWithNackEnabled) {
   vie_encoder_->Stop();
 }
 
-TEST_F(ViEEncoderTest, Vp9ResilienceIsOffFor1SL1TLWithNackEnabled) {
-  const bool kNackEnabled = true;
-  const size_t kNumStreams = 1;
-  const size_t kNumTl = 1;
-  const unsigned char kNumSl = 1;
-  ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled);
-  vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  // Capture a frame and wait for it to synchronize with the encoder thread.
-  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
-  sink_.WaitForEncodedFrame(1);
-  // The encoder have been configured once when the first frame is received.
-  EXPECT_EQ(1, sink_.number_of_reconfigurations());
-  EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType);
-  EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams);
-  EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers);
-  EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers);
-  // Resilience is off for no spatial and temporal layers with nack on.
-  EXPECT_FALSE(fake_encoder_.codec_config().VP9()->resilienceOn);
-  vie_encoder_->Stop();
-}
-
-TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor1SL1TLWithNackDisabled) {
-  const bool kNackEnabled = false;
-  const size_t kNumStreams = 1;
-  const size_t kNumTl = 1;
-  const unsigned char kNumSl = 1;
-  ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled);
-  vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  // Capture a frame and wait for it to synchronize with the encoder thread.
-  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
-  sink_.WaitForEncodedFrame(1);
-  // The encoder have been configured once when the first frame is received.
-  EXPECT_EQ(1, sink_.number_of_reconfigurations());
-  EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType);
-  EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams);
-  EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers);
-  EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers);
-  // Resilience is on if nack is off.
-  EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn);
-  vie_encoder_->Stop();
-}
-
-TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor2SL1TLWithNackEnabled) {
-  const bool kNackEnabled = true;
-  const size_t kNumStreams = 1;
-  const size_t kNumTl = 1;
-  const unsigned char kNumSl = 2;
-  ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled);
-  vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  // Capture a frame and wait for it to synchronize with the encoder thread.
-  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
-  sink_.WaitForEncodedFrame(1);
-  // The encoder have been configured once when the first frame is received.
-  EXPECT_EQ(1, sink_.number_of_reconfigurations());
-  EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType);
-  EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams);
-  EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers);
-  EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers);
-  // Resilience is on for spatial layers.
-  EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn);
-  vie_encoder_->Stop();
-}
-
-TEST_F(ViEEncoderTest, Vp9ResilienceIsOnFor1SL2TLWithNackEnabled) {
-  const bool kNackEnabled = true;
-  const size_t kNumStreams = 1;
-  const size_t kNumTl = 2;
-  const unsigned char kNumSl = 1;
-  ResetEncoder("VP9", kNumStreams, kNumTl, kNumSl, kNackEnabled);
-  vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
-
-  // Capture a frame and wait for it to synchronize with the encoder thread.
-  video_source_.IncomingCapturedFrame(CreateFrame(1, nullptr));
-  sink_.WaitForEncodedFrame(1);
-  // The encoder have been configured once when the first frame is received.
-  EXPECT_EQ(1, sink_.number_of_reconfigurations());
-  EXPECT_EQ(kVideoCodecVP9, fake_encoder_.codec_config().codecType);
-  EXPECT_EQ(kNumStreams, fake_encoder_.codec_config().numberOfSimulcastStreams);
-  EXPECT_EQ(kNumTl, fake_encoder_.codec_config().VP9()->numberOfTemporalLayers);
-  EXPECT_EQ(kNumSl, fake_encoder_.codec_config().VP9()->numberOfSpatialLayers);
-  // Resilience is on for temporal layers.
-  EXPECT_TRUE(fake_encoder_.codec_config().VP9()->resilienceOn);
-  vie_encoder_->Stop();
-}
-
 TEST_F(ViEEncoderTest, SwitchSourceDeregisterEncoderAsSink) {
   EXPECT_TRUE(video_source_.has_sinks());
   test::FrameForwarder new_video_source;