From b28bc83d02065d2173eadc2163cf9c378830ead1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Mon, 11 May 2020 16:37:41 +0200 Subject: [PATCH] [Adaptation] Make Manager's Resources not depend on encoder queue. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL makes the VideoStreamEncoderResourceManager's inner Resources (PreventAdaptUpDueToActiveCounts, PreventIncreaseResolutionDueToBitrateResource and PreventAdaptUpInBalancedResource) not directly depend on any of the manager's states that will continue to live on the encoder task queue when the adaptation task queue is introduced in the next CL. PreventAdaptUpDueToActiveCounts depends on effective degradation preference, which it can get from the Processor, and the active counts, which will move to the adaptation queue and is safe to use. PreventIncreaseResolutionDueToBitrateResource depends on encoder settings and target bitrate. This Resource now listens to these states being updated, which may be implemented with a PostTask when the adaptation queue is added. PreventAdaptUpInBalancedResource depends on the effective degradation preference, which it can get from the Processor; balanced settings, which is a const readonly struct (thread-safe); and encoder target bitrate, which it listens for being updated (to be PostTask'ed). All resources depends on GetReasonFromResource() which will be callable from the adaptation queue. Bug: webrtc:11542, webrtc:11520 Change-Id: Ifa7bd87d9d8729988073f78f6a37c6f3b8aa4db1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174807 Commit-Queue: Henrik Boström Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Evan Shrubsole Cr-Commit-Position: refs/heads/master@{#31220} --- .../video_stream_encoder_resource_manager.cc | 87 ++++++++++++++++--- .../video_stream_encoder_resource_manager.h | 39 +++++++-- 2 files changed, 106 insertions(+), 20 deletions(-) diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc index f5c812a761..c0103adbd6 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -140,7 +140,15 @@ class VideoStreamEncoderResourceManager::InitialFrameDropper { VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts:: PreventAdaptUpDueToActiveCounts(VideoStreamEncoderResourceManager* manager) - : rtc::RefCountedObject(), manager_(manager) {} + : rtc::RefCountedObject(), + manager_(manager), + adaptation_processor_(nullptr) {} + +void VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts:: + SetAdaptationProcessor( + ResourceAdaptationProcessorInterface* adaptation_processor) { + adaptation_processor_ = adaptation_processor; +} bool VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts:: IsAdaptationUpAllowed(const VideoStreamInputState& input_state, @@ -150,6 +158,7 @@ bool VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts:: // TODO(https://crbug.com/webrtc/11542): When we have an adaptation queue, // ensure that this is running on it instead. RTC_DCHECK_RUN_ON(manager_->encoder_queue_); + RTC_DCHECK(adaptation_processor_); VideoAdaptationReason reason = manager_->GetReasonFromResource(reason_resource); // We can't adapt up if we're already at the highest setting. @@ -162,7 +171,7 @@ bool VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts:: int num_downgrades = FilterVideoAdaptationCountersByDegradationPreference( manager_->active_counts_[reason], - manager_->adaptation_processor_->effective_degradation_preference()) + adaptation_processor_->effective_degradation_preference()) .Total(); RTC_DCHECK_GE(num_downgrades, 0); return num_downgrades > 0; @@ -172,7 +181,27 @@ VideoStreamEncoderResourceManager:: PreventIncreaseResolutionDueToBitrateResource:: PreventIncreaseResolutionDueToBitrateResource( VideoStreamEncoderResourceManager* manager) - : rtc::RefCountedObject(), manager_(manager) {} + : rtc::RefCountedObject(), + manager_(manager), + encoder_settings_(absl::nullopt), + encoder_target_bitrate_bps_(absl::nullopt) {} + +void VideoStreamEncoderResourceManager:: + PreventIncreaseResolutionDueToBitrateResource::OnEncoderSettingsUpdated( + absl::optional encoder_settings) { + // TODO(https://crbug.com/webrtc/11542): When we have an adaptation queue, + // update the state in a PostTask instead. + encoder_settings_ = std::move(encoder_settings); +} + +void VideoStreamEncoderResourceManager:: + PreventIncreaseResolutionDueToBitrateResource:: + OnEncoderTargetBitrateUpdated( + absl::optional encoder_target_bitrate_bps) { + // TODO(https://crbug.com/webrtc/11542): When we have an adaptation queue, + // update the state in a PostTask instead. + encoder_target_bitrate_bps_ = encoder_target_bitrate_bps; +} bool VideoStreamEncoderResourceManager:: PreventIncreaseResolutionDueToBitrateResource::IsAdaptationUpAllowed( @@ -191,10 +220,10 @@ bool VideoStreamEncoderResourceManager:: // due to CPU? Shouldn't this condition be checked regardless of reason? if (reason == VideoAdaptationReason::kQuality && DidIncreaseResolution(restrictions_before, restrictions_after)) { - uint32_t bitrate_bps = manager_->encoder_target_bitrate_bps_.value_or(0); + uint32_t bitrate_bps = encoder_target_bitrate_bps_.value_or(0); absl::optional bitrate_limits = - manager_->encoder_settings_.has_value() - ? manager_->encoder_settings_->encoder_info() + encoder_settings_.has_value() + ? encoder_settings_->encoder_info() .GetEncoderBitrateLimitsForResolution( // Need some sort of expected resulting pixels to be used // instead of unrestricted. @@ -213,7 +242,24 @@ bool VideoStreamEncoderResourceManager:: VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: PreventAdaptUpInBalancedResource(VideoStreamEncoderResourceManager* manager) - : rtc::RefCountedObject(), manager_(manager) {} + : rtc::RefCountedObject(), + manager_(manager), + adaptation_processor_(nullptr), + encoder_target_bitrate_bps_(absl::nullopt) {} + +void VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: + SetAdaptationProcessor( + ResourceAdaptationProcessorInterface* adaptation_processor) { + adaptation_processor_ = adaptation_processor; +} + +void VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: + OnEncoderTargetBitrateUpdated( + absl::optional encoder_target_bitrate_bps) { + // TODO(https://crbug.com/webrtc/11542): When we have an adaptation queue, + // update the state in a PostTask instead. + encoder_target_bitrate_bps_ = encoder_target_bitrate_bps; +} bool VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: IsAdaptationUpAllowed(const VideoStreamInputState& input_state, @@ -223,6 +269,7 @@ bool VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: // TODO(https://crbug.com/webrtc/11542): When we have an adaptation queue, // ensure that this is running on it instead. RTC_DCHECK_RUN_ON(manager_->encoder_queue_); + RTC_DCHECK(adaptation_processor_); VideoAdaptationReason reason = manager_->GetReasonFromResource(reason_resource); // Don't adapt if BalancedDegradationSettings applies and determines this will @@ -230,12 +277,12 @@ bool VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: // TODO(hbos): Why are we allowing violating balanced settings if adapting due // CPU? Shouldn't this condition be checked regardless of reason? if (reason == VideoAdaptationReason::kQuality && - manager_->adaptation_processor_->effective_degradation_preference() == + adaptation_processor_->effective_degradation_preference() == DegradationPreference::BALANCED && !manager_->balanced_settings_.CanAdaptUp( input_state.video_codec_type(), input_state.frame_size_pixels().value(), - manager_->encoder_target_bitrate_bps_.value_or(0))) { + encoder_target_bitrate_bps_.value_or(0))) { return false; } if (reason == VideoAdaptationReason::kQuality && @@ -243,7 +290,7 @@ bool VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource:: !manager_->balanced_settings_.CanAdaptUpResolution( input_state.video_codec_type(), input_state.frame_size_pixels().value(), - manager_->encoder_target_bitrate_bps_.value_or(0))) { + encoder_target_bitrate_bps_.value_or(0))) { return false; } return true; @@ -307,6 +354,10 @@ void VideoStreamEncoderResourceManager::SetAdaptationProcessor( ResourceAdaptationProcessorInterface* adaptation_processor) { RTC_DCHECK_RUN_ON(encoder_queue_); adaptation_processor_ = adaptation_processor; + prevent_adapt_up_due_to_active_counts_->SetAdaptationProcessor( + adaptation_processor); + prevent_adapt_up_in_balanced_resource_->SetAdaptationProcessor( + adaptation_processor); quality_scaler_resource_->SetAdaptationProcessor(adaptation_processor); } @@ -369,6 +420,8 @@ void VideoStreamEncoderResourceManager::SetEncoderSettings( EncoderSettings encoder_settings) { RTC_DCHECK_RUN_ON(encoder_queue_); encoder_settings_ = std::move(encoder_settings); + prevent_increase_resolution_due_to_bitrate_resource_ + ->OnEncoderSettingsUpdated(encoder_settings_); quality_rampup_experiment_.SetMaxBitrate( LastInputFrameSizeOrDefault(), @@ -379,8 +432,13 @@ void VideoStreamEncoderResourceManager::SetEncoderSettings( void VideoStreamEncoderResourceManager::SetStartBitrate( DataRate start_bitrate) { RTC_DCHECK_RUN_ON(encoder_queue_); - if (!start_bitrate.IsZero()) + if (!start_bitrate.IsZero()) { encoder_target_bitrate_bps_ = start_bitrate.bps(); + prevent_increase_resolution_due_to_bitrate_resource_ + ->OnEncoderTargetBitrateUpdated(encoder_target_bitrate_bps_); + prevent_adapt_up_in_balanced_resource_->OnEncoderTargetBitrateUpdated( + encoder_target_bitrate_bps_); + } initial_frame_dropper_->SetStartBitrate(start_bitrate, clock_->TimeInMicroseconds()); } @@ -388,8 +446,13 @@ void VideoStreamEncoderResourceManager::SetStartBitrate( void VideoStreamEncoderResourceManager::SetTargetBitrate( DataRate target_bitrate) { RTC_DCHECK_RUN_ON(encoder_queue_); - if (!target_bitrate.IsZero()) + if (!target_bitrate.IsZero()) { encoder_target_bitrate_bps_ = target_bitrate.bps(); + prevent_increase_resolution_due_to_bitrate_resource_ + ->OnEncoderTargetBitrateUpdated(encoder_target_bitrate_bps_); + prevent_adapt_up_in_balanced_resource_->OnEncoderTargetBitrateUpdated( + encoder_target_bitrate_bps_); + } initial_frame_dropper_->SetTargetBitrate(target_bitrate, clock_->TimeInMilliseconds()); } diff --git a/video/adaptation/video_stream_encoder_resource_manager.h b/video/adaptation/video_stream_encoder_resource_manager.h index 43364a8fff..ac20670727 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -183,10 +183,13 @@ class VideoStreamEncoderResourceManager VideoStreamEncoderResourceManager* manager); ~PreventAdaptUpDueToActiveCounts() override = default; + void SetAdaptationProcessor( + ResourceAdaptationProcessorInterface* adaptation_processor); + + // Resource overrides. std::string name() const override { return "PreventAdaptUpDueToActiveCounts"; } - bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, @@ -194,7 +197,10 @@ class VideoStreamEncoderResourceManager rtc::scoped_refptr reason_resource) const override; private: - VideoStreamEncoderResourceManager* manager_; + // The |manager_| must be alive as long as this resource is added to the + // ResourceAdaptationProcessor, i.e. when IsAdaptationUpAllowed() is called. + VideoStreamEncoderResourceManager* const manager_; + ResourceAdaptationProcessorInterface* adaptation_processor_; }; // Does not trigger adaptations, only prevents adapting up resolution. @@ -205,10 +211,15 @@ class VideoStreamEncoderResourceManager VideoStreamEncoderResourceManager* manager); ~PreventIncreaseResolutionDueToBitrateResource() override = default; + void OnEncoderSettingsUpdated( + absl::optional encoder_settings); + void OnEncoderTargetBitrateUpdated( + absl::optional encoder_target_bitrate_bps); + + // Resource overrides. std::string name() const override { return "PreventIncreaseResolutionDueToBitrateResource"; } - bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, @@ -216,7 +227,11 @@ class VideoStreamEncoderResourceManager rtc::scoped_refptr reason_resource) const override; private: - VideoStreamEncoderResourceManager* manager_; + // The |manager_| must be alive as long as this resource is added to the + // ResourceAdaptationProcessor, i.e. when IsAdaptationUpAllowed() is called. + VideoStreamEncoderResourceManager* const manager_; + absl::optional encoder_settings_; + absl::optional encoder_target_bitrate_bps_; }; // Does not trigger adaptations, only prevents adapting up in BALANCED. @@ -227,10 +242,15 @@ class VideoStreamEncoderResourceManager VideoStreamEncoderResourceManager* manager); ~PreventAdaptUpInBalancedResource() override = default; + void SetAdaptationProcessor( + ResourceAdaptationProcessorInterface* adaptation_processor); + void OnEncoderTargetBitrateUpdated( + absl::optional encoder_target_bitrate_bps); + + // Resource overrides. std::string name() const override { return "PreventAdaptUpInBalancedResource"; } - bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, @@ -238,7 +258,11 @@ class VideoStreamEncoderResourceManager rtc::scoped_refptr reason_resource) const override; private: - VideoStreamEncoderResourceManager* manager_; + // The |manager_| must be alive as long as this resource is added to the + // ResourceAdaptationProcessor, i.e. when IsAdaptationUpAllowed() is called. + VideoStreamEncoderResourceManager* const manager_; + ResourceAdaptationProcessorInterface* adaptation_processor_; + absl::optional encoder_target_bitrate_bps_; }; const rtc::scoped_refptr @@ -264,8 +288,7 @@ class VideoStreamEncoderResourceManager VideoSourceRestrictions video_source_restrictions_ RTC_GUARDED_BY(encoder_queue_); - const BalancedDegradationSettings balanced_settings_ - RTC_GUARDED_BY(encoder_queue_); + const BalancedDegradationSettings balanced_settings_; Clock* clock_ RTC_GUARDED_BY(encoder_queue_); const bool experiment_cpu_load_estimator_ RTC_GUARDED_BY(encoder_queue_); const std::unique_ptr initial_frame_dropper_