From f141470d385f33d5ed4e9e0b0a55d94b84884c33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 12 Sep 2018 15:04:08 +0200 Subject: [PATCH] Store qp limits for ScreenshareLayers only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:9745 Change-Id: Ie38b9d4991100657c1dc54660b39b80d86cc64fa Reviewed-on: https://webrtc-review.googlesource.com/99940 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#24716} --- .../codecs/vp8/screenshare_layers.cc | 22 +++++--- .../codecs/vp8/screenshare_layers_unittest.cc | 52 ++++++++++++++++++- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.cc b/modules/video_coding/codecs/vp8/screenshare_layers.cc index 0117d12ccb..12b74fa0d5 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers.cc +++ b/modules/video_coding/codecs/vp8/screenshare_layers.cc @@ -344,6 +344,13 @@ uint32_t ScreenshareLayers::GetCodecTargetBitrateKbps() const { } bool ScreenshareLayers::UpdateConfiguration(Vp8EncoderConfig* cfg) { + if (min_qp_ == -1 || max_qp_ == -1) { + // Store the valid qp range. This must not change during the lifetime of + // this class. + min_qp_ = cfg->rc_min_quantizer; + max_qp_ = cfg->rc_max_quantizer; + } + bool cfg_updated = false; uint32_t target_bitrate_kbps = GetCodecTargetBitrateKbps(); @@ -364,8 +371,6 @@ bool ScreenshareLayers::UpdateConfiguration(Vp8EncoderConfig* cfg) { // Don't reconfigure qp limits during quality boost frames. if (active_layer_ == -1 || layers_[active_layer_].state != TemporalLayer::State::kQualityBoost) { - min_qp_ = cfg->rc_min_quantizer; - max_qp_ = cfg->rc_max_quantizer; // After a dropped frame, a frame with max qp will be encoded and the // quality will then ramp up from there. To boost the speed of recovery, // encode the next frame with lower max qp, if there is sufficient @@ -408,13 +413,14 @@ bool ScreenshareLayers::UpdateConfiguration(Vp8EncoderConfig* cfg) { // If layer is in the quality boost state (following a dropped frame), update // the configuration with the adjusted (lower) qp and set the state back to // normal. - unsigned int adjusted_max_qp; - if (layers_[active_layer_].state == TemporalLayer::State::kQualityBoost && - layers_[active_layer_].enhanced_max_qp != -1) { - adjusted_max_qp = layers_[active_layer_].enhanced_max_qp; + unsigned int adjusted_max_qp = max_qp_; // Set the normal max qp. + if (layers_[active_layer_].state == TemporalLayer::State::kQualityBoost) { + if (layers_[active_layer_].enhanced_max_qp != -1) { + // Bitrate is high enough for quality boost, update max qp. + adjusted_max_qp = layers_[active_layer_].enhanced_max_qp; + } + // Regardless of qp, reset the boost state for the next frame. layers_[active_layer_].state = TemporalLayer::State::kNormal; - } else { - adjusted_max_qp = max_qp_; // Set the normal max qp. } if (adjusted_max_qp == cfg->rc_max_quantizer) diff --git a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc index 0853523557..098827c011 100644 --- a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc +++ b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc @@ -162,7 +162,7 @@ class ScreenshareLayerTest : public ::testing::Test { } int min_qp_; - int max_qp_; + uint32_t max_qp_; int frame_size_; SimulatedClock clock_; std::unique_ptr layers_; @@ -661,4 +661,54 @@ TEST_F(ScreenshareLayerTest, UpdatesConfigurationAfterRateChange) { EXPECT_TRUE(layers_->UpdateConfiguration(&cfg_)); } +TEST_F(ScreenshareLayerTest, MaxQpRestoredAfterDoubleDrop) { + // Run grace period so we have existing frames in both TL0 and Tl1. + EXPECT_TRUE(RunGracePeriod()); + + // Move ahead until we have a sync frame in TL1. + EXPECT_EQ(kTl1SyncFlags, SkipUntilTlAndSync(1, true)); + ASSERT_TRUE(vp8_info_.layerSync); + + // Simulate overshoot of this frame. + layers_->FrameEncoded(timestamp_, 0, -1); + + // Simulate re-encoded frame. + layers_->PopulateCodecSpecific(false, tl_config_, &vp8_info_, timestamp_); + layers_->FrameEncoded(timestamp_, 1, max_qp_); + + // Next frame, expect boosted quality. + // Slightly alter bitrate between each frame. + std::vector kDefault2TlBitratesBpsAlt = kDefault2TlBitratesBps; + kDefault2TlBitratesBpsAlt[1] += 4000; + layers_->OnRatesUpdated(kDefault2TlBitratesBpsAlt, kFrameRate); + EXPECT_EQ(kTl1Flags, SkipUntilTlAndSync(1, false)); + EXPECT_TRUE(config_updated_); + EXPECT_LT(cfg_.rc_max_quantizer, max_qp_); + uint32_t adjusted_qp = cfg_.rc_max_quantizer; + + // Simulate overshoot of this frame. + layers_->FrameEncoded(timestamp_, 0, -1); + + // Simulate re-encoded frame. + layers_->PopulateCodecSpecific(false, tl_config_, &vp8_info_, timestamp_); + layers_->FrameEncoded(timestamp_, frame_size_, max_qp_); + + // A third frame, expect boosted quality. + layers_->OnRatesUpdated(kDefault2TlBitratesBps, kFrameRate); + EXPECT_EQ(kTl1Flags, SkipUntilTlAndSync(1, false)); + EXPECT_TRUE(config_updated_); + EXPECT_LT(cfg_.rc_max_quantizer, max_qp_); + EXPECT_EQ(adjusted_qp, cfg_.rc_max_quantizer); + + // Frame encoded. + layers_->PopulateCodecSpecific(false, tl_config_, &vp8_info_, timestamp_); + layers_->FrameEncoded(timestamp_, frame_size_, max_qp_); + + // A fourth frame, max qp should be restored. + layers_->OnRatesUpdated(kDefault2TlBitratesBpsAlt, kFrameRate); + EXPECT_EQ(kTl1Flags, SkipUntilTlAndSync(1, false)); + EXPECT_TRUE(config_updated_); + EXPECT_EQ(cfg_.rc_max_quantizer, max_qp_); +} + } // namespace webrtc