From 4f7531e3681f02344a71c120e06e2e4129214cac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= <hbos@webrtc.org>
Date: Thu, 28 May 2020 10:08:57 +0200
Subject: [PATCH] [Adaptation] Add cooldown mechanism to prevent spammy
 kUnderuse from QP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This CL adds a 1 second cooldown period for QualityScalerResource to
signal kUnderuse due to being disabled.

If underuse is signaled every frame, any RTC_LOGging performed by the
ResourceAdaptationProcessor would become very spammy.

Plus we don't need to adapt every single frame.

Bug: webrtc:11616
Change-Id: Id76e5ca39a5e5dac9b71fdab79fb4f3dd5aeab1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176228
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31374}
---
 video/adaptation/quality_scaler_resource.cc | 25 ++++++++++++++++-----
 video/adaptation/quality_scaler_resource.h  |  6 +++++
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/video/adaptation/quality_scaler_resource.cc b/video/adaptation/quality_scaler_resource.cc
index 403f6080ca..c88bfa208f 100644
--- a/video/adaptation/quality_scaler_resource.cc
+++ b/video/adaptation/quality_scaler_resource.cc
@@ -13,12 +13,20 @@
 #include <utility>
 
 #include "rtc_base/experiments/balanced_degradation_settings.h"
+#include "rtc_base/time_utils.h"
 
 namespace webrtc {
 
+namespace {
+
+const int64_t kUnderuseDueToDisabledCooldownMs = 1000;
+
+}  // namespace
+
 QualityScalerResource::QualityScalerResource()
     : rtc::RefCountedObject<Resource>(),
       quality_scaler_(nullptr),
+      last_underuse_due_to_disabled_timestamp_ms_(absl::nullopt),
       num_handled_callbacks_(0),
       pending_callbacks_(),
       adaptation_processor_(nullptr),
@@ -82,11 +90,18 @@ void QualityScalerResource::OnEncodeCompleted(const EncodedImage& encoded_image,
     // mid call.
     // Instead it should be done at a higher layer in the same way for all
     // resources.
-    resource_adaptation_queue()->PostTask(
-        [this_ref = rtc::scoped_refptr<QualityScalerResource>(this)] {
-          RTC_DCHECK_RUN_ON(this_ref->resource_adaptation_queue());
-          this_ref->OnResourceUsageStateMeasured(ResourceUsageState::kUnderuse);
-        });
+    int64_t timestamp_ms = rtc::TimeMillis();
+    if (!last_underuse_due_to_disabled_timestamp_ms_.has_value() ||
+        timestamp_ms - last_underuse_due_to_disabled_timestamp_ms_.value() >=
+            kUnderuseDueToDisabledCooldownMs) {
+      last_underuse_due_to_disabled_timestamp_ms_ = timestamp_ms;
+      resource_adaptation_queue()->PostTask(
+          [this_ref = rtc::scoped_refptr<QualityScalerResource>(this)] {
+            RTC_DCHECK_RUN_ON(this_ref->resource_adaptation_queue());
+            this_ref->OnResourceUsageStateMeasured(
+                ResourceUsageState::kUnderuse);
+          });
+    }
   }
 }
 
diff --git a/video/adaptation/quality_scaler_resource.h b/video/adaptation/quality_scaler_resource.h
index 78685823c3..2632f2e124 100644
--- a/video/adaptation/quality_scaler_resource.h
+++ b/video/adaptation/quality_scaler_resource.h
@@ -15,6 +15,7 @@
 #include <queue>
 #include <string>
 
+#include "absl/types/optional.h"
 #include "api/video/video_adaptation_reason.h"
 #include "api/video_codecs/video_encoder.h"
 #include "call/adaptation/resource.h"
@@ -74,6 +75,11 @@ class QualityScalerResource : public rtc::RefCountedObject<Resource>,
   // Members accessed on the encoder queue.
   std::unique_ptr<QualityScaler> quality_scaler_
       RTC_GUARDED_BY(encoder_queue());
+  // The timestamp of the last time we reported underuse because this resource
+  // was disabled in order to prevent getting stuck with QP adaptations. Used to
+  // make sure underuse reporting is not too spammy.
+  absl::optional<int64_t> last_underuse_due_to_disabled_timestamp_ms_
+      RTC_GUARDED_BY(encoder_queue());
   // Every OnReportQpUsageHigh/Low() operation has a callback that MUST be
   // invoked on the |encoder_queue_|. Because usage measurements are reported on
   // the |encoder_queue_| but handled by the processor on the the