From ba8abbb630cdd9d05e22c830d0845e920762850d Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Thu, 13 Aug 2020 09:34:09 +0200 Subject: [PATCH] [Adaptation] Remove QualityScalerResource when disabled. Bug: webrtc:11843 Change-Id: I2d3e40356c266f189db0242f3c7590e6d83e4456 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181369 Commit-Queue: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#31924} --- .../resource_adaptation_processor.cc | 1 + video/adaptation/quality_scaler_resource.cc | 38 ++------- video/adaptation/quality_scaler_resource.h | 14 +-- .../quality_scaler_resource_unittest.cc | 6 +- .../video_stream_encoder_resource_manager.cc | 85 ++++++++++--------- .../video_stream_encoder_resource_manager.h | 26 ++---- video/video_stream_encoder.cc | 67 +++++++-------- video/video_stream_encoder.h | 8 +- video/video_stream_encoder_unittest.cc | 49 +++++++++++ 9 files changed, 147 insertions(+), 147 deletions(-) diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc index b988479d62..c164292e46 100644 --- a/call/adaptation/resource_adaptation_processor.cc +++ b/call/adaptation/resource_adaptation_processor.cc @@ -132,6 +132,7 @@ void ResourceAdaptationProcessor::AddResource( resources_.push_back(resource); } resource->SetResourceListener(resource_listener_delegate_); + RTC_LOG(INFO) << "Registered resource \"" << resource->Name() << "\"."; } std::vector> diff --git a/video/adaptation/quality_scaler_resource.cc b/video/adaptation/quality_scaler_resource.cc index ff8f1712d5..c438488182 100644 --- a/video/adaptation/quality_scaler_resource.cc +++ b/video/adaptation/quality_scaler_resource.cc @@ -12,6 +12,7 @@ #include +#include "rtc_base/checks.h" #include "rtc_base/experiments/balanced_degradation_settings.h" #include "rtc_base/ref_counted_object.h" #include "rtc_base/task_utils/to_queued_task.h" @@ -19,27 +20,14 @@ namespace webrtc { -namespace { - -const int64_t kUnderuseDueToDisabledCooldownMs = 1000; - -} // namespace - // static -rtc::scoped_refptr QualityScalerResource::Create( - DegradationPreferenceProvider* degradation_preference_provider) { - return new rtc::RefCountedObject( - degradation_preference_provider); +rtc::scoped_refptr QualityScalerResource::Create() { + return new rtc::RefCountedObject(); } -QualityScalerResource::QualityScalerResource( - DegradationPreferenceProvider* degradation_preference_provider) +QualityScalerResource::QualityScalerResource() : VideoStreamEncoderResource("QualityScalerResource"), - quality_scaler_(nullptr), - last_underuse_due_to_disabled_timestamp_ms_(absl::nullopt), - degradation_preference_provider_(degradation_preference_provider) { - RTC_CHECK(degradation_preference_provider_); -} + quality_scaler_(nullptr) {} QualityScalerResource::~QualityScalerResource() { RTC_DCHECK(!quality_scaler_); @@ -60,6 +48,7 @@ void QualityScalerResource::StartCheckForOveruse( void QualityScalerResource::StopCheckForOveruse() { RTC_DCHECK_RUN_ON(encoder_queue()); + RTC_DCHECK(is_started()); // Ensure we have no pending callbacks. This makes it safe to destroy the // QualityScaler and even task queues with tasks in-flight. quality_scaler_.reset(); @@ -83,21 +72,6 @@ void QualityScalerResource::OnEncodeCompleted(const EncodedImage& encoded_image, RTC_DCHECK_RUN_ON(encoder_queue()); if (quality_scaler_ && encoded_image.qp_ >= 0) { quality_scaler_->ReportQp(encoded_image.qp_, time_sent_in_us); - } else if (!quality_scaler_) { - // Reference counting guarantees that this object is still alive by the time - // the task is executed. - // TODO(webrtc:11553): this is a workaround to ensure that all quality - // scaler imposed limitations are removed once qualty scaler is disabled - // mid call. - // Instead it should be done at a higher layer in the same way for all - // resources. - int64_t timestamp_ms = rtc::TimeMillis(); - if (!last_underuse_due_to_disabled_timestamp_ms_.has_value() || - timestamp_ms - last_underuse_due_to_disabled_timestamp_ms_.value() >= - kUnderuseDueToDisabledCooldownMs) { - last_underuse_due_to_disabled_timestamp_ms_ = timestamp_ms; - OnResourceUsageStateMeasured(ResourceUsageState::kUnderuse); - } } } diff --git a/video/adaptation/quality_scaler_resource.h b/video/adaptation/quality_scaler_resource.h index 27c255567a..06c22ca3c6 100644 --- a/video/adaptation/quality_scaler_resource.h +++ b/video/adaptation/quality_scaler_resource.h @@ -32,18 +32,15 @@ namespace webrtc { class QualityScalerResource : public VideoStreamEncoderResource, public QualityScalerQpUsageHandlerInterface { public: - static rtc::scoped_refptr Create( - DegradationPreferenceProvider* degradation_preference_provider); + static rtc::scoped_refptr Create(); - explicit QualityScalerResource( - DegradationPreferenceProvider* degradation_preference_provider); + QualityScalerResource(); ~QualityScalerResource() override; bool is_started() const; void StartCheckForOveruse(VideoEncoder::QpThresholds qp_thresholds); void StopCheckForOveruse(); - void SetQpThresholds(VideoEncoder::QpThresholds qp_thresholds); bool QpFastFilterLow(); void OnEncodeCompleted(const EncodedImage& encoded_image, @@ -55,15 +52,8 @@ class QualityScalerResource : public VideoStreamEncoderResource, void OnReportQpUsageLow() override; private: - // Members accessed on the encoder queue. std::unique_ptr quality_scaler_ RTC_GUARDED_BY(encoder_queue()); - // The timestamp of the last time we reported underuse because this resource - // was disabled in order to prevent getting stuck with QP adaptations. Used to - // make sure underuse reporting is not too spammy. - absl::optional last_underuse_due_to_disabled_timestamp_ms_ - RTC_GUARDED_BY(encoder_queue()); - DegradationPreferenceProvider* const degradation_preference_provider_; }; } // namespace webrtc diff --git a/video/adaptation/quality_scaler_resource_unittest.cc b/video/adaptation/quality_scaler_resource_unittest.cc index 15605213c7..1a3175af00 100644 --- a/video/adaptation/quality_scaler_resource_unittest.cc +++ b/video/adaptation/quality_scaler_resource_unittest.cc @@ -41,17 +41,13 @@ class FakeDegradationPreferenceProvider : public DegradationPreferenceProvider { class QualityScalerResourceTest : public ::testing::Test { public: QualityScalerResourceTest() - : quality_scaler_resource_( - QualityScalerResource::Create(°radation_preference_provider_)) { + : quality_scaler_resource_(QualityScalerResource::Create()) { quality_scaler_resource_->RegisterEncoderTaskQueue( TaskQueueBase::Current()); quality_scaler_resource_->SetResourceListener(&fake_resource_listener_); - quality_scaler_resource_->StartCheckForOveruse( - VideoEncoder::QpThresholds()); } ~QualityScalerResourceTest() override { - quality_scaler_resource_->StopCheckForOveruse(); quality_scaler_resource_->SetResourceListener(nullptr); } diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc index 16c8b3d5b3..47bd1b4cc3 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -22,11 +22,15 @@ #include "api/video/video_adaptation_reason.h" #include "api/video/video_source_interface.h" #include "call/adaptation/video_source_restrictions.h" +#include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "rtc_base/numerics/safe_conversions.h" #include "rtc_base/ref_counted_object.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/synchronization/sequence_checker.h" +#include "rtc_base/task_utils/to_queued_task.h" #include "rtc_base/time_utils.h" +#include "video/adaptation/quality_scaler_resource.h" namespace webrtc { @@ -82,7 +86,6 @@ class VideoStreamEncoderResourceManager::InitialFrameDropper { void SetTargetBitrate(DataRate target_bitrate, int64_t now_ms) { if (set_start_bitrate_ > DataRate::Zero() && !has_seen_first_bwe_drop_ && - quality_scaler_resource_->is_started() && quality_scaler_settings_.InitialBitrateIntervalMs() && quality_scaler_settings_.InitialBitrateFactor()) { int64_t diff_ms = now_ms - set_start_bitrate_time_ms_; @@ -273,8 +276,7 @@ VideoStreamEncoderResourceManager::VideoStreamEncoderResourceManager( degradation_preference_provider_)), encode_usage_resource_( EncodeUsageResource::Create(std::move(overuse_detector))), - quality_scaler_resource_( - QualityScalerResource::Create(degradation_preference_provider_)), + quality_scaler_resource_(QualityScalerResource::Create()), encoder_queue_(nullptr), resource_adaptation_queue_(nullptr), input_state_provider_(input_state_provider), @@ -293,12 +295,10 @@ VideoStreamEncoderResourceManager::VideoStreamEncoderResourceManager( encoder_settings_(absl::nullopt) { RTC_CHECK(degradation_preference_provider_); RTC_CHECK(encoder_stats_observer_); - MapResourceToReason(encode_usage_resource_, VideoAdaptationReason::kCpu); - MapResourceToReason(quality_scaler_resource_, - VideoAdaptationReason::kQuality); } -VideoStreamEncoderResourceManager::~VideoStreamEncoderResourceManager() {} +VideoStreamEncoderResourceManager::~VideoStreamEncoderResourceManager() = + default; void VideoStreamEncoderResourceManager::Initialize( rtc::TaskQueue* encoder_queue, @@ -341,37 +341,49 @@ void VideoStreamEncoderResourceManager::EnsureEncodeUsageResourceStarted() { RTC_DCHECK(encoder_settings_.has_value()); if (encode_usage_resource_->is_started()) { encode_usage_resource_->StopCheckForOveruse(); + } else { + // If the resource has not yet started then it needs to be added. + AddResource(encode_usage_resource_, VideoAdaptationReason::kCpu); } encode_usage_resource_->StartCheckForOveruse(GetCpuOveruseOptions()); } void VideoStreamEncoderResourceManager::StopManagedResources() { RTC_DCHECK_RUN_ON(encoder_queue_); - encode_usage_resource_->StopCheckForOveruse(); - quality_scaler_resource_->StopCheckForOveruse(); + RTC_DCHECK(adaptation_processor_); + if (encode_usage_resource_->is_started()) { + encode_usage_resource_->StopCheckForOveruse(); + RemoveResource(encode_usage_resource_); + } + if (quality_scaler_resource_->is_started()) { + quality_scaler_resource_->StopCheckForOveruse(); + RemoveResource(quality_scaler_resource_); + } } -void VideoStreamEncoderResourceManager::MapResourceToReason( +void VideoStreamEncoderResourceManager::AddResource( rtc::scoped_refptr resource, VideoAdaptationReason reason) { MutexLock lock(&resource_lock_); RTC_DCHECK(resource); - RTC_DCHECK(absl::c_find_if(resources_, - [resource](const ResourceAndReason& r) { - return r.resource == resource; - }) == resources_.end()) - << "Resource " << resource->Name() << " already was inserted"; - resources_.emplace_back(resource, reason); + bool inserted; + std::tie(std::ignore, inserted) = resources_.emplace(resource, reason); + RTC_DCHECK(inserted) << "Resurce " << resource->Name() + << " already was inserted"; + adaptation_processor_->AddResource(resource); } -std::vector> -VideoStreamEncoderResourceManager::MappedResources() const { - MutexLock lock(&resource_lock_); - std::vector> resources; - for (auto const& resource_and_reason : resources_) { - resources.push_back(resource_and_reason.resource); +void VideoStreamEncoderResourceManager::RemoveResource( + rtc::scoped_refptr resource) { + { + MutexLock lock(&resource_lock_); + RTC_DCHECK(resource); + const auto& it = resources_.find(resource); + RTC_DCHECK(it != resources_.end()) + << "Resource \"" << resource->Name() << "\" not found."; + resources_.erase(it); } - return resources; + adaptation_processor_->RemoveResource(resource); } std::vector @@ -379,12 +391,6 @@ VideoStreamEncoderResourceManager::AdaptationConstraints() const { return {bitrate_constraint_, balanced_constraint_}; } -rtc::scoped_refptr -VideoStreamEncoderResourceManager::quality_scaler_resource_for_testing() { - MutexLock lock(&resource_lock_); - return quality_scaler_resource_; -} - void VideoStreamEncoderResourceManager::SetEncoderSettings( EncoderSettings encoder_settings) { RTC_DCHECK_RUN_ON(encoder_queue_); @@ -468,7 +474,6 @@ void VideoStreamEncoderResourceManager::OnEncodeCompleted( encoded_image.capture_time_ms_ * rtc::kNumMicrosecsPerMillisec; encode_usage_resource_->OnEncodeCompleted( timestamp, time_sent_in_us, capture_time_us, encode_duration_us); - // Inform |quality_scaler_resource_| of the encode completed event. quality_scaler_resource_->OnEncodeCompleted(encoded_image, time_sent_in_us); } @@ -486,7 +491,7 @@ bool VideoStreamEncoderResourceManager::DropInitialFrames() const { void VideoStreamEncoderResourceManager::OnMaybeEncodeFrame() { RTC_DCHECK_RUN_ON(encoder_queue_); initial_frame_dropper_->OnMaybeEncodeFrame(); - if (quality_rampup_experiment_) { + if (quality_rampup_experiment_ && quality_scaler_resource_->is_started()) { DataRate bandwidth = encoder_rates_.has_value() ? encoder_rates_->bandwidth_allocation : DataRate::Zero(); @@ -502,10 +507,15 @@ void VideoStreamEncoderResourceManager::UpdateQualityScalerSettings( absl::optional qp_thresholds) { RTC_DCHECK_RUN_ON(encoder_queue_); if (qp_thresholds.has_value()) { + if (quality_scaler_resource_->is_started()) { + quality_scaler_resource_->SetQpThresholds(qp_thresholds.value()); + } else { + quality_scaler_resource_->StartCheckForOveruse(qp_thresholds.value()); + AddResource(quality_scaler_resource_, VideoAdaptationReason::kQuality); + } + } else if (quality_scaler_resource_->is_started()) { quality_scaler_resource_->StopCheckForOveruse(); - quality_scaler_resource_->StartCheckForOveruse(qp_thresholds.value()); - } else { - quality_scaler_resource_->StopCheckForOveruse(); + RemoveResource(quality_scaler_resource_); } initial_frame_dropper_->OnQualityScalerSettingsUpdated(); } @@ -555,13 +565,10 @@ void VideoStreamEncoderResourceManager::ConfigureQualityScaler( VideoAdaptationReason VideoStreamEncoderResourceManager::GetReasonFromResource( rtc::scoped_refptr resource) const { MutexLock lock(&resource_lock_); - const auto& registered_resource = - absl::c_find_if(resources_, [&resource](const ResourceAndReason& r) { - return r.resource == resource; - }); + const auto& registered_resource = resources_.find(resource); RTC_DCHECK(registered_resource != resources_.end()) << resource->Name() << " not found."; - return registered_resource->reason; + return registered_resource->second; } // TODO(pbos): Lower these thresholds (to closer to 100%) when we handle diff --git a/video/adaptation/video_stream_encoder_resource_manager.h b/video/adaptation/video_stream_encoder_resource_manager.h index 10d0e66c1c..e7a45d9aa0 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -40,6 +40,7 @@ #include "rtc_base/strings/string_builder.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/task_queue.h" +#include "rtc_base/thread_annotations.h" #include "system_wrappers/include/clock.h" #include "video/adaptation/encode_usage_resource.h" #include "video/adaptation/overuse_frame_detector.h" @@ -115,12 +116,10 @@ class VideoStreamEncoderResourceManager // Resources need to be mapped to an AdaptReason (kCpu or kQuality) in order // to update legacy getStats(). - void MapResourceToReason(rtc::scoped_refptr resource, - VideoAdaptationReason reason); - std::vector> MappedResources() const; + void AddResource(rtc::scoped_refptr resource, + VideoAdaptationReason reason); + void RemoveResource(rtc::scoped_refptr resource); std::vector AdaptationConstraints() const; - rtc::scoped_refptr - quality_scaler_resource_for_testing(); // If true, the VideoStreamEncoder should eexecute its logic to maybe drop // frames baseed on size and bitrate. bool DropInitialFrames() const; @@ -237,8 +236,7 @@ class VideoStreamEncoderResourceManager rtc::TaskQueue* resource_adaptation_queue_; VideoStreamInputStateProvider* const input_state_provider_ RTC_GUARDED_BY(encoder_queue_); - ResourceAdaptationProcessorInterface* adaptation_processor_ - RTC_GUARDED_BY(resource_adaptation_queue_); + ResourceAdaptationProcessorInterface* adaptation_processor_; VideoStreamAdapter* stream_adapter_ RTC_GUARDED_BY(resource_adaptation_queue_); // Thread-safe. @@ -263,19 +261,11 @@ class VideoStreamEncoderResourceManager absl::optional encoder_settings_ RTC_GUARDED_BY(encoder_queue_); + mutable Mutex resource_lock_; // Ties a resource to a reason for statistical reporting. This AdaptReason is // also used by this module to make decisions about how to adapt up/down. - struct ResourceAndReason { - ResourceAndReason(rtc::scoped_refptr resource, - VideoAdaptationReason reason) - : resource(resource), reason(reason) {} - virtual ~ResourceAndReason() = default; - - const rtc::scoped_refptr resource; - const VideoAdaptationReason reason; - }; - mutable Mutex resource_lock_; - std::vector resources_ RTC_GUARDED_BY(&resource_lock_); + std::map, VideoAdaptationReason> resources_ + RTC_GUARDED_BY(&resource_lock_); }; } // namespace webrtc diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index e64e1e92ef..323c9fb39a 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -34,6 +34,7 @@ #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" #include "rtc_base/constructor_magic.h" +#include "rtc_base/event.h" #include "rtc_base/experiments/alr_experiment.h" #include "rtc_base/experiments/rate_control_settings.h" #include "rtc_base/location.h" @@ -386,9 +387,6 @@ VideoStreamEncoder::VideoStreamEncoder( // Add the stream resource manager's resources to the processor. adaptation_constraints_ = stream_resource_manager_.AdaptationConstraints(); - for (auto& resource : stream_resource_manager_.MappedResources()) { - resource_adaptation_processor_->AddResource(resource); - } for (auto* constraint : adaptation_constraints_) { video_stream_adapter_->AddAdaptationConstraint(constraint); } @@ -407,11 +405,19 @@ void VideoStreamEncoder::Stop() { RTC_DCHECK_RUN_ON(&thread_checker_); video_source_sink_controller_.SetSource(nullptr); - if (resource_adaptation_processor_) { - for (auto& resource : stream_resource_manager_.MappedResources()) { - resource_adaptation_processor_->RemoveResource(resource); - } - } + encoder_queue_.PostTask([this] { + RTC_DCHECK_RUN_ON(&encoder_queue_); + stream_resource_manager_.StopManagedResources(); + ShutdownResourceAdaptationQueue(); + rate_allocator_ = nullptr; + bitrate_observer_ = nullptr; + ReleaseEncoder(); + shutdown_event_.Set(); + }); + shutdown_event_.Wait(rtc::Event::kForever); +} + +void VideoStreamEncoder::ShutdownResourceAdaptationQueue() { rtc::Event shutdown_adaptation_processor_event; resource_adaptation_queue_.PostTask([this, &shutdown_adaptation_processor_event] { @@ -423,6 +429,10 @@ void VideoStreamEncoder::Stop() { for (auto* constraint : adaptation_constraints_) { video_stream_adapter_->RemoveAdaptationConstraint(constraint); } + for (auto& resource : additional_resources_) { + stream_resource_manager_.RemoveResource(resource); + } + additional_resources_.clear(); video_stream_adapter_->RemoveRestrictionsListener(this); video_stream_adapter_->RemoveRestrictionsListener( &stream_resource_manager_); @@ -435,15 +445,6 @@ void VideoStreamEncoder::Stop() { shutdown_adaptation_processor_event.Set(); }); shutdown_adaptation_processor_event.Wait(rtc::Event::kForever); - encoder_queue_.PostTask([this] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - stream_resource_manager_.StopManagedResources(); - rate_allocator_ = nullptr; - bitrate_observer_ = nullptr; - ReleaseEncoder(); - shutdown_event_.Set(); - }); - shutdown_event_.Wait(rtc::Event::kForever); } void VideoStreamEncoder::SetBitrateAllocationObserver( @@ -474,11 +475,10 @@ void VideoStreamEncoder::AddAdaptationResource( // TODO(hbos): Make the manager map any unknown resources to kCpu and get rid // of this MapResourceToReason() call. rtc::Event map_resource_event; - encoder_queue_.PostTask([this, resource, &map_resource_event] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - stream_resource_manager_.MapResourceToReason(resource, - VideoAdaptationReason::kCpu); - resource_adaptation_processor_->AddResource(resource); + resource_adaptation_queue_.PostTask([this, resource, &map_resource_event] { + RTC_DCHECK_RUN_ON(&resource_adaptation_queue_); + additional_resources_.push_back(resource); + stream_resource_manager_.AddResource(resource, VideoAdaptationReason::kCpu); map_resource_event.Set(); }); map_resource_event.Wait(rtc::Event::kForever); @@ -837,10 +837,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { } if (pending_encoder_creation_) { - // TODO(hbos): Stopping and restarting for backwards compatibility reasons. - // We may be able to change this to "EnsureStarted()" if it took care of - // reconfiguring the QualityScaler as well. (ConfigureQualityScaler() is - // invoked later in this method.) stream_resource_manager_.EnsureEncodeUsageResourceStarted(); pending_encoder_creation_ = false; } @@ -2084,12 +2080,13 @@ void VideoStreamEncoder::InjectAdaptationResource( rtc::scoped_refptr resource, VideoAdaptationReason reason) { rtc::Event map_resource_event; - encoder_queue_.PostTask([this, resource, reason, &map_resource_event] { - RTC_DCHECK_RUN_ON(&encoder_queue_); - stream_resource_manager_.MapResourceToReason(resource, reason); - resource_adaptation_processor_->AddResource(resource); - map_resource_event.Set(); - }); + resource_adaptation_queue_.PostTask( + [this, resource, reason, &map_resource_event] { + RTC_DCHECK_RUN_ON(&resource_adaptation_queue_); + additional_resources_.push_back(resource); + stream_resource_manager_.AddResource(resource, reason); + map_resource_event.Set(); + }); map_resource_event.Wait(rtc::Event::kForever); } @@ -2110,12 +2107,6 @@ void VideoStreamEncoder::InjectAdaptationConstraint( event.Wait(rtc::Event::kForever); } -rtc::scoped_refptr -VideoStreamEncoder::quality_scaler_resource_for_testing() { - RTC_DCHECK_RUN_ON(&encoder_queue_); - return stream_resource_manager_.quality_scaler_resource_for_testing(); -} - void VideoStreamEncoder::AddRestrictionsListenerForTesting( VideoSourceRestrictionsListener* restrictions_listener) { rtc::Event event; diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 5761896e1d..c8ef1fe6a3 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -40,6 +40,7 @@ #include "rtc_base/rate_statistics.h" #include "rtc_base/synchronization/sequence_checker.h" #include "rtc_base/task_queue.h" +#include "rtc_base/thread_annotations.h" #include "rtc_base/thread_checker.h" #include "system_wrappers/include/clock.h" #include "video/adaptation/video_stream_encoder_resource_manager.h" @@ -128,9 +129,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, VideoAdaptationReason reason); void InjectAdaptationConstraint(AdaptationConstraint* adaptation_constraint); - rtc::scoped_refptr - quality_scaler_resource_for_testing(); - void AddRestrictionsListenerForTesting( VideoSourceRestrictionsListener* restrictions_listener); void RemoveRestrictionsListenerForTesting( @@ -214,6 +212,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, DataSize frame_size); bool HasInternalSource() const RTC_RUN_ON(&encoder_queue_); void ReleaseEncoder() RTC_RUN_ON(&encoder_queue_); + // After calling this function |resource_adaptation_processor_| will be null. + void ShutdownResourceAdaptationQueue(); void CheckForAnimatedContent(const VideoFrame& frame, int64_t time_when_posted_in_ms) @@ -429,6 +429,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // tied to the VideoStreamEncoder (which is destroyed off the encoder queue) // and its resource list is accessible from any thread. VideoStreamEncoderResourceManager stream_resource_manager_; + std::vector> additional_resources_ + RTC_GUARDED_BY(&resource_adaptation_queue_); // Carries out the VideoSourceRestrictions provided by the // ResourceAdaptationProcessor, i.e. reconfigures the source of video frames // to provide us with different resolution or frame rate. diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 3007a6945e..c0719d4334 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -4211,6 +4211,55 @@ TEST_F(VideoStreamEncoderTest, RampsUpInQualityWhenBwIsHigh) { video_stream_encoder_->Stop(); } +TEST_F(VideoStreamEncoderTest, + QualityScalerAdaptationsRemovedWhenQualityScalingDisabled) { + AdaptingFrameForwarder source; + source.set_adaptation_enabled(true); + video_stream_encoder_->SetSource(&source, + DegradationPreference::MAINTAIN_FRAMERATE); + video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources( + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), + DataRate::BitsPerSec(kTargetBitrateBps), 0, 0, 0); + fake_encoder_.SetQp(kQpHigh + 1); + const int kWidth = 1280; + const int kHeight = 720; + const int64_t kFrameIntervalMs = 100; + int64_t timestamp_ms = kFrameIntervalMs; + for (size_t i = 1; i <= 100; i++) { + timestamp_ms += kFrameIntervalMs; + source.IncomingCapturedFrame(CreateFrame(timestamp_ms, kWidth, kHeight)); + WaitForEncodedFrame(timestamp_ms); + } + // Wait for QualityScaler, which will wait for 2000*2.5 ms until checking QP + // for the first time. + // TODO(eshr): We should avoid these waits by using threads with simulated + // time. + EXPECT_TRUE_WAIT(stats_proxy_->GetStats().bw_limited_resolution, + 2000 * 2.5 * 2); + timestamp_ms += kFrameIntervalMs; + source.IncomingCapturedFrame(CreateFrame(timestamp_ms, kWidth, kHeight)); + WaitForEncodedFrame(timestamp_ms); + video_stream_encoder_->WaitUntilTaskQueueIsIdle(); + video_stream_encoder_->WaitUntilAdaptationTaskQueueIsIdle(); + EXPECT_THAT(source.sink_wants(), WantsMaxPixels(Lt(kWidth * kHeight))); + EXPECT_TRUE(stats_proxy_->GetStats().bw_limited_resolution); + + // Disable Quality scaling by turning off scaler on the encoder and + // reconfiguring. + fake_encoder_.SetQualityScaling(false); + video_stream_encoder_->ConfigureEncoder(video_encoder_config_.Copy(), + kMaxPayloadLength); + video_stream_encoder_->WaitUntilTaskQueueIsIdle(); + video_stream_encoder_->WaitUntilAdaptationTaskQueueIsIdle(); + // Since we turned off the quality scaler, the adaptations made by it are + // removed. + EXPECT_THAT(source.sink_wants(), ResolutionMax()); + EXPECT_FALSE(stats_proxy_->GetStats().bw_limited_resolution); + + video_stream_encoder_->Stop(); +} + TEST_F(VideoStreamEncoderTest, ResolutionNotAdaptedForTooSmallFrame_MaintainFramerateMode) { const int kTooSmallWidth = 10;