FrameCadenceAdapter: survive layer updates for unconfigured layers.
The frame cadence adapter would previously crash on encountering
unconfigured layer updates. Fix this to silently ignore such updates.
Fixed: webrtc:14417
Change-Id: I524aa196f6aed10ffb8cd4ddcb4b053c71dfabba
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273325
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37980}
diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc
index c24a82c..efffa96 100644
--- a/video/frame_cadence_adapter.cc
+++ b/video/frame_cadence_adapter.cc
@@ -107,10 +107,11 @@
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 @@
absl::optional<ZeroHertzModeParams> params) override;
absl::optional<uint32_t> 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::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::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 149a915..d0eab7e 100644
--- a/video/frame_cadence_adapter.h
+++ b/video/frame_cadence_adapter.h
@@ -47,7 +47,7 @@
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 @@
// 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 0824ac3..afc675f 100644
--- a/video/frame_cadence_adapter_unittest.cc
+++ b/video/frame_cadence_adapter_unittest.cc
@@ -499,6 +499,24 @@
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<int> {
public:
@@ -514,7 +532,7 @@
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 4400a27..3a5b03d 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -777,11 +777,11 @@
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 @@
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 @@
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);
