diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc index c24a82cbe4..efffa9672a 100644 --- a/video/frame_cadence_adapter.cc +++ b/video/frame_cadence_adapter.cc @@ -107,10 +107,11 @@ class ZeroHertzAdapterMode : public AdapterMode { const FrameCadenceAdapterInterface::ZeroHertzModeParams& params); // Updates spatial layer quality convergence status. - void UpdateLayerQualityConvergence(int spatial_index, bool quality_converged); + void UpdateLayerQualityConvergence(size_t spatial_index, + bool quality_converged); // Updates spatial layer enabled status. - void UpdateLayerStatus(int spatial_index, bool enabled); + void UpdateLayerStatus(size_t spatial_index, bool enabled); // Adapter overrides. void OnFrame(Timestamp post_time, @@ -232,9 +233,9 @@ class FrameCadenceAdapterImpl : public FrameCadenceAdapterInterface { absl::optional params) override; absl::optional GetInputFrameRateFps() override; void UpdateFrameRate() override; - void UpdateLayerQualityConvergence(int spatial_index, + void UpdateLayerQualityConvergence(size_t spatial_index, bool quality_converged) override; - void UpdateLayerStatus(int spatial_index, bool enabled) override; + void UpdateLayerStatus(size_t spatial_index, bool enabled) override; void ProcessKeyFrameRequest() override; // VideoFrameSink overrides. @@ -323,19 +324,22 @@ void ZeroHertzAdapterMode::ReconfigureParameters( } void ZeroHertzAdapterMode::UpdateLayerQualityConvergence( - int spatial_index, + size_t spatial_index, bool quality_converged) { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_DCHECK_LT(spatial_index, layer_trackers_.size()); RTC_LOG(LS_INFO) << __func__ << " this " << this << " layer " << spatial_index << " quality has converged: " << quality_converged; + if (spatial_index >= layer_trackers_.size()) + return; if (layer_trackers_[spatial_index].quality_converged.has_value()) layer_trackers_[spatial_index].quality_converged = quality_converged; } -void ZeroHertzAdapterMode::UpdateLayerStatus(int spatial_index, bool enabled) { +void ZeroHertzAdapterMode::UpdateLayerStatus(size_t spatial_index, + bool enabled) { RTC_DCHECK_RUN_ON(&sequence_checker_); - RTC_DCHECK_LT(spatial_index, layer_trackers_.size()); + if (spatial_index >= layer_trackers_.size()) + return; if (enabled) { if (!layer_trackers_[spatial_index].quality_converged.has_value()) { // Assume quality has not converged until hearing otherwise. @@ -624,14 +628,14 @@ void FrameCadenceAdapterImpl::UpdateFrameRate() { } void FrameCadenceAdapterImpl::UpdateLayerQualityConvergence( - int spatial_index, + size_t spatial_index, bool quality_converged) { if (zero_hertz_adapter_.has_value()) zero_hertz_adapter_->UpdateLayerQualityConvergence(spatial_index, quality_converged); } -void FrameCadenceAdapterImpl::UpdateLayerStatus(int spatial_index, +void FrameCadenceAdapterImpl::UpdateLayerStatus(size_t spatial_index, bool enabled) { if (zero_hertz_adapter_.has_value()) zero_hertz_adapter_->UpdateLayerStatus(spatial_index, enabled); diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h index 149a915624..d0eab7e770 100644 --- a/video/frame_cadence_adapter.h +++ b/video/frame_cadence_adapter.h @@ -47,7 +47,7 @@ class FrameCadenceAdapterInterface struct ZeroHertzModeParams { // The number of simulcast layers used in this configuration. - int num_simulcast_layers = 0; + size_t num_simulcast_layers = 0; }; // Callback interface used to inform instance owners. @@ -106,11 +106,11 @@ class FrameCadenceAdapterInterface // Updates quality convergence status for an enabled spatial layer. // Convergence means QP has dropped to a low-enough level to warrant ceasing // to send identical frames at high frequency. - virtual void UpdateLayerQualityConvergence(int spatial_index, + virtual void UpdateLayerQualityConvergence(size_t spatial_index, bool converged) = 0; // Updates spatial layer enabled status. - virtual void UpdateLayerStatus(int spatial_index, bool enabled) = 0; + virtual void UpdateLayerStatus(size_t spatial_index, bool enabled) = 0; // Conditionally requests a refresh frame via // Callback::RequestRefreshFrame. diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc index 0824ac309b..afc675ffde 100644 --- a/video/frame_cadence_adapter_unittest.cc +++ b/video/frame_cadence_adapter_unittest.cc @@ -499,6 +499,24 @@ TEST(FrameCadenceAdapterTest, OmitsRefreshAfterFrameDropWithTimelyFrameEntry) { Mock::VerifyAndClearExpectations(&callback); } +TEST(FrameCadenceAdapterTest, AcceptsUnconfiguredLayerFeedback) { + // This is a regression test for bugs.webrtc.org/14417. + ZeroHertzFieldTrialEnabler enabler; + MockCallback callback; + GlobalSimulatedTimeController time_controller(Timestamp::Zero()); + auto adapter = CreateAdapter(enabler, time_controller.GetClock()); + adapter->Initialize(&callback); + adapter->SetZeroHertzModeEnabled( + FrameCadenceAdapterInterface::ZeroHertzModeParams{.num_simulcast_layers = + 1}); + constexpr int kMaxFps = 10; + adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps}); + time_controller.AdvanceTime(TimeDelta::Zero()); + + adapter->UpdateLayerQualityConvergence(2, false); + adapter->UpdateLayerStatus(2, false); +} + class FrameCadenceAdapterSimulcastLayersParamTest : public ::testing::TestWithParam { public: @@ -514,7 +532,7 @@ class FrameCadenceAdapterSimulcastLayersParamTest time_controller_.AdvanceTime(TimeDelta::Zero()); adapter_->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{}); - const int num_spatial_layers = GetParam(); + const size_t num_spatial_layers = GetParam(); adapter_->SetZeroHertzModeEnabled( FrameCadenceAdapterInterface::ZeroHertzModeParams{num_spatial_layers}); } diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index 4400a27415..3a5b03d1db 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -777,11 +777,11 @@ class MockFrameCadenceAdapter : public FrameCadenceAdapterInterface { MOCK_METHOD(void, UpdateFrameRate, (), (override)); MOCK_METHOD(void, UpdateLayerQualityConvergence, - (int spatial_index, bool converged), + (size_t spatial_index, bool converged), (override)); MOCK_METHOD(void, UpdateLayerStatus, - (int spatial_index, bool enabled), + (size_t spatial_index, bool enabled), (override)); MOCK_METHOD(void, ProcessKeyFrameRequest, (), (override)); }; @@ -9174,7 +9174,7 @@ TEST(VideoStreamEncoderFrameCadenceTest, ActivatesFrameCadenceOnContentType) { EXPECT_CALL(*adapter_ptr, SetZeroHertzModeEnabled(Optional(Field( &FrameCadenceAdapterInterface:: ZeroHertzModeParams::num_simulcast_layers, - Eq(0))))); + Eq(0u))))); VideoEncoderConfig config; test::FillEncoderConfiguration(kVideoCodecVP8, 1, &config); config.content_type = VideoEncoderConfig::ContentType::kScreen; @@ -9186,7 +9186,7 @@ TEST(VideoStreamEncoderFrameCadenceTest, ActivatesFrameCadenceOnContentType) { EXPECT_CALL(*adapter_ptr, SetZeroHertzModeEnabled(Optional(Field( &FrameCadenceAdapterInterface:: ZeroHertzModeParams::num_simulcast_layers, - Gt(0))))); + Gt(0u))))); PassAFrame(encoder_queue, video_stream_encoder_callback, /*ntp_time_ms=*/1); factory.DepleteTaskQueues(); Mock::VerifyAndClearExpectations(adapter_ptr);