[Adaptation] Make Manager's Resources not depend on encoder queue.
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 <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31220}
diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc
index f5c812a..c0103ad 100644
--- a/video/adaptation/video_stream_encoder_resource_manager.cc
+++ b/video/adaptation/video_stream_encoder_resource_manager.cc
@@ -140,7 +140,15 @@
VideoStreamEncoderResourceManager::PreventAdaptUpDueToActiveCounts::
PreventAdaptUpDueToActiveCounts(VideoStreamEncoderResourceManager* manager)
- : rtc::RefCountedObject<Resource>(), manager_(manager) {}
+ : rtc::RefCountedObject<Resource>(),
+ 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 @@
// 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 @@
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 @@
PreventIncreaseResolutionDueToBitrateResource::
PreventIncreaseResolutionDueToBitrateResource(
VideoStreamEncoderResourceManager* manager)
- : rtc::RefCountedObject<Resource>(), manager_(manager) {}
+ : rtc::RefCountedObject<Resource>(),
+ manager_(manager),
+ encoder_settings_(absl::nullopt),
+ encoder_target_bitrate_bps_(absl::nullopt) {}
+
+void VideoStreamEncoderResourceManager::
+ PreventIncreaseResolutionDueToBitrateResource::OnEncoderSettingsUpdated(
+ absl::optional<EncoderSettings> 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<uint32_t> 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 @@
// 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<VideoEncoder::ResolutionBitrateLimits> 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 @@
VideoStreamEncoderResourceManager::PreventAdaptUpInBalancedResource::
PreventAdaptUpInBalancedResource(VideoStreamEncoderResourceManager* manager)
- : rtc::RefCountedObject<Resource>(), manager_(manager) {}
+ : rtc::RefCountedObject<Resource>(),
+ 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<uint32_t> 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 @@
// 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 @@
// 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 @@
!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 @@
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 @@
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::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::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 43364a8..ac20670 100644
--- a/video/adaptation/video_stream_encoder_resource_manager.h
+++ b/video/adaptation/video_stream_encoder_resource_manager.h
@@ -183,10 +183,13 @@
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 @@
rtc::scoped_refptr<Resource> 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 @@
VideoStreamEncoderResourceManager* manager);
~PreventIncreaseResolutionDueToBitrateResource() override = default;
+ void OnEncoderSettingsUpdated(
+ absl::optional<EncoderSettings> encoder_settings);
+ void OnEncoderTargetBitrateUpdated(
+ absl::optional<uint32_t> 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 @@
rtc::scoped_refptr<Resource> 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<EncoderSettings> encoder_settings_;
+ absl::optional<uint32_t> encoder_target_bitrate_bps_;
};
// Does not trigger adaptations, only prevents adapting up in BALANCED.
@@ -227,10 +242,15 @@
VideoStreamEncoderResourceManager* manager);
~PreventAdaptUpInBalancedResource() override = default;
+ void SetAdaptationProcessor(
+ ResourceAdaptationProcessorInterface* adaptation_processor);
+ void OnEncoderTargetBitrateUpdated(
+ absl::optional<uint32_t> 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 @@
rtc::scoped_refptr<Resource> 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<uint32_t> encoder_target_bitrate_bps_;
};
const rtc::scoped_refptr<PreventAdaptUpDueToActiveCounts>
@@ -264,8 +288,7 @@
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<InitialFrameDropper> initial_frame_dropper_
