diff --git a/call/adaptation/resource_adaptation_processor_interface.h b/call/adaptation/resource_adaptation_processor_interface.h index 8f46d7b9e0..5d84ffa088 100644 --- a/call/adaptation/resource_adaptation_processor_interface.h +++ b/call/adaptation/resource_adaptation_processor_interface.h @@ -13,6 +13,7 @@ #include "absl/types/optional.h" #include "api/rtp_parameters.h" +#include "api/video/video_adaptation_counters.h" #include "api/video/video_frame.h" #include "call/adaptation/encoder_settings.h" #include "call/adaptation/resource.h" @@ -26,8 +27,13 @@ class ResourceAdaptationProcessorListener { public: virtual ~ResourceAdaptationProcessorListener(); + // The |restrictions| are filtered by degradation preference but not the + // |adaptation_counters|, which are currently only reported for legacy stats + // calculation purposes. virtual void OnVideoSourceRestrictionsUpdated( - VideoSourceRestrictions restrictions) = 0; + VideoSourceRestrictions restrictions, + const VideoAdaptationCounters& adaptation_counters, + const Resource* reason) = 0; }; // Responsible for reconfiguring encoded streams based on resource consumption, diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc index b720de3074..8b6388f3d3 100644 --- a/call/adaptation/video_stream_adapter.cc +++ b/call/adaptation/video_stream_adapter.cc @@ -361,22 +361,17 @@ void VideoStreamAdapter::ClearRestrictions() { last_adaptation_request_.reset(); } -VideoStreamAdapter::SetDegradationPreferenceResult -VideoStreamAdapter::SetDegradationPreference( +void VideoStreamAdapter::SetDegradationPreference( DegradationPreference degradation_preference) { if (degradation_preference_ == degradation_preference) - return SetDegradationPreferenceResult::kRestrictionsNotCleared; + return; // Invalidate any previously returned Adaptation. ++adaptation_validation_id_; - bool did_clear = false; if (degradation_preference == DegradationPreference::BALANCED || degradation_preference_ == DegradationPreference::BALANCED) { ClearRestrictions(); - did_clear = true; } degradation_preference_ = degradation_preference; - return did_clear ? SetDegradationPreferenceResult::kRestrictionsCleared - : SetDegradationPreferenceResult::kRestrictionsNotCleared; } void VideoStreamAdapter::SetInput(VideoStreamInputState input_state) { diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h index 80ea869e17..5c41845182 100644 --- a/call/adaptation/video_stream_adapter.h +++ b/call/adaptation/video_stream_adapter.h @@ -109,11 +109,6 @@ class Adaptation final { // 3. Modify the stream's restrictions in one of the valid ways. class VideoStreamAdapter { public: - enum class SetDegradationPreferenceResult { - kRestrictionsNotCleared, - kRestrictionsCleared, - }; - VideoStreamAdapter(); ~VideoStreamAdapter(); @@ -129,8 +124,7 @@ class VideoStreamAdapter { // TODO(hbos): Setting the degradation preference should not clear // restrictions! This is not defined in the spec and is unexpected, there is a // tiny risk that people would discover and rely on this behavior. - SetDegradationPreferenceResult SetDegradationPreference( - DegradationPreference degradation_preference); + void SetDegradationPreference(DegradationPreference degradation_preference); // The adaptaiton logic depends on these inputs. void SetInput(VideoStreamInputState input_state); diff --git a/call/adaptation/video_stream_adapter_unittest.cc b/call/adaptation/video_stream_adapter_unittest.cc index 6bf2a35a80..55d604e917 100644 --- a/call/adaptation/video_stream_adapter_unittest.cc +++ b/call/adaptation/video_stream_adapter_unittest.cc @@ -588,18 +588,13 @@ TEST(VideoStreamAdapterTest, PeekNextRestrictions) { TEST(VideoStreamAdapterTest, SetDegradationPreferenceToOrFromBalancedClearsRestrictions) { VideoStreamAdapter adapter; - EXPECT_EQ(VideoStreamAdapter::SetDegradationPreferenceResult:: - kRestrictionsNotCleared, - adapter.SetDegradationPreference( - DegradationPreference::MAINTAIN_FRAMERATE)); + adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE); adapter.SetInput(InputState(1280 * 720, 30, kDefaultMinPixelsPerFrame)); adapter.ApplyAdaptation(adapter.GetAdaptationDown()); EXPECT_NE(VideoSourceRestrictions(), adapter.source_restrictions()); EXPECT_NE(0, adapter.adaptation_counters().Total()); // Changing from non-balanced to balanced clears the restrictions. - EXPECT_EQ( - VideoStreamAdapter::SetDegradationPreferenceResult::kRestrictionsCleared, - adapter.SetDegradationPreference(DegradationPreference::BALANCED)); + adapter.SetDegradationPreference(DegradationPreference::BALANCED); EXPECT_EQ(VideoSourceRestrictions(), adapter.source_restrictions()); EXPECT_EQ(0, adapter.adaptation_counters().Total()); // Apply adaptation again. @@ -607,10 +602,7 @@ TEST(VideoStreamAdapterTest, EXPECT_NE(VideoSourceRestrictions(), adapter.source_restrictions()); EXPECT_NE(0, adapter.adaptation_counters().Total()); // Changing from balanced to non-balanced clears the restrictions. - EXPECT_EQ( - VideoStreamAdapter::SetDegradationPreferenceResult::kRestrictionsCleared, - adapter.SetDegradationPreference( - DegradationPreference::MAINTAIN_RESOLUTION)); + adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION); EXPECT_EQ(VideoSourceRestrictions(), adapter.source_restrictions()); EXPECT_EQ(0, adapter.adaptation_counters().Total()); } diff --git a/video/adaptation/resource_adaptation_processor.cc b/video/adaptation/resource_adaptation_processor.cc index 388a82a92e..0e329af05d 100644 --- a/video/adaptation/resource_adaptation_processor.cc +++ b/video/adaptation/resource_adaptation_processor.cc @@ -11,6 +11,7 @@ #include "video/adaptation/resource_adaptation_processor.h" #include +#include #include #include #include @@ -323,6 +324,7 @@ void ResourceAdaptationProcessor::AddResource(Resource* resource, void ResourceAdaptationProcessor::SetDegradationPreference( DegradationPreference degradation_preference) { degradation_preference_ = degradation_preference; + UpdateStatsAdaptationSettings(); MaybeUpdateEffectiveDegradationPreference(); } @@ -358,9 +360,7 @@ void ResourceAdaptationProcessor::SetEncoderRates( void ResourceAdaptationProcessor::ResetVideoSourceRestrictions() { stream_adapter_->ClearRestrictions(); - ResetActiveCounts(); - encoder_stats_observer_->ClearAdaptationStats(); - MaybeUpdateVideoSourceRestrictions(); + MaybeUpdateVideoSourceRestrictions(nullptr); } void ResourceAdaptationProcessor::OnFrameDroppedDueToSize() { @@ -533,10 +533,7 @@ void ResourceAdaptationProcessor::OnResourceUnderuse( stream_adapter_->ApplyAdaptation(adaptation); // Update VideoSourceRestrictions based on adaptation. This also informs the // |adaptation_listener_|. - MaybeUpdateVideoSourceRestrictions(); - // Stats and logging. - UpdateAdaptationStats(GetReasonFromResource(reason_resource)); - RTC_LOG(LS_INFO) << ActiveCountsToString(); + MaybeUpdateVideoSourceRestrictions(&reason_resource); } ResourceListenerResponse ResourceAdaptationProcessor::OnResourceOveruse( @@ -562,10 +559,7 @@ ResourceListenerResponse ResourceAdaptationProcessor::OnResourceOveruse( stream_adapter_->ApplyAdaptation(adaptation); // Update VideoSourceRestrictions based on adaptation. This also informs the // |adaptation_listener_|. - MaybeUpdateVideoSourceRestrictions(); - // Stats and logging. - UpdateAdaptationStats(GetReasonFromResource(reason_resource)); - RTC_LOG(INFO) << ActiveCountsToString(); + MaybeUpdateVideoSourceRestrictions(&reason_resource); return response; } @@ -604,28 +598,59 @@ void ResourceAdaptationProcessor::MaybeUpdateEffectiveDegradationPreference() { degradation_preference_ == DegradationPreference::BALANCED) ? DegradationPreference::MAINTAIN_RESOLUTION : degradation_preference_; - if (stream_adapter_->SetDegradationPreference( - effective_degradation_preference_) == - VideoStreamAdapter::SetDegradationPreferenceResult:: - kRestrictionsCleared) { - ResetActiveCounts(); - encoder_stats_observer_->ClearAdaptationStats(); - } - MaybeUpdateVideoSourceRestrictions(); + stream_adapter_->SetDegradationPreference(effective_degradation_preference_); + MaybeUpdateVideoSourceRestrictions(nullptr); } -void ResourceAdaptationProcessor::MaybeUpdateVideoSourceRestrictions() { +void ResourceAdaptationProcessor::MaybeUpdateVideoSourceRestrictions( + const Resource* reason_resource) { VideoSourceRestrictions new_restrictions = FilterRestrictionsByDegradationPreference( stream_adapter_->source_restrictions(), degradation_preference_); if (video_source_restrictions_ != new_restrictions) { video_source_restrictions_ = std::move(new_restrictions); + // TODO(https://crbug.com/webrtc/11172): Support multiple listeners and + // loop through them here instead of calling two hardcoded listeners (|this| + // and |adaptation_listener_|). + OnVideoSourceRestrictionsUpdated(video_source_restrictions_, + stream_adapter_->adaptation_counters(), + reason_resource); adaptation_listener_->OnVideoSourceRestrictionsUpdated( - video_source_restrictions_); - MaybeUpdateTargetFrameRate(); + video_source_restrictions_, stream_adapter_->adaptation_counters(), + reason_resource); } } +void ResourceAdaptationProcessor::OnVideoSourceRestrictionsUpdated( + VideoSourceRestrictions restrictions, + const VideoAdaptationCounters& adaptation_counters, + const Resource* reason) { + VideoAdaptationCounters previous_adaptation_counters = + active_counts_[VideoAdaptationReason::kQuality] + + active_counts_[VideoAdaptationReason::kCpu]; + int adaptation_counters_total_abs_diff = std::abs( + adaptation_counters.Total() - previous_adaptation_counters.Total()); + if (reason) { + // A resource signal triggered this adaptation. The adaptation counters have + // to be updated every time the adaptation counter is incremented or + // decremented due to a resource. + RTC_DCHECK_EQ(adaptation_counters_total_abs_diff, 1); + VideoAdaptationReason reason_type = GetReasonFromResource(*reason); + UpdateAdaptationStats(adaptation_counters, reason_type); + } else if (adaptation_counters.Total() == 0) { + // Adaptation was manually reset - clear the per-reason counters too. + ResetActiveCounts(); + encoder_stats_observer_->ClearAdaptationStats(); + } else { + // If a reason did not increase or decrease the Total() by 1 and the + // restrictions were not just reset, the adaptation counters MUST not have + // been modified and there is nothing to do stats-wise. + RTC_DCHECK_EQ(adaptation_counters_total_abs_diff, 0); + } + RTC_LOG(LS_INFO) << ActiveCountsToString(); + MaybeUpdateTargetFrameRate(); +} + void ResourceAdaptationProcessor::MaybeUpdateTargetFrameRate() { absl::optional codec_max_frame_rate = encoder_settings_.has_value() @@ -710,12 +735,11 @@ void ResourceAdaptationProcessor::OnAdaptationCountChanged( } void ResourceAdaptationProcessor::UpdateAdaptationStats( + const VideoAdaptationCounters& total_counts, VideoAdaptationReason reason) { // Update active counts VideoAdaptationCounters& active_count = active_counts_[reason]; VideoAdaptationCounters& other_active = active_counts_[OtherReason(reason)]; - const VideoAdaptationCounters total_counts = - stream_adapter_->adaptation_counters(); OnAdaptationCountChanged(total_counts, &active_count, &other_active); diff --git a/video/adaptation/resource_adaptation_processor.h b/video/adaptation/resource_adaptation_processor.h index b91f8a0673..0037ab5204 100644 --- a/video/adaptation/resource_adaptation_processor.h +++ b/video/adaptation/resource_adaptation_processor.h @@ -59,7 +59,8 @@ extern const int kDefaultInputPixelsHeight; // indirectly in video_stream_encoder_unittest.cc and other tests exercising // VideoStreamEncoder. class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, - public ResourceListener { + public ResourceListener, + public ResourceAdaptationProcessorListener { public: // The processor can be constructed on any sequence, but must be initialized // and used on a single sequence, e.g. the encoder queue. @@ -83,7 +84,7 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, void StartResourceAdaptation( ResourceAdaptationProcessorListener* adaptation_listener) override; void StopResourceAdaptation() override; - // Uses a default AdaptReason of kCpu. + // Uses a default VideoAdaptationReason of kCpu. void AddResource(Resource* resource) override; void AddResource(Resource* resource, VideoAdaptationReason reason); void SetDegradationPreference( @@ -116,6 +117,11 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, ResourceListenerResponse OnResourceUsageStateMeasured( const Resource& resource) override; + void OnVideoSourceRestrictionsUpdated( + VideoSourceRestrictions restrictions, + const VideoAdaptationCounters& adaptation_counters, + const Resource* reason) override; + // For reasons of adaptation and statistics, we not only count the total // number of adaptations, but we also count the number of adaptations per // reason. @@ -157,7 +163,7 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, // Makes |video_source_restrictions_| up-to-date and informs the // |adaptation_listener_| if restrictions are changed, allowing the listener // to reconfigure the source accordingly. - void MaybeUpdateVideoSourceRestrictions(); + void MaybeUpdateVideoSourceRestrictions(const Resource* reason_resource); // Calculates an up-to-date value of the target frame rate and informs the // |encode_usage_resource_| of the new value. void MaybeUpdateTargetFrameRate(); @@ -166,7 +172,8 @@ class ResourceAdaptationProcessor : public ResourceAdaptationProcessorInterface, void UpdateQualityScalerSettings( absl::optional qp_thresholds); - void UpdateAdaptationStats(VideoAdaptationReason reason); + void UpdateAdaptationStats(const VideoAdaptationCounters& total_counts, + VideoAdaptationReason reason); void UpdateStatsAdaptationSettings() const; // Checks to see if we should execute the quality rampup experiment. The diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 157bf661b5..592260102f 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1657,7 +1657,9 @@ bool VideoStreamEncoder::DropDueToSize(uint32_t pixel_count) const { } void VideoStreamEncoder::OnVideoSourceRestrictionsUpdated( - VideoSourceRestrictions restrictions) { + VideoSourceRestrictions restrictions, + const VideoAdaptationCounters& adaptation_counters, + const Resource* reason) { RTC_DCHECK_RUN_ON(&encoder_queue_); video_source_sink_controller_->SetRestrictions(std::move(restrictions)); video_source_sink_controller_->PushSourceSinkSettings(); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 8e36f517d4..1bb3bcf9e0 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -108,7 +108,9 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, rtc::TaskQueue* encoder_queue() { return &encoder_queue_; } void OnVideoSourceRestrictionsUpdated( - VideoSourceRestrictions restrictions) override; + VideoSourceRestrictions restrictions, + const VideoAdaptationCounters& adaptation_counters, + const Resource* reason) override; // Used for injected test resources. // TODO(eshr): Move all adaptation tests out of VideoStreamEncoder tests.