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}
This commit is contained in:
Markus Handell 2022-09-01 12:51:50 +00:00 committed by WebRTC LUCI CQ
parent 6670e4e059
commit 5a77e51c17
4 changed files with 40 additions and 18 deletions

View File

@ -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<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::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);

View File

@ -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.

View File

@ -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<int> {
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});
}

View File

@ -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);