From 99b0f8d26cbea54fbbae1bdcf0388c42a4e48fce Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Tue, 25 Aug 2020 13:12:28 +0200 Subject: [PATCH] Reland "[Adaptation] Remove QualityScalerResource when disabled." MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a reland of ba8abbb630cdd9d05e22c830d0845e920762850d This can be relanded as the queuing issues that were causing a crash in the WebRTC roll in Chromium have been resolved. I have added the Chromium failing targets to the CQ for this commit and they have succeeded. 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} Bug: webrtc:11843 Change-Id: I228331293060ef996f1dd7f8e18d52b0818f526b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182080 Commit-Queue: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/master@{#31996} --- .../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 | 83 ++++++++++--------- .../video_stream_encoder_resource_manager.h | 23 ++--- video/video_stream_encoder.cc | 32 +++---- video/video_stream_encoder.h | 10 ++- video/video_stream_encoder_unittest.cc | 47 +++++++++++ 9 files changed, 124 insertions(+), 130 deletions(-) diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc index 7ce28777e7..ac1b1db174 100644 --- a/call/adaptation/resource_adaptation_processor.cc +++ b/call/adaptation/resource_adaptation_processor.cc @@ -129,6 +129,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 8ccbc6f3d5..b0b6488954 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -22,12 +22,14 @@ #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/time_utils.h" +#include "video/adaptation/quality_scaler_resource.h" namespace webrtc { @@ -142,8 +144,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), input_state_provider_(input_state_provider), adaptation_processor_(nullptr), @@ -158,14 +159,13 @@ VideoStreamEncoderResourceManager::VideoStreamEncoderResourceManager( encoder_target_bitrate_bps_(absl::nullopt), quality_rampup_experiment_( QualityRampUpExperimentHelper::CreateIfEnabled(this, clock_)), - encoder_settings_(absl::nullopt), - resources_{{encode_usage_resource_, VideoAdaptationReason::kCpu}, - {quality_scaler_resource_, VideoAdaptationReason::kQuality}} { + encoder_settings_(absl::nullopt) { RTC_CHECK(degradation_preference_provider_); RTC_CHECK(encoder_stats_observer_); } -VideoStreamEncoderResourceManager::~VideoStreamEncoderResourceManager() {} +VideoStreamEncoderResourceManager::~VideoStreamEncoderResourceManager() = + default; void VideoStreamEncoderResourceManager::Initialize( rtc::TaskQueue* encoder_queue) { @@ -202,37 +202,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) { RTC_DCHECK_RUN_ON(encoder_queue_); 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 { - RTC_DCHECK_RUN_ON(encoder_queue_); - std::vector> resources; - for (auto const& resource_and_reason : resources_) { - resources.push_back(resource_and_reason.resource); +void VideoStreamEncoderResourceManager::RemoveResource( + rtc::scoped_refptr resource) { + { + RTC_DCHECK_RUN_ON(encoder_queue_); + 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 @@ -241,12 +253,6 @@ VideoStreamEncoderResourceManager::AdaptationConstraints() const { return {bitrate_constraint_.get(), balanced_constraint_.get()}; } -rtc::scoped_refptr -VideoStreamEncoderResourceManager::quality_scaler_resource_for_testing() { - RTC_DCHECK_RUN_ON(encoder_queue_); - return quality_scaler_resource_; -} - void VideoStreamEncoderResourceManager::SetEncoderSettings( EncoderSettings encoder_settings) { RTC_DCHECK_RUN_ON(encoder_queue_); @@ -318,7 +324,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); } @@ -336,7 +341,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(); @@ -352,10 +357,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(); } @@ -405,13 +415,10 @@ void VideoStreamEncoderResourceManager::ConfigureQualityScaler( VideoAdaptationReason VideoStreamEncoderResourceManager::GetReasonFromResource( rtc::scoped_refptr resource) const { RTC_DCHECK_RUN_ON(encoder_queue_); - 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 54b35a45c7..932d90c209 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -117,12 +117,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; @@ -176,8 +174,7 @@ class VideoStreamEncoderResourceManager rtc::TaskQueue* encoder_queue_; VideoStreamInputStateProvider* const input_state_provider_ RTC_GUARDED_BY(encoder_queue_); - ResourceAdaptationProcessorInterface* adaptation_processor_ - RTC_GUARDED_BY(encoder_queue_); + ResourceAdaptationProcessorInterface* adaptation_processor_; VideoStreamAdapter* stream_adapter_ RTC_GUARDED_BY(encoder_queue_); // Thread-safe. VideoStreamEncoderObserver* const encoder_stats_observer_; @@ -203,16 +200,8 @@ class VideoStreamEncoderResourceManager // 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; - }; - std::vector resources_ RTC_GUARDED_BY(encoder_queue_); + std::map, VideoAdaptationReason> resources_ + RTC_GUARDED_BY(encoder_queue_); }; } // namespace webrtc diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index a209361a00..cb6080b456 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" @@ -359,9 +360,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); } @@ -383,12 +381,14 @@ void VideoStreamEncoder::Stop() { encoder_queue_.PostTask([this] { RTC_DCHECK_RUN_ON(&encoder_queue_); if (resource_adaptation_processor_) { - for (auto& resource : stream_resource_manager_.MappedResources()) { - resource_adaptation_processor_->RemoveResource(resource); - } + stream_resource_manager_.StopManagedResources(); 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_); @@ -397,7 +397,6 @@ void VideoStreamEncoder::Stop() { stream_resource_manager_.SetAdaptationProcessor(nullptr, nullptr); resource_adaptation_processor_.reset(); } - stream_resource_manager_.StopManagedResources(); rate_allocator_ = nullptr; bitrate_observer_ = nullptr; ReleaseEncoder(); @@ -436,9 +435,8 @@ void VideoStreamEncoder::AddAdaptationResource( 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); + additional_resources_.push_back(resource); + stream_resource_manager_.AddResource(resource, VideoAdaptationReason::kCpu); map_resource_event.Set(); }); map_resource_event.Wait(rtc::Event::kForever); @@ -797,10 +795,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; } @@ -2045,8 +2039,8 @@ void VideoStreamEncoder::InjectAdaptationResource( 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); + additional_resources_.push_back(resource); + stream_resource_manager_.AddResource(resource, reason); map_resource_event.Set(); }); map_resource_event.Wait(rtc::Event::kForever); @@ -2069,12 +2063,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 1b7376f17b..3a294a97ee 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -126,9 +126,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( @@ -211,6 +208,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) @@ -423,9 +422,12 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, // scaling". Also involved with various mitigations such as inital frame // dropping. // The manager primarily operates on the |encoder_queue_| but its lifetime is - // tied to the VideoStreamEncoder (which is destroyed off the encoder queue). + // tied to the VideoStreamEncoder (which is destroyed off the encoder queue) + // and its resource list is accessible from any thread. VideoStreamEncoderResourceManager stream_resource_manager_ RTC_GUARDED_BY(&encoder_queue_); + std::vector> additional_resources_ + RTC_GUARDED_BY(&encoder_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 ceea7082bb..0bab521202 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -4236,6 +4236,53 @@ 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(); + 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(); + // 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;