Don't await adaptation after deg-pref change
Now we only await a previous adaptation if the degradataion
preference is the same as our current degradation preference.
Without this guard we can get stuck as detailed in the attached bug.
Bug: webrtc:11562
Change-Id: I91be48546446ef8d01fe901bc6889201a5b97ba6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174805
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31236}
diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc
index 7b15473..4ebe00f 100644
--- a/call/adaptation/video_stream_adapter.cc
+++ b/call/adaptation/video_stream_adapter.cc
@@ -315,22 +315,6 @@
VideoAdaptationCounters adaptations_;
};
-// static
-VideoStreamAdapter::AdaptationRequest::Mode
-VideoStreamAdapter::AdaptationRequest::GetModeFromAdaptationAction(
- Adaptation::StepType step_type) {
- switch (step_type) {
- case Adaptation::StepType::kIncreaseResolution:
- return AdaptationRequest::Mode::kAdaptUp;
- case Adaptation::StepType::kDecreaseResolution:
- return AdaptationRequest::Mode::kAdaptDown;
- case Adaptation::StepType::kIncreaseFrameRate:
- return AdaptationRequest::Mode::kAdaptUp;
- case Adaptation::StepType::kDecreaseFrameRate:
- return AdaptationRequest::Mode::kAdaptDown;
- }
-}
-
VideoStreamAdapter::VideoStreamAdapter()
: source_restrictor_(std::make_unique<VideoSourceRestrictor>()),
balanced_settings_(),
@@ -381,10 +365,10 @@
RTC_DCHECK_NE(degradation_preference_, DegradationPreference::DISABLED);
RTC_DCHECK(input_state_.HasInputFrameSizeAndFramesPerSecond());
// Don't adapt if we're awaiting a previous adaptation to have an effect.
- bool last_adaptation_was_up =
- last_adaptation_request_ &&
- last_adaptation_request_->mode_ == AdaptationRequest::Mode::kAdaptUp;
- if (last_adaptation_was_up &&
+ bool last_request_increased_resolution =
+ last_adaptation_request_ && last_adaptation_request_->step_type_ ==
+ Adaptation::StepType::kIncreaseResolution;
+ if (last_request_increased_resolution &&
degradation_preference_ == DegradationPreference::MAINTAIN_FRAMERATE &&
input_state_.frame_size_pixels().value() <=
last_adaptation_request_->input_pixel_count_) {
@@ -453,12 +437,12 @@
Adaptation VideoStreamAdapter::GetAdaptationDown() const {
RTC_DCHECK_NE(degradation_preference_, DegradationPreference::DISABLED);
RTC_DCHECK(input_state_.HasInputFrameSizeAndFramesPerSecond());
- // Don't adapt adaptation is disabled.
- bool last_adaptation_was_down =
- last_adaptation_request_ &&
- last_adaptation_request_->mode_ == AdaptationRequest::Mode::kAdaptDown;
- // Don't adapt if we're awaiting a previous adaptation to have an effect.
- if (last_adaptation_was_down &&
+ // Don't adapt if we're awaiting a previous adaptation to have an effect or
+ // if we switched degradation preference.
+ bool last_request_decreased_resolution =
+ last_adaptation_request_ && last_adaptation_request_->step_type_ ==
+ Adaptation::StepType::kDecreaseResolution;
+ if (last_request_decreased_resolution &&
degradation_preference_ == DegradationPreference::MAINTAIN_FRAMERATE &&
input_state_.frame_size_pixels().value() >=
last_adaptation_request_->input_pixel_count_) {
@@ -536,8 +520,7 @@
// adapting again before this adaptation has had an effect.
last_adaptation_request_.emplace(AdaptationRequest{
input_state_.frame_size_pixels().value(),
- input_state_.frames_per_second(),
- AdaptationRequest::GetModeFromAdaptationAction(adaptation.step().type)});
+ input_state_.frames_per_second(), adaptation.step().type});
// Adapt!
source_restrictor_->ApplyAdaptationStep(adaptation.step(),
degradation_preference_);
diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h
index 1f44dd5..f313e6b 100644
--- a/call/adaptation/video_stream_adapter.h
+++ b/call/adaptation/video_stream_adapter.h
@@ -147,12 +147,8 @@
int input_pixel_count_;
// Framerate received from the source at the time of the adaptation.
int framerate_fps_;
- // Indicates if request was to adapt up or down.
- enum class Mode { kAdaptUp, kAdaptDown } mode_;
-
- // This is a static method rather than an anonymous namespace function due
- // to namespace visiblity.
- static Mode GetModeFromAdaptationAction(Adaptation::StepType step_type);
+ // Degradation preference for the request.
+ Adaptation::StepType step_type_;
};
// Owner and modifier of the VideoSourceRestriction of this stream adaptor.
diff --git a/call/adaptation/video_stream_adapter_unittest.cc b/call/adaptation/video_stream_adapter_unittest.cc
index 55d604e..79247a7 100644
--- a/call/adaptation/video_stream_adapter_unittest.cc
+++ b/call/adaptation/video_stream_adapter_unittest.cc
@@ -552,6 +552,130 @@
}
}
+TEST(VideoStreamAdapterTest,
+ MaintainResolution_AdaptsUpAfterSwitchingDegradationPreference) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Adapt down in fps for later.
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ EXPECT_EQ(1, adapter.adaptation_counters().fps_adaptations);
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationUp());
+ EXPECT_EQ(1, adapter.adaptation_counters().fps_adaptations);
+ EXPECT_EQ(0, adapter.adaptation_counters().resolution_adaptations);
+
+ // We should be able to adapt in framerate one last time after the change of
+ // degradation preference.
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ Adaptation adaptation = adapter.GetAdaptationUp();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationUp());
+ EXPECT_EQ(0, adapter.adaptation_counters().fps_adaptations);
+}
+
+TEST(VideoStreamAdapterTest,
+ MaintainFramerate_AdaptsUpAfterSwitchingDegradationPreference) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Adapt down in resolution for later.
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ EXPECT_EQ(1, adapter.adaptation_counters().resolution_adaptations);
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationUp());
+ EXPECT_EQ(1, adapter.adaptation_counters().resolution_adaptations);
+ EXPECT_EQ(0, adapter.adaptation_counters().fps_adaptations);
+
+ // We should be able to adapt in framerate one last time after the change of
+ // degradation preference.
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ Adaptation adaptation = adapter.GetAdaptationUp();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationUp());
+ EXPECT_EQ(0, adapter.adaptation_counters().resolution_adaptations);
+}
+
+TEST(VideoStreamAdapterTest,
+ PendingResolutionIncreaseAllowsAdaptUpAfterSwitchToMaintainResolution) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Adapt fps down so we can adapt up later in the test.
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ // Apply adaptation up but don't update input.
+ adapter.ApplyAdaptation(adapter.GetAdaptationUp());
+ EXPECT_EQ(Adaptation::Status::kAwaitingPreviousAdaptation,
+ adapter.GetAdaptationUp().status());
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ Adaptation adaptation = adapter.GetAdaptationUp();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+}
+
+TEST(VideoStreamAdapterTest,
+ MaintainFramerate_AdaptsDownAfterSwitchingDegradationPreference) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Adapt down once, should change FPS.
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ EXPECT_EQ(1, adapter.adaptation_counters().fps_adaptations);
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ // Adaptation down should apply after the degradation prefs change.
+ Adaptation adaptation = adapter.GetAdaptationDown();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+ fake_stream.ApplyAdaptation(adaptation);
+ EXPECT_EQ(1, adapter.adaptation_counters().fps_adaptations);
+ EXPECT_EQ(1, adapter.adaptation_counters().resolution_adaptations);
+}
+
+TEST(VideoStreamAdapterTest,
+ MaintainResolution_AdaptsDownAfterSwitchingDegradationPreference) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Adapt down once, should change FPS.
+ fake_stream.ApplyAdaptation(adapter.GetAdaptationDown());
+ EXPECT_EQ(1, adapter.adaptation_counters().resolution_adaptations);
+
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ Adaptation adaptation = adapter.GetAdaptationDown();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+ fake_stream.ApplyAdaptation(adaptation);
+
+ EXPECT_EQ(1, adapter.adaptation_counters().fps_adaptations);
+ EXPECT_EQ(1, adapter.adaptation_counters().resolution_adaptations);
+}
+
+TEST(VideoStreamAdapterTest,
+ PendingResolutionDecreaseAllowsAdaptDownAfterSwitchToMaintainResolution) {
+ VideoStreamAdapter adapter;
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ FakeVideoStream fake_stream(&adapter, 1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ // Apply adaptation but don't update the input.
+ adapter.ApplyAdaptation(adapter.GetAdaptationDown());
+ EXPECT_EQ(Adaptation::Status::kAwaitingPreviousAdaptation,
+ adapter.GetAdaptationDown().status());
+ adapter.SetDegradationPreference(DegradationPreference::MAINTAIN_RESOLUTION);
+ Adaptation adaptation = adapter.GetAdaptationDown();
+ EXPECT_EQ(Adaptation::Status::kValid, adaptation.status());
+}
+
TEST(VideoStreamAdapterTest, PeekNextRestrictions) {
VideoStreamAdapter adapter;
// Any non-disabled DegradationPreference will do.
