Refactor OveruseFrameDetector to use timing in us units

Refactor OveruseFrameDetector to use timing in us units

Use rtc::TimeMicros, and don't refer to ntp time.

BUG=webrtc:6977
Review-Url: https://codereview.webrtc.org/2633673002
Cr-Commit-Position: refs/heads/master@{#16138}
Committed: https://chromium.googlesource.com/external/webrtc/+/e0e3bdfbbfedda7a8547a13106da17734366a9e7

[nisse-webrtc, 2017-01-13 13:38:40]

nisse@webrtc.org changed reviewers:
+ asapersson@webrtc.org

[nisse-webrtc, 2017-01-13 13:38:41]

Hi,

I'm trying to eliminate use of the ntp_time_ms attribute on VideoFrame. The
overuse detector is one of the users.

Not entirely sure how it was used though, if I read the code correctly, the ntp
time attached to the frame was used only for the call to
encoder_timing_->OnEncodeTiming. I don't know if that is broken when I change to
time base on rtc::TimeMicros, with a different epoch.

I also took out the dependency on the system_wrappers Clock class, instead using
rtc:TimeMicros everywhere, and ScopedFakeClock in the unit tests.

[åsapersson, 2017-01-16 15:07:53]

lgtm

[nisse-webrtc, 2017-01-16 15:13:15]

The CQ bit was checked by nisse@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-16 15:13:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-16 18:21:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-16 18:21:38]

Dry run: Try jobs failed on following builders:
  linux_msan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/16734)

[nisse-webrtc, 2017-01-17 08:31:04]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2017-01-17 08:31:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-17 10:34:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 10:34:21]

Try jobs failed on following builders:
  linux_msan on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/16760)

[nisse-webrtc, 2017-01-17 15:02:24]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2017-01-17 15:02:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-17 16:27:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-17 16:27:29]

Failed to apply patch for webrtc/video/overuse_frame_detector.cc:
While running git apply --index -p1;
  error: patch failed: webrtc/video/overuse_frame_detector.cc:22
  error: webrtc/video/overuse_frame_detector.cc: patch does not apply

Patch:       webrtc/video/overuse_frame_detector.cc
Index: webrtc/video/overuse_frame_detector.cc
diff --git a/webrtc/video/overuse_frame_detector.cc
b/webrtc/video/overuse_frame_detector.cc
index
ad81fcc08546038e08c298cc8c38a28e9333b34d..8b7580d5a2aa7b5a2709daceb12d2515a6850885
100644
--- a/webrtc/video/overuse_frame_detector.cc
+++ b/webrtc/video/overuse_frame_detector.cc
@@ -22,7 +22,6 @@
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/common_video/include/frame_callback.h"
-#include "webrtc/system_wrappers/include/clock.h"
 
 #if defined(WEBRTC_MAC) && !defined(WEBRTC_IOS)
 #include <mach/mach.h>
@@ -203,7 +202,6 @@ class OveruseFrameDetector::CheckOveruseTask : public
rtc::QueuedTask {
 };
 
 OveruseFrameDetector::OveruseFrameDetector(
-    Clock* clock,
     const CpuOveruseOptions& options,
     ScalingObserverInterface* observer,
     EncodedFrameObserver* encoder_timing,
@@ -213,10 +211,10 @@ OveruseFrameDetector::OveruseFrameDetector(
       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),
+      // TODO(nisse): Use rtc::Optional
+      last_capture_time_us_(-1),
+      last_processed_capture_time_us_(-1),
       num_pixels_(0),
       last_overuse_time_ms_(-1),
       checks_above_threshold_(0),
@@ -260,11 +258,12 @@ bool OveruseFrameDetector::FrameSizeChanged(int
num_pixels) const {
   return false;
 }
 
-bool OveruseFrameDetector::FrameTimeoutDetected(int64_t now) const {
+bool OveruseFrameDetector::FrameTimeoutDetected(int64_t now_us) const {
   RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
-  if (last_capture_time_ms_ == -1)
+  if (last_capture_time_us_ == -1)
     return false;
-  return (now - last_capture_time_ms_) > options_.frame_timeout_interval_ms;
+  return (now_us - last_capture_time_us_) >
+      options_.frame_timeout_interval_ms * rtc::kNumMicrosecsPerMillisec;
 }
 
 void OveruseFrameDetector::ResetAll(int num_pixels) {
@@ -272,32 +271,33 @@ void OveruseFrameDetector::ResetAll(int num_pixels) {
   num_pixels_ = num_pixels;
   usage_->Reset();
   frame_timing_.clear();
-  last_capture_time_ms_ = -1;
-  last_processed_capture_time_ms_ = -1;
+  last_capture_time_us_ = -1;
+  last_processed_capture_time_us_ = -1;
   num_process_times_ = 0;
   metrics_ = rtc::Optional<CpuOveruseMetrics>();
 }
 
 void OveruseFrameDetector::FrameCaptured(const VideoFrame& frame,
-                                         int64_t time_when_first_seen_ms) {
+                                         int64_t time_when_first_seen_us) {
   RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_);
 
   if (FrameSizeChanged(frame.width() * frame.height()) ||
-      FrameTimeoutDetected(time_when_first_seen_ms)) {
+      FrameTimeoutDetected(time_when_first_seen_us)) {
     ResetAll(frame.width() * frame.height());
   }
 
-  if (last_capture_time_ms_ != -1)
-    usage_->AddCaptureSample(time_when_first_seen_ms - last_capture_time_ms_);
+  if (last_capture_time_us_ != -1)
+    usage_->AddCaptureSample(
+        1e-3 * (time_when_first_seen_us - last_capture_time_us_));
 
-  last_capture_time_ms_ = time_when_first_seen_ms;
+  last_capture_time_us_ = time_when_first_seen_us;
 
-  frame_timing_.push_back(FrameTiming(frame.ntp_time_ms(), frame.timestamp(),
-                                      time_when_first_seen_ms));
+  frame_timing_.push_back(FrameTiming(frame.timestamp_us(), frame.timestamp(),
+                                      time_when_first_seen_us));
 }
 
 void OveruseFrameDetector::FrameSent(uint32_t timestamp,
-                                     int64_t time_sent_in_ms) {
+                                     int64_t time_sent_in_us) {
   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
@@ -307,7 +307,7 @@ void OveruseFrameDetector::FrameSent(uint32_t timestamp,
   static const int64_t kEncodingTimeMeasureWindowMs = 1000;
   for (auto& it : frame_timing_) {
     if (it.timestamp == timestamp) {
-      it.last_send_ms = time_sent_in_ms;
+      it.last_send_us = time_sent_in_us;
       break;
     }
   }
@@ -319,21 +319,26 @@ void OveruseFrameDetector::FrameSent(uint32_t timestamp,
   // https://crbug.com/350106
   while (!frame_timing_.empty()) {
     FrameTiming timing = frame_timing_.front();
-    if (time_sent_in_ms - timing.capture_ms < kEncodingTimeMeasureWindowMs)
+    if (time_sent_in_us - timing.capture_us <
+        kEncodingTimeMeasureWindowMs * rtc::kNumMicrosecsPerMillisec)
       break;
-    if (timing.last_send_ms != -1) {
-      int encode_duration_ms =
-          static_cast<int>(timing.last_send_ms - timing.capture_ms);
+    if (timing.last_send_us != -1) {
+      int encode_duration_us =
+          static_cast<int>(timing.last_send_us - timing.capture_us);
       if (encoder_timing_) {
-        encoder_timing_->OnEncodeTiming(timing.capture_ntp_ms,
-                                        encode_duration_ms);
+        // TODO(nisse): Update encoder_timing_ to also use us units.
+        encoder_timing_->OnEncodeTiming(timing.capture_time_us /
+                                        rtc::kNumMicrosecsPerMillisec,
+                                        encode_duration_us /
+                                        rtc::kNumMicrosecsPerMillisec);
       }
-      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);
+      if (last_processed_capture_time_us_ != -1) {
+        int64_t diff_us = timing.capture_us - last_processed_capture_time_us_;
+        usage_->AddSample(1e-3 * encode_duration_us, 1e-3 * diff_us);
       }
-      last_processed_capture_time_ms_ = timing.capture_ms;
-      EncodedFrameTimeMeasured(encode_duration_ms);
+      last_processed_capture_time_us_ = timing.capture_us;
+      EncodedFrameTimeMeasured(encode_duration_us /
+                               rtc::kNumMicrosecsPerMillisec);
     }
     frame_timing_.pop_front();
   }
@@ -345,7 +350,7 @@ void OveruseFrameDetector::CheckForOveruse() {
   if (num_process_times_ <= options_.min_process_count || !metrics_)
     return;
 
-  int64_t now = clock_->TimeInMilliseconds();
+  int64_t now_ms = rtc::TimeMillis();
 
   if (IsOverusing(*metrics_)) {
     // If the last thing we did was going up, and now have to back down, we
need
@@ -353,7 +358,7 @@ void OveruseFrameDetector::CheckForOveruse() {
     // 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 ||
+      if (now_ms - last_rampup_time_ms_ < kStandardRampUpDelayMs ||
           num_overuse_detections_ > kMaxOverusesBeforeApplyRampupDelay) {
         // Going up was not ok for very long, back off.
         current_rampup_delay_ms_ *= kRampUpBackoffFactor;
@@ -365,15 +370,15 @@ void OveruseFrameDetector::CheckForOveruse() {
       }
     }
 
-    last_overuse_time_ms_ = now;
+    last_overuse_time_ms_ = now_ms;
     in_quick_rampup_ = false;
     checks_above_threshold_ = 0;
     ++num_overuse_detections_;
 
     if (observer_)
       observer_->ScaleDown(kScaleReasonCpu);
-  } else if (IsUnderusing(*metrics_, now)) {
-    last_rampup_time_ms_ = now;
+  } else if (IsUnderusing(*metrics_, now_ms)) {
+    last_rampup_time_ms_ = now_ms;
     in_quick_rampup_ = true;
 
     if (observer_)

[nisse-webrtc, 2017-01-18 09:09:14]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-01-18 09:09:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
asapersson@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2633673002/#ps20001 (title:
"Rebased.")

[commit-bot: I haz the power, 2017-01-18 09:09:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-01-18 10:16:21]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484730554586230,
"parent_rev": "d3fabe51b201761b3959d612eab4577f0c2a88e1", "commit_rev":
"e0e3bdfbbfedda7a8547a13106da17734366a9e7"}

[commit-bot: I haz the power, 2017-01-18 10:16:23]

Description was changed from

==========
Refactor OveruseFrameDetector to use timing in us units

Use rtc::TimeMicros, and don't refer to ntp time.

BUG=webrtc:6977
==========

to

==========
Refactor OveruseFrameDetector to use timing in us units

Use rtc::TimeMicros, and don't refer to ntp time.

BUG=webrtc:6977

Review-Url: https://codereview.webrtc.org/2633673002
Cr-Commit-Position: refs/heads/master@{#16138}
Committed:
https://chromium.googlesource.com/external/webrtc/+/e0e3bdfbbfedda7a8547a1310...
==========

[commit-bot: I haz the power, 2017-01-18 10:16:24]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/e0e3bdfbbfedda7a8547a1310...