From 61b2993aafc8c89e7c4c4d43b7306a694ed03be2 Mon Sep 17 00:00:00 2001 From: Guido Urdaneta Date: Mon, 17 Aug 2020 13:04:15 +0000 Subject: [PATCH] Revert "[Adaptation] Remove QualityScalerResource when disabled." This reverts commit ba8abbb630cdd9d05e22c830d0845e920762850d. Reason for revert: Suspect of causing Chormium trybots to fail, preventing rolls. Will reland if the revert does not fix it. Original change's description: > [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} TBR=ilnik@webrtc.org,eshr@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:11843 Change-Id: Idc3950be209c6edce0dbe72d98c9b4becae0049f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181880 Reviewed-by: Guido Urdaneta Commit-Queue: Guido Urdaneta Cr-Commit-Position: refs/heads/master@{#31952} --- .../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 | 84 +++++++++---------- .../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, 146 insertions(+), 147 deletions(-) diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc index b303d348dc..c9d324c31c 100644 --- a/call/adaptation/resource_adaptation_processor.cc +++ b/call/adaptation/resource_adaptation_processor.cc @@ -130,7 +130,6 @@ 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 c438488182..ff8f1712d5 100644 --- a/video/adaptation/quality_scaler_resource.cc +++ b/video/adaptation/quality_scaler_resource.cc @@ -12,7 +12,6 @@ #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" @@ -20,14 +19,27 @@ namespace webrtc { +namespace { + +const int64_t kUnderuseDueToDisabledCooldownMs = 1000; + +} // namespace + // static -rtc::scoped_refptr QualityScalerResource::Create() { - return new rtc::RefCountedObject(); +rtc::scoped_refptr QualityScalerResource::Create( + DegradationPreferenceProvider* degradation_preference_provider) { + return new rtc::RefCountedObject( + degradation_preference_provider); } -QualityScalerResource::QualityScalerResource() +QualityScalerResource::QualityScalerResource( + DegradationPreferenceProvider* degradation_preference_provider) : VideoStreamEncoderResource("QualityScalerResource"), - quality_scaler_(nullptr) {} + quality_scaler_(nullptr), + last_underuse_due_to_disabled_timestamp_ms_(absl::nullopt), + degradation_preference_provider_(degradation_preference_provider) { + RTC_CHECK(degradation_preference_provider_); +} QualityScalerResource::~QualityScalerResource() { RTC_DCHECK(!quality_scaler_); @@ -48,7 +60,6 @@ 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(); @@ -72,6 +83,21 @@ 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 06c22ca3c6..27c255567a 100644 --- a/video/adaptation/quality_scaler_resource.h +++ b/video/adaptation/quality_scaler_resource.h @@ -32,15 +32,18 @@ namespace webrtc { class QualityScalerResource : public VideoStreamEncoderResource, public QualityScalerQpUsageHandlerInterface { public: - static rtc::scoped_refptr Create(); + static rtc::scoped_refptr Create( + DegradationPreferenceProvider* degradation_preference_provider); - QualityScalerResource(); + explicit QualityScalerResource( + DegradationPreferenceProvider* degradation_preference_provider); ~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, @@ -52,8 +55,15 @@ 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 1a3175af00..15605213c7 100644 --- a/video/adaptation/quality_scaler_resource_unittest.cc +++ b/video/adaptation/quality_scaler_resource_unittest.cc @@ -41,13 +41,17 @@ class FakeDegradationPreferenceProvider : public DegradationPreferenceProvider { class QualityScalerResourceTest : public ::testing::Test { public: QualityScalerResourceTest() - : quality_scaler_resource_(QualityScalerResource::Create()) { + : quality_scaler_resource_( + QualityScalerResource::Create(°radation_preference_provider_)) { 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 1d8296bb3a..b19ce04c6c 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -22,15 +22,11 @@ #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 { @@ -145,7 +141,8 @@ VideoStreamEncoderResourceManager::VideoStreamEncoderResourceManager( degradation_preference_provider_)), encode_usage_resource_( EncodeUsageResource::Create(std::move(overuse_detector))), - quality_scaler_resource_(QualityScalerResource::Create()), + quality_scaler_resource_( + QualityScalerResource::Create(degradation_preference_provider_)), encoder_queue_(nullptr), resource_adaptation_queue_(nullptr), input_state_provider_(input_state_provider), @@ -164,10 +161,12 @@ 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() = - default; +VideoStreamEncoderResourceManager::~VideoStreamEncoderResourceManager() {} void VideoStreamEncoderResourceManager::Initialize( rtc::TaskQueue* encoder_queue, @@ -210,49 +209,37 @@ 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_); - 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_); - } + encode_usage_resource_->StopCheckForOveruse(); + quality_scaler_resource_->StopCheckForOveruse(); } -void VideoStreamEncoderResourceManager::AddResource( +void VideoStreamEncoderResourceManager::MapResourceToReason( rtc::scoped_refptr resource, VideoAdaptationReason reason) { MutexLock lock(&resource_lock_); RTC_DCHECK(resource); - bool inserted; - std::tie(std::ignore, inserted) = resources_.emplace(resource, reason); - RTC_DCHECK(inserted) << "Resurce " << resource->Name() - << " already was inserted"; - adaptation_processor_->AddResource(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); } -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); +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); } - adaptation_processor_->RemoveResource(resource); + return resources; } std::vector @@ -260,6 +247,12 @@ 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_); @@ -343,6 +336,7 @@ 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); } @@ -360,7 +354,7 @@ bool VideoStreamEncoderResourceManager::DropInitialFrames() const { void VideoStreamEncoderResourceManager::OnMaybeEncodeFrame() { RTC_DCHECK_RUN_ON(encoder_queue_); initial_frame_dropper_->OnMaybeEncodeFrame(); - if (quality_rampup_experiment_ && quality_scaler_resource_->is_started()) { + if (quality_rampup_experiment_) { DataRate bandwidth = encoder_rates_.has_value() ? encoder_rates_->bandwidth_allocation : DataRate::Zero(); @@ -376,15 +370,10 @@ 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(); - RemoveResource(quality_scaler_resource_); + quality_scaler_resource_->StartCheckForOveruse(qp_thresholds.value()); + } else { + quality_scaler_resource_->StopCheckForOveruse(); } initial_frame_dropper_->OnQualityScalerSettingsUpdated(); } @@ -434,10 +423,13 @@ void VideoStreamEncoderResourceManager::ConfigureQualityScaler( VideoAdaptationReason VideoStreamEncoderResourceManager::GetReasonFromResource( rtc::scoped_refptr resource) const { MutexLock lock(&resource_lock_); - const auto& registered_resource = resources_.find(resource); + const auto& registered_resource = + absl::c_find_if(resources_, [&resource](const ResourceAndReason& r) { + return r.resource == resource; + }); RTC_DCHECK(registered_resource != resources_.end()) << resource->Name() << " not found."; - return registered_resource->second; + return registered_resource->reason; } // 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 471dd13324..a070fb9a33 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -40,7 +40,6 @@ #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/balanced_constraint.h" #include "video/adaptation/bitrate_constraint.h" @@ -118,10 +117,12 @@ class VideoStreamEncoderResourceManager // Resources need to be mapped to an AdaptReason (kCpu or kQuality) in order // to update legacy getStats(). - void AddResource(rtc::scoped_refptr resource, - VideoAdaptationReason reason); - void RemoveResource(rtc::scoped_refptr resource); + void MapResourceToReason(rtc::scoped_refptr resource, + VideoAdaptationReason reason); + std::vector> MappedResources() const; 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; @@ -174,7 +175,8 @@ class VideoStreamEncoderResourceManager rtc::TaskQueue* resource_adaptation_queue_; VideoStreamInputStateProvider* const input_state_provider_ RTC_GUARDED_BY(encoder_queue_); - ResourceAdaptationProcessorInterface* adaptation_processor_; + ResourceAdaptationProcessorInterface* adaptation_processor_ + RTC_GUARDED_BY(resource_adaptation_queue_); VideoStreamAdapter* stream_adapter_ RTC_GUARDED_BY(resource_adaptation_queue_); // Thread-safe. @@ -199,11 +201,19 @@ 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. - std::map, VideoAdaptationReason> resources_ - RTC_GUARDED_BY(&resource_lock_); + 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_); }; } // namespace webrtc diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 45e5840f89..7a66b21577 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -34,7 +34,6 @@ #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" @@ -387,6 +386,9 @@ 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); } @@ -405,19 +407,11 @@ void VideoStreamEncoder::Stop() { RTC_DCHECK_RUN_ON(&thread_checker_); video_source_sink_controller_.SetSource(nullptr); - 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() { + if (resource_adaptation_processor_) { + for (auto& resource : stream_resource_manager_.MappedResources()) { + resource_adaptation_processor_->RemoveResource(resource); + } + } rtc::Event shutdown_adaptation_processor_event; resource_adaptation_queue_.PostTask([this, &shutdown_adaptation_processor_event] { @@ -429,10 +423,6 @@ void VideoStreamEncoder::ShutdownResourceAdaptationQueue() { 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_); @@ -445,6 +435,15 @@ void VideoStreamEncoder::ShutdownResourceAdaptationQueue() { 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( @@ -475,10 +474,11 @@ 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; - 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); + 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); map_resource_event.Set(); }); map_resource_event.Wait(rtc::Event::kForever); @@ -837,6 +837,10 @@ 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; } @@ -2080,13 +2084,12 @@ void VideoStreamEncoder::InjectAdaptationResource( rtc::scoped_refptr resource, VideoAdaptationReason reason) { rtc::Event map_resource_event; - 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(); - }); + 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(); + }); map_resource_event.Wait(rtc::Event::kForever); } @@ -2107,6 +2110,12 @@ 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 c8ef1fe6a3..5761896e1d 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -40,7 +40,6 @@ #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" @@ -129,6 +128,9 @@ 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( @@ -212,8 +214,6 @@ 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,8 +429,6 @@ 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 0cacbbea2f..fc64f2c286 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -4254,55 +4254,6 @@ 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;