From d6fb409d463dc3eb1f09dc4c6f879d15e5503ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 25 Feb 2020 17:51:08 +0100 Subject: [PATCH] [Overuse] Make some should-be-const methods const. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fact that they weren't const is probably a remenant of the good ol' days this class being multi-threaded and having to acquire mutexes. Now they can properly be made const. In order to make GetConstAdaptCounter() const, this CL makes sure a cleared adapt_counters_ map contains mappings for all degradation preferences to default-constructed AdaptCounters. Previously, if the mapping was missing it was implicitly inserted by GetConstAdaptCounter(). Now it can DCHECK that mappings always exists instead, and it always has something to return. Bug: webrtc:11222 Change-Id: If33227fe6572eb1d6cc6b9f851d6d174d035c110 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168953 Commit-Queue: Henrik Boström Reviewed-by: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#30611} --- ...ame_detector_resource_adaptation_module.cc | 40 +++++++++++++------ ...rame_detector_resource_adaptation_module.h | 12 +++--- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/video/overuse_frame_detector_resource_adaptation_module.cc b/video/overuse_frame_detector_resource_adaptation_module.cc index 21cdf28514..a4d4c04471 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.cc +++ b/video/overuse_frame_detector_resource_adaptation_module.cc @@ -417,6 +417,7 @@ OveruseFrameDetectorResourceAdaptationModule:: encoder_stats_observer_(encoder_stats_observer) { RTC_DCHECK(adaptation_listener_); RTC_DCHECK(encoder_stats_observer_); + ClearAdaptCounters(); AddResource(encode_usage_resource_.get(), AdaptationObserverInterface::AdaptReason::kCpu); AddResource(quality_scaler_resource_.get(), @@ -486,7 +487,7 @@ void OveruseFrameDetectorResourceAdaptationModule::SetDegradationPreference( // TODO(asapersson): Consider removing |adapt_counters_| map and use one // AdaptCounter for all modes. source_restrictor_->ClearRestrictions(); - adapt_counters_.clear(); + ClearAdaptCounters(); } } degradation_preference_ = degradation_preference; @@ -528,7 +529,7 @@ void OveruseFrameDetectorResourceAdaptationModule:: ResetVideoSourceRestrictions() { last_adaptation_request_.reset(); source_restrictor_->ClearRestrictions(); - adapt_counters_.clear(); + ClearAdaptCounters(); MaybeUpdateVideoSourceRestrictions(); } @@ -678,7 +679,7 @@ OveruseFrameDetectorResourceAdaptationModule::OnResourceUsageStateMeasured( bool OveruseFrameDetectorResourceAdaptationModule::CanAdaptUp( AdaptationObserverInterface::AdaptReason reason, - const AdaptationRequest& adaptation_request) { + const AdaptationRequest& adaptation_request) const { if (!has_input_video_) return false; // We can't adapt up if we're already at the highest setting. @@ -701,7 +702,7 @@ bool OveruseFrameDetectorResourceAdaptationModule::CanAdaptUp( // We shouldn't adapt up if BalancedSettings doesn't allow it, which is only // applicable if reason is kQuality and preference is BALANCED. if (reason == AdaptationObserverInterface::AdaptReason::kQuality && - EffectiveDegradataionPreference() == DegradationPreference::BALANCED && + EffectiveDegradationPreference() == DegradationPreference::BALANCED && !balanced_settings_.CanAdaptUp(GetVideoCodecTypeOrGeneric(), LastInputFrameSizeOrDefault(), encoder_target_bitrate_bps_.value_or(0))) { @@ -722,7 +723,7 @@ void OveruseFrameDetectorResourceAdaptationModule::OnResourceUnderuse( if (!CanAdaptUp(reason, adaptation_request)) return; - switch (EffectiveDegradataionPreference()) { + switch (EffectiveDegradationPreference()) { case DegradationPreference::BALANCED: { // Try scale up framerate, if higher. int fps = balanced_settings_.MaxFps(GetVideoCodecTypeOrGeneric(), @@ -803,7 +804,7 @@ void OveruseFrameDetectorResourceAdaptationModule::OnResourceUnderuse( } bool OveruseFrameDetectorResourceAdaptationModule::CanAdaptDown( - const AdaptationRequest& adaptation_request) { + const AdaptationRequest& adaptation_request) const { if (!has_input_video_) return false; // TODO(hbos): Don't support DISABLED, it doesn't exist in the spec and it @@ -817,7 +818,7 @@ bool OveruseFrameDetectorResourceAdaptationModule::CanAdaptDown( last_adaptation_request_->mode_ == AdaptationRequest::Mode::kAdaptDown; // We shouldn't adapt down if our frame rate is below the minimum or if its // currently unknown. - if (EffectiveDegradataionPreference() == + if (EffectiveDegradationPreference() == DegradationPreference::MAINTAIN_RESOLUTION) { // TODO(hbos): This usage of |last_adaptation_was_down| looks like a mistake // - delete it. @@ -856,7 +857,7 @@ OveruseFrameDetectorResourceAdaptationModule::OnResourceOveruse( ResourceListenerResponse response = ResourceListenerResponse::kNothing; - switch (EffectiveDegradataionPreference()) { + switch (EffectiveDegradationPreference()) { case DegradationPreference::BALANCED: { // Try scale down framerate, if lower. int fps = balanced_settings_.MinFps(GetVideoCodecTypeOrGeneric(), @@ -1048,8 +1049,9 @@ OveruseFrameDetectorResourceAdaptationModule::GetActiveCounts( return counts; } -DegradationPreference OveruseFrameDetectorResourceAdaptationModule:: - EffectiveDegradataionPreference() { +DegradationPreference +OveruseFrameDetectorResourceAdaptationModule::EffectiveDegradationPreference() + const { // Balanced mode for screenshare works via automatic animation detection: // Resolution is capped for fullscreen animated content. // Adapatation is done only via framerate downgrade. @@ -1067,9 +1069,23 @@ OveruseFrameDetectorResourceAdaptationModule::GetAdaptCounter() { return adapt_counters_[degradation_preference_]; } +void OveruseFrameDetectorResourceAdaptationModule::ClearAdaptCounters() { + adapt_counters_.clear(); + adapt_counters_.insert( + std::make_pair(DegradationPreference::DISABLED, AdaptCounter())); + adapt_counters_.insert(std::make_pair( + DegradationPreference::MAINTAIN_FRAMERATE, AdaptCounter())); + adapt_counters_.insert(std::make_pair( + DegradationPreference::MAINTAIN_RESOLUTION, AdaptCounter())); + adapt_counters_.insert( + std::make_pair(DegradationPreference::BALANCED, AdaptCounter())); +} + const OveruseFrameDetectorResourceAdaptationModule::AdaptCounter& -OveruseFrameDetectorResourceAdaptationModule::GetConstAdaptCounter() { - return adapt_counters_[degradation_preference_]; +OveruseFrameDetectorResourceAdaptationModule::GetConstAdaptCounter() const { + auto it = adapt_counters_.find(degradation_preference_); + RTC_DCHECK(it != adapt_counters_.cend()); + return it->second; } bool OveruseFrameDetectorResourceAdaptationModule::CanAdaptUpResolution( diff --git a/video/overuse_frame_detector_resource_adaptation_module.h b/video/overuse_frame_detector_resource_adaptation_module.h index e62530320c..1f75f66f20 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.h +++ b/video/overuse_frame_detector_resource_adaptation_module.h @@ -130,12 +130,9 @@ class OveruseFrameDetectorResourceAdaptationModule enum class Mode { kAdaptUp, kAdaptDown } mode_; }; - // TODO(https://crbug.com/webrtc/11222): Make CanAdaptUp/CanAdaptDown const. - // This requires making other private methods const like GetConstAdaptCounter - // and EffectiveDegradataionPreference. // Preconditions for OnResourceUnderuse() to adapt up. bool CanAdaptUp(AdaptationObserverInterface::AdaptReason reason, - const AdaptationRequest& adaptation_request); + const AdaptationRequest& adaptation_request) const; // Adapts up if preconditions apply and VideoSourceRestrictor allows it. // TODO(https://crbug.com/webrtc/11222): This method is still a "Maybe" method // due to the remaining VideoSourceRestrictor logic and it implicitly @@ -145,7 +142,7 @@ class OveruseFrameDetectorResourceAdaptationModule // returns a valid target (or null if there is no next target). void OnResourceUnderuse(AdaptationObserverInterface::AdaptReason reason); // Preconditions for OnResourceOveruse() to adapt down. - bool CanAdaptDown(const AdaptationRequest& adaptation_request); + bool CanAdaptDown(const AdaptationRequest& adaptation_request) const; // Adapts down if preconditions apply and VideoSourceRestrictor allows it. ResourceListenerResponse OnResourceOveruse( AdaptationObserverInterface::AdaptReason reason); @@ -155,7 +152,8 @@ class OveruseFrameDetectorResourceAdaptationModule int LastInputFrameSizeOrDefault() const; VideoStreamEncoderObserver::AdaptationSteps GetActiveCounts( AdaptationObserverInterface::AdaptReason reason); - const AdaptCounter& GetConstAdaptCounter(); + void ClearAdaptCounters(); + const AdaptCounter& GetConstAdaptCounter() const; // Makes |video_source_restrictions_| up-to-date and informs the // |adaptation_listener_| if restrictions are changed, allowing the listener @@ -170,7 +168,7 @@ class OveruseFrameDetectorResourceAdaptationModule absl::optional qp_thresholds); void UpdateAdaptationStats(AdaptationObserverInterface::AdaptReason reason); - DegradationPreference EffectiveDegradataionPreference(); + DegradationPreference EffectiveDegradationPreference() const; AdaptCounter& GetAdaptCounter(); bool CanAdaptUpResolution(int pixels, uint32_t bitrate_bps) const;