[Adaptation] Move AdaptationListeners to VideoStreamAdapter
This is a step needed for multi-stream and new mitigations. It also
cleans up needing to signal adaptation changes in mutiple places
from ResourceAdaptationProcessor.
R=hbos@webrtc.org
Bug: webrtc:11754
Change-Id: Ib185dc9f66fbb4a087eb9e970c68c3f47eafb17f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178874
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31684}
diff --git a/call/adaptation/resource_adaptation_processor.cc b/call/adaptation/resource_adaptation_processor.cc
index dd273ac..bea8334 100644
--- a/call/adaptation/resource_adaptation_processor.cc
+++ b/call/adaptation/resource_adaptation_processor.cc
@@ -90,9 +90,6 @@
RTC_DCHECK(adaptation_constraints_.empty())
<< "There are constaint(s) attached to a ResourceAdaptationProcessor "
<< "being destroyed.";
- RTC_DCHECK(adaptation_listeners_.empty())
- << "There are listener(s) attached to a ResourceAdaptationProcessor "
- << "being destroyed.";
stream_adapter_->RemoveRestrictionsListener(this);
resource_listener_delegate_->OnProcessorDestroyed();
}
@@ -182,24 +179,6 @@
adaptation_constraints_.erase(it);
}
-void ResourceAdaptationProcessor::AddAdaptationListener(
- AdaptationListener* adaptation_listener) {
- RTC_DCHECK_RUN_ON(resource_adaptation_queue_);
- RTC_DCHECK(std::find(adaptation_listeners_.begin(),
- adaptation_listeners_.end(),
- adaptation_listener) == adaptation_listeners_.end());
- adaptation_listeners_.push_back(adaptation_listener);
-}
-
-void ResourceAdaptationProcessor::RemoveAdaptationListener(
- AdaptationListener* adaptation_listener) {
- RTC_DCHECK_RUN_ON(resource_adaptation_queue_);
- auto it = std::find(adaptation_listeners_.begin(),
- adaptation_listeners_.end(), adaptation_listener);
- RTC_DCHECK(it != adaptation_listeners_.end());
- adaptation_listeners_.erase(it);
-}
-
void ResourceAdaptationProcessor::OnResourceUsageStateMeasured(
rtc::scoped_refptr<Resource> resource,
ResourceUsageState usage_state) {
@@ -308,11 +287,6 @@
}
// Apply adaptation.
stream_adapter_->ApplyAdaptation(adaptation, reason_resource);
- for (auto* adaptation_listener : adaptation_listeners_) {
- adaptation_listener->OnAdaptationApplied(
- adaptation.input_state(), restrictions_before, restrictions_after,
- reason_resource);
- }
processing_in_progress_ = false;
rtc::StringBuilder message;
message << "Adapted up successfully. Unfiltered adaptations: "
@@ -341,17 +315,9 @@
message.Release());
}
// Apply adaptation.
- VideoSourceRestrictions restrictions_before =
- stream_adapter_->source_restrictions();
- VideoSourceRestrictions restrictions_after = adaptation.restrictions();
UpdateResourceLimitations(reason_resource, adaptation.restrictions(),
adaptation.counters());
- stream_adapter_->ApplyAdaptation(adaptation, nullptr);
- for (auto* adaptation_listener : adaptation_listeners_) {
- adaptation_listener->OnAdaptationApplied(
- adaptation.input_state(), restrictions_before, restrictions_after,
- reason_resource);
- }
+ stream_adapter_->ApplyAdaptation(adaptation, reason_resource);
processing_in_progress_ = false;
rtc::StringBuilder message;
message << "Adapted down successfully. Unfiltered adaptations: "
@@ -429,11 +395,6 @@
most_limited.counters, most_limited.restrictions);
RTC_DCHECK_EQ(adapt_to.status(), Adaptation::Status::kValid);
stream_adapter_->ApplyAdaptation(adapt_to, nullptr);
- for (auto* adaptation_listener : adaptation_listeners_) {
- adaptation_listener->OnAdaptationApplied(
- adapt_to.input_state(), removed_limitations.restrictions,
- most_limited.restrictions, nullptr);
- }
RTC_LOG(INFO) << "Most limited resource removed. Restoring restrictions to "
"next most limited restrictions: "
diff --git a/call/adaptation/resource_adaptation_processor.h b/call/adaptation/resource_adaptation_processor.h
index f519fcc..9f20bdb 100644
--- a/call/adaptation/resource_adaptation_processor.h
+++ b/call/adaptation/resource_adaptation_processor.h
@@ -26,7 +26,6 @@
#include "api/video/video_frame.h"
#include "api/video/video_stream_encoder_observer.h"
#include "call/adaptation/adaptation_constraint.h"
-#include "call/adaptation/adaptation_listener.h"
#include "call/adaptation/resource_adaptation_processor_interface.h"
#include "call/adaptation/video_source_restrictions.h"
#include "call/adaptation/video_stream_adapter.h"
@@ -75,9 +74,6 @@
AdaptationConstraint* adaptation_constraint) override;
void RemoveAdaptationConstraint(
AdaptationConstraint* adaptation_constraint) override;
- void AddAdaptationListener(AdaptationListener* adaptation_listener) override;
- void RemoveAdaptationListener(
- AdaptationListener* adaptation_listener) override;
// ResourceListener implementation.
// Triggers OnResourceUnderuse() or OnResourceOveruse().
@@ -165,8 +161,6 @@
RTC_GUARDED_BY(resource_adaptation_queue_);
std::vector<AdaptationConstraint*> adaptation_constraints_
RTC_GUARDED_BY(resource_adaptation_queue_);
- std::vector<AdaptationListener*> adaptation_listeners_
- RTC_GUARDED_BY(resource_adaptation_queue_);
// Purely used for statistics, does not ensure mapped resources stay alive.
std::map<rtc::scoped_refptr<Resource>,
VideoStreamAdapter::RestrictionsWithCounters>
diff --git a/call/adaptation/resource_adaptation_processor_interface.h b/call/adaptation/resource_adaptation_processor_interface.h
index cd68441..fe500c9 100644
--- a/call/adaptation/resource_adaptation_processor_interface.h
+++ b/call/adaptation/resource_adaptation_processor_interface.h
@@ -68,10 +68,6 @@
AdaptationConstraint* adaptation_constraint) = 0;
virtual void RemoveAdaptationConstraint(
AdaptationConstraint* adaptation_constraint) = 0;
- virtual void AddAdaptationListener(
- AdaptationListener* adaptation_listener) = 0;
- virtual void RemoveAdaptationListener(
- AdaptationListener* adaptation_listener) = 0;
};
} // namespace webrtc
diff --git a/call/adaptation/resource_adaptation_processor_unittest.cc b/call/adaptation/resource_adaptation_processor_unittest.cc
index 0b16fab..42fb497 100644
--- a/call/adaptation/resource_adaptation_processor_unittest.cc
+++ b/call/adaptation/resource_adaptation_processor_unittest.cc
@@ -102,7 +102,7 @@
processor_->AddResource(resource_);
processor_->AddResource(other_resource_);
processor_->AddAdaptationConstraint(&adaptation_constraint_);
- processor_->AddAdaptationListener(&adaptation_listener_);
+ video_stream_adapter_->AddAdaptationListener(&adaptation_listener_);
}
~ResourceAdaptationProcessorTest() override {
if (processor_) {
@@ -132,7 +132,7 @@
processor_->RemoveResource(other_resource_);
}
processor_->RemoveAdaptationConstraint(&adaptation_constraint_);
- processor_->RemoveAdaptationListener(&adaptation_listener_);
+ video_stream_adapter_->RemoveAdaptationListener(&adaptation_listener_);
video_stream_adapter_->RemoveRestrictionsListener(&restrictions_listener_);
processor_.reset();
}
diff --git a/call/adaptation/video_stream_adapter.cc b/call/adaptation/video_stream_adapter.cc
index 51433ef..438b64e 100644
--- a/call/adaptation/video_stream_adapter.cc
+++ b/call/adaptation/video_stream_adapter.cc
@@ -210,7 +210,11 @@
sequence_checker_.Detach();
}
-VideoStreamAdapter::~VideoStreamAdapter() {}
+VideoStreamAdapter::~VideoStreamAdapter() {
+ RTC_DCHECK(adaptation_listeners_.empty())
+ << "There are listener(s) attached to a VideoStreamAdapter being "
+ "destroyed.";
+}
VideoSourceRestrictions VideoStreamAdapter::source_restrictions() const {
RTC_DCHECK_RUN_ON(&sequence_checker_);
@@ -230,7 +234,8 @@
current_restrictions_ = {VideoSourceRestrictions(),
VideoAdaptationCounters()};
awaiting_frame_size_change_ = absl::nullopt;
- BroadcastVideoRestrictionsUpdate(nullptr);
+ BroadcastVideoRestrictionsUpdate(input_state_provider_->InputState(),
+ nullptr);
}
void VideoStreamAdapter::AddRestrictionsListener(
@@ -251,6 +256,24 @@
restrictions_listeners_.erase(it);
}
+void VideoStreamAdapter::AddAdaptationListener(
+ AdaptationListener* adaptation_listener) {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DCHECK(std::find(adaptation_listeners_.begin(),
+ adaptation_listeners_.end(),
+ adaptation_listener) == adaptation_listeners_.end());
+ adaptation_listeners_.push_back(adaptation_listener);
+}
+
+void VideoStreamAdapter::RemoveAdaptationListener(
+ AdaptationListener* adaptation_listener) {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ auto it = std::find(adaptation_listeners_.begin(),
+ adaptation_listeners_.end(), adaptation_listener);
+ RTC_DCHECK(it != adaptation_listeners_.end());
+ adaptation_listeners_.erase(it);
+}
+
void VideoStreamAdapter::SetDegradationPreference(
DegradationPreference degradation_preference) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
@@ -266,7 +289,8 @@
// ClearRestrictions() calls BroadcastVideoRestrictionsUpdate(nullptr).
ClearRestrictions();
} else {
- BroadcastVideoRestrictionsUpdate(nullptr);
+ BroadcastVideoRestrictionsUpdate(input_state_provider_->InputState(),
+ nullptr);
}
}
@@ -584,7 +608,7 @@
awaiting_frame_size_change_ = absl::nullopt;
}
current_restrictions_ = {adaptation.restrictions(), adaptation.counters()};
- BroadcastVideoRestrictionsUpdate(resource);
+ BroadcastVideoRestrictionsUpdate(adaptation.input_state(), resource);
}
Adaptation VideoStreamAdapter::GetAdaptationTo(
@@ -598,6 +622,7 @@
}
void VideoStreamAdapter::BroadcastVideoRestrictionsUpdate(
+ const VideoStreamInputState& input_state,
const rtc::scoped_refptr<Resource>& resource) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
VideoSourceRestrictions filtered = FilterRestrictionsByDegradationPreference(
@@ -610,6 +635,11 @@
filtered, current_restrictions_.counters, resource,
source_restrictions());
}
+ for (auto* adaptation_listener : adaptation_listeners_) {
+ adaptation_listener->OnAdaptationApplied(
+ input_state, last_video_source_restrictions_,
+ current_restrictions_.restrictions, resource);
+ }
last_video_source_restrictions_ = current_restrictions_.restrictions;
last_filtered_restrictions_ = filtered;
}
diff --git a/call/adaptation/video_stream_adapter.h b/call/adaptation/video_stream_adapter.h
index 00a52a9..f91be3c 100644
--- a/call/adaptation/video_stream_adapter.h
+++ b/call/adaptation/video_stream_adapter.h
@@ -20,6 +20,7 @@
#include "api/adaptation/resource.h"
#include "api/rtp_parameters.h"
#include "api/video/video_adaptation_counters.h"
+#include "call/adaptation/adaptation_listener.h"
#include "call/adaptation/degradation_preference_provider.h"
#include "call/adaptation/video_source_restrictions.h"
#include "call/adaptation/video_stream_input_state.h"
@@ -135,6 +136,8 @@
VideoSourceRestrictionsListener* restrictions_listener);
void RemoveRestrictionsListener(
VideoSourceRestrictionsListener* restrictions_listener);
+ void AddAdaptationListener(AdaptationListener* adaptation_listener);
+ void RemoveAdaptationListener(AdaptationListener* adaptation_listener);
// TODO(hbos): Setting the degradation preference should not clear
// restrictions! This is not defined in the spec and is unexpected, there is a
@@ -164,6 +167,7 @@
private:
void BroadcastVideoRestrictionsUpdate(
+ const VideoStreamInputState& input_state,
const rtc::scoped_refptr<Resource>& resource);
bool HasSufficientInputForAdaptation(const VideoStreamInputState& input_state)
@@ -244,6 +248,8 @@
std::vector<VideoSourceRestrictionsListener*> restrictions_listeners_
RTC_GUARDED_BY(&sequence_checker_);
+ std::vector<AdaptationListener*> adaptation_listeners_
+ RTC_GUARDED_BY(&sequence_checker_);
RestrictionsWithCounters current_restrictions_
RTC_GUARDED_BY(&sequence_checker_);
diff --git a/call/adaptation/video_stream_adapter_unittest.cc b/call/adaptation/video_stream_adapter_unittest.cc
index 86c53a9..cdda03c 100644
--- a/call/adaptation/video_stream_adapter_unittest.cc
+++ b/call/adaptation/video_stream_adapter_unittest.cc
@@ -18,7 +18,9 @@
#include "api/video_codecs/video_codec.h"
#include "api/video_codecs/video_encoder.h"
#include "api/video_codecs/video_encoder_config.h"
+#include "call/adaptation/adaptation_listener.h"
#include "call/adaptation/encoder_settings.h"
+#include "call/adaptation/test/fake_adaptation_listener.h"
#include "call/adaptation/video_source_restrictions.h"
#include "call/adaptation/video_stream_input_state.h"
#include "rtc_base/string_encode.h"
@@ -29,6 +31,10 @@
namespace webrtc {
+using ::testing::_;
+using ::testing::DoAll;
+using ::testing::SaveArg;
+
namespace {
const int kBalancedHighResolutionPixels = 1280 * 720;
@@ -145,6 +151,17 @@
VideoSourceRestrictions last_restrictions_;
};
+class MockAdaptationListener : public AdaptationListener {
+ public:
+ MOCK_METHOD(void,
+ OnAdaptationApplied,
+ (const VideoStreamInputState& input_state,
+ const VideoSourceRestrictions& restrictions_before,
+ const VideoSourceRestrictions& restrictions_after,
+ rtc::scoped_refptr<Resource> reason_resource),
+ (override));
+};
+
} // namespace
class VideoStreamAdapterTest : public ::testing::Test {
@@ -870,6 +887,26 @@
adapter_.GetAdaptDownResolution().status());
}
+TEST_F(VideoStreamAdapterTest, AdaptationListenerReceivesSignalOnAdaptation) {
+ testing::StrictMock<MockAdaptationListener> adaptation_listener;
+ adapter_.SetDegradationPreference(DegradationPreference::MAINTAIN_FRAMERATE);
+ adapter_.AddAdaptationListener(&adaptation_listener);
+ input_state_provider_.SetInputState(1280 * 720, 30,
+ kDefaultMinPixelsPerFrame);
+ VideoSourceRestrictions restrictions_before;
+ VideoSourceRestrictions restrictions_after;
+ EXPECT_CALL(adaptation_listener, OnAdaptationApplied)
+ .WillOnce(DoAll(SaveArg<1>(&restrictions_before),
+ SaveArg<2>(&restrictions_after)));
+ auto adaptation = adapter_.GetAdaptationDown();
+ adapter_.ApplyAdaptation(adaptation, nullptr);
+ EXPECT_EQ(VideoSourceRestrictions(), restrictions_before);
+ EXPECT_EQ(adaptation.restrictions(), restrictions_after);
+
+ // Clean up.
+ adapter_.RemoveAdaptationListener(&adaptation_listener);
+}
+
// Death tests.
// Disabled on Android because death tests misbehave on Android, see
// base/test/gtest_util.h.
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 84b5aa3..255405e 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -348,7 +348,6 @@
degradation_preference_manager_(
std::make_unique<DegradationPreferenceManager>()),
adaptation_constraints_(),
- adaptation_listeners_(),
stream_resource_manager_(&input_state_provider_,
encoder_stats_observer,
clock_,
@@ -387,15 +386,14 @@
// Add the stream resource manager's resources to the processor.
adaptation_constraints_ = stream_resource_manager_.AdaptationConstraints();
- adaptation_listeners_ = stream_resource_manager_.AdaptationListeners();
for (auto& resource : stream_resource_manager_.MappedResources()) {
resource_adaptation_processor_->AddResource(resource);
}
for (auto* constraint : adaptation_constraints_) {
resource_adaptation_processor_->AddAdaptationConstraint(constraint);
}
- for (auto* listener : adaptation_listeners_) {
- resource_adaptation_processor_->AddAdaptationListener(listener);
+ for (auto* listener : stream_resource_manager_.AdaptationListeners()) {
+ video_stream_adapter_->AddAdaptationListener(listener);
}
initialize_processor_event.Set();
});
@@ -423,8 +421,8 @@
for (auto* constraint : adaptation_constraints_) {
resource_adaptation_processor_->RemoveAdaptationConstraint(constraint);
}
- for (auto* listener : adaptation_listeners_) {
- resource_adaptation_processor_->RemoveAdaptationListener(listener);
+ for (auto* listener : stream_resource_manager_.AdaptationListeners()) {
+ video_stream_adapter_->RemoveAdaptationListener(listener);
}
video_stream_adapter_->RemoveRestrictionsListener(this);
video_stream_adapter_->RemoveRestrictionsListener(
@@ -2153,23 +2151,6 @@
event.Wait(rtc::Event::kForever);
}
-void VideoStreamEncoder::InjectAdaptationListener(
- AdaptationListener* adaptation_listener) {
- rtc::Event event;
- resource_adaptation_queue_.PostTask([this, adaptation_listener, &event] {
- RTC_DCHECK_RUN_ON(&resource_adaptation_queue_);
- if (!resource_adaptation_processor_) {
- // The VideoStreamEncoder was stopped and the processor destroyed before
- // this task had a chance to execute. No action needed.
- return;
- }
- adaptation_listeners_.push_back(adaptation_listener);
- resource_adaptation_processor_->AddAdaptationListener(adaptation_listener);
- event.Set();
- });
- event.Wait(rtc::Event::kForever);
-}
-
rtc::scoped_refptr<QualityScalerResource>
VideoStreamEncoder::quality_scaler_resource_for_testing() {
RTC_DCHECK_RUN_ON(&encoder_queue_);
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index 5af0111..95d4dcb 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -129,7 +129,6 @@
void InjectAdaptationResource(rtc::scoped_refptr<Resource> resource,
VideoAdaptationReason reason);
void InjectAdaptationConstraint(AdaptationConstraint* adaptation_constraint);
- void InjectAdaptationListener(AdaptationListener* adaptation_listener);
rtc::scoped_refptr<QualityScalerResource>
quality_scaler_resource_for_testing();
@@ -423,8 +422,6 @@
std::unique_ptr<DegradationPreferenceManager> degradation_preference_manager_;
std::vector<AdaptationConstraint*> adaptation_constraints_
RTC_GUARDED_BY(&resource_adaptation_queue_);
- std::vector<AdaptationListener*> adaptation_listeners_
- RTC_GUARDED_BY(&resource_adaptation_queue_);
// Handles input, output and stats reporting related to VideoStreamEncoder
// specific resources, such as "encode usage percent" measurements and "QP
// scaling". Also involved with various mitigations such as inital frame
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 6bdcbd0..6a11bf6 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -322,13 +322,11 @@
task_queue_factory),
fake_cpu_resource_(FakeResource::Create("FakeResource[CPU]")),
fake_quality_resource_(FakeResource::Create("FakeResource[QP]")),
- fake_adaptation_constraint_("FakeAdaptationConstraint"),
- fake_adaptation_listener_() {
+ fake_adaptation_constraint_("FakeAdaptationConstraint") {
InjectAdaptationResource(fake_quality_resource_,
VideoAdaptationReason::kQuality);
InjectAdaptationResource(fake_cpu_resource_, VideoAdaptationReason::kCpu);
InjectAdaptationConstraint(&fake_adaptation_constraint_);
- InjectAdaptationListener(&fake_adaptation_listener_);
}
void SetSourceAndWaitForRestrictionsUpdated(
@@ -435,7 +433,6 @@
rtc::scoped_refptr<FakeResource> fake_cpu_resource_;
rtc::scoped_refptr<FakeResource> fake_quality_resource_;
FakeAdaptationConstraint fake_adaptation_constraint_;
- FakeAdaptationListener fake_adaptation_listener_;
};
class VideoStreamFactory
