Change OverUseFrameDetector to use a task queue

webrtc/video/overuse_frame_detector.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 13 matching lines @@
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/video_frame.h"
 
 #if defined(WEBRTC_MAC) && !defined(WEBRTC_IOS)
 #include <mach/mach.h>
 #endif  // defined(WEBRTC_MAC) && !defined(WEBRTC_IOS)
 
 namespace webrtc {
 
 namespace {
-const int64_t kProcessIntervalMs = 5000;
+const int64_t ktCheckForOveruseIntervalMs = 5000;

[åsapersson, 2016/08/24 08:59:04]

kCheck..?

[perkj_webrtc, 2016/09/01 10:03:29]

Done.

+const int64_t kTimeToFirstCheckForOveruseMs = 100;
 
 // Delay between consecutive rampups. (Used for quick recovery.)
 const int kQuickRampUpDelayMs = 10 * 1000;
 // Delay between rampup attempts. Initially uses standard, scales up to max.
 const int kStandardRampUpDelayMs = 40 * 1000;
 const int kMaxRampUpDelayMs = 240 * 1000;
 // Expontential back-off factor, to prevent annoying up-down behaviour.
 const double kRampUpBackoffFactor = 2.0;
 
 // Max number of overuses detected before always applying the rampup delay.
@@ skipping 118 matching lines @@
   const float kWeightFactorFrameDiff;
   const float kWeightFactorProcessing;
   const float kInitialSampleDiffMs;
   const float kMaxSampleDiffMs;
   uint64_t count_;
   const CpuOveruseOptions options_;
   std::unique_ptr<rtc::ExpFilter> filtered_processing_ms_;
   std::unique_ptr<rtc::ExpFilter> filtered_frame_diff_ms_;
 };
 
+class OveruseFrameDetector::CheckOveruseTask : public rtc::QueuedTask {
+ public:
+  explicit CheckOveruseTask(OveruseFrameDetector* overuse_detector)
+      : overuse_detector_(overuse_detector) {
+    rtc::TaskQueue::Current()->PostDelayedTask(
+        std::unique_ptr<rtc::QueuedTask>(this), kTimeToFirstCheckForOveruseMs);
+  }
+
+  void Stop() {
+    RTC_CHECK(task_checker_.CalledSequentially());
+    overuse_detector_ = nullptr;
+  }
+
+ private:
+  bool Run() override {
+    RTC_CHECK(task_checker_.CalledSequentially());
+    if (!overuse_detector_)
+      return true;  // This will make the task queue delete this task.
+    overuse_detector_->CheckForOveruse();
+
+    rtc::TaskQueue::Current()->PostDelayedTask(
+        std::unique_ptr<rtc::QueuedTask>(this), ktCheckForOveruseIntervalMs);
+    // Return false to prevent this task from being deleted. Ownership has been
+    // transferred to the task queue when PostDelayedTask was called.
+    return false;
+  }
+  rtc::SequencedTaskChecker task_checker_;
+  OveruseFrameDetector* overuse_detector_;
+};
+
 OveruseFrameDetector::OveruseFrameDetector(
     Clock* clock,
     const CpuOveruseOptions& options,
     CpuOveruseObserver* observer,
     EncodedFrameObserver* encoder_timing,
     CpuOveruseMetricsObserver* metrics_observer)
-    : options_(options),
+    : check_over_use_task_(nullptr),
+      options_(options),
       observer_(observer),
       encoder_timing_(encoder_timing),
       metrics_observer_(metrics_observer),
       clock_(clock),
       num_process_times_(0),
       last_capture_time_ms_(-1),
       last_processed_capture_time_ms_(-1),
       num_pixels_(0),
-      next_process_time_ms_(clock_->TimeInMilliseconds()),
       last_overuse_time_ms_(-1),
       checks_above_threshold_(0),
       num_overuse_detections_(0),
       last_rampup_time_ms_(-1),
       in_quick_rampup_(false),
       current_rampup_delay_ms_(kStandardRampUpDelayMs),
       usage_(new SendProcessingUsage(options)) {
-  processing_thread_.DetachFromThread();
+  task_checker_.Detach();
 }
 
 OveruseFrameDetector::~OveruseFrameDetector() {
+  RTC_DCHECK(!check_over_use_task_) << "StopCheckForOverUse must be called.";

[åsapersson, 2016/08/24 08:59:04]

nit: OverUse->Overuse

[perkj_webrtc, 2016/09/01 10:03:29]

Done.

+}
+
+void OveruseFrameDetector::StartCheckForOveruse() {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
+  RTC_DCHECK(!check_over_use_task_);
+  check_over_use_task_ = new CheckOveruseTask(this);
+}
+void OveruseFrameDetector::StopCheckForOveruse() {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
+  check_over_use_task_->Stop();
+  check_over_use_task_ = nullptr;
 }
 
 void OveruseFrameDetector::EncodedFrameTimeMeasured(int encode_duration_ms) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   if (!metrics_)
     metrics_ = rtc::Optional<CpuOveruseMetrics>(CpuOveruseMetrics());
   metrics_->encode_usage_percent = usage_->Value();
 
   metrics_observer_->OnEncodedFrameTimeMeasured(encode_duration_ms, *metrics_);
 }
 
-int64_t OveruseFrameDetector::TimeUntilNextProcess() {
-  RTC_DCHECK(processing_thread_.CalledOnValidThread());
-  return next_process_time_ms_ - clock_->TimeInMilliseconds();
-}
-
 bool OveruseFrameDetector::FrameSizeChanged(int num_pixels) const {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);

[åsapersson, 2016/08/24 08:59:04]

Is the check needed for private methods?

[perkj_webrtc, 2016/09/01 10:03:29]

Yes- , if you use GUARDED_BY(..) this is needed on all methods that access that
variable.

   if (num_pixels != num_pixels_) {
     return true;
   }
   return false;
 }
 
 bool OveruseFrameDetector::FrameTimeoutDetected(int64_t now) const {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   if (last_capture_time_ms_ == -1)
     return false;
   return (now - last_capture_time_ms_) > options_.frame_timeout_interval_ms;
 }
 
 void OveruseFrameDetector::ResetAll(int num_pixels) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   num_pixels_ = num_pixels;
   usage_->Reset();
   frame_timing_.clear();
   last_capture_time_ms_ = -1;
   last_processed_capture_time_ms_ = -1;
   num_process_times_ = 0;
   metrics_ = rtc::Optional<CpuOveruseMetrics>();
 }
 
-void OveruseFrameDetector::FrameCaptured(const VideoFrame& frame) {
-  rtc::CritScope cs(&crit_);
+void OveruseFrameDetector::FrameCaptured(const VideoFrame& frame,
+                                         int64_t time_when_first_seen) {

[åsapersson, 2016/08/24 08:59:04]

time_when_first_seen_ms

[perkj_webrtc, 2016/09/01 10:03:29]

Done.

+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
 
-  int64_t now = clock_->TimeInMilliseconds();
   if (FrameSizeChanged(frame.width() * frame.height()) ||
-      FrameTimeoutDetected(now)) {
+      FrameTimeoutDetected(time_when_first_seen)) {
     ResetAll(frame.width() * frame.height());
   }
 
   if (last_capture_time_ms_ != -1)
-    usage_->AddCaptureSample(now - last_capture_time_ms_);
+    usage_->AddCaptureSample(time_when_first_seen - last_capture_time_ms_);
 
-  last_capture_time_ms_ = now;
+  last_capture_time_ms_ = time_when_first_seen;
 
-  frame_timing_.push_back(
-      FrameTiming(frame.ntp_time_ms(), frame.timestamp(), now));
+  frame_timing_.push_back(FrameTiming(frame.ntp_time_ms(), frame.timestamp(),
+                                      time_when_first_seen));
 }
 
-void OveruseFrameDetector::FrameSent(uint32_t timestamp) {
-  rtc::CritScope cs(&crit_);
+void OveruseFrameDetector::FrameSent(uint32_t timestamp,
+                                     int64_t time_sent_in_ms) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   // Delay before reporting actual encoding time, used to have the ability to
   // detect total encoding time when encoding more than one layer. Encoding is
   // here assumed to finish within a second (or that we get enough long-time
   // samples before one second to trigger an overuse even when this is not the
   // case).
   static const int64_t kEncodingTimeMeasureWindowMs = 1000;
-  int64_t now = clock_->TimeInMilliseconds();
   for (auto& it : frame_timing_) {
     if (it.timestamp == timestamp) {
-      it.last_send_ms = now;
+      it.last_send_ms = time_sent_in_ms;
       break;
     }
   }
   // TODO(pbos): Handle the case/log errors when not finding the corresponding
   // frame (either very slow encoding or incorrect wrong timestamps returned
   // from the encoder).
   // This is currently the case for all frames on ChromeOS, so logging them
   // would be spammy, and triggering overuse would be wrong.
   // https://crbug.com/350106
   while (!frame_timing_.empty()) {
     FrameTiming timing = frame_timing_.front();
-    if (now - timing.capture_ms < kEncodingTimeMeasureWindowMs)
+    if (time_sent_in_ms - timing.capture_ms < kEncodingTimeMeasureWindowMs)
       break;
     if (timing.last_send_ms != -1) {
       int encode_duration_ms =
           static_cast<int>(timing.last_send_ms - timing.capture_ms);
       if (encoder_timing_) {
         encoder_timing_->OnEncodeTiming(timing.capture_ntp_ms,
                                         encode_duration_ms);
       }
       if (last_processed_capture_time_ms_ != -1) {
         int64_t diff_ms = timing.capture_ms - last_processed_capture_time_ms_;
         usage_->AddSample(encode_duration_ms, diff_ms);
       }
       last_processed_capture_time_ms_ = timing.capture_ms;
       EncodedFrameTimeMeasured(encode_duration_ms);
     }
     frame_timing_.pop_front();
   }
 }
 
-void OveruseFrameDetector::Process() {
-  RTC_DCHECK(processing_thread_.CalledOnValidThread());
+void OveruseFrameDetector::CheckForOveruse() {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
+  ++num_process_times_;
+  if (num_process_times_ <= options_.min_process_count || !metrics_)
+    return;
 
   int64_t now = clock_->TimeInMilliseconds();
-
-  // Used to protect against Process() being called too often.
-  if (now < next_process_time_ms_)
-    return;
-
-  next_process_time_ms_ = now + kProcessIntervalMs;
-
-  CpuOveruseMetrics current_metrics;
-  {
-    rtc::CritScope cs(&crit_);
-    ++num_process_times_;
-    if (num_process_times_ <= options_.min_process_count || !metrics_)
-      return;
-
-    current_metrics = *metrics_;
-  }
+  CpuOveruseMetrics current_metrics = *metrics_;

[åsapersson, 2016/08/24 08:59:04]

remove current_metrics and use metrics_ instead?

[perkj_webrtc, 2016/09/01 10:03:29]

Done.

 
   if (IsOverusing(current_metrics)) {
     // If the last thing we did was going up, and now have to back down, we need
     // to check if this peak was short. If so we should back off to avoid going
     // back and forth between this load, the system doesn't seem to handle it.
     bool check_for_backoff = last_rampup_time_ms_ > last_overuse_time_ms_;
     if (check_for_backoff) {
       if (now - last_rampup_time_ms_ < kStandardRampUpDelayMs ||
           num_overuse_detections_ > kMaxOverusesBeforeApplyRampupDelay) {
         // Going up was not ok for very long, back off.
@@ skipping 24 matching lines @@
   int rampup_delay =
       in_quick_rampup_ ? kQuickRampUpDelayMs : current_rampup_delay_ms_;
 
   LOG(LS_VERBOSE) << " Frame stats: "
                   << " encode usage " << current_metrics.encode_usage_percent
                   << " overuse detections " << num_overuse_detections_
                   << " rampup delay " << rampup_delay;
 }
 
 bool OveruseFrameDetector::IsOverusing(const CpuOveruseMetrics& metrics) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   if (metrics.encode_usage_percent >=
       options_.high_encode_usage_threshold_percent) {
     ++checks_above_threshold_;
   } else {
     checks_above_threshold_ = 0;
   }
   return checks_above_threshold_ >= options_.high_threshold_consecutive_count;
 }
 
 bool OveruseFrameDetector::IsUnderusing(const CpuOveruseMetrics& metrics,
                                         int64_t time_now) {
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
   int delay = in_quick_rampup_ ? kQuickRampUpDelayMs : current_rampup_delay_ms_;
   if (time_now < last_rampup_time_ms_ + delay)
     return false;
 
   return metrics.encode_usage_percent <
          options_.low_encode_usage_threshold_percent;
 }
 }  // namespace webrtc