From b93ad75c94fc5907fa2984737d45424ed8da18e6 Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Thu, 13 Aug 2020 11:35:24 +0200 Subject: [PATCH] [Adaptation] Apply AdaptationConstraints to all resources Some restrictions previously were preventing adaptation up caused by the quality resource. However, it makes sense to use the same restrictions in the case of other resources. This CL removes now unneeded wire-up of reason/resource causing adaptation. Bug: webrtc:11771 Change-Id: Iec301a59d2a41d32d23b6be340f3b5637d697e52 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181580 Commit-Queue: Evan Shrubsole Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#31926} --- call/adaptation/adaptation_constraint.h | 3 +- .../resource_adaptation_processor.cc | 2 +- .../test/fake_adaptation_constraint.cc | 5 +- .../test/fake_adaptation_constraint.h | 3 +- call/adaptation/video_stream_adapter.cc | 15 ++-- call/adaptation/video_stream_adapter.h | 9 +-- .../video_stream_adapter_unittest.cc | 72 +++++++++---------- .../video_stream_encoder_resource_manager.cc | 38 ++++------ .../video_stream_encoder_resource_manager.h | 6 +- 9 files changed, 61 insertions(+), 92 deletions(-) diff --git a/call/adaptation/adaptation_constraint.h b/call/adaptation/adaptation_constraint.h index 9ff15d6b86..e6646902c2 100644 --- a/call/adaptation/adaptation_constraint.h +++ b/call/adaptation/adaptation_constraint.h @@ -34,8 +34,7 @@ class AdaptationConstraint { virtual bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const = 0; + const VideoSourceRestrictions& restrictions_after) const = 0; }; } // namespace webrtc diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc index c164292e46..94a5b7f963 100644 --- a/call/adaptation/resource_adaptation_processor.cc +++ b/call/adaptation/resource_adaptation_processor.cc @@ -245,7 +245,7 @@ ResourceAdaptationProcessor::OnResourceUnderuse( rtc::scoped_refptr reason_resource) { RTC_DCHECK_RUN_ON(resource_adaptation_queue_); // How can this stream be adapted up? - Adaptation adaptation = stream_adapter_->GetAdaptationUp(reason_resource); + Adaptation adaptation = stream_adapter_->GetAdaptationUp(); if (adaptation.status() != Adaptation::Status::kValid) { rtc::StringBuilder message; message << "Not adapting up because VideoStreamAdapter returned " diff --git a/call/adaptation/test/fake_adaptation_constraint.cc b/call/adaptation/test/fake_adaptation_constraint.cc index 983885e58a..18b8e8b696 100644 --- a/call/adaptation/test/fake_adaptation_constraint.cc +++ b/call/adaptation/test/fake_adaptation_constraint.cc @@ -17,7 +17,7 @@ namespace webrtc { FakeAdaptationConstraint::FakeAdaptationConstraint(std::string name) : name_(std::move(name)), is_adaptation_up_allowed_(true) {} -FakeAdaptationConstraint::~FakeAdaptationConstraint() {} +FakeAdaptationConstraint::~FakeAdaptationConstraint() = default; void FakeAdaptationConstraint::set_is_adaptation_up_allowed( bool is_adaptation_up_allowed) { @@ -31,8 +31,7 @@ std::string FakeAdaptationConstraint::Name() const { bool FakeAdaptationConstraint::IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const { + const VideoSourceRestrictions& restrictions_after) const { return is_adaptation_up_allowed_; } diff --git a/call/adaptation/test/fake_adaptation_constraint.h b/call/adaptation/test/fake_adaptation_constraint.h index 74637f48fd..021e46a501 100644 --- a/call/adaptation/test/fake_adaptation_constraint.h +++ b/call/adaptation/test/fake_adaptation_constraint.h @@ -29,8 +29,7 @@ class FakeAdaptationConstraint : public AdaptationConstraint { bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const override; + const VideoSourceRestrictions& restrictions_after) const override; private: const std::string name_; diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc index 6209c05844..744cd22cd4 100644 --- a/call/adaptation/video_stream_adapter.cc +++ b/call/adaptation/video_stream_adapter.cc @@ -326,17 +326,16 @@ Adaptation VideoStreamAdapter::RestrictionsOrStateToAdaptation( } Adaptation VideoStreamAdapter::GetAdaptationUp( - const VideoStreamInputState& input_state, - rtc::scoped_refptr resource) const { + const VideoStreamInputState& input_state) const { RestrictionsOrState step = GetAdaptationUpStep(input_state); // If an adaptation proposed, check with the constraints that it is ok. if (absl::holds_alternative(step)) { RestrictionsWithCounters restrictions = absl::get(step); for (const auto* constraint : adaptation_constraints_) { - if (!constraint->IsAdaptationUpAllowed( - input_state, current_restrictions_.restrictions, - restrictions.restrictions, resource)) { + if (!constraint->IsAdaptationUpAllowed(input_state, + current_restrictions_.restrictions, + restrictions.restrictions)) { RTC_LOG(INFO) << "Not adapting up because constraint \"" << constraint->Name() << "\" disallowed it"; step = Adaptation::Status::kRejectedByConstraint; @@ -346,13 +345,11 @@ Adaptation VideoStreamAdapter::GetAdaptationUp( return RestrictionsOrStateToAdaptation(step, input_state); } -Adaptation VideoStreamAdapter::GetAdaptationUp( - rtc::scoped_refptr resource) { +Adaptation VideoStreamAdapter::GetAdaptationUp() { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_DCHECK(resource); VideoStreamInputState input_state = input_state_provider_->InputState(); ++adaptation_validation_id_; - Adaptation adaptation = GetAdaptationUp(input_state, resource); + Adaptation adaptation = GetAdaptationUp(input_state); return adaptation; } diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h index 1f6f252b33..bbcbdfe444 100644 --- a/call/adaptation/video_stream_adapter.h +++ b/call/adaptation/video_stream_adapter.h @@ -148,9 +148,7 @@ class VideoStreamAdapter { // Returns an adaptation that we are guaranteed to be able to apply, or a // status code indicating the reason why we cannot adapt. - // TODO(https://crbug.com/webrtc/11771) |resource| is needed by the - // AdaptationConstraint resources. Remove this parameter when it's removed. - Adaptation GetAdaptationUp(rtc::scoped_refptr resource); + Adaptation GetAdaptationUp(); Adaptation GetAdaptationDown(); Adaptation GetAdaptationTo(const VideoAdaptationCounters& counters, const VideoSourceRestrictions& restrictions); @@ -194,10 +192,7 @@ class VideoStreamAdapter { 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. - Adaptation GetAdaptationUp(const VideoStreamInputState& input_state, - rtc::scoped_refptr resource) const + Adaptation GetAdaptationUp(const VideoStreamInputState& input_state) const RTC_RUN_ON(&sequence_checker_); Adaptation GetAdaptationDown(const VideoStreamInputState& input_state) const RTC_RUN_ON(&sequence_checker_); diff --git a/call/adaptation/video_stream_adapter_unittest.cc b/call/adaptation/video_stream_adapter_unittest.cc index 073ff825c5..5aeff845dd 100644 --- a/call/adaptation/video_stream_adapter_unittest.cc +++ b/call/adaptation/video_stream_adapter_unittest.cc @@ -159,8 +159,7 @@ class MockAdaptationConstraint : public AdaptationConstraint { IsAdaptationUpAllowed, (const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource), + const VideoSourceRestrictions& restrictions_after), (const, override)); // MOCK_METHOD(std::string, Name, (), (const, override)); @@ -234,7 +233,7 @@ TEST_F(VideoStreamAdapterTest, MaintainFramerate_IncreasePixelsToFiveThirds) { int input_pixels = fake_stream.input_pixels(); // Go up once. The target is 5/3 and the max is 12/5 of the target. const int target = (input_pixels * 5) / 3; - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(static_cast((target * 12) / 5), adapter_.source_restrictions().max_pixels_per_frame()); EXPECT_EQ(static_cast(target), @@ -249,11 +248,11 @@ TEST_F(VideoStreamAdapterTest, MaintainFramerate_IncreasePixelsToUnrestricted) { kDefaultMinPixelsPerFrame); // We are unrestricted by default and should not be able to adapt up. EXPECT_EQ(Adaptation::Status::kLimitReached, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); // If we go down once and then back up we should not have any restrictions. fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); EXPECT_EQ(1, adapter_.adaptation_counters().resolution_adaptations); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(VideoSourceRestrictions(), adapter_.source_restrictions()); EXPECT_EQ(0, adapter_.adaptation_counters().Total()); } @@ -303,7 +302,7 @@ TEST_F(VideoStreamAdapterTest, MaintainResolution_IncreaseFpsToThreeHalves) { EXPECT_EQ(2, adapter_.adaptation_counters().fps_adaptations); int input_fps = fake_stream.input_fps(); // Go up once. The target is 3/2 of the input. - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(absl::nullopt, @@ -321,11 +320,11 @@ TEST_F(VideoStreamAdapterTest, MaintainResolution_IncreaseFpsToUnrestricted) { kDefaultMinPixelsPerFrame); // We are unrestricted by default and should not be able to adapt up. EXPECT_EQ(Adaptation::Status::kLimitReached, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); // If we go down once and then back up we should not have any restrictions. fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); EXPECT_EQ(1, adapter_.adaptation_counters().fps_adaptations); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(VideoSourceRestrictions(), adapter_.source_restrictions()); EXPECT_EQ(0, adapter_.adaptation_counters().Total()); } @@ -472,7 +471,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_IncreaseFrameRateAndResolution) { // the next resolution configuration up ("high") is kBalancedHighFrameRateFps // and "balanced" prefers adapting frame rate if not already applied. { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(static_cast(kBalancedHighFrameRateFps), @@ -488,7 +487,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_IncreaseFrameRateAndResolution) { constexpr size_t kReducedPixelsSecondStepUp = (kReducedPixelsThirdStep * 5) / 3; { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(kReducedPixelsSecondStepUp, @@ -499,7 +498,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_IncreaseFrameRateAndResolution) { // Now that our resolution is back in the high-range, the next frame rate to // try out is "unlimited". { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(absl::nullopt, adapter_.source_restrictions().max_frame_rate()); @@ -510,7 +509,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_IncreaseFrameRateAndResolution) { constexpr size_t kReducedPixelsFirstStepUp = (kReducedPixelsSecondStepUp * 5) / 3; { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(kReducedPixelsFirstStepUp, @@ -520,7 +519,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_IncreaseFrameRateAndResolution) { } // The last step up should make us entirely unrestricted. { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(VideoSourceRestrictions(), adapter_.source_restrictions()); @@ -535,7 +534,7 @@ TEST_F(VideoStreamAdapterTest, Balanced_LimitReached) { kBalancedLowFrameRateFps, kDefaultMinPixelsPerFrame); // Attempting to adapt up while unrestricted should result in kLimitReached. EXPECT_EQ(Adaptation::Status::kLimitReached, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); // Adapting down once result in restricted frame rate, in this case we reach // the lowest possible frame rate immediately: kBalancedLowFrameRateFps. fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); @@ -597,12 +596,12 @@ TEST_F(VideoStreamAdapterTest, MaintainFramerate_AwaitingPreviousAdaptationUp) { fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); EXPECT_EQ(2, adapter_.adaptation_counters().resolution_adaptations); // Adapt up once, but don't update the input. - adapter_.ApplyAdaptation(adapter_.GetAdaptationUp(resource_), nullptr); + adapter_.ApplyAdaptation(adapter_.GetAdaptationUp(), nullptr); EXPECT_EQ(1, adapter_.adaptation_counters().resolution_adaptations); { // Having performed the adaptation, but not updated the input based on the // new restrictions, adapting again in the same direction will not work. - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kAwaitingPreviousAdaptation, adaptation.status()); } @@ -619,16 +618,16 @@ TEST_F(VideoStreamAdapterTest, adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE); fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(1, adapter_.adaptation_counters().fps_adaptations); EXPECT_EQ(0, adapter_.adaptation_counters().resolution_adaptations); // We should be able to adapt in framerate one last time after the change of // degradation preference. adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION); - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(0, adapter_.adaptation_counters().fps_adaptations); } @@ -643,16 +642,16 @@ TEST_F(VideoStreamAdapterTest, adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION); fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(1, adapter_.adaptation_counters().resolution_adaptations); EXPECT_EQ(0, adapter_.adaptation_counters().fps_adaptations); // We should be able to adapt in framerate one last time after the change of // degradation preference. adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE); - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); - fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp(resource_)); + fake_stream.ApplyAdaptation(adapter_.GetAdaptationUp()); EXPECT_EQ(0, adapter_.adaptation_counters().resolution_adaptations); } @@ -667,12 +666,12 @@ TEST_F(VideoStreamAdapterTest, adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE); fake_stream.ApplyAdaptation(adapter_.GetAdaptationDown()); // Apply adaptation up but don't update input. - adapter_.ApplyAdaptation(adapter_.GetAdaptationUp(resource_), nullptr); + adapter_.ApplyAdaptation(adapter_.GetAdaptationUp(), nullptr); EXPECT_EQ(Adaptation::Status::kAwaitingPreviousAdaptation, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION); - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); } @@ -735,7 +734,7 @@ TEST_F(VideoStreamAdapterTest, RestrictionBroadcasted) { kDefaultMinPixelsPerFrame); // Not broadcast on invalid ApplyAdaptation. { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); adapter_.ApplyAdaptation(adaptation, nullptr); EXPECT_EQ(0, listener.calls()); } @@ -761,7 +760,7 @@ TEST_F(VideoStreamAdapterTest, AdaptationHasNextRestrcitions) { kDefaultMinPixelsPerFrame); // When adaptation is not possible. { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kLimitReached, adaptation.status()); EXPECT_EQ(adaptation.restrictions(), adapter_.source_restrictions()); EXPECT_EQ(0, adaptation.counters().Total()); @@ -776,7 +775,7 @@ TEST_F(VideoStreamAdapterTest, AdaptationHasNextRestrcitions) { } // When we adapt up. { - Adaptation adaptation = adapter_.GetAdaptationUp(resource_); + Adaptation adaptation = adapter_.GetAdaptationUp(); EXPECT_EQ(Adaptation::Status::kValid, adaptation.status()); fake_stream.ApplyAdaptation(adaptation); EXPECT_EQ(adaptation.restrictions(), adapter_.source_restrictions()); @@ -889,7 +888,7 @@ TEST_F(VideoStreamAdapterTest, EXPECT_EQ(Adaptation::Status::kAdaptationDisabled, adapter_.GetAdaptationDown().status()); EXPECT_EQ(Adaptation::Status::kAdaptationDisabled, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); EXPECT_EQ(Adaptation::Status::kAdaptationDisabled, adapter_.GetAdaptDownResolution().status()); } @@ -906,12 +905,10 @@ TEST_F(VideoStreamAdapterTest, AdaptationConstraintAllowsAdaptationsUp) { auto first_adaptation = adapter_.GetAdaptationDown(); fake_stream.ApplyAdaptation(first_adaptation); - EXPECT_CALL( - adaptation_constraint, - IsAdaptationUpAllowed(_, first_adaptation.restrictions(), _, resource_)) + EXPECT_CALL(adaptation_constraint, + IsAdaptationUpAllowed(_, first_adaptation.restrictions(), _)) .WillOnce(Return(true)); - EXPECT_EQ(Adaptation::Status::kValid, - adapter_.GetAdaptationUp(resource_).status()); + EXPECT_EQ(Adaptation::Status::kValid, adapter_.GetAdaptationUp().status()); adapter_.RemoveAdaptationConstraint(&adaptation_constraint); } @@ -927,12 +924,11 @@ TEST_F(VideoStreamAdapterTest, AdaptationConstraintDisallowsAdaptationsUp) { auto first_adaptation = adapter_.GetAdaptationDown(); fake_stream.ApplyAdaptation(first_adaptation); - EXPECT_CALL( - adaptation_constraint, - IsAdaptationUpAllowed(_, first_adaptation.restrictions(), _, resource_)) + EXPECT_CALL(adaptation_constraint, + IsAdaptationUpAllowed(_, first_adaptation.restrictions(), _)) .WillOnce(Return(false)); EXPECT_EQ(Adaptation::Status::kRejectedByConstraint, - adapter_.GetAdaptationUp(resource_).status()); + adapter_.GetAdaptationUp().status()); adapter_.RemoveAdaptationConstraint(&adaptation_constraint); } diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc index 47bd1b4cc3..c68851fa7e 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -167,20 +167,13 @@ void VideoStreamEncoderResourceManager::BitrateConstraint:: } bool VideoStreamEncoderResourceManager::BitrateConstraint:: - IsAdaptationUpAllowed(const VideoStreamInputState& input_state, - const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const { + IsAdaptationUpAllowed( + const VideoStreamInputState& input_state, + const VideoSourceRestrictions& restrictions_before, + const VideoSourceRestrictions& restrictions_after) const { RTC_DCHECK_RUN_ON(resource_adaptation_queue_); - VideoAdaptationReason reason = - manager_->GetReasonFromResource(reason_resource); - // If increasing resolution due to kQuality, make sure bitrate limits are not - // violated. - // TODO(https://crbug.com/webrtc/11771): Why are we allowing violating bitrate - // constraints if adapting due to CPU? Shouldn't this condition be checked - // regardless of reason? - if (reason == VideoAdaptationReason::kQuality && - DidIncreaseResolution(restrictions_before, restrictions_after)) { + // Make sure bitrate limits are not violated. + if (DidIncreaseResolution(restrictions_before, restrictions_after)) { uint32_t bitrate_bps = encoder_target_bitrate_bps_.value_or(0); absl::optional bitrate_limits = encoder_settings_.has_value() @@ -230,20 +223,14 @@ void VideoStreamEncoderResourceManager::BalancedConstraint:: } bool VideoStreamEncoderResourceManager::BalancedConstraint:: - IsAdaptationUpAllowed(const VideoStreamInputState& input_state, - const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const { + IsAdaptationUpAllowed( + const VideoStreamInputState& input_state, + const VideoSourceRestrictions& restrictions_before, + const VideoSourceRestrictions& restrictions_after) const { RTC_DCHECK_RUN_ON(resource_adaptation_queue_); - VideoAdaptationReason reason = - manager_->GetReasonFromResource(reason_resource); // Don't adapt if BalancedDegradationSettings applies and determines this will // exceed bitrate constraints. - // TODO(https://crbug.com/webrtc/11771): Why are we allowing violating - // balanced settings if adapting due CPU? Shouldn't this condition be checked - // regardless of reason? - if (reason == VideoAdaptationReason::kQuality && - degradation_preference_provider_->degradation_preference() == + if (degradation_preference_provider_->degradation_preference() == DegradationPreference::BALANCED && !manager_->balanced_settings_.CanAdaptUp( input_state.video_codec_type(), @@ -251,8 +238,7 @@ bool VideoStreamEncoderResourceManager::BalancedConstraint:: encoder_target_bitrate_bps_.value_or(0))) { return false; } - if (reason == VideoAdaptationReason::kQuality && - DidIncreaseResolution(restrictions_before, restrictions_after) && + if (DidIncreaseResolution(restrictions_before, restrictions_after) && !manager_->balanced_settings_.CanAdaptUpResolution( input_state.video_codec_type(), input_state.frame_size_pixels().value(), diff --git a/video/adaptation/video_stream_encoder_resource_manager.h b/video/adaptation/video_stream_encoder_resource_manager.h index e7a45d9aa0..c2e14acde5 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -181,8 +181,7 @@ class VideoStreamEncoderResourceManager bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const override; + const VideoSourceRestrictions& restrictions_after) const override; private: // The |manager_| must be alive as long as this resource is added to the @@ -213,8 +212,7 @@ class VideoStreamEncoderResourceManager bool IsAdaptationUpAllowed( const VideoStreamInputState& input_state, const VideoSourceRestrictions& restrictions_before, - const VideoSourceRestrictions& restrictions_after, - rtc::scoped_refptr reason_resource) const override; + const VideoSourceRestrictions& restrictions_after) const override; private: // The |manager_| must be alive as long as this resource is added to the