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