Always update libvpx configuration if bitrate has changed.
OnRatesUpdated() is called every time the bitrate estimate, or once per
second. However, since we don't want to reconfigure libvpx too often,
just in case it interferes with the rate controller, so
ScreenshareLayers contains a boolean |bitrate_update_| which indicate
if the configuration should be updated on a call to
UpdateConfiguration().
However, it two rate updates happened between two frames, the first of
which changes the rates and second one does not, |bitrate_update_| will
be reset to false and the encoder won't get the desired config.
This CL makes sure we update the configuration iff the rate has changed
at any time since the last call to UpdateConfiguration().
Bug: webrtc:9012
Change-Id: I62af36cffe20ecb7d3f403b3eb11f23a9692d719
Reviewed-on: https://webrtc-review.googlesource.com/69040
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22826}diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.cc b/modules/video_coding/codecs/vp8/screenshare_layers.cc
index 733ca0a..cd24490 100644
--- a/modules/video_coding/codecs/vp8/screenshare_layers.cc
+++ b/modules/video_coding/codecs/vp8/screenshare_layers.cc
@@ -221,10 +221,12 @@
capture_framerate_ = target_framerate_;
bitrate_updated_ = true;
} else {
- bitrate_updated_ = capture_framerate_ &&
- framerate_fps != static_cast<int>(*capture_framerate_);
- bitrate_updated_ |= tl0_kbps != layers_[0].target_rate_kbps_;
- bitrate_updated_ |= tl1_kbps != layers_[1].target_rate_kbps_;
+ if ((capture_framerate_ &&
+ framerate_fps != static_cast<int>(*capture_framerate_)) ||
+ (tl0_kbps != layers_[0].target_rate_kbps_) ||
+ (tl1_kbps != layers_[1].target_rate_kbps_)) {
+ bitrate_updated_ = true;
+ }
if (framerate_fps < 0) {
capture_framerate_.reset();
diff --git a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
index 04762a3..e6f121b 100644
--- a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
+++ b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
@@ -642,4 +642,23 @@
EXPECT_EQ(cfg_.rc_target_bitrate, default_bitrate * 2);
}
+TEST_F(ScreenshareLayerTest, UpdatesConfigurationAfterRateChange) {
+ // Set inital rate again, no need to update configuration.
+ layers_->OnRatesUpdated(kDefault2TlBitratesBps, kFrameRate);
+ EXPECT_FALSE(layers_->UpdateConfiguration(&cfg_));
+
+ // Rate changed, now update config.
+ std::vector<uint32_t> bitrates = kDefault2TlBitratesBps;
+ bitrates[1] -= 100000;
+ layers_->OnRatesUpdated(bitrates, 5);
+ EXPECT_TRUE(layers_->UpdateConfiguration(&cfg_));
+
+ // Changed rate, but then set changed rate again before trying to update
+ // configuration, update should still apply.
+ bitrates[1] -= 100000;
+ layers_->OnRatesUpdated(bitrates, 5);
+ layers_->OnRatesUpdated(bitrates, 5);
+ EXPECT_TRUE(layers_->UpdateConfiguration(&cfg_));
+}
+
} // namespace webrtc
