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}
diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
index 335ba9f..7694dae 100644
--- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
+++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
@@ -203,6 +203,11 @@
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 @@
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.
- for (size_t si = 0, encoder_idx = encoders_.size() - 1; si < encoders_.size();
- ++si, --encoder_idx) {
- info.fps_allocation[si].clear();
- if ((codec_.numberOfSimulcastStreams > si &&
- !codec_.simulcastStream[si].active) ||
- (si == 0 && SimulcastUtility::IsConferenceModeScreenshare(codec_))) {
- // No defined frame rate fractions if not active or if using
- // ScreenshareLayers, leave vector empty and continue;
- continue;
- }
- if (vpx_configs_[encoder_idx].ts_number_layers <= 1) {
- info.fps_allocation[si].push_back(EncoderInfo::kMaxFramerateFraction);
- } else {
- for (size_t ti = 0; ti < vpx_configs_[encoder_idx].ts_number_layers;
- ++ti) {
- RTC_DCHECK_GT(vpx_configs_[encoder_idx].ts_rate_decimator[ti], 0);
- info.fps_allocation[si].push_back(rtc::saturated_cast<uint8_t>(
- EncoderInfo::kMaxFramerateFraction /
- vpx_configs_[encoder_idx].ts_rate_decimator[ti] +
- 0.5));
+
+ if (inited_) {
+ // |encoder_idx| is libvpx index where 0 is highest resolution.
+ // |si| is simulcast index, where 0 is lowest resolution.
+ for (size_t si = 0, encoder_idx = encoders_.size() - 1;
+ si < encoders_.size(); ++si, --encoder_idx) {
+ info.fps_allocation[si].clear();
+ if ((codec_.numberOfSimulcastStreams > si &&
+ !codec_.simulcastStream[si].active) ||
+ (si == 0 && SimulcastUtility::IsConferenceModeScreenshare(codec_))) {
+ // No defined frame rate fractions if not active or if using
+ // ScreenshareLayers, leave vector empty and continue;
+ continue;
+ }
+ if (vpx_configs_[encoder_idx].ts_number_layers <= 1) {
+ info.fps_allocation[si].push_back(EncoderInfo::kMaxFramerateFraction);
+ } else {
+ for (size_t ti = 0; ti < vpx_configs_[encoder_idx].ts_number_layers;
+ ++ti) {
+ RTC_DCHECK_GT(vpx_configs_[encoder_idx].ts_rate_decimator[ti], 0);
+ info.fps_allocation[si].push_back(rtc::saturated_cast<uint8_t>(
+ EncoderInfo::kMaxFramerateFraction /
+ vpx_configs_[encoder_idx].ts_rate_decimator[ti] +
+ 0.5));
+ }
}
}
}
diff --git a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc
index a1eb684..5159526 100644
--- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc
+++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc
@@ -692,6 +692,28 @@
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
diff --git a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc
index 78411eb..58805934 100644
--- a/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc
+++ b/modules/video_coding/codecs/vp9/test/vp9_impl_unittest.cc
@@ -1376,6 +1376,7 @@
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 @@
// 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 @@
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
diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc
index 99d1abe..42afb36 100644
--- a/modules/video_coding/codecs/vp9/vp9_impl.cc
+++ b/modules/video_coding/codecs/vp9/vp9_impl.cc
@@ -1550,20 +1550,33 @@
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) {
- info.fps_allocation[si].clear();
- if (!codec_.spatialLayers[si].active) {
- continue;
+ if (inited_) {
+ // Find the max configured fps of any active spatial layer.
+ float max_fps = 0.0;
+ 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 =
- codec_.spatialLayers[si].maxFramerate / codec_.maxFramerate;
- 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)));
+
+ for (size_t si = 0; si < num_spatial_layers_; ++si) {
+ info.fps_allocation[si].clear();
+ if (!codec_.spatialLayers[si].active) {
+ continue;
+ }
+
+ // This spatial layer may already use a fraction of the total frame rate.
+ 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;
diff --git a/video/encoder_bitrate_adjuster.cc b/video/encoder_bitrate_adjuster.cc
index e6c8739..45d8887 100644
--- a/video/encoder_bitrate_adjuster.cc
+++ b/video/encoder_bitrate_adjuster.cc
@@ -282,6 +282,13 @@
(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);
