Gather history of capture and encode time determine next recoring delay

remoting/host/capture_scheduler.cc

Index: remoting/host/capture_scheduler.cc
diff --git a/remoting/host/capture_scheduler.cc b/remoting/host/capture_scheduler.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1a26b8511f9c7e8904232884d8d68425eeacc0b2
--- /dev/null
+++ b/remoting/host/capture_scheduler.cc
@@ -0,0 +1,61 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "remoting/host/capture_scheduler.h"
+
+#include <algorithm>
+
+#include "base/logging.h"
+#include "base/sys_info.h"
+#include "base/time.h"
+
+namespace {
+
+// Number of samples to average the most recent capture and encode time.

[Wez, 2011/10/31 22:05:53]

nit: You need "over" at the end of the sentence. :)

+const int kStatisticsWindow = 3;
+
+// The hard limit is 20fps or 50ms per recording cycle.
+const int64 kMinimumRecordingDelay = 50;
+
+// Controls how much CPU time we can use for encode and capture.
+// Range of this value is between 0 to 1.

[Wez, 2011/10/31 22:05:53]

This comment still doesn't make it clear whether the value 1 means "use all of
one CPU" or "use all of all CPUs".  The comment should indicate what the value
_means_, not just the syntax.

+const double kRecordingCpuConsumption = 0.5;
+
+}  // namespace
+
+namespace remoting {
+
+// Number of processors is read when this class is constructed so it doesn't
+// change dynamically.

[Wez, 2011/10/31 22:05:53]

Is the comment to address my nit?  If so then please reword to something like
"We assume that the number of available cores is constant".

+CaptureScheduler::CaptureScheduler()
+    : num_of_processors_(base::SysInfo::NumberOfProcessors()),
+      capture_time_(kStatisticsWindow),
+      encode_time_(kStatisticsWindow) {
+  DCHECK(num_of_processors_);
+}
+
+CaptureScheduler::~CaptureScheduler() {
+}
+
+base::TimeDelta CaptureScheduler::NextCaptureDelay() {
+  // The algorithm only uses CPU time spent in capture and encode,
+  // and number of processors to determine the next capture interval.

[Wez, 2011/10/31 22:05:53]

Could this comment indicate the intended behaviour of the algorithm, rather than
just telling me which factors it takes into account?  e.g. Delay by an amount
chosen such that if capture and encode times continue to follow the averages,
then we'll consume the target fraction of CPU across all cores.

+  double delay =
+      (capture_time_.Average() + encode_time_.Average()) /
+      (kRecordingCpuConsumption * num_of_processors_);
+
+  if (delay < kMinimumRecordingDelay)
+    return base::TimeDelta::FromMilliseconds(kMinimumRecordingDelay);
+  return base::TimeDelta::FromMilliseconds(delay);
+}
+
+void CaptureScheduler::RecordCaptureTime(base::TimeDelta capture_time) {
+  capture_time_.Record(capture_time.InMilliseconds());
+}
+
+void CaptureScheduler::RecordEncodeTime(base::TimeDelta encode_time) {
+  encode_time_.Record(encode_time.InMilliseconds());
+}
+
+}  // namespace remoting