From a2cb93d8b9659292f7ec73db53421d481f84c22c Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Tue, 10 Mar 2020 08:09:38 +0000 Subject: [PATCH] Revert "Wire up internal libvpx VP9 scaler to statistics proxy" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 50327a51007c3e25bc3bcd35b5d0945fe0f27d05. Reason for revert: Breaks downstream tests Original change's description: > Wire up internal libvpx VP9 scaler to statistics proxy > > Bug: webrtc:11396 > Change-Id: I5ac69208b00cc75d4e5dbb3ab86f234b3e1f29f8 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169922 > Reviewed-by: Niels Moller > Reviewed-by: Henrik Boström > Commit-Queue: Ilya Nikolaevskiy > Cr-Commit-Position: refs/heads/master@{#30725} TBR=ilnik@webrtc.org,hbos@webrtc.org,nisse@webrtc.org Change-Id: I53dcb41bdf8f8dccfcd43b717509ec047f590648 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:11396 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170102 Reviewed-by: Sebastian Jansson Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#30734} --- api/video/video_stream_encoder_observer.h | 5 ---- video/send_statistics_proxy.cc | 16 +----------- video/send_statistics_proxy.h | 4 --- video/send_statistics_proxy_unittest.cc | 9 ------- video/video_quality_test.cc | 6 +---- video/video_send_stream_tests.cc | 1 - video/video_stream_encoder.cc | 32 ----------------------- 7 files changed, 2 insertions(+), 71 deletions(-) diff --git a/api/video/video_stream_encoder_observer.h b/api/video/video_stream_encoder_observer.h index 9fd462ca3e..0d639f3c05 100644 --- a/api/video/video_stream_encoder_observer.h +++ b/api/video/video_stream_encoder_observer.h @@ -95,11 +95,6 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver { const VideoCodec& codec, const VideoBitrateAllocation& allocation) {} - // Informes observer if an internal encoder scaler has reduced video - // resolution or not. |is_scaled| is a flag indicating if the video is scaled - // down. - virtual void OnEncoderInternalScalerUpdate(bool is_scaled) {} - // TODO(nisse): VideoStreamEncoder wants to query the stats, which makes this // not a pure observer. GetInputFrameRate is needed for the cpu adaptation, so // can be deleted if that responsibility is moved out to a VideoStreamAdaptor diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 60d84f14c7..094baa3157 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -148,7 +148,6 @@ SendStatisticsProxy::SendStatisticsProxy( last_num_simulcast_streams_(0), last_spatial_layer_use_{}, bw_limited_layers_(false), - internal_encoder_scaler_(false), uma_container_( new UmaSamplesContainer(GetUmaPrefix(content_type_), stats_, clock)) { } @@ -1084,7 +1083,7 @@ void SendStatisticsProxy::UpdateAdaptationStats() { cpu_counts_.num_framerate_reductions > 0; bool is_bandwidth_limited = quality_counts_.num_resolution_reductions > 0 || quality_counts_.num_framerate_reductions > 0 || - bw_limited_layers_ || internal_encoder_scaler_; + bw_limited_layers_; if (is_bandwidth_limited) { // We may be both CPU limited and bandwidth limited at the same time but // there is no way to express this in standardized stats. Heuristically, @@ -1118,10 +1117,6 @@ void SendStatisticsProxy::UpdateAdaptationStats() { } } } - if (internal_encoder_scaler_) { - stats_.bw_limited_resolution = true; - } - stats_.quality_limitation_reason = quality_limitation_reason_tracker_.current_reason(); @@ -1169,15 +1164,6 @@ void SendStatisticsProxy::OnBitrateAllocationUpdated( last_num_simulcast_streams_ = num_simulcast_streams; } -// Informes observer if an internal encoder scaler has reduced video -// resolution or not. |is_scaled| is a flag indicating if the video is scaled -// down. -void SendStatisticsProxy::OnEncoderInternalScalerUpdate(bool is_scaled) { - rtc::CritScope lock(&crit_); - internal_encoder_scaler_ = is_scaled; - UpdateAdaptationStats(); -} - // TODO(asapersson): Include fps changes. void SendStatisticsProxy::OnInitialQualityResolutionAdaptDown() { rtc::CritScope lock(&crit_); diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h index abe39992cd..a67725e17a 100644 --- a/video/send_statistics_proxy.h +++ b/video/send_statistics_proxy.h @@ -78,8 +78,6 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, const VideoCodec& codec, const VideoBitrateAllocation& allocation) override; - void OnEncoderInternalScalerUpdate(bool is_scaled) override; - void OnMinPixelLimitReached() override; void OnInitialQualityResolutionAdaptDown() override; @@ -266,8 +264,6 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, // Indicates if the latest bitrate allocation had layers disabled by low // available bandwidth. bool bw_limited_layers_ RTC_GUARDED_BY(crit_); - // Indicastes if the encoder internally downscales input image. - bool internal_encoder_scaler_ RTC_GUARDED_BY(crit_); AdaptationSteps cpu_counts_ RTC_GUARDED_BY(crit_); AdaptationSteps quality_counts_ RTC_GUARDED_BY(crit_); diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index 3f5ebd53b4..db5c94b5cb 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -2147,15 +2147,6 @@ TEST_F(SendStatisticsProxyTest, GetStatsReportsBandwidthLimitedResolution) { allocation.set_bw_limited(true); statistics_proxy_->OnBitrateAllocationUpdated(codec, allocation); EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution); - - // Revert for the next test. - allocation.set_bw_limited(false); - statistics_proxy_->OnBitrateAllocationUpdated(codec, allocation); - EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution); - - // Internal encoder scaler reduced resolution. - statistics_proxy_->OnEncoderInternalScalerUpdate(/*scaled=*/true); - EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution); } TEST_F(SendStatisticsProxyTest, GetStatsReportsTargetMediaBitrate) { diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 826567c21f..b870f7c793 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -870,7 +870,6 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings(); vp9_settings.denoisingOn = false; vp9_settings.frameDroppingOn = false; - vp9_settings.automaticResizeOn = false; vp9_settings.numberOfTemporalLayers = static_cast( params_.video[video_idx].num_temporal_layers); vp9_settings.numberOfSpatialLayers = static_cast( @@ -893,7 +892,6 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, vp9_settings.numberOfSpatialLayers = static_cast(params_.ss[video_idx].num_spatial_layers); vp9_settings.interLayerPred = params_.ss[video_idx].inter_layer_pred; - vp9_settings.automaticResizeOn = false; video_encoder_configs_[video_idx].encoder_specific_settings = new rtc::RefCountedObject< VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings); @@ -906,9 +904,7 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, VideoEncoderConfig::Vp8EncoderSpecificSettings>(vp8_settings); } else if (params_.video[video_idx].codec == "VP9") { VideoCodecVP9 vp9_settings = VideoEncoder::GetDefaultVp9Settings(); - // Only enable quality scaler for single spatial layer. - vp9_settings.automaticResizeOn = - params_.ss[video_idx].num_spatial_layers == 1; + vp9_settings.automaticResizeOn = true; video_encoder_configs_[video_idx].encoder_specific_settings = new rtc::RefCountedObject< VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings); diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 27bf0f08bf..7ceb9db136 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -3479,7 +3479,6 @@ void VideoSendStreamTest::TestVp9NonFlexMode(uint8_t num_temporal_layers, vp9_settings_.flexibleMode = false; vp9_settings_.frameDroppingOn = false; - vp9_settings_.automaticResizeOn = false; vp9_settings_.keyFrameInterval = kKeyFrameInterval; vp9_settings_.numberOfTemporalLayers = num_temporal_layers_; vp9_settings_.numberOfSpatialLayers = num_spatial_layers_; diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index d9eda8e302..58fba37bae 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -169,7 +169,6 @@ VideoBitrateAllocation UpdateAllocationFromEncoderInfo( new_allocation.set_bw_limited(allocation.is_bw_limited()); return new_allocation; } - } // namespace const int VideoStreamEncoder::kDefaultLastFrameInfoWidth = 176; @@ -1361,37 +1360,6 @@ EncodedImageCallback::Result VideoStreamEncoder::OnEncodedImage( RTC_CHECK(videocontenttypehelpers::SetSimulcastId( &image_copy.content_type_, static_cast(spatial_idx + 1))); - // Currently internal quality scaler is used for VP9 instead of webrtc qp - // scaler (in no-svc case or if only a single spatial layer is encoded). - // It has to be explicitly detected and reported to adaptation metrics. - // Post a task because |send_codec_| requires |encoder_queue_| lock. - unsigned int image_width = image_copy._encodedWidth; - unsigned int image_height = image_copy._encodedHeight; - VideoCodecType codec = codec_specific_info - ? codec_specific_info->codecType - : VideoCodecType::kVideoCodecGeneric; - encoder_queue_.PostTask([this, codec, image_width, image_height] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - if (codec == VideoCodecType::kVideoCodecVP9 && - send_codec_.VP9()->automaticResizeOn) { - unsigned int expected_width = send_codec_.width; - unsigned int expected_height = send_codec_.height; - int num_active_layers = 0; - for (int i = 0; i < send_codec_.VP9()->numberOfSpatialLayers; ++i) { - if (send_codec_.spatialLayers[i].active) { - ++num_active_layers; - expected_width = send_codec_.spatialLayers[i].width; - expected_height = send_codec_.spatialLayers[i].height; - } - } - RTC_DCHECK_LE(num_active_layers, 1) - << "VP9 quality scaling is enabled for " - "SVC with several active layers."; - encoder_stats_observer_->OnEncoderInternalScalerUpdate( - image_width < expected_width || image_height < expected_height); - } - }); - // 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.