Skip to content

Commit

Permalink
Don't await adaptation after deg-pref change
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Henrik Boström <[email protected]>
Cr-Commit-Position: refs/heads/master@{#31236}
  • Loading branch information
eshrubs authored and Commit Bot committed May 13, 2020
1 parent 1d6e70f commit 84afe46
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 34 deletions.
39 changes: 11 additions & 28 deletions call/adaptation/video_stream_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -315,22 +315,6 @@ class VideoStreamAdapter::VideoSourceRestrictor {
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_(),
Expand Down Expand Up @@ -381,10 +365,10 @@ Adaptation VideoStreamAdapter::GetAdaptationUp() const {
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_) {
Expand Down Expand Up @@ -453,12 +437,12 @@ Adaptation VideoStreamAdapter::GetAdaptationUp() const {
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_) {
Expand Down Expand Up @@ -536,8 +520,7 @@ void VideoStreamAdapter::ApplyAdaptation(const Adaptation& adaptation) {
// 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_);
Expand Down
8 changes: 2 additions & 6 deletions call/adaptation/video_stream_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,8 @@ class VideoStreamAdapter {
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.
Expand Down
124 changes: 124 additions & 0 deletions call/adaptation/video_stream_adapter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,130 @@ TEST(VideoStreamAdapterTest, MaintainFramerate_AwaitingPreviousAdaptationUp) {
}
}

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.
Expand Down

0 comments on commit 84afe46

Please sign in to comment.