From 8d9f7505802b17882148ea4cdd142e65157f1335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Fri, 6 Mar 2020 14:51:34 +0100 Subject: [PATCH] [Overuse] Make EffectiveDegradationPreference() private. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The EffectiveDegradationPreference() exposed an "implementation detail" of the VideoStreamAdapter - how degradation preference may be modified. By changing the return value of ApplyAdaptationTarget() this dependency could be removed. We still have a TODO to get rid of the ResourceListenerResponse enum, but that is QualityScaler related work. This CL does the following: - Module's GetAdaptUpTarget/GetAdaptDownTarget/ApplyAdaptationTarget methods are removed in favor if invoking the VideoStreamAdapter's version of these methods directly. - Removing the EffectiveDegradationPreference() usage in OveruseFrameDetectorResourceAdaptationModule meant moving that usage to VideoStreamAdapter. - MinPixelsPerFrame() is moved to VideoStreamAdapter; this is "can adapt?" logic, i.e. the adapter's responsibility. Bug: webrtc:11393 Change-Id: I75091ce97093bfa48a6d883492de30ed4b004492 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169859 Reviewed-by: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#30714} --- video/adaptation/video_stream_adapter.cc | 76 ++++++++++------ video/adaptation/video_stream_adapter.h | 26 +++--- ...ame_detector_resource_adaptation_module.cc | 90 +++++-------------- ...rame_detector_resource_adaptation_module.h | 18 ---- 4 files changed, 88 insertions(+), 122 deletions(-) diff --git a/video/adaptation/video_stream_adapter.cc b/video/adaptation/video_stream_adapter.cc index 8b589c383b..078410a94b 100644 --- a/video/adaptation/video_stream_adapter.cc +++ b/video/adaptation/video_stream_adapter.cc @@ -25,6 +25,13 @@ namespace { const int kMinFramerateFps = 2; +int MinPixelsPerFrame(const absl::optional& encoder_settings) { + return encoder_settings.has_value() + ? encoder_settings->encoder_info() + .scaling_settings.min_pixels_per_frame + : kDefaultMinPixelsPerFrame; +} + // Generate suggested higher and lower frame rates and resolutions, to be // applied to the VideoSourceRestrictor. These are used in "maintain-resolution" // and "maintain-framerate". The "balanced" degradation preference also makes @@ -263,21 +270,6 @@ VideoStreamAdapter::SetDegradationPreference( : SetDegradationPreferenceResult::kRestrictionsNotCleared; } -DegradationPreference VideoStreamAdapter::EffectiveDegradationPreference( - VideoInputMode input_mode) const { - // Balanced mode for screenshare works via automatic animation detection: - // Resolution is capped for fullscreen animated content. - // Adapatation is done only via framerate downgrade. - // Thus effective degradation preference is MAINTAIN_RESOLUTION. - // TODO(hbos): Don't do this. This is not what "balanced" means. If the - // application wants to maintain resolution it should set that degradation - // preference rather than depend on non-standard behaviors. - return (input_mode == VideoInputMode::kScreenshareVideo && - degradation_preference_ == DegradationPreference::BALANCED) - ? DegradationPreference::MAINTAIN_RESOLUTION - : degradation_preference_; -} - absl::optional VideoStreamAdapter::GetAdaptUpTarget( const absl::optional& encoder_settings, @@ -376,8 +368,8 @@ VideoStreamAdapter::GetAdaptDownTarget( VideoInputMode input_mode, int input_pixels, int input_fps, - int min_pixels_per_frame, VideoStreamEncoderObserver* encoder_stats_observer) const { + const int min_pixels_per_frame = MinPixelsPerFrame(encoder_settings); // Preconditions for being able to adapt down: if (input_mode == VideoInputMode::kNoVideo) return absl::nullopt; @@ -451,24 +443,27 @@ VideoStreamAdapter::GetAdaptDownTarget( } } -void VideoStreamAdapter::ApplyAdaptationTarget(const AdaptationTarget& target, - VideoInputMode input_mode, - int input_pixels, - int input_fps, - int min_pixels_per_frame) { +ResourceListenerResponse VideoStreamAdapter::ApplyAdaptationTarget( + const AdaptationTarget& target, + const absl::optional& encoder_settings, + VideoInputMode input_mode, + int input_pixels, + int input_fps) { + const int min_pixels_per_frame = MinPixelsPerFrame(encoder_settings); // Remember the input pixels and fps of this adaptation. Used to avoid // adapting again before this adaptation has had an effect. last_adaptation_request_.emplace(AdaptationRequest{ input_pixels, input_fps, AdaptationRequest::GetModeFromAdaptationAction(target.action)}); + // Adapt! switch (target.action) { case AdaptationAction::kIncreaseResolution: source_restrictor_->IncreaseResolutionTo(target.value); - return; + break; case AdaptationAction::kDecreaseResolution: source_restrictor_->DecreaseResolutionTo(target.value, min_pixels_per_frame); - return; + break; case AdaptationAction::kIncreaseFrameRate: source_restrictor_->IncreaseFrameRateTo(target.value); // TODO(https://crbug.com/webrtc/11222): Don't adapt in two steps. @@ -483,11 +478,42 @@ void VideoStreamAdapter::ApplyAdaptationTarget(const AdaptationTarget& target, source_restrictor_->IncreaseFrameRateTo( std::numeric_limits::max()); } - return; + break; case AdaptationAction::kDecreaseFrameRate: source_restrictor_->DecreaseFrameRateTo(target.value); - return; + break; } + // In BALANCED, if requested FPS is higher or close to input FPS to the target + // we tell the QualityScaler to increase its frequency. + // TODO(hbos): Don't have QualityScaler-specific logic here. If the + // QualityScaler wants to add special logic depending on what effects + // adaptation had, it should listen to changes to the VideoSourceRestrictions + // instead. + if (EffectiveDegradationPreference(input_mode) == + DegradationPreference::BALANCED && + target.action == + VideoStreamAdapter::AdaptationAction::kDecreaseFrameRate) { + absl::optional min_diff = balanced_settings_.MinFpsDiff(input_pixels); + if (min_diff && input_fps > 0) { + int fps_diff = input_fps - target.value; + if (fps_diff < min_diff.value()) { + return ResourceListenerResponse::kQualityScalerShouldIncreaseFrequency; + } + } + } + return ResourceListenerResponse::kNothing; +} + +DegradationPreference VideoStreamAdapter::EffectiveDegradationPreference( + VideoInputMode input_mode) const { + // Balanced mode for screenshare works via automatic animation detection: + // Resolution is capped for fullscreen animated content. + // Adapatation is done only via framerate downgrade. + // Thus effective degradation preference is MAINTAIN_RESOLUTION. + return (input_mode == VideoInputMode::kScreenshareVideo && + degradation_preference_ == DegradationPreference::BALANCED) + ? DegradationPreference::MAINTAIN_RESOLUTION + : degradation_preference_; } } // namespace webrtc diff --git a/video/adaptation/video_stream_adapter.h b/video/adaptation/video_stream_adapter.h index 40e35ecc77..d4d9ff25b7 100644 --- a/video/adaptation/video_stream_adapter.h +++ b/video/adaptation/video_stream_adapter.h @@ -17,6 +17,7 @@ #include "api/rtp_parameters.h" #include "api/video/video_stream_encoder_observer.h" #include "call/adaptation/encoder_settings.h" +#include "call/adaptation/resource.h" #include "call/adaptation/video_source_restrictions.h" #include "modules/video_coding/utility/quality_scaler.h" #include "rtc_base/experiments/balanced_degradation_settings.h" @@ -85,11 +86,6 @@ class VideoStreamAdapter { // tiny risk that people would discover and rely on this behavior. SetDegradationPreferenceResult SetDegradationPreference( DegradationPreference degradation_preference); - // TODO(hbos): This is only used in one place externally by - // OveruseFrameDetectorResourceAdaptationModule - can we get rid of that - // usage? This is exposing an implementation detail. - DegradationPreference EffectiveDegradationPreference( - VideoInputMode input_mode) const; // Returns a target that we are guaranteed to be able to adapt to, or null if // adaptation is not desired or not possible. @@ -108,14 +104,15 @@ class VideoStreamAdapter { VideoInputMode input_mode, int input_pixels, int input_fps, - int min_pixels_per_frame, VideoStreamEncoderObserver* encoder_stats_observer) const; // Applies the |target| to |source_restrictor_|. - void ApplyAdaptationTarget(const AdaptationTarget& target, - VideoInputMode input_mode, - int input_pixels, - int input_fps, - int min_pixels_per_frame); + // TODO(hbos): Delete ResourceListenerResponse! + ResourceListenerResponse ApplyAdaptationTarget( + const AdaptationTarget& target, + const absl::optional& encoder_settings, + VideoInputMode input_mode, + int input_pixels, + int input_fps); private: class VideoSourceRestrictor; @@ -137,6 +134,13 @@ class VideoStreamAdapter { static Mode GetModeFromAdaptationAction(AdaptationAction action); }; + // Reinterprets "balanced + screenshare" as "maintain-resolution". + // TODO(hbos): Don't do this. This is not what "balanced" means. If the + // application wants to maintain resolution it should set that degradation + // preference rather than depend on non-standard behaviors. + DegradationPreference EffectiveDegradationPreference( + VideoInputMode input_mode) const; + // Owner and modifier of the VideoSourceRestriction of this stream adaptor. const std::unique_ptr source_restrictor_; // Decides the next adaptation target in DegradationPreference::BALANCED. diff --git a/video/overuse_frame_detector_resource_adaptation_module.cc b/video/overuse_frame_detector_resource_adaptation_module.cc index d86b7d90ca..4dc2876d55 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.cc +++ b/video/overuse_frame_detector_resource_adaptation_module.cc @@ -443,11 +443,8 @@ OveruseFrameDetectorResourceAdaptationModule::OnResourceUsageStateMeasured( } } -absl::optional -OveruseFrameDetectorResourceAdaptationModule::GetAdaptUpTarget( - int input_pixels, - int input_fps, - AdaptationObserverInterface::AdaptReason reason) const { +void OveruseFrameDetectorResourceAdaptationModule::OnResourceUnderuse( + AdaptationObserverInterface::AdaptReason reason) { // We can't adapt up if we're already at the highest setting. // Note that this only includes counts relevant to the current degradation // preference. e.g. we previously adapted resolution, now prefer adpating fps, @@ -464,45 +461,21 @@ OveruseFrameDetectorResourceAdaptationModule::GetAdaptUpTarget( .Total(); RTC_DCHECK_GE(num_downgrades, 0); if (num_downgrades == 0) - return absl::nullopt; - return stream_adapter_->GetAdaptUpTarget( - encoder_settings_, encoder_target_bitrate_bps_, GetVideoInputMode(), - input_pixels, input_fps, reason); -} - -absl::optional -OveruseFrameDetectorResourceAdaptationModule::GetAdaptDownTarget( - int input_pixels, - int input_fps, - int min_pixels_per_frame) const { - return stream_adapter_->GetAdaptDownTarget( - encoder_settings_, GetVideoInputMode(), input_pixels, input_fps, - min_pixels_per_frame, encoder_stats_observer_); -} - -void OveruseFrameDetectorResourceAdaptationModule::ApplyAdaptationTarget( - const VideoStreamAdapter::AdaptationTarget& target, - int input_pixels, - int input_fps, - int min_pixels_per_frame) { - stream_adapter_->ApplyAdaptationTarget(target, GetVideoInputMode(), - input_pixels, input_fps, - min_pixels_per_frame); -} - -void OveruseFrameDetectorResourceAdaptationModule::OnResourceUnderuse( - AdaptationObserverInterface::AdaptReason reason) { - int input_pixels = LastInputFrameSizeOrDefault(); - int input_fps = encoder_stats_observer_->GetInputFrameRate(); - int min_pixels_per_frame = MinPixelsPerFrame(); + return; + // Current video input states used by VideoStreamAdapter. + const VideoStreamAdapter::VideoInputMode input_mode = GetVideoInputMode(); + const int input_pixels = LastInputFrameSizeOrDefault(); + const int input_fps = encoder_stats_observer_->GetInputFrameRate(); // Should we adapt, if so to what target? absl::optional target = - GetAdaptUpTarget(input_pixels, input_fps, reason); + stream_adapter_->GetAdaptUpTarget(encoder_settings_, + encoder_target_bitrate_bps_, input_mode, + input_pixels, input_fps, reason); if (!target.has_value()) return; // Apply target. - ApplyAdaptationTarget(target.value(), input_pixels, input_fps, - min_pixels_per_frame); + stream_adapter_->ApplyAdaptationTarget(target.value(), encoder_settings_, + input_mode, input_pixels, input_fps); // Update VideoSourceRestrictions based on adaptation. This also informs the // |adaptation_listener_|. MaybeUpdateVideoSourceRestrictions(); @@ -516,39 +489,27 @@ OveruseFrameDetectorResourceAdaptationModule::OnResourceOveruse( AdaptationObserverInterface::AdaptReason reason) { if (!has_input_video_) return ResourceListenerResponse::kQualityScalerShouldIncreaseFrequency; - int input_pixels = LastInputFrameSizeOrDefault(); - int input_fps = encoder_stats_observer_->GetInputFrameRate(); - int min_pixels_per_frame = MinPixelsPerFrame(); + // Current video input states used by VideoStreamAdapter. + const VideoStreamAdapter::VideoInputMode input_mode = GetVideoInputMode(); + const int input_pixels = LastInputFrameSizeOrDefault(); + const int input_fps = encoder_stats_observer_->GetInputFrameRate(); // Should we adapt, if so to what target? absl::optional target = - GetAdaptDownTarget(input_pixels, input_fps, min_pixels_per_frame); + stream_adapter_->GetAdaptDownTarget(encoder_settings_, input_mode, + input_pixels, input_fps, + encoder_stats_observer_); if (!target.has_value()) return ResourceListenerResponse::kNothing; // Apply target. - ApplyAdaptationTarget(target.value(), input_pixels, input_fps, - min_pixels_per_frame); + ResourceListenerResponse response = stream_adapter_->ApplyAdaptationTarget( + target.value(), encoder_settings_, input_mode, input_pixels, input_fps); // Update VideoSourceRestrictions based on adaptation. This also informs the // |adaptation_listener_|. MaybeUpdateVideoSourceRestrictions(); // Stats and logging. UpdateAdaptationStats(reason); RTC_LOG(INFO) << ActiveCountsToString(); - // In BALANCED, if requested FPS is higher or close to input FPS to the target - // we tell the QualityScaler to increase its frequency. - if (stream_adapter_->EffectiveDegradationPreference(GetVideoInputMode()) == - DegradationPreference::BALANCED && - target->action == - VideoStreamAdapter::AdaptationAction::kDecreaseFrameRate) { - absl::optional min_diff = - stream_adapter_->balanced_settings().MinFpsDiff(input_pixels); - if (min_diff && input_fps > 0) { - int fps_diff = input_fps - target->value; - if (fps_diff < min_diff.value()) { - return ResourceListenerResponse::kQualityScalerShouldIncreaseFrequency; - } - } - } - return ResourceListenerResponse::kNothing; + return response; } // TODO(pbos): Lower these thresholds (to closer to 100%) when we handle @@ -587,13 +548,6 @@ int OveruseFrameDetectorResourceAdaptationModule::LastInputFrameSizeOrDefault() VideoStreamEncoder::kDefaultLastFrameInfoHeight); } -int OveruseFrameDetectorResourceAdaptationModule::MinPixelsPerFrame() const { - return encoder_settings_.has_value() - ? encoder_settings_->encoder_info() - .scaling_settings.min_pixels_per_frame - : kDefaultMinPixelsPerFrame; -} - void OveruseFrameDetectorResourceAdaptationModule:: MaybeUpdateVideoSourceRestrictions() { VideoSourceRestrictions new_restrictions = ApplyDegradationPreference( diff --git a/video/overuse_frame_detector_resource_adaptation_module.h b/video/overuse_frame_detector_resource_adaptation_module.h index e80bd6f1f6..e959e2a8e0 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.h +++ b/video/overuse_frame_detector_resource_adaptation_module.h @@ -27,7 +27,6 @@ #include "api/video_codecs/video_encoder_config.h" #include "call/adaptation/resource.h" #include "call/adaptation/resource_adaptation_module_interface.h" -#include "rtc_base/experiments/balanced_degradation_settings.h" #include "rtc_base/experiments/quality_rampup_experiment.h" #include "rtc_base/experiments/quality_scaler_settings.h" #include "rtc_base/strings/string_builder.h" @@ -132,22 +131,6 @@ class OveruseFrameDetectorResourceAdaptationModule enum class State { kStopped, kStarted }; - // Returns a target that we are guaranteed to be able to adapt to, or null if - // adaptation is not desired or not possible. - absl::optional GetAdaptUpTarget( - int input_pixels, - int input_fps, - AdaptationObserverInterface::AdaptReason reason) const; - absl::optional GetAdaptDownTarget( - int input_pixels, - int input_fps, - int min_pixels_per_frame) const; - // Applies the |target| to |source_restrictor_|. - void ApplyAdaptationTarget(const VideoStreamAdapter::AdaptationTarget& target, - int input_pixels, - int input_fps, - int min_pixels_per_frame); - // Performs the adaptation by getting the next target, applying it and // informing listeners of the new VideoSourceRestriction and adapt counters. void OnResourceUnderuse(AdaptationObserverInterface::AdaptReason reason); @@ -156,7 +139,6 @@ class OveruseFrameDetectorResourceAdaptationModule CpuOveruseOptions GetCpuOveruseOptions() const; int LastInputFrameSizeOrDefault() const; - int MinPixelsPerFrame() const; VideoStreamEncoderObserver::AdaptationSteps GetActiveCounts( AdaptationObserverInterface::AdaptReason reason); VideoStreamAdapter::VideoInputMode GetVideoInputMode() const;