Revert of Properly report number of quality downscales in stats. (patchset #11 id:220001 of https://codereview.webrtc.org/2564373002/ )
Reason for revert:
Breaks perf tests
Original issue's description:
> Properly report number of quality downscales in stats.
>
> A regression was introduced in 876222f that caused these stats to
> be reported incorrectly. This used to be only implemented for VP8
> but should now be available for all codecs.
>
> BUG=webrtc:6860
>
> Review-Url: https://codereview.webrtc.org/2564373002
> Cr-Commit-Position: refs/heads/master@{#15673}
> Committed: https://chromium.googlesource.com/external/webrtc/+/0c8c5388355f5df085595d9ea24fa38995708120
TBR=asapersson@webrtc.org,stefan@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6860
Review-Url: https://codereview.webrtc.org/2586783003
Cr-Commit-Position: refs/heads/master@{#15678}
diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
index bbb3f09..cb35fdc 100644
--- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
@@ -466,7 +466,7 @@
VideoEncoder::ScalingSettings SimulcastEncoderAdapter::GetScalingSettings()
const {
// Turn off quality scaling for simulcast.
- if (!Initialized() || NumberOfStreams(codec_) != 1)
+ if (NumberOfStreams(codec_) != 1)
return VideoEncoder::ScalingSettings(false);
return streaminfos_[0].encoder->GetScalingSettings();
}
diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc
index 6c249f7..82d7f0d 100644
--- a/webrtc/video/send_statistics_proxy.cc
+++ b/webrtc/video/send_statistics_proxy.cc
@@ -506,13 +506,17 @@
uma_container_->key_frame_counter_.Add(encoded_image._frameType ==
kVideoFrameKey);
stats_.bw_limited_resolution =
- encoded_image.adapt_reason_.bw_resolutions_disabled > 0 ||
- quality_downscales_ > 0;
+ stats_.bw_limited_resolution ||
+ encoded_image.adapt_reason_.bw_resolutions_disabled > 0;
- if (quality_downscales_ != -1) {
- uma_container_->quality_limited_frame_counter_.Add(quality_downscales_ > 0);
- if (quality_downscales_ > 0)
- uma_container_->quality_downscales_counter_.Add(quality_downscales_);
+ if (encoded_image.adapt_reason_.quality_resolution_downscales != -1) {
+ bool downscaled =
+ encoded_image.adapt_reason_.quality_resolution_downscales > 0;
+ uma_container_->quality_limited_frame_counter_.Add(downscaled);
+ if (downscaled) {
+ uma_container_->quality_downscales_counter_.Add(
+ encoded_image.adapt_reason_.quality_resolution_downscales);
+ }
}
if (encoded_image.adapt_reason_.bw_resolutions_disabled != -1) {
bool bw_limited = encoded_image.adapt_reason_.bw_resolutions_disabled > 0;
@@ -583,20 +587,11 @@
uma_container_->cpu_limited_frame_counter_.Add(stats_.cpu_limited_resolution);
}
-void SendStatisticsProxy::SetResolutionRestrictionStats(
- bool scaling_enabled,
- bool cpu_restricted,
- int num_quality_downscales) {
+void SendStatisticsProxy::SetResolutionRestrictionStats(bool bandwidth,
+ bool cpu) {
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_.bw_limited_resolution = bandwidth;
+ stats_.cpu_limited_resolution = cpu;
}
void SendStatisticsProxy::OnCpuRestrictedResolutionChanged(
@@ -607,10 +602,10 @@
}
void SendStatisticsProxy::OnQualityRestrictedResolutionChanged(
- int num_quality_downscales) {
+ bool restricted) {
rtc::CritScope lock(&crit_);
- quality_downscales_ = num_quality_downscales;
- stats_.bw_limited_resolution = quality_downscales_ > 0;
+ uma_container_->quality_downscales_counter_.Add(restricted);
+ stats_.bw_limited_resolution = restricted;
}
void SendStatisticsProxy::RtcpPacketTypesCounterUpdated(
diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h
index ec8bb6d..934dab2 100644
--- a/webrtc/video/send_statistics_proxy.h
+++ b/webrtc/video/send_statistics_proxy.h
@@ -58,10 +58,8 @@
void OnIncomingFrame(int width, int height);
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 OnQualityRestrictedResolutionChanged(bool restricted);
+ void SetResolutionRestrictionStats(bool bandwidth, bool cpu);
void OnEncoderStatsUpdate(uint32_t framerate, uint32_t bitrate);
void OnSuspendChange(bool is_suspended);
@@ -156,7 +154,6 @@
uint32_t last_sent_frame_timestamp_ GUARDED_BY(crit_);
std::map<uint32_t, StatsUpdateTimes> update_times_ GUARDED_BY(crit_);
rtc::ExpFilter encode_time_ GUARDED_BY(crit_);
- int quality_downscales_ GUARDED_BY(crit_) = 0;
// Contains stats used for UMA histograms. These stats will be reset if
// content type changes between real-time video and screenshare, since these
diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc
index acb8a30..4cee104 100644
--- a/webrtc/video/send_statistics_proxy_unittest.cc
+++ b/webrtc/video/send_statistics_proxy_unittest.cc
@@ -31,12 +31,6 @@
const int kHeight = 480;
const int kQpIdx0 = 21;
const int kQpIdx1 = 39;
-const CodecSpecificInfo kDefaultCodecInfo = []() {
- CodecSpecificInfo codec_info;
- codec_info.codecType = kVideoCodecVP8;
- codec_info.codecSpecific.VP8.simulcastIdx = 0;
- return codec_info;
-}();
} // namespace
class SendStatisticsProxyTest : public ::testing::Test {
@@ -665,10 +659,10 @@
TEST_F(SendStatisticsProxyTest,
QualityLimitedHistogramsNotUpdatedWhenDisabled) {
EncodedImage encoded_image;
- statistics_proxy_->SetResolutionRestrictionStats(false /* scaling_enabled */,
- 0, 0);
+ // encoded_image.adapt_reason_.quality_resolution_downscales disabled by
+ // default: -1
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
- statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
+ statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
// Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset();
@@ -680,9 +674,11 @@
TEST_F(SendStatisticsProxyTest,
QualityLimitedHistogramsUpdatedWhenEnabled_NoResolutionDownscale) {
+ const int kDownscales = 0;
EncodedImage encoded_image;
+ encoded_image.adapt_reason_.quality_resolution_downscales = kDownscales;
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
- statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
+ statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
// Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset();
@@ -699,9 +695,10 @@
QualityLimitedHistogramsUpdatedWhenEnabled_TwoResolutionDownscales) {
const int kDownscales = 2;
EncodedImage encoded_image;
- statistics_proxy_->OnQualityRestrictedResolutionChanged(kDownscales);
+ encoded_image.adapt_reason_.quality_resolution_downscales = kDownscales;
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
- statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
+ statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
+
// Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset();
EXPECT_EQ(
@@ -723,18 +720,12 @@
EncodedImage encoded_image;
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
-
- // Simulcast disabled resolutions
- encoded_image.adapt_reason_.bw_resolutions_disabled = 1;
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
-
+ // Resolution not scaled.
encoded_image.adapt_reason_.bw_resolutions_disabled = 0;
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
-
- // Resolution scaled due to quality.
- statistics_proxy_->OnQualityRestrictedResolutionChanged(1);
+ // Resolution scaled due to bandwidth.
+ encoded_image.adapt_reason_.bw_resolutions_disabled = 1;
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
}
diff --git a/webrtc/video/vie_encoder.cc b/webrtc/video/vie_encoder.cc
index 36db9f8..cc33a79 100644
--- a/webrtc/video/vie_encoder.cc
+++ b/webrtc/video/vie_encoder.cc
@@ -15,7 +15,6 @@
#include <utility>
#include "webrtc/modules/video_coding/include/video_codec_initializer.h"
-#include "webrtc/base/arraysize.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/logging.h"
#include "webrtc/base/trace_event.h"
@@ -269,7 +268,6 @@
has_received_rpsi_(false),
picture_id_rpsi_(0),
clock_(Clock::GetRealTimeClock()),
- scale_counter_(kScaleReasonSize, 0),
last_frame_width_(0),
last_frame_height_(0),
last_captured_timestamp_(0),
@@ -341,10 +339,12 @@
source_proxy_->SetSource(source, degradation_preference);
encoder_queue_.PostTask([this, degradation_preference] {
RTC_DCHECK_RUN_ON(&encoder_queue_);
- scaling_enabled_ = (degradation_preference !=
+ scaling_enabled_ =
+ (degradation_preference !=
VideoSendStream::DegradationPreference::kMaintainResolution);
stats_proxy_->SetResolutionRestrictionStats(
- scaling_enabled_, scale_counter_[kCpu] > 0, scale_counter_[kQuality]);
+ scaling_enabled_ && scale_counter_[kQuality] > 0,
+ scaling_enabled_ && scale_counter_[kCpu] > 0);
});
}
@@ -440,8 +440,7 @@
std::move(streams), encoder_config_.min_transmit_bitrate_bps);
const auto scaling_settings = settings_.encoder->GetScalingSettings();
- scaling_enabled_ &= scaling_settings.enabled;
- if (scaling_enabled_) {
+ if (scaling_settings.enabled && scaling_enabled_) {
if (scaling_settings.thresholds) {
quality_scaler_.reset(
new QualityScaler(this, *(scaling_settings.thresholds)));
@@ -450,8 +449,6 @@
}
} else {
quality_scaler_.reset(nullptr);
- stats_proxy_->SetResolutionRestrictionStats(
- false, scale_counter_[kCpu] > 0, scale_counter_[kQuality]);
}
}
@@ -599,8 +596,9 @@
// Encoded is called on whatever thread the real encoder implementation run
// on. In the case of hardware encoders, there might be several encoders
// running in parallel on different threads.
- if (stats_proxy_)
+ if (stats_proxy_) {
stats_proxy_->OnSendEncodedImage(encoded_image, codec_specific_info);
+ }
EncodedImageCallback::Result result =
sink_->OnEncodedImage(encoded_image, codec_specific_info, fragmentation);
@@ -709,8 +707,7 @@
return;
switch (reason) {
case kQuality:
- stats_proxy_->OnQualityRestrictedResolutionChanged(
- scale_counter_[reason] + 1);
+ stats_proxy_->OnQualityRestrictedResolutionChanged(true);
break;
case kCpu:
if (scale_counter_[reason] >= kMaxCpuDowngrades)
@@ -742,7 +739,7 @@
switch (reason) {
case kQuality:
stats_proxy_->OnQualityRestrictedResolutionChanged(
- scale_counter_[reason] - 1);
+ scale_counter_[reason] > 1);
break;
case kCpu:
// Update stats accordingly.
diff --git a/webrtc/video/vie_encoder.h b/webrtc/video/vie_encoder.h
index c086644..ca7bd93 100644
--- a/webrtc/video/vie_encoder.h
+++ b/webrtc/video/vie_encoder.h
@@ -211,7 +211,7 @@
Clock* const clock_;
// Counters used for deciding if the video resolution is currently
// restricted, and if so, why.
- std::vector<int> scale_counter_ ACCESS_ON(&encoder_queue_);
+ int scale_counter_[kScaleReasonSize] ACCESS_ON(&encoder_queue_) = {0};
// Set depending on degradation preferences
bool scaling_enabled_ ACCESS_ON(&encoder_queue_) = false;
diff --git a/webrtc/video_frame.h b/webrtc/video_frame.h
index bec43f8..5fd62a6 100644
--- a/webrtc/video_frame.h
+++ b/webrtc/video_frame.h
@@ -136,10 +136,14 @@
EncodedImage(uint8_t* buffer, size_t length, size_t size)
: _buffer(buffer), _length(length), _size(size) {}
- // TODO(kthelgason): get rid of this struct as it only has a single member
- // remaining.
struct AdaptReason {
- AdaptReason() : bw_resolutions_disabled(-1) {}
+ AdaptReason()
+ : quality_resolution_downscales(-1),
+ bw_resolutions_disabled(-1) {}
+
+ int quality_resolution_downscales; // Number of times this frame is down
+ // scaled in resolution due to quality.
+ // Or -1 if information is not provided.
int bw_resolutions_disabled; // Number of resolutions that are not sent
// due to bandwidth for this frame.
// Or -1 if information is not provided.
