[Adaptation] Make ResourceUsageState nullable, remove kStable.

This CL is part of the Call-Level Adaptation Processing design doc:
https://docs.google.com/document/d/1ZyC26yOCknrrcYa839ZWLxD6o6Gig5A3lVTh4E41074/edit?usp=sharing

The ResourceUsageState was written as: {kOveruse, kStable, kUnderuse}.
The assumption was that if a resource neither wanted to adapt up or
down it would report kStable. But with the addition of
Resource::IsAdaptationUpAllowed() (prior CL) the notion of being
"stable" was already captured outside of ResourceUsageState.
Furthermore, kStable failed to capture what IsAdaptationUpAllowed() did
not: whether we can go up depends on the resulting resolution or frame
rate (restrictions_after). Perhaps we can go up a little, but not a lot.

This CL also adds Resource::ClearUsageState(). After applying an
adaptation, all usage states become invalidated (new measurements are
needed to know if we are still over- or underusing). This was always
the case, but prior to this CL this was not accurately reflected in the
Resource::usage_state() in-between measurements.

Bug: webrtc:11172
Change-Id: I140ff3114025b7732e530564690783e168d2509b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173088
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31110}

call/adaptation/resource.cc

@@ -17,30 +17,25 @@ namespace webrtc {
 
 ResourceListener::~ResourceListener() {}
 
-Resource::Resource() : usage_state_(ResourceUsageState::kStable) {}
+Resource::Resource() : usage_state_(absl::nullopt), listener_(nullptr) {}
 
-Resource::~Resource() {
+Resource::~Resource() {}
-  RTC_DCHECK(listeners_.empty());
+
+void Resource::SetResourceListener(ResourceListener* listener) {
+  // If you want to change listener you need to unregister the old listener by
+  // setting it to null first.
+  RTC_DCHECK(!listener_ || !listener) << "A listener is already set";
+  listener_ = listener;
 }
 
-void Resource::RegisterListener(ResourceListener* listener) {
+absl::optional<ResourceUsageState> Resource::usage_state() const {
-  RTC_DCHECK(listener);
-  RTC_DCHECK(absl::c_find(listeners_, listener) == listeners_.end())
-      << "ResourceListener was added twice.";
-  listeners_.push_back(listener);
-}
-
-void Resource::UnregisterListener(ResourceListener* listener) {
-  RTC_DCHECK(listener);
-  auto it = absl::c_find(listeners_, listener);
-  if (it != listeners_.end())
-    listeners_.erase(it);
-}
-
-ResourceUsageState Resource::usage_state() const {
   return usage_state_;
 }
 
+void Resource::ClearUsageState() {
+  usage_state_ = absl::nullopt;
+}
+
 bool Resource::IsAdaptationUpAllowed(
     const VideoStreamInputState& input_state,
     const VideoSourceRestrictions& restrictions_before,
@@ -51,15 +46,10 @@ bool Resource::IsAdaptationUpAllowed(
 
 ResourceListenerResponse Resource::OnResourceUsageStateMeasured(
     ResourceUsageState usage_state) {
-  ResourceListenerResponse response = ResourceListenerResponse::kNothing;
   usage_state_ = usage_state;
-  for (auto* listener : listeners_) {
+  if (!listener_)
-    ResourceListenerResponse listener_response =
+    return ResourceListenerResponse::kNothing;
-        listener->OnResourceUsageStateMeasured(*this);
+  return listener_->OnResourceUsageStateMeasured(*this);
-    if (listener_response != ResourceListenerResponse::kNothing)
-      response = listener_response;
-  }
-  return response;
 }
 
 }  // namespace webrtc

call/adaptation/resource.h

@@ -25,10 +25,7 @@ class Resource;
 enum class ResourceUsageState {
   // Action is needed to minimze the load on this resource.
   kOveruse,
-  // No action needed for this resource, increasing the load on this resource
+  // Increasing the load on this resource is desired, if possible.
-  // is not allowed.
-  kStable,
-  // Increasing the load on this resource is allowed.
   kUnderuse,
 };
 
@@ -69,22 +66,17 @@ class ResourceListener {
       const Resource& resource) = 0;
 };
 
-// A Resource is something which can be measured as "overused", "stable" or
-// "underused". When the resource usage changes, listeners of the resource are
-// informed.
-//
-// Implementations of this interface are responsible for performing resource
-// usage measurements and invoking OnResourceUsageStateMeasured().
 class Resource {
  public:
-  // By default, usage_state() is kStable until a measurement is made.
+  // By default, usage_state() is null until a measurement is made.
   Resource();
   virtual ~Resource();
 
-  void RegisterListener(ResourceListener* listener);
+  void SetResourceListener(ResourceListener* listener);
-  void UnregisterListener(ResourceListener* listener);
+
+  absl::optional<ResourceUsageState> usage_state() const;
+  void ClearUsageState();
 
-  ResourceUsageState usage_state() const;
   // This method allows the Resource to reject a proposed adaptation in the "up"
   // direction if it predicts this would cause overuse of this resource. The
   // default implementation unconditionally returns true (= allowed).
@@ -104,8 +96,8 @@ class Resource {
       ResourceUsageState usage_state);
 
  private:
-  ResourceUsageState usage_state_;
+  absl::optional<ResourceUsageState> usage_state_;
-  std::vector<ResourceListener*> listeners_;
+  ResourceListener* listener_;
 };
 
 }  // namespace webrtc

call/adaptation/resource_adaptation_processor.cc

@@ -42,13 +42,13 @@ ResourceAdaptationProcessor::effective_degradation_preference() const {
 
 void ResourceAdaptationProcessor::StartResourceAdaptation() {
   for (auto* resource : resources_) {
-    resource->RegisterListener(this);
+    resource->SetResourceListener(this);
   }
 }
 
 void ResourceAdaptationProcessor::StopResourceAdaptation() {
   for (auto* resource : resources_) {
-    resource->UnregisterListener(this);
+    resource->SetResourceListener(nullptr);
   }
 }
 
@@ -106,13 +106,10 @@ void ResourceAdaptationProcessor::MaybeUpdateVideoSourceRestrictions(
 ResourceListenerResponse
 ResourceAdaptationProcessor::OnResourceUsageStateMeasured(
     const Resource& resource) {
-  switch (resource.usage_state()) {
+  RTC_DCHECK(resource.usage_state().has_value());
+  switch (resource.usage_state().value()) {
     case ResourceUsageState::kOveruse:
       return OnResourceOveruse(resource);
-    case ResourceUsageState::kStable:
-      // TODO(https://crbug.com/webrtc/11172): Delete kStable in favor of null.
-      RTC_NOTREACHED();
-      return ResourceListenerResponse::kNothing;
     case ResourceUsageState::kUnderuse:
       OnResourceUnderuse(resource);
       return ResourceListenerResponse::kNothing;
@@ -129,6 +126,13 @@ bool ResourceAdaptationProcessor::HasSufficientInputForAdaptation(
 
 void ResourceAdaptationProcessor::OnResourceUnderuse(
     const Resource& reason_resource) {
+  // Clear all usage states. In order to re-run adaptation logic, resources need
+  // to provide new resource usage measurements.
+  // TODO(hbos): Support not unconditionally clearing usage states by having the
+  // ResourceAdaptationProcessor check in on its resources at certain intervals.
+  for (Resource* resource : resources_) {
+    resource->ClearUsageState();
+  }
   VideoStreamInputState input_state = input_state_provider_->InputState();
   if (effective_degradation_preference_ == DegradationPreference::DISABLED ||
       !HasSufficientInputForAdaptation(input_state)) {
@@ -163,6 +167,13 @@ void ResourceAdaptationProcessor::OnResourceUnderuse(
 
 ResourceListenerResponse ResourceAdaptationProcessor::OnResourceOveruse(
     const Resource& reason_resource) {
+  // Clear all usage states. In order to re-run adaptation logic, resources need
+  // to provide new resource usage measurements.
+  // TODO(hbos): Support not unconditionally clearing usage states by having the
+  // ResourceAdaptationProcessor check in on its resources at certain intervals.
+  for (Resource* resource : resources_) {
+    resource->ClearUsageState();
+  }
   VideoStreamInputState input_state = input_state_provider_->InputState();
   if (!input_state.has_input()) {
     return ResourceListenerResponse::kQualityScalerShouldIncreaseFrequency;

call/adaptation/resource_unittest.cc

@@ -26,10 +26,10 @@ class MockResourceListener : public ResourceListener {
               (const Resource& resource));
 };
 
-TEST(ResourceTest, AddingListenerReceivesCallbacks) {
+TEST(ResourceTest, RegisteringListenerReceivesCallbacks) {
   StrictMock<MockResourceListener> resource_listener;
-  FakeResource fake_resource(ResourceUsageState::kStable);
+  FakeResource fake_resource("FakeResource");
-  fake_resource.RegisterListener(&resource_listener);
+  fake_resource.SetResourceListener(&resource_listener);
   EXPECT_CALL(resource_listener, OnResourceUsageStateMeasured(_))
       .Times(1)
       .WillOnce([](const Resource& resource) {
@@ -37,14 +37,14 @@ TEST(ResourceTest, AddingListenerReceivesCallbacks) {
         return ResourceListenerResponse::kNothing;
       });
   fake_resource.set_usage_state(ResourceUsageState::kOveruse);
-  fake_resource.UnregisterListener(&resource_listener);
+  fake_resource.SetResourceListener(nullptr);
 }
 
-TEST(ResourceTest, RemovingListenerStopsCallbacks) {
+TEST(ResourceTest, UnregisteringListenerStopsCallbacks) {
   StrictMock<MockResourceListener> resource_listener;
-  FakeResource fake_resource(ResourceUsageState::kStable);
+  FakeResource fake_resource("FakeResource");
-  fake_resource.RegisterListener(&resource_listener);
+  fake_resource.SetResourceListener(&resource_listener);
-  fake_resource.UnregisterListener(&resource_listener);
+  fake_resource.SetResourceListener(nullptr);
   EXPECT_CALL(resource_listener, OnResourceUsageStateMeasured(_)).Times(0);
   fake_resource.set_usage_state(ResourceUsageState::kOveruse);
 }

call/adaptation/test/fake_resource.cc

@@ -14,18 +14,13 @@
 
 namespace webrtc {
 
-FakeResource::FakeResource(ResourceUsageState usage_state)
+FakeResource::FakeResource(std::string name)
-    : FakeResource(usage_state, "FakeResource") {}
+    : Resource(), name_(std::move(name)) {}
 
 FakeResource::~FakeResource() {}
 
 void FakeResource::set_usage_state(ResourceUsageState usage_state) {
   last_response_ = OnResourceUsageStateMeasured(usage_state);
 }
-FakeResource::FakeResource(ResourceUsageState usage_state,
-                           const std::string& name)
-    : Resource(), name_(name) {
-  set_usage_state(usage_state);
-}
 
 }  // namespace webrtc

call/adaptation/test/fake_resource.h

@@ -20,8 +20,7 @@ namespace webrtc {
 // Fake resource used for testing.
 class FakeResource : public Resource {
  public:
-  explicit FakeResource(ResourceUsageState usage_state);
+  explicit FakeResource(std::string name);
-  FakeResource(ResourceUsageState usage_state, const std::string& name);
   ~FakeResource() override;
 
   void set_usage_state(ResourceUsageState usage_state);

video/video_stream_encoder_unittest.cc

@@ -159,12 +159,9 @@ class VideoStreamEncoderUnderTest : public VideoStreamEncoder {
                                overuse_detector_proxy_ =
                                    new CpuOveruseDetectorProxy(stats_proxy)),
                            task_queue_factory),
-        fake_cpu_resource_(
+        fake_cpu_resource_(std::make_unique<FakeResource>("FakeResource[CPU]")),
-            std::make_unique<FakeResource>(ResourceUsageState::kStable,
-                                           "FakeResource[CPU]")),
         fake_quality_resource_(
-            std::make_unique<FakeResource>(ResourceUsageState::kStable,
+            std::make_unique<FakeResource>("FakeResource[QP]")) {
-                                           "FakeResource[QP]")) {
     InjectAdaptationResource(fake_quality_resource_.get(),
                              VideoAdaptationReason::kQuality);
     InjectAdaptationResource(fake_cpu_resource_.get(),