From a1c77f6d0de621069736b5068f984b7f4241eefb Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Mon, 10 Aug 2020 11:01:06 +0200 Subject: [PATCH] [Adaptation] Move Balanced MinFpsDiff logic to VideoStreamAdapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This way can double adapt right away instead of relying on the qp scaler checking soon into the future. Bug: webrtc:11830 Change-Id: I8e878168303cf6a4c3edcf3997dd8ac2413a4479 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181060 Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Åsa Persson Commit-Queue: Evan Shrubsole Cr-Commit-Position: refs/heads/master@{#31895} --- call/adaptation/video_stream_adapter.cc | 44 ++++- call/adaptation/video_stream_adapter.h | 7 +- .../video_coding/utility/quality_scaler.cc | 96 ++--------- modules/video_coding/utility/quality_scaler.h | 34 +--- .../utility/quality_scaler_unittest.cc | 57 +------ video/adaptation/BUILD.gn | 2 +- video/adaptation/quality_scaler_resource.cc | 101 +---------- video/adaptation/quality_scaler_resource.h | 33 +--- .../quality_scaler_resource_unittest.cc | 158 +++++------------- .../video_stream_encoder_resource_manager.cc | 5 - .../video_stream_encoder_resource_manager.h | 1 - video/video_stream_encoder.cc | 6 - video/video_stream_encoder_unittest.cc | 69 ++------ 13 files changed, 128 insertions(+), 485 deletions(-) diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc index ec80a13a08..ad63cc1d42 100644 --- a/call/adaptation/video_stream_adapter.cc +++ b/call/adaptation/video_stream_adapter.cc @@ -421,13 +421,43 @@ Adaptation VideoStreamAdapter::GetAdaptationDown() { RTC_DCHECK_RUN_ON(&sequence_checker_); VideoStreamInputState input_state = input_state_provider_->InputState(); ++adaptation_validation_id_; - return RestrictionsOrStateToAdaptation(GetAdaptationDownStep(input_state), - input_state); + RestrictionsOrState restrictions_or_state = + GetAdaptationDownStep(input_state, current_restrictions_); + + // Check for min_fps + if (degradation_preference_ == DegradationPreference::BALANCED && + absl::holds_alternative( + restrictions_or_state)) { + restrictions_or_state = AdaptIfFpsDiffInsufficient( + input_state, + absl::get(restrictions_or_state)); + } + return RestrictionsOrStateToAdaptation(restrictions_or_state, input_state); +} + +VideoStreamAdapter::RestrictionsOrState +VideoStreamAdapter::AdaptIfFpsDiffInsufficient( + const VideoStreamInputState& input_state, + const RestrictionsWithCounters& restrictions) const { + RTC_DCHECK_EQ(degradation_preference_, DegradationPreference::BALANCED); + absl::optional min_fps_diff = + balanced_settings_.MinFpsDiff(input_state.frame_size_pixels().value()); + if (current_restrictions_.counters.fps_adaptations < + restrictions.counters.fps_adaptations && + min_fps_diff && input_state.frames_per_second() > 0) { + int fps_diff = input_state.frames_per_second() - + restrictions.restrictions.max_frame_rate().value(); + if (fps_diff < min_fps_diff.value()) { + return GetAdaptationDownStep(input_state, restrictions); + } + } + return restrictions; } VideoStreamAdapter::RestrictionsOrState VideoStreamAdapter::GetAdaptationDownStep( - const VideoStreamInputState& input_state) const { + const VideoStreamInputState& input_state, + const RestrictionsWithCounters& current_restrictions) const { if (!HasSufficientInputForAdaptation(input_state)) { return Adaptation::Status::kInsufficientInput; } @@ -445,7 +475,7 @@ VideoStreamAdapter::GetAdaptationDownStep( case DegradationPreference::BALANCED: { // Try scale down framerate, if lower. RestrictionsOrState decrease_frame_rate = - DecreaseFramerate(input_state, current_restrictions_); + DecreaseFramerate(input_state, current_restrictions); if (absl::holds_alternative( decrease_frame_rate)) { return decrease_frame_rate; @@ -454,10 +484,10 @@ VideoStreamAdapter::GetAdaptationDownStep( ABSL_FALLTHROUGH_INTENDED; } case DegradationPreference::MAINTAIN_FRAMERATE: { - return DecreaseResolution(input_state, current_restrictions_); + return DecreaseResolution(input_state, current_restrictions); } case DegradationPreference::MAINTAIN_RESOLUTION: { - return DecreaseFramerate(input_state, current_restrictions_); + return DecreaseFramerate(input_state, current_restrictions); } case DegradationPreference::DISABLED: return Adaptation::Status::kAdaptationDisabled; @@ -608,7 +638,7 @@ VideoStreamAdapter::RestrictionsOrState VideoStreamAdapter::GetAdaptDownResolutionStepForBalanced( const VideoStreamInputState& input_state) const { // Adapt twice if the first adaptation did not decrease resolution. - auto first_step = GetAdaptationDownStep(input_state); + auto first_step = GetAdaptationDownStep(input_state, current_restrictions_); if (!absl::holds_alternative(first_step)) { return first_step; } diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h index 27699e6aa8..5e54a65dff 100644 --- a/call/adaptation/video_stream_adapter.h +++ b/call/adaptation/video_stream_adapter.h @@ -186,11 +186,16 @@ class VideoStreamAdapter { const VideoStreamInputState& input_state) const RTC_RUN_ON(&sequence_checker_); RestrictionsOrState GetAdaptationDownStep( - const VideoStreamInputState& input_state) const + const VideoStreamInputState& input_state, + const RestrictionsWithCounters& current_restrictions) const RTC_RUN_ON(&sequence_checker_); RestrictionsOrState GetAdaptDownResolutionStepForBalanced( const VideoStreamInputState& input_state) const RTC_RUN_ON(&sequence_checker_); + RestrictionsOrState AdaptIfFpsDiffInsufficient( + const VideoStreamInputState& input_state, + const RestrictionsWithCounters& restrictions) const + RTC_RUN_ON(&sequence_checker_); // TODO(https://crbug.com/webrtc/11771) |resource| is needed by the // AdaptationConstraint resources. Remove this parameter when it's removed. diff --git a/modules/video_coding/utility/quality_scaler.cc b/modules/video_coding/utility/quality_scaler.cc index e909b2f88e..71bf934295 100644 --- a/modules/video_coding/utility/quality_scaler.cc +++ b/modules/video_coding/utility/quality_scaler.cc @@ -86,7 +86,6 @@ class QualityScaler::CheckQpTask { struct Result { bool observed_enough_frames = false; bool qp_usage_reported = false; - bool clear_qp_samples = false; }; CheckQpTask(QualityScaler* quality_scaler, Result previous_task_result) @@ -110,49 +109,36 @@ class QualityScaler::CheckQpTask { case QualityScaler::CheckQpResult::kInsufficientSamples: { result_.observed_enough_frames = false; // After this line, |this| may be deleted. - DoCompleteTask(); - return; + break; } case QualityScaler::CheckQpResult::kNormalQp: { result_.observed_enough_frames = true; - // After this line, |this| may be deleted. - DoCompleteTask(); - return; + break; } case QualityScaler::CheckQpResult::kHighQp: { result_.observed_enough_frames = true; result_.qp_usage_reported = true; - state_ = State::kAwaitingQpUsageHandled; - rtc::scoped_refptr - callback = ConstructCallback(); quality_scaler_->fast_rampup_ = false; - // After this line, |this| may be deleted. - quality_scaler_->handler_->OnReportQpUsageHigh(callback); - return; + quality_scaler_->handler_->OnReportQpUsageHigh(); + quality_scaler_->ClearSamples(); + break; } case QualityScaler::CheckQpResult::kLowQp: { result_.observed_enough_frames = true; result_.qp_usage_reported = true; - state_ = State::kAwaitingQpUsageHandled; - rtc::scoped_refptr - callback = ConstructCallback(); - // After this line, |this| may be deleted. - quality_scaler_->handler_->OnReportQpUsageLow(callback); - return; + quality_scaler_->handler_->OnReportQpUsageLow(); + quality_scaler_->ClearSamples(); + break; } } + state_ = State::kCompleted; + // Starting the next task deletes the pending task. After this line, + // |this| has been deleted. + quality_scaler_->StartNextCheckQpTask(); }), GetCheckingQpDelayMs()); } - void OnQpUsageHandled(bool clear_qp_samples) { - RTC_DCHECK_EQ(state_, State::kAwaitingQpUsageHandled); - result_.clear_qp_samples = clear_qp_samples; - if (clear_qp_samples) - quality_scaler_->ClearSamples(); - DoCompleteTask(); - } - bool HasCompletedTask() const { return state_ == State::kCompleted; } Result result() const { @@ -164,15 +150,9 @@ class QualityScaler::CheckQpTask { enum class State { kNotStarted, kCheckingQp, - kAwaitingQpUsageHandled, kCompleted, }; - // Defined after the definition of QualityScaler::CheckQpTaskHandlerCallback. - // Gets around a forward declaration issue. - rtc::scoped_refptr - ConstructCallback(); - // Determines the sampling period of CheckQpTasks. int64_t GetCheckingQpDelayMs() const { RTC_DCHECK_RUN_ON(&quality_scaler_->task_checker_); @@ -184,10 +164,6 @@ class QualityScaler::CheckQpTask { // Use half the interval while waiting for enough frames. return quality_scaler_->sampling_period_ms_ / 2; } - if (!previous_task_result_.clear_qp_samples) { - // Check shortly again. - return quality_scaler_->sampling_period_ms_ / 8; - } if (quality_scaler_->scale_factor_ && !previous_task_result_.qp_usage_reported) { // Last CheckQp did not call AdaptDown/Up, possibly reduce interval. @@ -198,15 +174,6 @@ class QualityScaler::CheckQpTask { quality_scaler_->initial_scale_factor_; } - void DoCompleteTask() { - RTC_DCHECK(state_ == State::kCheckingQp || - state_ == State::kAwaitingQpUsageHandled); - state_ = State::kCompleted; - // Starting the next task deletes the pending task. After this line, |this| - // has been deleted. - quality_scaler_->StartNextCheckQpTask(); - } - QualityScaler* const quality_scaler_; State state_; const Result previous_task_result_; @@ -215,39 +182,6 @@ class QualityScaler::CheckQpTask { rtc::WeakPtrFactory weak_ptr_factory_; }; -class QualityScaler::CheckQpTaskHandlerCallback - : public QualityScalerQpUsageHandlerCallbackInterface { - public: - CheckQpTaskHandlerCallback( - rtc::WeakPtr check_qp_task) - : QualityScalerQpUsageHandlerCallbackInterface(), - check_qp_task_(std::move(check_qp_task)), - was_handled_(false) {} - - ~CheckQpTaskHandlerCallback() { RTC_DCHECK(was_handled_); } - - void OnQpUsageHandled(bool clear_qp_samples) { - RTC_DCHECK(!was_handled_); - was_handled_ = true; - if (!check_qp_task_) { - // The task has been cancelled through destruction; the result of the - // operation is ignored. - return; - } - check_qp_task_->OnQpUsageHandled(clear_qp_samples); - } - - private: - // The callback may outlive the QualityScaler and its task. - rtc::WeakPtr const check_qp_task_; - bool was_handled_; -}; - -rtc::scoped_refptr -QualityScaler::CheckQpTask::ConstructCallback() { - return new CheckQpTaskHandlerCallback(weak_ptr_factory_.GetWeakPtr()); -} - QualityScaler::QualityScaler(QualityScalerQpUsageHandlerInterface* handler, VideoEncoder::QpThresholds thresholds) : QualityScaler(handler, thresholds, kMeasureMs) {} @@ -401,10 +335,4 @@ void QualityScaler::ClearSamples() { QualityScalerQpUsageHandlerInterface::~QualityScalerQpUsageHandlerInterface() {} -QualityScalerQpUsageHandlerCallbackInterface:: - QualityScalerQpUsageHandlerCallbackInterface() {} - -QualityScalerQpUsageHandlerCallbackInterface:: - ~QualityScalerQpUsageHandlerCallbackInterface() {} - } // namespace webrtc diff --git a/modules/video_coding/utility/quality_scaler.h b/modules/video_coding/utility/quality_scaler.h index cfd2fced3f..28f225f397 100644 --- a/modules/video_coding/utility/quality_scaler.h +++ b/modules/video_coding/utility/quality_scaler.h @@ -112,38 +112,8 @@ class QualityScalerQpUsageHandlerInterface { public: virtual ~QualityScalerQpUsageHandlerInterface(); - // Reacts to QP usage being too high or too low. The |callback| MUST be - // invoked when the handler is done, allowing the QualityScaler to resume - // checking for QP. - virtual void OnReportQpUsageHigh( - rtc::scoped_refptr - callback) = 0; - virtual void OnReportQpUsageLow( - rtc::scoped_refptr - callback) = 0; -}; - -// When QP is reported as high or low by the QualityScaler, it pauses checking -// for QP until the QP usage has been handled. When OnQpUsageHandled() is -// invoked, the QualityScaler resumes checking for QP. This ensures that if the -// stream is reconfigured in response to QP usage we do not include QP samples -// from before the reconfiguration the next time we check for QP. -// -// OnQpUsageHandled() MUST be invoked exactly once before this object is -// destroyed. -class QualityScalerQpUsageHandlerCallbackInterface - : public rtc::RefCountedObject { - public: - virtual ~QualityScalerQpUsageHandlerCallbackInterface(); - - // If |clear_qp_samples| is true, existing QP samples are cleared before the - // next time QualityScaler checks for QP. This is usually a good idea when the - // stream is reconfigured. If |clear_qp_samples| is false, samples are not - // cleared and QualityScaler increases its frequency of checking for QP. - virtual void OnQpUsageHandled(bool clear_qp_samples) = 0; - - protected: - QualityScalerQpUsageHandlerCallbackInterface(); + virtual void OnReportQpUsageHigh() = 0; + virtual void OnReportQpUsageLow() = 0; }; } // namespace webrtc diff --git a/modules/video_coding/utility/quality_scaler_unittest.cc b/modules/video_coding/utility/quality_scaler_unittest.cc index 275b327960..d5b22a8a29 100644 --- a/modules/video_coding/utility/quality_scaler_unittest.cc +++ b/modules/video_coding/utility/quality_scaler_unittest.cc @@ -28,37 +28,24 @@ static const int kMinFramesNeededToScale = 60; // From quality_scaler.cc. static const size_t kDefaultTimeoutMs = 150; } // namespace -class MockQpUsageHandler : public QualityScalerQpUsageHandlerInterface { +class FakeQpUsageHandler : public QualityScalerQpUsageHandlerInterface { public: - virtual ~MockQpUsageHandler() {} + ~FakeQpUsageHandler() override = default; // QualityScalerQpUsageHandlerInterface implementation. - void OnReportQpUsageHigh( - rtc::scoped_refptr callback) - override { - callback_ = callback; + void OnReportQpUsageHigh() override { adapt_down_events_++; event.Set(); - if (synchronously_invoke_callback) - callback_->OnQpUsageHandled(true); } - void OnReportQpUsageLow( - rtc::scoped_refptr callback) - override { - callback_ = callback; + void OnReportQpUsageLow() override { adapt_up_events_++; event.Set(); - if (synchronously_invoke_callback) - callback_->OnQpUsageHandled(true); } rtc::Event event; int adapt_up_events_ = 0; int adapt_down_events_ = 0; - bool synchronously_invoke_callback = true; - rtc::scoped_refptr callback_ = - nullptr; }; // Pass a lower sampling period to speed up the tests. @@ -83,7 +70,7 @@ class QualityScalerTest : public ::testing::Test, QualityScalerTest() : scoped_field_trial_(GetParam()), task_queue_("QualityScalerTestQueue"), - handler_(new MockQpUsageHandler()) { + handler_(std::make_unique()) { task_queue_.SendTask( [this] { qs_ = std::unique_ptr(new QualityScalerUnderTest( @@ -92,7 +79,7 @@ class QualityScalerTest : public ::testing::Test, RTC_FROM_HERE); } - ~QualityScalerTest() { + ~QualityScalerTest() override { task_queue_.SendTask([this] { qs_ = nullptr; }, RTC_FROM_HERE); } @@ -121,7 +108,7 @@ class QualityScalerTest : public ::testing::Test, test::ScopedFieldTrials scoped_field_trial_; TaskQueueForTest task_queue_; std::unique_ptr qs_; - std::unique_ptr handler_; + std::unique_ptr handler_; }; INSTANTIATE_TEST_SUITE_P( @@ -282,34 +269,4 @@ TEST_P(QualityScalerTest, ScalesDownAndBackUpWithMinFramesNeeded) { EXPECT_EQ(1, handler_->adapt_up_events_); } -TEST_P(QualityScalerTest, CheckingQpAgainRequiresResolvingCallback) { - handler_->synchronously_invoke_callback = false; - task_queue_.SendTask([this] { TriggerScale(kScaleDown); }, RTC_FROM_HERE); - EXPECT_TRUE(handler_->event.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(1, handler_->adapt_down_events_); - // Without invoking the callback, another downscale should not happen. - handler_->event.Reset(); - rtc::Event event; - task_queue_.SendTask( - [this, &event] { - TriggerScale(kScaleDown); - event.Set(); - }, - RTC_FROM_HERE); - EXPECT_TRUE(event.Wait(kDefaultTimeoutMs)); - EXPECT_FALSE(handler_->event.Wait(0)); - EXPECT_EQ(1, handler_->adapt_down_events_); - // Resume checking for QP again by invoking the callback. - task_queue_.SendTask( - [this] { - handler_->callback_->OnQpUsageHandled(true); - TriggerScale(kScaleDown); - }, - RTC_FROM_HERE); - EXPECT_TRUE(handler_->event.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(2, handler_->adapt_down_events_); - task_queue_.SendTask([this] { handler_->callback_->OnQpUsageHandled(true); }, - RTC_FROM_HERE); -} - } // namespace webrtc diff --git a/video/adaptation/BUILD.gn b/video/adaptation/BUILD.gn index 12e111bbc9..95df9c5887 100644 --- a/video/adaptation/BUILD.gn +++ b/video/adaptation/BUILD.gn @@ -74,13 +74,13 @@ if (rtc_include_tests) { deps = [ ":video_adaptation", "../../api:scoped_refptr", - "../../api/task_queue:default_task_queue_factory", "../../api/task_queue:task_queue", "../../api/video:encoded_image", "../../api/video:video_adaptation", "../../api/video:video_frame_i420", "../../api/video_codecs:video_codecs_api", "../../call/adaptation:resource_adaptation", + "../../call/adaptation:resource_adaptation_test_utilities", "../../modules/video_coding:video_coding_utility", "../../rtc_base:checks", "../../rtc_base:logging", diff --git a/video/adaptation/quality_scaler_resource.cc b/video/adaptation/quality_scaler_resource.cc index c26c2e193d..ffa34b3f58 100644 --- a/video/adaptation/quality_scaler_resource.cc +++ b/video/adaptation/quality_scaler_resource.cc @@ -37,16 +37,12 @@ QualityScalerResource::QualityScalerResource( : VideoStreamEncoderResource("QualityScalerResource"), quality_scaler_(nullptr), last_underuse_due_to_disabled_timestamp_ms_(absl::nullopt), - num_handled_callbacks_(0), - pending_callbacks_(), - degradation_preference_provider_(degradation_preference_provider), - clear_qp_samples_(false) { + degradation_preference_provider_(degradation_preference_provider) { RTC_CHECK(degradation_preference_provider_); } QualityScalerResource::~QualityScalerResource() { RTC_DCHECK(!quality_scaler_); - RTC_DCHECK(pending_callbacks_.empty()); } bool QualityScalerResource::is_started() const { @@ -66,7 +62,6 @@ void QualityScalerResource::StopCheckForOveruse() { RTC_DCHECK_RUN_ON(encoder_queue()); // Ensure we have no pending callbacks. This makes it safe to destroy the // QualityScaler and even task queues with tasks in-flight. - AbortPendingCallbacks(); quality_scaler_.reset(); } @@ -126,115 +121,29 @@ void QualityScalerResource::OnFrameDropped( } } -void QualityScalerResource::OnReportQpUsageHigh( - rtc::scoped_refptr callback) { +void QualityScalerResource::OnReportQpUsageHigh() { RTC_DCHECK_RUN_ON(encoder_queue()); - size_t callback_id = QueuePendingCallback(callback); // Reference counting guarantees that this object is still alive by the time // the task is executed. MaybePostTaskToResourceAdaptationQueue( - [this_ref = rtc::scoped_refptr(this), - callback_id] { + [this_ref = rtc::scoped_refptr(this)] { RTC_DCHECK_RUN_ON(this_ref->resource_adaptation_queue()); - this_ref->clear_qp_samples_ = false; // If this OnResourceUsageStateMeasured() triggers an adaptation, // OnAdaptationApplied() will occur between this line and the next. This // allows modifying |clear_qp_samples_| based on the adaptation. this_ref->OnResourceUsageStateMeasured(ResourceUsageState::kOveruse); - this_ref->HandlePendingCallback(callback_id, - this_ref->clear_qp_samples_); }); } -void QualityScalerResource::OnReportQpUsageLow( - rtc::scoped_refptr callback) { +void QualityScalerResource::OnReportQpUsageLow() { RTC_DCHECK_RUN_ON(encoder_queue()); - size_t callback_id = QueuePendingCallback(callback); // Reference counting guarantees that this object is still alive by the time // the task is executed. MaybePostTaskToResourceAdaptationQueue( - [this_ref = rtc::scoped_refptr(this), - callback_id] { + [this_ref = rtc::scoped_refptr(this)] { RTC_DCHECK_RUN_ON(this_ref->resource_adaptation_queue()); this_ref->OnResourceUsageStateMeasured(ResourceUsageState::kUnderuse); - this_ref->HandlePendingCallback(callback_id, true); }); } -void QualityScalerResource::OnAdaptationApplied( - const VideoStreamInputState& input_state, - const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) { - RTC_DCHECK_RUN_ON(resource_adaptation_queue()); - // We only clear QP samples on adaptations triggered by the QualityScaler. - if (reason_resource != this) - return; - clear_qp_samples_ = true; - // If we're in "balanced" and the frame rate before and after adaptation did - // not differ that much, don't clear the QP samples and instead check for QP - // again in a short amount of time. This may trigger adapting down again soon. - // TODO(hbos): Can this be simplified by getting rid of special casing logic? - // For example, we could decide whether or not to clear QP samples based on - // how big the adaptation step was alone (regardless of degradation preference - // or what resource triggered the adaptation) and the QualityScaler could - // check for QP when it had enough QP samples rather than at a variable - // interval whose delay is calculated based on events such as these. Now there - // is much dependency on a specific OnReportQpUsageHigh() event and "balanced" - // but adaptations happening might not align with QualityScaler's CheckQpTask. - if (degradation_preference_provider_->degradation_preference() == - DegradationPreference::BALANCED && - DidDecreaseFrameRate(restrictions_before, restrictions_after)) { - absl::optional min_diff = BalancedDegradationSettings().MinFpsDiff( - input_state.frame_size_pixels().value()); - if (min_diff && input_state.frames_per_second() > 0) { - int fps_diff = input_state.frames_per_second() - - restrictions_after.max_frame_rate().value(); - if (fps_diff < min_diff.value()) { - clear_qp_samples_ = false; - } - } - } -} - -size_t QualityScalerResource::QueuePendingCallback( - rtc::scoped_refptr callback) { - RTC_DCHECK_RUN_ON(encoder_queue()); - pending_callbacks_.push(callback); - // The ID of a callback is its sequence number (1, 2, 3...). - return num_handled_callbacks_ + pending_callbacks_.size(); -} - -void QualityScalerResource::HandlePendingCallback(size_t callback_id, - bool clear_qp_samples) { - RTC_DCHECK_RUN_ON(resource_adaptation_queue()); - // Reference counting guarantees that this object is still alive by the time - // the task is executed. - encoder_queue()->PostTask( - ToQueuedTask([this_ref = rtc::scoped_refptr(this), - callback_id, clear_qp_samples] { - RTC_DCHECK_RUN_ON(this_ref->encoder_queue()); - if (this_ref->num_handled_callbacks_ >= callback_id) { - // The callback with this ID has already been handled. - // This happens if AbortPendingCallbacks() is called while the task is - // in flight. - return; - } - RTC_DCHECK(!this_ref->pending_callbacks_.empty()); - this_ref->pending_callbacks_.front()->OnQpUsageHandled( - clear_qp_samples); - ++this_ref->num_handled_callbacks_; - this_ref->pending_callbacks_.pop(); - })); -} - -void QualityScalerResource::AbortPendingCallbacks() { - RTC_DCHECK_RUN_ON(encoder_queue()); - while (!pending_callbacks_.empty()) { - pending_callbacks_.front()->OnQpUsageHandled(false); - ++num_handled_callbacks_; - pending_callbacks_.pop(); - } -} - } // namespace webrtc diff --git a/video/adaptation/quality_scaler_resource.h b/video/adaptation/quality_scaler_resource.h index 286413132a..9317c79d8b 100644 --- a/video/adaptation/quality_scaler_resource.h +++ b/video/adaptation/quality_scaler_resource.h @@ -31,7 +31,6 @@ namespace webrtc { // Handles interaction with the QualityScaler. class QualityScalerResource : public VideoStreamEncoderResource, - public AdaptationListener, public QualityScalerQpUsageHandlerInterface { public: static rtc::scoped_refptr Create( @@ -53,27 +52,10 @@ class QualityScalerResource : public VideoStreamEncoderResource, void OnFrameDropped(EncodedImageCallback::DropReason reason); // QualityScalerQpUsageHandlerInterface implementation. - void OnReportQpUsageHigh( - rtc::scoped_refptr callback) - override; - void OnReportQpUsageLow( - rtc::scoped_refptr callback) - override; - - // AdaptationListener implementation. - void OnAdaptationApplied( - const VideoStreamInputState& input_state, - const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) override; + void OnReportQpUsageHigh() override; + void OnReportQpUsageLow() override; private: - size_t QueuePendingCallback( - rtc::scoped_refptr - callback); - void HandlePendingCallback(size_t callback_id, bool clear_qp_samples); - void AbortPendingCallbacks(); - // Members accessed on the encoder queue. std::unique_ptr quality_scaler_ RTC_GUARDED_BY(encoder_queue()); @@ -82,18 +64,7 @@ class QualityScalerResource : public VideoStreamEncoderResource, // make sure underuse reporting is not too spammy. absl::optional last_underuse_due_to_disabled_timestamp_ms_ RTC_GUARDED_BY(encoder_queue()); - // Every OnReportQpUsageHigh/Low() operation has a callback that MUST be - // invoked on the encoder_queue(). Because usage measurements are reported on - // the encoder_queue() but handled by the processor on the the - // resource_adaptation_queue_(), handling a measurement entails a task queue - // "ping" round-trip. Multiple callbacks in-flight is thus possible. - size_t num_handled_callbacks_ RTC_GUARDED_BY(encoder_queue()); - std::queue> - pending_callbacks_ RTC_GUARDED_BY(encoder_queue()); DegradationPreferenceProvider* const degradation_preference_provider_; - - // Members accessed on the adaptation queue. - bool clear_qp_samples_ RTC_GUARDED_BY(resource_adaptation_queue()); }; } // namespace webrtc diff --git a/video/adaptation/quality_scaler_resource_unittest.cc b/video/adaptation/quality_scaler_resource_unittest.cc index 0dca7327fd..707a5755a9 100644 --- a/video/adaptation/quality_scaler_resource_unittest.cc +++ b/video/adaptation/quality_scaler_resource_unittest.cc @@ -13,58 +13,26 @@ #include #include "absl/types/optional.h" -#include "api/task_queue/default_task_queue_factory.h" -#include "api/task_queue/task_queue_factory.h" #include "api/video_codecs/video_encoder.h" +#include "call/adaptation/test/mock_resource_listener.h" #include "rtc_base/event.h" +#include "rtc_base/location.h" #include "rtc_base/task_queue.h" +#include "rtc_base/task_queue_for_test.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { +using testing::_; +using testing::Eq; +using testing::StrictMock; + namespace { -const int kDefaultTimeout = 5000; - -class FakeQualityScalerQpUsageHandlerCallback - : public QualityScalerQpUsageHandlerCallbackInterface { - public: - explicit FakeQualityScalerQpUsageHandlerCallback( - rtc::TaskQueue* encoder_queue) - : QualityScalerQpUsageHandlerCallbackInterface(), - encoder_queue_(encoder_queue), - is_handled_(false), - qp_usage_handled_event_(true /* manual_reset */, false), - clear_qp_samples_result_(absl::nullopt) {} - ~FakeQualityScalerQpUsageHandlerCallback() override { - RTC_DCHECK(is_handled_) - << "The callback was destroyed without being invoked."; - } - - void OnQpUsageHandled(bool clear_qp_samples) override { - ASSERT_TRUE(encoder_queue_->IsCurrent()); - RTC_DCHECK(!is_handled_); - clear_qp_samples_result_ = clear_qp_samples; - is_handled_ = true; - qp_usage_handled_event_.Set(); - } - - bool is_handled() const { return is_handled_; } - rtc::Event* qp_usage_handled_event() { return &qp_usage_handled_event_; } - absl::optional clear_qp_samples_result() const { - return clear_qp_samples_result_; - } - - private: - rtc::TaskQueue* const encoder_queue_; - bool is_handled_; - rtc::Event qp_usage_handled_event_; - absl::optional clear_qp_samples_result_; -}; - class FakeDegradationPreferenceProvider : public DegradationPreferenceProvider { public: - ~FakeDegradationPreferenceProvider() override {} + ~FakeDegradationPreferenceProvider() override = default; DegradationPreference degradation_preference() const override { return DegradationPreference::MAINTAIN_FRAMERATE; @@ -76,95 +44,61 @@ class FakeDegradationPreferenceProvider : public DegradationPreferenceProvider { class QualityScalerResourceTest : public ::testing::Test { public: QualityScalerResourceTest() - : task_queue_factory_(CreateDefaultTaskQueueFactory()), - resource_adaptation_queue_(task_queue_factory_->CreateTaskQueue( - "ResourceAdaptationQueue", - TaskQueueFactory::Priority::NORMAL)), - encoder_queue_(task_queue_factory_->CreateTaskQueue( - "EncoderQueue", - TaskQueueFactory::Priority::NORMAL)), - degradation_preference_provider_(), + : resource_adaptation_queue_("ResourceAdaptationQueue", + TaskQueueFactory::Priority::NORMAL), + encoder_queue_("EncoderQueue", TaskQueueFactory::Priority::NORMAL), quality_scaler_resource_( QualityScalerResource::Create(°radation_preference_provider_)) { quality_scaler_resource_->RegisterEncoderTaskQueue(encoder_queue_.Get()); quality_scaler_resource_->RegisterAdaptationTaskQueue( resource_adaptation_queue_.Get()); - rtc::Event event; - encoder_queue_.PostTask([this, &event] { - quality_scaler_resource_->StartCheckForOveruse( - VideoEncoder::QpThresholds()); - event.Set(); - }); - event.Wait(kDefaultTimeout); + encoder_queue_.SendTask( + [this] { + quality_scaler_resource_->SetResourceListener( + &fake_resource_listener_); + quality_scaler_resource_->StartCheckForOveruse( + VideoEncoder::QpThresholds()); + }, + RTC_FROM_HERE); } - ~QualityScalerResourceTest() { - rtc::Event event; - encoder_queue_.PostTask([this, &event] { - quality_scaler_resource_->StopCheckForOveruse(); - event.Set(); - }); - event.Wait(kDefaultTimeout); + ~QualityScalerResourceTest() override { + encoder_queue_.SendTask( + [this] { + quality_scaler_resource_->StopCheckForOveruse(); + quality_scaler_resource_->SetResourceListener(nullptr); + }, + RTC_FROM_HERE); } protected: - const std::unique_ptr task_queue_factory_; - rtc::TaskQueue resource_adaptation_queue_; - rtc::TaskQueue encoder_queue_; + TaskQueueForTest resource_adaptation_queue_; + TaskQueueForTest encoder_queue_; + StrictMock fake_resource_listener_; FakeDegradationPreferenceProvider degradation_preference_provider_; rtc::scoped_refptr quality_scaler_resource_; }; TEST_F(QualityScalerResourceTest, ReportQpHigh) { - rtc::scoped_refptr callback = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - encoder_queue_.PostTask([this, callback] { - quality_scaler_resource_->OnReportQpUsageHigh(callback); - }); - callback->qp_usage_handled_event()->Wait(kDefaultTimeout); + EXPECT_CALL(fake_resource_listener_, + OnResourceUsageStateMeasured(Eq(quality_scaler_resource_), + Eq(ResourceUsageState::kOveruse))); + encoder_queue_.SendTask( + [this] { quality_scaler_resource_->OnReportQpUsageHigh(); }, + RTC_FROM_HERE); + // Wait for adapt queue to clear since that signals the resource listener. + resource_adaptation_queue_.WaitForPreviouslyPostedTasks(); } TEST_F(QualityScalerResourceTest, ReportQpLow) { - rtc::scoped_refptr callback = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - encoder_queue_.PostTask([this, callback] { - quality_scaler_resource_->OnReportQpUsageLow(callback); - }); - callback->qp_usage_handled_event()->Wait(kDefaultTimeout); -} - -TEST_F(QualityScalerResourceTest, MultipleCallbacksInFlight) { - rtc::scoped_refptr callback1 = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - rtc::scoped_refptr callback2 = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - rtc::scoped_refptr callback3 = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - encoder_queue_.PostTask([this, callback1, callback2, callback3] { - quality_scaler_resource_->OnReportQpUsageHigh(callback1); - quality_scaler_resource_->OnReportQpUsageLow(callback2); - quality_scaler_resource_->OnReportQpUsageHigh(callback3); - }); - callback1->qp_usage_handled_event()->Wait(kDefaultTimeout); - callback2->qp_usage_handled_event()->Wait(kDefaultTimeout); - callback3->qp_usage_handled_event()->Wait(kDefaultTimeout); -} - -TEST_F(QualityScalerResourceTest, AbortPendingCallbacksAndStartAgain) { - rtc::scoped_refptr callback1 = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - rtc::scoped_refptr callback2 = - new FakeQualityScalerQpUsageHandlerCallback(&encoder_queue_); - encoder_queue_.PostTask([this, callback1, callback2] { - quality_scaler_resource_->OnReportQpUsageHigh(callback1); - quality_scaler_resource_->StopCheckForOveruse(); - EXPECT_TRUE(callback1->qp_usage_handled_event()->Wait(0)); - quality_scaler_resource_->StartCheckForOveruse( - VideoEncoder::QpThresholds()); - quality_scaler_resource_->OnReportQpUsageHigh(callback2); - }); - callback1->qp_usage_handled_event()->Wait(kDefaultTimeout); - callback2->qp_usage_handled_event()->Wait(kDefaultTimeout); + EXPECT_CALL(fake_resource_listener_, + OnResourceUsageStateMeasured(Eq(quality_scaler_resource_), + Eq(ResourceUsageState::kUnderuse))); + encoder_queue_.SendTask( + [this] { quality_scaler_resource_->OnReportQpUsageLow(); }, + RTC_FROM_HERE); + // Wait for adapt queue to clear since that signals the resource listener. + resource_adaptation_queue_.WaitForPreviouslyPostedTasks(); } } // namespace webrtc diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc index dc19e1c787..5c13b3e80a 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -381,11 +381,6 @@ VideoStreamEncoderResourceManager::AdaptationConstraints() const { return {bitrate_constraint_, balanced_constraint_}; } -std::vector -VideoStreamEncoderResourceManager::AdaptationListeners() const { - return {quality_scaler_resource_}; -} - rtc::scoped_refptr VideoStreamEncoderResourceManager::quality_scaler_resource_for_testing() { MutexLock lock(&resource_lock_); diff --git a/video/adaptation/video_stream_encoder_resource_manager.h b/video/adaptation/video_stream_encoder_resource_manager.h index 742d87bf1c..a0a53eafc9 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -121,7 +121,6 @@ class VideoStreamEncoderResourceManager VideoAdaptationReason reason); std::vector> MappedResources() const; std::vector AdaptationConstraints() const; - std::vector AdaptationListeners() const; rtc::scoped_refptr quality_scaler_resource_for_testing(); // If true, the VideoStreamEncoder should eexecute its logic to maybe drop diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index f38d2a2990..2e9e453c78 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -392,9 +392,6 @@ VideoStreamEncoder::VideoStreamEncoder( for (auto* constraint : adaptation_constraints_) { video_stream_adapter_->AddAdaptationConstraint(constraint); } - for (auto* listener : stream_resource_manager_.AdaptationListeners()) { - video_stream_adapter_->AddAdaptationListener(listener); - } initialize_processor_event.Set(); }); initialize_processor_event.Wait(rtc::Event::kForever); @@ -426,9 +423,6 @@ void VideoStreamEncoder::Stop() { for (auto* constraint : adaptation_constraints_) { video_stream_adapter_->RemoveAdaptationConstraint(constraint); } - for (auto* listener : stream_resource_manager_.AdaptationListeners()) { - video_stream_adapter_->RemoveAdaptationListener(listener); - } video_stream_adapter_->RemoveRestrictionsListener(this); video_stream_adapter_->RemoveRestrictionsListener( &stream_resource_manager_); diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index b651c7bf6d..46104569c8 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -161,34 +161,6 @@ class CpuOveruseDetectorProxy : public OveruseFrameDetector { rtc::Event framerate_updated_event_; }; -class FakeQualityScalerQpUsageHandlerCallback - : public QualityScalerQpUsageHandlerCallbackInterface { - public: - FakeQualityScalerQpUsageHandlerCallback() - : QualityScalerQpUsageHandlerCallbackInterface(), - qp_usage_handled_event_(/*manual_reset=*/true, - /*initially_signaled=*/false), - clear_qp_samples_result_(absl::nullopt) {} - ~FakeQualityScalerQpUsageHandlerCallback() override { - RTC_DCHECK(clear_qp_samples_result_.has_value()); - } - - void OnQpUsageHandled(bool clear_qp_samples) override { - clear_qp_samples_result_ = clear_qp_samples; - qp_usage_handled_event_.Set(); - } - - bool WaitForQpUsageHandled() { return qp_usage_handled_event_.Wait(5000); } - - absl::optional clear_qp_samples_result() const { - return clear_qp_samples_result_; - } - - private: - rtc::Event qp_usage_handled_event_; - absl::optional clear_qp_samples_result_; -}; - class FakeVideoSourceRestrictionsListener : public VideoSourceRestrictionsListener { public: @@ -413,22 +385,6 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder { ASSERT_TRUE(event.Wait(5000)); } - // Fakes high QP resource usage measurements on the real - // QualityScalerResource. Returns whether or not QP samples would have been - // cleared if this had been a real signal from the QualityScaler. - bool TriggerQualityScalerHighQpAndReturnIfQpSamplesShouldBeCleared() { - rtc::scoped_refptr callback = - new FakeQualityScalerQpUsageHandlerCallback(); - encoder_queue()->PostTask([this, callback] { - // This will cause a "ping" between adaptation task queue and encoder - // queue. When we have the result, the |callback| will be notified. - quality_scaler_resource_for_testing()->OnReportQpUsageHigh(callback); - }); - EXPECT_TRUE(callback->WaitForQpUsageHandled()); - EXPECT_TRUE(callback->clear_qp_samples_result().has_value()); - return callback->clear_qp_samples_result().value(); - } - CpuOveruseDetectorProxy* overuse_detector_proxy_; rtc::scoped_refptr fake_cpu_resource_; rtc::scoped_refptr fake_quality_resource_; @@ -3282,7 +3238,7 @@ class BalancedDegradationTest : public VideoStreamEncoderTest { AdaptingFrameForwarder source_; }; -TEST_F(BalancedDegradationTest, AdaptDownReturnsFalseIfFpsDiffLtThreshold) { +TEST_F(BalancedDegradationTest, AdaptDownTwiceIfMinFpsDiffLtThreshold) { test::ScopedFieldTrials field_trials( "WebRTC-Video-BalancedDegradationSettings/" "pixels:57600|129600|230400,fps:7|10|24,fps_diff:1|1|1/"); @@ -3297,18 +3253,16 @@ TEST_F(BalancedDegradationTest, AdaptDownReturnsFalseIfFpsDiffLtThreshold) { InsertFrameAndWaitForEncoded(); EXPECT_THAT(source_.sink_wants(), FpsMaxResolutionMax()); - // Trigger adapt down, expect scaled down framerate (640x360@24fps). - // Fps diff (input-requested:0) < threshold, expect adapting down not to clear - // QP samples. - EXPECT_FALSE( - video_stream_encoder_ - ->TriggerQualityScalerHighQpAndReturnIfQpSamplesShouldBeCleared()); - EXPECT_THAT(source_.sink_wants(), FpsMatchesResolutionMax(Eq(24))); + // Trigger adapt down, expect scaled down framerate and resolution, + // since Fps diff (input-requested:0) < threshold. + video_stream_encoder_->TriggerQualityLow(); + EXPECT_THAT(source_.sink_wants(), + AllOf(WantsFps(Eq(24)), WantsMaxPixels(Le(230400)))); video_stream_encoder_->Stop(); } -TEST_F(BalancedDegradationTest, AdaptDownReturnsTrueIfFpsDiffGeThreshold) { +TEST_F(BalancedDegradationTest, AdaptDownOnceIfFpsDiffGeThreshold) { test::ScopedFieldTrials field_trials( "WebRTC-Video-BalancedDegradationSettings/" "pixels:57600|129600|230400,fps:7|10|24,fps_diff:1|1|1/"); @@ -3323,12 +3277,9 @@ TEST_F(BalancedDegradationTest, AdaptDownReturnsTrueIfFpsDiffGeThreshold) { InsertFrameAndWaitForEncoded(); EXPECT_THAT(source_.sink_wants(), FpsMaxResolutionMax()); - // Trigger adapt down, expect scaled down framerate (640x360@24fps). - // Fps diff (input-requested:1) == threshold, expect adapting down to clear QP - // samples. - EXPECT_TRUE( - video_stream_encoder_ - ->TriggerQualityScalerHighQpAndReturnIfQpSamplesShouldBeCleared()); + // Trigger adapt down, expect scaled down framerate only (640x360@24fps). + // Fps diff (input-requested:1) == threshold. + video_stream_encoder_->TriggerQualityLow(); EXPECT_THAT(source_.sink_wants(), FpsMatchesResolutionMax(Eq(24))); video_stream_encoder_->Stop();