diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc index dd273acb26..bea8334efd 100644 --- a/call/adaptation/resource_adaptation_processor.cc +++ b/call/adaptation/resource_adaptation_processor.cc @@ -90,9 +90,6 @@ ResourceAdaptationProcessor::~ResourceAdaptationProcessor() { RTC_DCHECK(adaptation_constraints_.empty()) << "There are constaint(s) attached to a ResourceAdaptationProcessor " << "being destroyed."; - RTC_DCHECK(adaptation_listeners_.empty()) - << "There are listener(s) attached to a ResourceAdaptationProcessor " - << "being destroyed."; stream_adapter_->RemoveRestrictionsListener(this); resource_listener_delegate_->OnProcessorDestroyed(); } @@ -182,24 +179,6 @@ void ResourceAdaptationProcessor::RemoveAdaptationConstraint( adaptation_constraints_.erase(it); } -void ResourceAdaptationProcessor::AddAdaptationListener( - AdaptationListener* adaptation_listener) { - RTC_DCHECK_RUN_ON(resource_adaptation_queue_); - RTC_DCHECK(std::find(adaptation_listeners_.begin(), - adaptation_listeners_.end(), - adaptation_listener) == adaptation_listeners_.end()); - adaptation_listeners_.push_back(adaptation_listener); -} - -void ResourceAdaptationProcessor::RemoveAdaptationListener( - AdaptationListener* adaptation_listener) { - RTC_DCHECK_RUN_ON(resource_adaptation_queue_); - auto it = std::find(adaptation_listeners_.begin(), - adaptation_listeners_.end(), adaptation_listener); - RTC_DCHECK(it != adaptation_listeners_.end()); - adaptation_listeners_.erase(it); -} - void ResourceAdaptationProcessor::OnResourceUsageStateMeasured( rtc::scoped_refptr resource, ResourceUsageState usage_state) { @@ -308,11 +287,6 @@ ResourceAdaptationProcessor::OnResourceUnderuse( } // Apply adaptation. stream_adapter_->ApplyAdaptation(adaptation, reason_resource); - for (auto* adaptation_listener : adaptation_listeners_) { - adaptation_listener->OnAdaptationApplied( - adaptation.input_state(), restrictions_before, restrictions_after, - reason_resource); - } processing_in_progress_ = false; rtc::StringBuilder message; message << "Adapted up successfully. Unfiltered adaptations: " @@ -341,17 +315,9 @@ ResourceAdaptationProcessor::OnResourceOveruse( message.Release()); } // Apply adaptation. - VideoSourceRestrictions restrictions_before = - stream_adapter_->source_restrictions(); - VideoSourceRestrictions restrictions_after = adaptation.restrictions(); UpdateResourceLimitations(reason_resource, adaptation.restrictions(), adaptation.counters()); - stream_adapter_->ApplyAdaptation(adaptation, nullptr); - for (auto* adaptation_listener : adaptation_listeners_) { - adaptation_listener->OnAdaptationApplied( - adaptation.input_state(), restrictions_before, restrictions_after, - reason_resource); - } + stream_adapter_->ApplyAdaptation(adaptation, reason_resource); processing_in_progress_ = false; rtc::StringBuilder message; message << "Adapted down successfully. Unfiltered adaptations: " @@ -429,11 +395,6 @@ void ResourceAdaptationProcessor:: most_limited.counters, most_limited.restrictions); RTC_DCHECK_EQ(adapt_to.status(), Adaptation::Status::kValid); stream_adapter_->ApplyAdaptation(adapt_to, nullptr); - for (auto* adaptation_listener : adaptation_listeners_) { - adaptation_listener->OnAdaptationApplied( - adapt_to.input_state(), removed_limitations.restrictions, - most_limited.restrictions, nullptr); - } RTC_LOG(INFO) << "Most limited resource removed. Restoring restrictions to " "next most limited restrictions: " diff --git a/call/adaptation/resource_adaptation_processor.h b/call/adaptation/resource_adaptation_processor.h index f519fcc860..9f20bdbc60 100644 --- a/call/adaptation/resource_adaptation_processor.h +++ b/call/adaptation/resource_adaptation_processor.h @@ -26,7 +26,6 @@ #include "api/video/video_frame.h" #include "api/video/video_stream_encoder_observer.h" #include "call/adaptation/adaptation_constraint.h" -#include "call/adaptation/adaptation_listener.h" #include "call/adaptation/resource_adaptation_processor_interface.h" #include "call/adaptation/video_source_restrictions.h" #include "call/adaptation/video_stream_adapter.h" @@ -75,9 +74,6 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, AdaptationConstraint* adaptation_constraint) override; void RemoveAdaptationConstraint( AdaptationConstraint* adaptation_constraint) override; - void AddAdaptationListener(AdaptationListener* adaptation_listener) override; - void RemoveAdaptationListener( - AdaptationListener* adaptation_listener) override; // ResourceListener implementation. // Triggers OnResourceUnderuse() or OnResourceOveruse(). @@ -165,8 +161,6 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, RTC_GUARDED_BY(resource_adaptation_queue_); std::vector adaptation_constraints_ RTC_GUARDED_BY(resource_adaptation_queue_); - std::vector adaptation_listeners_ - RTC_GUARDED_BY(resource_adaptation_queue_); // Purely used for statistics, does not ensure mapped resources stay alive. std::map, VideoStreamAdapter::RestrictionsWithCounters> diff --git a/call/adaptation/resource_adaptation_processor_interface.h b/call/adaptation/resource_adaptation_processor_interface.h index cd684419c8..fe500c9d89 100644 --- a/call/adaptation/resource_adaptation_processor_interface.h +++ b/call/adaptation/resource_adaptation_processor_interface.h @@ -68,10 +68,6 @@ class ResourceAdaptationProcessorInterface { AdaptationConstraint* adaptation_constraint) = 0; virtual void RemoveAdaptationConstraint( AdaptationConstraint* adaptation_constraint) = 0; - virtual void AddAdaptationListener( - AdaptationListener* adaptation_listener) = 0; - virtual void RemoveAdaptationListener( - AdaptationListener* adaptation_listener) = 0; }; } // namespace webrtc diff --git a/call/adaptation/resource_adaptation_processor_unittest.cc b/call/adaptation/resource_adaptation_processor_unittest.cc index 0b16fabca7..42fb497159 100644 --- a/call/adaptation/resource_adaptation_processor_unittest.cc +++ b/call/adaptation/resource_adaptation_processor_unittest.cc @@ -102,7 +102,7 @@ class ResourceAdaptationProcessorTest : public ::testing::Test { processor_->AddResource(resource_); processor_->AddResource(other_resource_); processor_->AddAdaptationConstraint(&adaptation_constraint_); - processor_->AddAdaptationListener(&adaptation_listener_); + video_stream_adapter_->AddAdaptationListener(&adaptation_listener_); } ~ResourceAdaptationProcessorTest() override { if (processor_) { @@ -132,7 +132,7 @@ class ResourceAdaptationProcessorTest : public ::testing::Test { processor_->RemoveResource(other_resource_); } processor_->RemoveAdaptationConstraint(&adaptation_constraint_); - processor_->RemoveAdaptationListener(&adaptation_listener_); + video_stream_adapter_->RemoveAdaptationListener(&adaptation_listener_); video_stream_adapter_->RemoveRestrictionsListener(&restrictions_listener_); processor_.reset(); } diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc index 51433efee4..438b64e883 100644 --- a/call/adaptation/video_stream_adapter.cc +++ b/call/adaptation/video_stream_adapter.cc @@ -210,7 +210,11 @@ VideoStreamAdapter::VideoStreamAdapter( sequence_checker_.Detach(); } -VideoStreamAdapter::~VideoStreamAdapter() {} +VideoStreamAdapter::~VideoStreamAdapter() { + RTC_DCHECK(adaptation_listeners_.empty()) + << "There are listener(s) attached to a VideoStreamAdapter being " + "destroyed."; +} VideoSourceRestrictions VideoStreamAdapter::source_restrictions() const { RTC_DCHECK_RUN_ON(&sequence_checker_); @@ -230,7 +234,8 @@ void VideoStreamAdapter::ClearRestrictions() { current_restrictions_ = {VideoSourceRestrictions(), VideoAdaptationCounters()}; awaiting_frame_size_change_ = absl::nullopt; - BroadcastVideoRestrictionsUpdate(nullptr); + BroadcastVideoRestrictionsUpdate(input_state_provider_->InputState(), + nullptr); } void VideoStreamAdapter::AddRestrictionsListener( @@ -251,6 +256,24 @@ void VideoStreamAdapter::RemoveRestrictionsListener( restrictions_listeners_.erase(it); } +void VideoStreamAdapter::AddAdaptationListener( + AdaptationListener* adaptation_listener) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + RTC_DCHECK(std::find(adaptation_listeners_.begin(), + adaptation_listeners_.end(), + adaptation_listener) == adaptation_listeners_.end()); + adaptation_listeners_.push_back(adaptation_listener); +} + +void VideoStreamAdapter::RemoveAdaptationListener( + AdaptationListener* adaptation_listener) { + RTC_DCHECK_RUN_ON(&sequence_checker_); + auto it = std::find(adaptation_listeners_.begin(), + adaptation_listeners_.end(), adaptation_listener); + RTC_DCHECK(it != adaptation_listeners_.end()); + adaptation_listeners_.erase(it); +} + void VideoStreamAdapter::SetDegradationPreference( DegradationPreference degradation_preference) { RTC_DCHECK_RUN_ON(&sequence_checker_); @@ -266,7 +289,8 @@ void VideoStreamAdapter::SetDegradationPreference( // ClearRestrictions() calls BroadcastVideoRestrictionsUpdate(nullptr). ClearRestrictions(); } else { - BroadcastVideoRestrictionsUpdate(nullptr); + BroadcastVideoRestrictionsUpdate(input_state_provider_->InputState(), + nullptr); } } @@ -584,7 +608,7 @@ void VideoStreamAdapter::ApplyAdaptation( awaiting_frame_size_change_ = absl::nullopt; } current_restrictions_ = {adaptation.restrictions(), adaptation.counters()}; - BroadcastVideoRestrictionsUpdate(resource); + BroadcastVideoRestrictionsUpdate(adaptation.input_state(), resource); } Adaptation VideoStreamAdapter::GetAdaptationTo( @@ -598,6 +622,7 @@ Adaptation VideoStreamAdapter::GetAdaptationTo( } void VideoStreamAdapter::BroadcastVideoRestrictionsUpdate( + const VideoStreamInputState& input_state, const rtc::scoped_refptr& resource) { RTC_DCHECK_RUN_ON(&sequence_checker_); VideoSourceRestrictions filtered = FilterRestrictionsByDegradationPreference( @@ -610,6 +635,11 @@ void VideoStreamAdapter::BroadcastVideoRestrictionsUpdate( filtered, current_restrictions_.counters, resource, source_restrictions()); } + for (auto* adaptation_listener : adaptation_listeners_) { + adaptation_listener->OnAdaptationApplied( + input_state, last_video_source_restrictions_, + current_restrictions_.restrictions, resource); + } last_video_source_restrictions_ = current_restrictions_.restrictions; last_filtered_restrictions_ = filtered; } diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h index 00a52a9ef5..f91be3cead 100644 --- a/call/adaptation/video_stream_adapter.h +++ b/call/adaptation/video_stream_adapter.h @@ -20,6 +20,7 @@ #include "api/adaptation/resource.h" #include "api/rtp_parameters.h" #include "api/video/video_adaptation_counters.h" +#include "call/adaptation/adaptation_listener.h" #include "call/adaptation/degradation_preference_provider.h" #include "call/adaptation/video_source_restrictions.h" #include "call/adaptation/video_stream_input_state.h" @@ -135,6 +136,8 @@ class VideoStreamAdapter { VideoSourceRestrictionsListener* restrictions_listener); void RemoveRestrictionsListener( VideoSourceRestrictionsListener* restrictions_listener); + void AddAdaptationListener(AdaptationListener* adaptation_listener); + void RemoveAdaptationListener(AdaptationListener* adaptation_listener); // TODO(hbos): Setting the degradation preference should not clear // restrictions! This is not defined in the spec and is unexpected, there is a @@ -164,6 +167,7 @@ class VideoStreamAdapter { private: void BroadcastVideoRestrictionsUpdate( + const VideoStreamInputState& input_state, const rtc::scoped_refptr& resource); bool HasSufficientInputForAdaptation(const VideoStreamInputState& input_state) @@ -244,6 +248,8 @@ class VideoStreamAdapter { std::vector restrictions_listeners_ RTC_GUARDED_BY(&sequence_checker_); + std::vector adaptation_listeners_ + RTC_GUARDED_BY(&sequence_checker_); RestrictionsWithCounters current_restrictions_ RTC_GUARDED_BY(&sequence_checker_); diff --git a/call/adaptation/video_stream_adapter_unittest.cc b/call/adaptation/video_stream_adapter_unittest.cc index 86c53a96bc..cdda03cc3e 100644 --- a/call/adaptation/video_stream_adapter_unittest.cc +++ b/call/adaptation/video_stream_adapter_unittest.cc @@ -18,7 +18,9 @@ #include "api/video_codecs/video_codec.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_config.h" +#include "call/adaptation/adaptation_listener.h" #include "call/adaptation/encoder_settings.h" +#include "call/adaptation/test/fake_adaptation_listener.h" #include "call/adaptation/video_source_restrictions.h" #include "call/adaptation/video_stream_input_state.h" #include "rtc_base/string_encode.h" @@ -29,6 +31,10 @@ namespace webrtc { +using ::testing::_; +using ::testing::DoAll; +using ::testing::SaveArg; + namespace { const int kBalancedHighResolutionPixels = 1280 * 720; @@ -145,6 +151,17 @@ class FakeVideoStreamAdapterListner : public VideoSourceRestrictionsListener { VideoSourceRestrictions last_restrictions_; }; +class MockAdaptationListener : public AdaptationListener { + public: + MOCK_METHOD(void, + OnAdaptationApplied, + (const VideoStreamInputState& input_state, + const VideoSourceRestrictions& restrictions_before, + const VideoSourceRestrictions& restrictions_after, + rtc::scoped_refptr reason_resource), + (override)); +}; + } // namespace class VideoStreamAdapterTest : public ::testing::Test { @@ -870,6 +887,26 @@ TEST_F(VideoStreamAdapterTest, adapter_.GetAdaptDownResolution().status()); } +TEST_F(VideoStreamAdapterTest, AdaptationListenerReceivesSignalOnAdaptation) { + testing::StrictMock adaptation_listener; + adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE); + adapter_.AddAdaptationListener(&adaptation_listener); + input_state_provider_.SetInputState(1280 * 720, 30, + kDefaultMinPixelsPerFrame); + VideoSourceRestrictions restrictions_before; + VideoSourceRestrictions restrictions_after; + EXPECT_CALL(adaptation_listener, OnAdaptationApplied) + .WillOnce(DoAll(SaveArg<1>(&restrictions_before), + SaveArg<2>(&restrictions_after))); + auto adaptation = adapter_.GetAdaptationDown(); + adapter_.ApplyAdaptation(adaptation, nullptr); + EXPECT_EQ(VideoSourceRestrictions(), restrictions_before); + EXPECT_EQ(adaptation.restrictions(), restrictions_after); + + // Clean up. + adapter_.RemoveAdaptationListener(&adaptation_listener); +} + // Death tests. // Disabled on Android because death tests misbehave on Android, see // base/test/gtest_util.h. diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 84b5aa327c..255405ee92 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -348,7 +348,6 @@ VideoStreamEncoder::VideoStreamEncoder( degradation_preference_manager_( std::make_unique()), adaptation_constraints_(), - adaptation_listeners_(), stream_resource_manager_(&input_state_provider_, encoder_stats_observer, clock_, @@ -387,15 +386,14 @@ VideoStreamEncoder::VideoStreamEncoder( // Add the stream resource manager's resources to the processor. adaptation_constraints_ = stream_resource_manager_.AdaptationConstraints(); - adaptation_listeners_ = stream_resource_manager_.AdaptationListeners(); for (auto& resource : stream_resource_manager_.MappedResources()) { resource_adaptation_processor_->AddResource(resource); } for (auto* constraint : adaptation_constraints_) { resource_adaptation_processor_->AddAdaptationConstraint(constraint); } - for (auto* listener : adaptation_listeners_) { - resource_adaptation_processor_->AddAdaptationListener(listener); + for (auto* listener : stream_resource_manager_.AdaptationListeners()) { + video_stream_adapter_->AddAdaptationListener(listener); } initialize_processor_event.Set(); }); @@ -423,8 +421,8 @@ void VideoStreamEncoder::Stop() { for (auto* constraint : adaptation_constraints_) { resource_adaptation_processor_->RemoveAdaptationConstraint(constraint); } - for (auto* listener : adaptation_listeners_) { - resource_adaptation_processor_->RemoveAdaptationListener(listener); + for (auto* listener : stream_resource_manager_.AdaptationListeners()) { + video_stream_adapter_->RemoveAdaptationListener(listener); } video_stream_adapter_->RemoveRestrictionsListener(this); video_stream_adapter_->RemoveRestrictionsListener( @@ -2153,23 +2151,6 @@ void VideoStreamEncoder::InjectAdaptationConstraint( event.Wait(rtc::Event::kForever); } -void VideoStreamEncoder::InjectAdaptationListener( - AdaptationListener* adaptation_listener) { - rtc::Event event; - resource_adaptation_queue_.PostTask([this, adaptation_listener, &event] { - RTC_DCHECK_RUN_ON(&resource_adaptation_queue_); - if (!resource_adaptation_processor_) { - // The VideoStreamEncoder was stopped and the processor destroyed before - // this task had a chance to execute. No action needed. - return; - } - adaptation_listeners_.push_back(adaptation_listener); - resource_adaptation_processor_->AddAdaptationListener(adaptation_listener); - event.Set(); - }); - event.Wait(rtc::Event::kForever); -} - rtc::scoped_refptr VideoStreamEncoder::quality_scaler_resource_for_testing() { RTC_DCHECK_RUN_ON(&encoder_queue_); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 5af011113d..95d4dcb69e 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -129,7 +129,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, void InjectAdaptationResource(rtc::scoped_refptr resource, VideoAdaptationReason reason); void InjectAdaptationConstraint(AdaptationConstraint* adaptation_constraint); - void InjectAdaptationListener(AdaptationListener* adaptation_listener); rtc::scoped_refptr quality_scaler_resource_for_testing(); @@ -423,8 +422,6 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, std::unique_ptr degradation_preference_manager_; std::vector adaptation_constraints_ RTC_GUARDED_BY(&resource_adaptation_queue_); - std::vector adaptation_listeners_ - RTC_GUARDED_BY(&resource_adaptation_queue_); // Handles input, output and stats reporting related to VideoStreamEncoder // specific resources, such as "encode usage percent" measurements and "QP // scaling". Also involved with various mitigations such as inital frame diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 6bdcbd09c5..6a11bf6b83 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -322,13 +322,11 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder { task_queue_factory), fake_cpu_resource_(FakeResource::Create("FakeResource[CPU]")), fake_quality_resource_(FakeResource::Create("FakeResource[QP]")), - fake_adaptation_constraint_("FakeAdaptationConstraint"), - fake_adaptation_listener_() { + fake_adaptation_constraint_("FakeAdaptationConstraint") { InjectAdaptationResource(fake_quality_resource_, VideoAdaptationReason::kQuality); InjectAdaptationResource(fake_cpu_resource_, VideoAdaptationReason::kCpu); InjectAdaptationConstraint(&fake_adaptation_constraint_); - InjectAdaptationListener(&fake_adaptation_listener_); } void SetSourceAndWaitForRestrictionsUpdated( @@ -435,7 +433,6 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder { rtc::scoped_refptr fake_cpu_resource_; rtc::scoped_refptr fake_quality_resource_; FakeAdaptationConstraint fake_adaptation_constraint_; - FakeAdaptationListener fake_adaptation_listener_; }; class VideoStreamFactory