From 36e9eb4773d1a27d67ef632b9537bae82e3d6b6b Mon Sep 17 00:00:00 2001 From: asapersson Date: Fri, 31 Mar 2017 05:29:12 -0700 Subject: [PATCH] Do not report quality limited resolution stats when quality scaler is disabled. WebRTC.Video.QualityLimitedResolutionInPercent is reported as zero for calls when the quality scaler is off (and the degradation preference allows scaling). Update SetResolutionRestrictionStats to specify if quality scaler is enabled. BUG=webrtc:7432 Review-Url: https://codereview.webrtc.org/2783213002 Cr-Commit-Position: refs/heads/master@{#17487} --- webrtc/video/send_statistics_proxy.cc | 21 ++++------ webrtc/video/send_statistics_proxy.h | 5 +-- .../video/send_statistics_proxy_unittest.cc | 22 +++++++++- webrtc/video/vie_encoder.cc | 12 +++--- webrtc/video/vie_encoder_unittest.cc | 42 +++++++++++++++++++ 5 files changed, 79 insertions(+), 23 deletions(-) diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 1f12b2d640..7cefa294e6 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -677,20 +677,15 @@ void SendStatisticsProxy::OnIncomingFrame(int width, int height) { "ssrc", rtp_config_.ssrcs[0]); } -void SendStatisticsProxy::SetResolutionRestrictionStats( - bool scaling_enabled, - bool cpu_restricted, - int num_quality_downscales) { +void SendStatisticsProxy::SetCpuScalingStats(bool cpu_restricted_resolution) { rtc::CritScope lock(&crit_); - if (scaling_enabled) { - quality_downscales_ = num_quality_downscales; - stats_.bw_limited_resolution = quality_downscales_ > 0; - stats_.cpu_limited_resolution = cpu_restricted; - } else { - stats_.bw_limited_resolution = false; - stats_.cpu_limited_resolution = false; - quality_downscales_ = -1; - } + stats_.cpu_limited_resolution = cpu_restricted_resolution; +} + +void SendStatisticsProxy::SetQualityScalingStats(int num_quality_downscales) { + rtc::CritScope lock(&crit_); + quality_downscales_ = num_quality_downscales; + stats_.bw_limited_resolution = quality_downscales_ > 0; } void SendStatisticsProxy::OnCpuRestrictedResolutionChanged( diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index 37db3b0a09..5e45bf5e7a 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -59,9 +59,8 @@ class SendStatisticsProxy : public CpuOveruseMetricsObserver, void OnCpuRestrictedResolutionChanged(bool cpu_restricted_resolution); void OnQualityRestrictedResolutionChanged(int num_quality_downscales); - void SetResolutionRestrictionStats(bool scaling_enabled, - bool cpu_restricted, - int num_quality_downscales); + void SetCpuScalingStats(bool cpu_restricted_resolution); + void SetQualityScalingStats(int num_quality_downscales); // -1: disabled. void OnEncoderStatsUpdate(uint32_t framerate, uint32_t bitrate); void OnSuspendChange(bool is_suspended); diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc index 778514a083..08a2fd26f0 100644 --- a/webrtc/video/send_statistics_proxy_unittest.cc +++ b/webrtc/video/send_statistics_proxy_unittest.cc @@ -367,6 +367,24 @@ TEST_F(SendStatisticsProxyTest, OnSendEncodedImageWithoutQpQpSumWontExist) { EXPECT_EQ(rtc::Optional(), statistics_proxy_->GetStats().qp_sum); } +TEST_F(SendStatisticsProxyTest, SetCpuScalingUpdatesStats) { + EXPECT_FALSE(statistics_proxy_->GetStats().cpu_limited_resolution); + statistics_proxy_->SetCpuScalingStats(true); + EXPECT_TRUE(statistics_proxy_->GetStats().cpu_limited_resolution); + statistics_proxy_->SetCpuScalingStats(false); + EXPECT_FALSE(statistics_proxy_->GetStats().cpu_limited_resolution); +} + +TEST_F(SendStatisticsProxyTest, SetQualityScalingUpdatesStats) { + EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); + statistics_proxy_->SetQualityScalingStats(-1); + EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); + statistics_proxy_->SetQualityScalingStats(0); + EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); + statistics_proxy_->SetQualityScalingStats(1); + EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution); +} + TEST_F(SendStatisticsProxyTest, SwitchContentTypeUpdatesHistograms) { for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i) statistics_proxy_->OnIncomingFrame(kWidth, kHeight); @@ -779,9 +797,9 @@ TEST_F(SendStatisticsProxyTest, TEST_F(SendStatisticsProxyTest, QualityLimitedHistogramsNotUpdatedWhenDisabled) { + const int kNumDownscales = -1; EncodedImage encoded_image; - statistics_proxy_->SetResolutionRestrictionStats(false /* scaling_enabled */, - 0, 0); + statistics_proxy_->SetQualityScalingStats(kNumDownscales); for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i) statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo); diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc index 9940963f83..c816dc272f 100644 --- a/webrtc/video/vie_encoder.cc +++ b/webrtc/video/vie_encoder.cc @@ -461,13 +461,15 @@ void ViEEncoder::ConfigureQualityScaler() { const auto scaling_settings = settings_.encoder->GetScalingSettings(); const bool degradation_preference_allows_scaling = degradation_preference_ != DegradationPreference::kMaintainResolution; + const bool quality_scaling_allowed = + degradation_preference_allows_scaling && scaling_settings.enabled; - stats_proxy_->SetResolutionRestrictionStats( - degradation_preference_allows_scaling, scale_counter_[kCpu] > 0, - scale_counter_[kQuality]); + stats_proxy_->SetCpuScalingStats( + degradation_preference_allows_scaling ? scale_counter_[kCpu] > 0 : false); + stats_proxy_->SetQualityScalingStats( + quality_scaling_allowed ? scale_counter_[kQuality] : -1); - if (degradation_preference_allows_scaling && - scaling_settings.enabled) { + if (quality_scaling_allowed) { // Abort if quality scaler has already been configured. if (quality_scaler_.get() != nullptr) return; diff --git a/webrtc/video/vie_encoder_unittest.cc b/webrtc/video/vie_encoder_unittest.cc index 4c19d5806d..f037600e7d 100644 --- a/webrtc/video/vie_encoder_unittest.cc +++ b/webrtc/video/vie_encoder_unittest.cc @@ -930,6 +930,48 @@ TEST_F(ViEEncoderTest, SwitchingSourceKeepsQualityAdaptation) { vie_encoder_->Stop(); } +TEST_F(ViEEncoderTest, QualityAdaptationStatsAreResetWhenScalerIsDisabled) { + vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); + + const int kWidth = 1280; + const int kHeight = 720; + video_source_.set_adaptation_enabled(true); + video_source_.IncomingCapturedFrame(CreateFrame(1, kWidth, kHeight)); + sink_.WaitForEncodedFrame(1); + EXPECT_FALSE(stats_proxy_->GetStats().cpu_limited_resolution); + EXPECT_FALSE(stats_proxy_->GetStats().bw_limited_resolution); + EXPECT_EQ(0, stats_proxy_->GetStats().number_of_cpu_adapt_changes); + + // Trigger adapt down. + vie_encoder_->TriggerQualityLow(); + video_source_.IncomingCapturedFrame(CreateFrame(2, kWidth, kHeight)); + sink_.WaitForEncodedFrame(2); + EXPECT_FALSE(stats_proxy_->GetStats().cpu_limited_resolution); + EXPECT_TRUE(stats_proxy_->GetStats().bw_limited_resolution); + EXPECT_EQ(0, stats_proxy_->GetStats().number_of_cpu_adapt_changes); + + // Trigger overuse. + vie_encoder_->TriggerCpuOveruse(); + video_source_.IncomingCapturedFrame(CreateFrame(3, kWidth, kHeight)); + sink_.WaitForEncodedFrame(3); + EXPECT_TRUE(stats_proxy_->GetStats().cpu_limited_resolution); + EXPECT_TRUE(stats_proxy_->GetStats().bw_limited_resolution); + EXPECT_EQ(1, stats_proxy_->GetStats().number_of_cpu_adapt_changes); + + // Set source with adaptation still enabled but quality scaler is off. + fake_encoder_.SetQualityScaling(false); + vie_encoder_->SetSource(&video_source_, + VideoSendStream::DegradationPreference::kBalanced); + + video_source_.IncomingCapturedFrame(CreateFrame(4, kWidth, kHeight)); + sink_.WaitForEncodedFrame(4); + EXPECT_TRUE(stats_proxy_->GetStats().cpu_limited_resolution); + EXPECT_FALSE(stats_proxy_->GetStats().bw_limited_resolution); + EXPECT_EQ(1, stats_proxy_->GetStats().number_of_cpu_adapt_changes); + + vie_encoder_->Stop(); +} + TEST_F(ViEEncoderTest, StatsTracksAdaptationStatsWhenSwitchingSource) { vie_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);