Reland of Properly report number of quality downscales in stats. (patchset #1 id:1 of https://codereview.webrtc.org/2586783003/ )

Reason for revert:
Bug affecting perf tests has been fixed. The issue was that I had accidentally disabled cpu overuse adaptation based on the encoders ScalingSettings, not just quality-based scaling.

Original issue's description:
> 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: 0c8c538835
>
> 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}
> Committed: fe04bd43cc

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/2588743002
Cr-Commit-Position: refs/heads/master@{#15680}
This commit is contained in:
kthelgason
2016-12-19 06:32:16 -08:00
committed by Commit bot
parent 8c1dd8d32f
commit 0cd27ba088
7 changed files with 64 additions and 49 deletions

View File

@ -466,7 +466,7 @@ bool SimulcastEncoderAdapter::SupportsNativeHandle() const {
VideoEncoder::ScalingSettings SimulcastEncoderAdapter::GetScalingSettings() VideoEncoder::ScalingSettings SimulcastEncoderAdapter::GetScalingSettings()
const { const {
// Turn off quality scaling for simulcast. // Turn off quality scaling for simulcast.
if (NumberOfStreams(codec_) != 1) if (!Initialized() || NumberOfStreams(codec_) != 1)
return VideoEncoder::ScalingSettings(false); return VideoEncoder::ScalingSettings(false);
return streaminfos_[0].encoder->GetScalingSettings(); return streaminfos_[0].encoder->GetScalingSettings();
} }

View File

@ -506,17 +506,13 @@ void SendStatisticsProxy::OnSendEncodedImage(
uma_container_->key_frame_counter_.Add(encoded_image._frameType == uma_container_->key_frame_counter_.Add(encoded_image._frameType ==
kVideoFrameKey); kVideoFrameKey);
stats_.bw_limited_resolution = stats_.bw_limited_resolution =
stats_.bw_limited_resolution || encoded_image.adapt_reason_.bw_resolutions_disabled > 0 ||
encoded_image.adapt_reason_.bw_resolutions_disabled > 0; quality_downscales_ > 0;
if (encoded_image.adapt_reason_.quality_resolution_downscales != -1) { if (quality_downscales_ != -1) {
bool downscaled = uma_container_->quality_limited_frame_counter_.Add(quality_downscales_ > 0);
encoded_image.adapt_reason_.quality_resolution_downscales > 0; if (quality_downscales_ > 0)
uma_container_->quality_limited_frame_counter_.Add(downscaled); uma_container_->quality_downscales_counter_.Add(quality_downscales_);
if (downscaled) {
uma_container_->quality_downscales_counter_.Add(
encoded_image.adapt_reason_.quality_resolution_downscales);
}
} }
if (encoded_image.adapt_reason_.bw_resolutions_disabled != -1) { if (encoded_image.adapt_reason_.bw_resolutions_disabled != -1) {
bool bw_limited = encoded_image.adapt_reason_.bw_resolutions_disabled > 0; bool bw_limited = encoded_image.adapt_reason_.bw_resolutions_disabled > 0;
@ -587,11 +583,20 @@ void SendStatisticsProxy::OnIncomingFrame(int width, int height) {
uma_container_->cpu_limited_frame_counter_.Add(stats_.cpu_limited_resolution); uma_container_->cpu_limited_frame_counter_.Add(stats_.cpu_limited_resolution);
} }
void SendStatisticsProxy::SetResolutionRestrictionStats(bool bandwidth, void SendStatisticsProxy::SetResolutionRestrictionStats(
bool cpu) { bool scaling_enabled,
bool cpu_restricted,
int num_quality_downscales) {
rtc::CritScope lock(&crit_); rtc::CritScope lock(&crit_);
stats_.bw_limited_resolution = bandwidth; if (scaling_enabled) {
stats_.cpu_limited_resolution = cpu; 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;
}
} }
void SendStatisticsProxy::OnCpuRestrictedResolutionChanged( void SendStatisticsProxy::OnCpuRestrictedResolutionChanged(
@ -602,10 +607,10 @@ void SendStatisticsProxy::OnCpuRestrictedResolutionChanged(
} }
void SendStatisticsProxy::OnQualityRestrictedResolutionChanged( void SendStatisticsProxy::OnQualityRestrictedResolutionChanged(
bool restricted) { int num_quality_downscales) {
rtc::CritScope lock(&crit_); rtc::CritScope lock(&crit_);
uma_container_->quality_downscales_counter_.Add(restricted); quality_downscales_ = num_quality_downscales;
stats_.bw_limited_resolution = restricted; stats_.bw_limited_resolution = quality_downscales_ > 0;
} }
void SendStatisticsProxy::RtcpPacketTypesCounterUpdated( void SendStatisticsProxy::RtcpPacketTypesCounterUpdated(

View File

@ -58,8 +58,10 @@ class SendStatisticsProxy : public CpuOveruseMetricsObserver,
void OnIncomingFrame(int width, int height); void OnIncomingFrame(int width, int height);
void OnCpuRestrictedResolutionChanged(bool cpu_restricted_resolution); void OnCpuRestrictedResolutionChanged(bool cpu_restricted_resolution);
void OnQualityRestrictedResolutionChanged(bool restricted); void OnQualityRestrictedResolutionChanged(int num_quality_downscales);
void SetResolutionRestrictionStats(bool bandwidth, bool cpu); void SetResolutionRestrictionStats(bool scaling_enabled,
bool cpu_restricted,
int num_quality_downscales);
void OnEncoderStatsUpdate(uint32_t framerate, uint32_t bitrate); void OnEncoderStatsUpdate(uint32_t framerate, uint32_t bitrate);
void OnSuspendChange(bool is_suspended); void OnSuspendChange(bool is_suspended);
@ -154,6 +156,7 @@ class SendStatisticsProxy : public CpuOveruseMetricsObserver,
uint32_t last_sent_frame_timestamp_ GUARDED_BY(crit_); uint32_t last_sent_frame_timestamp_ GUARDED_BY(crit_);
std::map<uint32_t, StatsUpdateTimes> update_times_ GUARDED_BY(crit_); std::map<uint32_t, StatsUpdateTimes> update_times_ GUARDED_BY(crit_);
rtc::ExpFilter encode_time_ 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 // Contains stats used for UMA histograms. These stats will be reset if
// content type changes between real-time video and screenshare, since these // content type changes between real-time video and screenshare, since these

View File

@ -31,6 +31,12 @@ const int kWidth = 640;
const int kHeight = 480; const int kHeight = 480;
const int kQpIdx0 = 21; const int kQpIdx0 = 21;
const int kQpIdx1 = 39; const int kQpIdx1 = 39;
const CodecSpecificInfo kDefaultCodecInfo = []() {
CodecSpecificInfo codec_info;
codec_info.codecType = kVideoCodecVP8;
codec_info.codecSpecific.VP8.simulcastIdx = 0;
return codec_info;
}();
} // namespace } // namespace
class SendStatisticsProxyTest : public ::testing::Test { class SendStatisticsProxyTest : public ::testing::Test {
@ -659,10 +665,10 @@ TEST_F(SendStatisticsProxyTest,
TEST_F(SendStatisticsProxyTest, TEST_F(SendStatisticsProxyTest,
QualityLimitedHistogramsNotUpdatedWhenDisabled) { QualityLimitedHistogramsNotUpdatedWhenDisabled) {
EncodedImage encoded_image; EncodedImage encoded_image;
// encoded_image.adapt_reason_.quality_resolution_downscales disabled by statistics_proxy_->SetResolutionRestrictionStats(false /* scaling_enabled */,
// default: -1 0, 0);
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i) for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
// Histograms are updated when the statistics_proxy_ is deleted. // Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset(); statistics_proxy_.reset();
@ -674,11 +680,9 @@ TEST_F(SendStatisticsProxyTest,
TEST_F(SendStatisticsProxyTest, TEST_F(SendStatisticsProxyTest,
QualityLimitedHistogramsUpdatedWhenEnabled_NoResolutionDownscale) { QualityLimitedHistogramsUpdatedWhenEnabled_NoResolutionDownscale) {
const int kDownscales = 0;
EncodedImage encoded_image; EncodedImage encoded_image;
encoded_image.adapt_reason_.quality_resolution_downscales = kDownscales;
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i) for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
// Histograms are updated when the statistics_proxy_ is deleted. // Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset(); statistics_proxy_.reset();
@ -695,10 +699,9 @@ TEST_F(SendStatisticsProxyTest,
QualityLimitedHistogramsUpdatedWhenEnabled_TwoResolutionDownscales) { QualityLimitedHistogramsUpdatedWhenEnabled_TwoResolutionDownscales) {
const int kDownscales = 2; const int kDownscales = 2;
EncodedImage encoded_image; EncodedImage encoded_image;
encoded_image.adapt_reason_.quality_resolution_downscales = kDownscales; statistics_proxy_->OnQualityRestrictedResolutionChanged(kDownscales);
for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i) for (int i = 0; i < SendStatisticsProxy::kMinRequiredMetricsSamples; ++i)
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, &kDefaultCodecInfo);
// Histograms are updated when the statistics_proxy_ is deleted. // Histograms are updated when the statistics_proxy_ is deleted.
statistics_proxy_.reset(); statistics_proxy_.reset();
EXPECT_EQ( EXPECT_EQ(
@ -720,12 +723,18 @@ TEST_F(SendStatisticsProxyTest, GetStatsReportsBandwidthLimitedResolution) {
EncodedImage encoded_image; EncodedImage encoded_image;
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
// Resolution not scaled.
// 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);
encoded_image.adapt_reason_.bw_resolutions_disabled = 0; encoded_image.adapt_reason_.bw_resolutions_disabled = 0;
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
// Resolution scaled due to bandwidth.
encoded_image.adapt_reason_.bw_resolutions_disabled = 1; // Resolution scaled due to quality.
statistics_proxy_->OnQualityRestrictedResolutionChanged(1);
statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr); statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution); EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
} }

View File

@ -15,6 +15,7 @@
#include <utility> #include <utility>
#include "webrtc/modules/video_coding/include/video_codec_initializer.h" #include "webrtc/modules/video_coding/include/video_codec_initializer.h"
#include "webrtc/base/arraysize.h"
#include "webrtc/base/checks.h" #include "webrtc/base/checks.h"
#include "webrtc/base/logging.h" #include "webrtc/base/logging.h"
#include "webrtc/base/trace_event.h" #include "webrtc/base/trace_event.h"
@ -268,6 +269,7 @@ ViEEncoder::ViEEncoder(uint32_t number_of_cores,
has_received_rpsi_(false), has_received_rpsi_(false),
picture_id_rpsi_(0), picture_id_rpsi_(0),
clock_(Clock::GetRealTimeClock()), clock_(Clock::GetRealTimeClock()),
scale_counter_(kScaleReasonSize, 0),
last_frame_width_(0), last_frame_width_(0),
last_frame_height_(0), last_frame_height_(0),
last_captured_timestamp_(0), last_captured_timestamp_(0),
@ -339,12 +341,10 @@ void ViEEncoder::SetSource(
source_proxy_->SetSource(source, degradation_preference); source_proxy_->SetSource(source, degradation_preference);
encoder_queue_.PostTask([this, degradation_preference] { encoder_queue_.PostTask([this, degradation_preference] {
RTC_DCHECK_RUN_ON(&encoder_queue_); RTC_DCHECK_RUN_ON(&encoder_queue_);
scaling_enabled_ = scaling_enabled_ = (degradation_preference !=
(degradation_preference !=
VideoSendStream::DegradationPreference::kMaintainResolution); VideoSendStream::DegradationPreference::kMaintainResolution);
stats_proxy_->SetResolutionRestrictionStats( stats_proxy_->SetResolutionRestrictionStats(
scaling_enabled_ && scale_counter_[kQuality] > 0, scaling_enabled_, scale_counter_[kCpu] > 0, scale_counter_[kQuality]);
scaling_enabled_ && scale_counter_[kCpu] > 0);
}); });
} }
@ -440,7 +440,7 @@ void ViEEncoder::ReconfigureEncoder() {
std::move(streams), encoder_config_.min_transmit_bitrate_bps); std::move(streams), encoder_config_.min_transmit_bitrate_bps);
const auto scaling_settings = settings_.encoder->GetScalingSettings(); const auto scaling_settings = settings_.encoder->GetScalingSettings();
if (scaling_settings.enabled && scaling_enabled_) { if (scaling_enabled_ && scaling_settings.enabled) {
if (scaling_settings.thresholds) { if (scaling_settings.thresholds) {
quality_scaler_.reset( quality_scaler_.reset(
new QualityScaler(this, *(scaling_settings.thresholds))); new QualityScaler(this, *(scaling_settings.thresholds)));
@ -449,6 +449,8 @@ void ViEEncoder::ReconfigureEncoder() {
} }
} else { } else {
quality_scaler_.reset(nullptr); quality_scaler_.reset(nullptr);
stats_proxy_->SetResolutionRestrictionStats(
false, scale_counter_[kCpu] > 0, scale_counter_[kQuality]);
} }
} }
@ -596,9 +598,8 @@ EncodedImageCallback::Result ViEEncoder::OnEncodedImage(
// Encoded is called on whatever thread the real encoder implementation run // Encoded is called on whatever thread the real encoder implementation run
// on. In the case of hardware encoders, there might be several encoders // on. In the case of hardware encoders, there might be several encoders
// running in parallel on different threads. // running in parallel on different threads.
if (stats_proxy_) { if (stats_proxy_)
stats_proxy_->OnSendEncodedImage(encoded_image, codec_specific_info); stats_proxy_->OnSendEncodedImage(encoded_image, codec_specific_info);
}
EncodedImageCallback::Result result = EncodedImageCallback::Result result =
sink_->OnEncodedImage(encoded_image, codec_specific_info, fragmentation); sink_->OnEncodedImage(encoded_image, codec_specific_info, fragmentation);
@ -707,7 +708,8 @@ void ViEEncoder::ScaleDown(ScaleReason reason) {
return; return;
switch (reason) { switch (reason) {
case kQuality: case kQuality:
stats_proxy_->OnQualityRestrictedResolutionChanged(true); stats_proxy_->OnQualityRestrictedResolutionChanged(
scale_counter_[reason] + 1);
break; break;
case kCpu: case kCpu:
if (scale_counter_[reason] >= kMaxCpuDowngrades) if (scale_counter_[reason] >= kMaxCpuDowngrades)
@ -739,7 +741,7 @@ void ViEEncoder::ScaleUp(ScaleReason reason) {
switch (reason) { switch (reason) {
case kQuality: case kQuality:
stats_proxy_->OnQualityRestrictedResolutionChanged( stats_proxy_->OnQualityRestrictedResolutionChanged(
scale_counter_[reason] > 1); scale_counter_[reason] - 1);
break; break;
case kCpu: case kCpu:
// Update stats accordingly. // Update stats accordingly.

View File

@ -211,7 +211,7 @@ class ViEEncoder : public rtc::VideoSinkInterface<VideoFrame>,
Clock* const clock_; Clock* const clock_;
// Counters used for deciding if the video resolution is currently // Counters used for deciding if the video resolution is currently
// restricted, and if so, why. // restricted, and if so, why.
int scale_counter_[kScaleReasonSize] ACCESS_ON(&encoder_queue_) = {0}; std::vector<int> scale_counter_ ACCESS_ON(&encoder_queue_);
// Set depending on degradation preferences // Set depending on degradation preferences
bool scaling_enabled_ ACCESS_ON(&encoder_queue_) = false; bool scaling_enabled_ ACCESS_ON(&encoder_queue_) = false;

View File

@ -136,14 +136,10 @@ class EncodedImage {
EncodedImage(uint8_t* buffer, size_t length, size_t size) EncodedImage(uint8_t* buffer, size_t length, size_t size)
: _buffer(buffer), _length(length), _size(size) {} : _buffer(buffer), _length(length), _size(size) {}
// TODO(kthelgason): get rid of this struct as it only has a single member
// remaining.
struct AdaptReason { struct AdaptReason {
AdaptReason() AdaptReason() : bw_resolutions_disabled(-1) {}
: 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 int bw_resolutions_disabled; // Number of resolutions that are not sent
// due to bandwidth for this frame. // due to bandwidth for this frame.
// Or -1 if information is not provided. // Or -1 if information is not provided.