Fixes temporal rate allocation issues.

This CL fixes a few issues where the reported fraction of frames
allocated to various temporal layers could be incorrect:
* In LibvpxVp8Encoder, calling GetEncoderInfo() while not initialized,
  or when first configuring with temporal layers and then without,
  could trigger incorrect fps allocations.
* In VP9 when different spatial layers have different max framerates,
  the layer fps should be compared to the layer with the highest
  configured fps, not codec_.maxFramerate which is updated to the
  current input fps on SetRates().
* In EncoderBitrateAdjuster, just warn and ignore if a layer has
  non-zero bps but zero fps, rather than passing down the chain and
  risk weird behavior or divide by zero.

Bug: b/152040235
Change-Id: I548fb3e099b1ec9f536a7b93313fb40c4d32e596
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/171516
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30880}

modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc

@@ -203,6 +203,11 @@ void ApplyVp8EncoderConfigToVpxConfig(const Vp8EncoderConfig& encoder_config,
     vpx_config->ts_periodicity = ts_config.ts_periodicity;
     std::copy(ts_config.ts_layer_id.begin(), ts_config.ts_layer_id.end(),
               std::begin(vpx_config->ts_layer_id));
+  } else {
+    vpx_config->ts_number_layers = 1;
+    vpx_config->ts_rate_decimator[0] = 1;
+    vpx_config->ts_periodicity = 1;
+    vpx_config->ts_layer_id[0] = 0;
   }
 
   if (encoder_config.rc_target_bitrate.has_value()) {
@@ -1248,28 +1253,31 @@ VideoEncoder::EncoderInfo LibvpxVp8Encoder::GetEncoderInfo() const {
     info.scaling_settings.min_pixels_per_frame =
         rate_control_settings_.LibvpxVp8MinPixels().value();
   }
-  // |encoder_idx| is libvpx index where 0 is highest resolution.
+
-  // |si| is simulcast index, where 0 is lowest resolution.
+  if (inited_) {
-  for (size_t si = 0, encoder_idx = encoders_.size() - 1; si < encoders_.size();
+    // |encoder_idx| is libvpx index where 0 is highest resolution.
-       ++si, --encoder_idx) {
+    // |si| is simulcast index, where 0 is lowest resolution.
-    info.fps_allocation[si].clear();
+    for (size_t si = 0, encoder_idx = encoders_.size() - 1;
-    if ((codec_.numberOfSimulcastStreams > si &&
+         si < encoders_.size(); ++si, --encoder_idx) {
-         !codec_.simulcastStream[si].active) ||
+      info.fps_allocation[si].clear();
-        (si == 0 && SimulcastUtility::IsConferenceModeScreenshare(codec_))) {
+      if ((codec_.numberOfSimulcastStreams > si &&
-      // No defined frame rate fractions if not active or if using
+           !codec_.simulcastStream[si].active) ||
-      // ScreenshareLayers, leave vector empty and continue;
+          (si == 0 && SimulcastUtility::IsConferenceModeScreenshare(codec_))) {
-      continue;
+        // No defined frame rate fractions if not active or if using
-    }
+        // ScreenshareLayers, leave vector empty and continue;
-    if (vpx_configs_[encoder_idx].ts_number_layers <= 1) {
+        continue;
-      info.fps_allocation[si].push_back(EncoderInfo::kMaxFramerateFraction);
+      }
-    } else {
+      if (vpx_configs_[encoder_idx].ts_number_layers <= 1) {
-      for (size_t ti = 0; ti < vpx_configs_[encoder_idx].ts_number_layers;
+        info.fps_allocation[si].push_back(EncoderInfo::kMaxFramerateFraction);
-           ++ti) {
+      } else {
-        RTC_DCHECK_GT(vpx_configs_[encoder_idx].ts_rate_decimator[ti], 0);
+        for (size_t ti = 0; ti < vpx_configs_[encoder_idx].ts_number_layers;
-        info.fps_allocation[si].push_back(rtc::saturated_cast<uint8_t>(
+             ++ti) {
-            EncoderInfo::kMaxFramerateFraction /
+          RTC_DCHECK_GT(vpx_configs_[encoder_idx].ts_rate_decimator[ti], 0);
-                vpx_configs_[encoder_idx].ts_rate_decimator[ti] +
+          info.fps_allocation[si].push_back(rtc::saturated_cast<uint8_t>(
-            0.5));
+              EncoderInfo::kMaxFramerateFraction /
+                  vpx_configs_[encoder_idx].ts_rate_decimator[ti] +
+              0.5));
+        }
       }
     }
   }

modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc

@@ -692,6 +692,28 @@ TEST_F(TestVp8Impl, GetEncoderInfoFpsAllocationSimulcastVideo) {
   expected_fps_allocation[2] = expected_fps_allocation[0];
   EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
               ::testing::ElementsAreArray(expected_fps_allocation));
+
+  // Release encoder and re-init without temporal layers.
+  EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->Release());
+
+  // Sanity check fps allocation when not inited.
+  FramerateFractions default_fps_fraction[kMaxSpatialLayers];
+  default_fps_fraction[0].push_back(EncoderInfo::kMaxFramerateFraction);
+  EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
+              ::testing::ElementsAreArray(default_fps_fraction));
+
+  for (int i = 0; i < codec_settings_.numberOfSimulcastStreams; ++i) {
+    codec_settings_.simulcastStream[i].numberOfTemporalLayers = 1;
+  }
+  EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
+            encoder_->InitEncode(&codec_settings_, kSettings));
+
+  for (size_t i = 0; i < 3; ++i) {
+    expected_fps_allocation[i].clear();
+    expected_fps_allocation[i].push_back(EncoderInfo::kMaxFramerateFraction);
+  }
+  EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
+              ::testing::ElementsAreArray(expected_fps_allocation));
 }
 
 }  // namespace webrtc

modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc

@@ -1376,6 +1376,7 @@ TEST_F(TestVp9Impl, EncoderInfoFpsAllocationFlexibleMode) {
   codec_settings_.VP9()->numberOfTemporalLayers = 1;
   codec_settings_.VP9()->flexibleMode = true;
 
+  VideoEncoder::RateControlParameters rate_params;
   for (uint8_t sl_idx = 0; sl_idx < kNumSpatialLayers; ++sl_idx) {
     codec_settings_.spatialLayers[sl_idx].width = codec_settings_.width;
     codec_settings_.spatialLayers[sl_idx].height = codec_settings_.height;
@@ -1390,7 +1391,12 @@ TEST_F(TestVp9Impl, EncoderInfoFpsAllocationFlexibleMode) {
     // fraction is correct.
     codec_settings_.spatialLayers[sl_idx].maxFramerate =
         codec_settings_.maxFramerate / (kNumSpatialLayers - sl_idx);
+    rate_params.bitrate.SetBitrate(sl_idx, 0,
+                                   codec_settings_.startBitrate * 1000);
   }
+  rate_params.bandwidth_allocation =
+      DataRate::BitsPerSec(rate_params.bitrate.get_sum_bps());
+  rate_params.framerate_fps = codec_settings_.maxFramerate;
 
   EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK,
             encoder_->InitEncode(&codec_settings_, kSettings));
@@ -1402,6 +1408,17 @@ TEST_F(TestVp9Impl, EncoderInfoFpsAllocationFlexibleMode) {
   expected_fps_allocation[2].push_back(EncoderInfo::kMaxFramerateFraction);
   EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
               ::testing::ElementsAreArray(expected_fps_allocation));
+
+  // SetRates with current fps does not alter outcome.
+  encoder_->SetRates(rate_params);
+  EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
+              ::testing::ElementsAreArray(expected_fps_allocation));
+
+  // Higher fps than the codec wants, should still not affect outcome.
+  rate_params.framerate_fps *= 2;
+  encoder_->SetRates(rate_params);
+  EXPECT_THAT(encoder_->GetEncoderInfo().fps_allocation,
+              ::testing::ElementsAreArray(expected_fps_allocation));
 }
 
 class TestVp9ImplWithLayering

modules/video_coding/codecs/vp9/vp9_impl.cc

@@ -1550,20 +1550,33 @@ VideoEncoder::EncoderInfo VP9EncoderImpl::GetEncoderInfo() const {
   info.has_trusted_rate_controller = trusted_rate_controller_;
   info.is_hardware_accelerated = false;
   info.has_internal_source = false;
-  for (size_t si = 0; si < num_spatial_layers_; ++si) {
+  if (inited_) {
-    info.fps_allocation[si].clear();
+    // Find the max configured fps of any active spatial layer.
-    if (!codec_.spatialLayers[si].active) {
+    float max_fps = 0.0;
-      continue;
+    for (size_t si = 0; si < num_spatial_layers_; ++si) {
+      if (codec_.spatialLayers[si].active &&
+          codec_.spatialLayers[si].maxFramerate > max_fps) {
+        max_fps = codec_.spatialLayers[si].maxFramerate;
+      }
     }
-    // This spatial layer may already use a fraction of the total frame rate.
+
-    const float sl_fps_fraction =
+    for (size_t si = 0; si < num_spatial_layers_; ++si) {
-        codec_.spatialLayers[si].maxFramerate / codec_.maxFramerate;
+      info.fps_allocation[si].clear();
-    for (size_t ti = 0; ti < num_temporal_layers_; ++ti) {
+      if (!codec_.spatialLayers[si].active) {
-      const uint32_t decimator =
+        continue;
-          num_temporal_layers_ <= 1 ? 1 : config_->ts_rate_decimator[ti];
+      }
-      RTC_DCHECK_GT(decimator, 0);
+
-      info.fps_allocation[si].push_back(rtc::saturated_cast<uint8_t>(
+      // This spatial layer may already use a fraction of the total frame rate.
-          EncoderInfo::kMaxFramerateFraction * (sl_fps_fraction / decimator)));
+      const float sl_fps_fraction =
+          codec_.spatialLayers[si].maxFramerate / max_fps;
+      for (size_t ti = 0; ti < num_temporal_layers_; ++ti) {
+        const uint32_t decimator =
+            num_temporal_layers_ <= 1 ? 1 : config_->ts_rate_decimator[ti];
+        RTC_DCHECK_GT(decimator, 0);
+        info.fps_allocation[si].push_back(
+            rtc::saturated_cast<uint8_t>(EncoderInfo::kMaxFramerateFraction *
+                                         (sl_fps_fraction / decimator)));
+      }
     }
   }
   return info;

video/encoder_bitrate_adjuster.cc

@@ -282,6 +282,13 @@ VideoBitrateAllocation EncoderBitrateAdjuster::AdjustRateAllocation(
                 (ti == 0 ? 0 : current_fps_allocation_[si][ti - 1])) /
             VideoEncoder::EncoderInfo::kMaxFramerateFraction;
 
+        if (fps_fraction <= 0.0) {
+          RTC_LOG(LS_WARNING)
+              << "Encoder config has temporal layer with non-zero bitrate "
+                 "allocation but zero framerate allocation.";
+          continue;
+        }
+
         overshoot_detectors_[si][ti]->SetTargetRate(
             DataRate::BitsPerSec(layer_bitrate_bps),
             fps_fraction * rates.framerate_fps, now_ms);