Gather history of capture and encode time determine next recoring delay

remoting/host/screen_recorder.cc

 // 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/screen_recorder.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop_proxy.h"
 #include "base/stl_util.h"
+#include "base/sys_info.h"
 #include "base/task.h"
 #include "base/time.h"
 #include "remoting/base/capture_data.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/video.pb.h"
 #include "remoting/protocol/client_stub.h"
 #include "remoting/protocol/connection_to_client.h"
 #include "remoting/protocol/message_decoder.h"
 #include "remoting/protocol/util.h"
 
 using remoting::protocol::ConnectionToClient;
 
 namespace remoting {
 
-// By default we capture 20 times a second. This number is obtained by
-// experiment to provide good latency.
-static const double kDefaultCaptureRate = 20.0;
-
 // Maximum number of frames that can be processed similtaneously.
-// TODO(sergeyu): Should this be set to 1? Or should we change
-// dynamically depending on how fast network and CPU are? Experement
-// with it.
 static const int kMaxRecordings = 2;

[Sergey Ulanov, 2011/10/20 00:17:00]

Can we move this to rate controller class? E.g. you can pass |recordings_| to
NextRecordingDelay(), and it would return -1 if |recordings_| is to high and we
need to wait for one next frame to be processed.

[Alpha Left Google, 2011/10/31 18:47:24]

I think it'll be better to do it separately. It's not a requirement to this
improvement.

[Sergey Ulanov, 2011/10/31 20:17:18]

add TODO for this?

 
 ScreenRecorder::ScreenRecorder(
     MessageLoop* capture_loop,
     MessageLoop* encode_loop,
     base::MessageLoopProxy* network_loop,
     Capturer* capturer,
     Encoder* encoder)
     : capture_loop_(capture_loop),
       encode_loop_(encode_loop),
       network_loop_(network_loop),
       capturer_(capturer),
       encoder_(encoder),
       is_recording_(false),
       network_stopped_(false),
       encoder_stopped_(false),
+      max_recordings_(kMaxRecordings),
       recordings_(0),
       frame_skipped_(false),
-      max_rate_(kDefaultCaptureRate),
       sequence_number_(0) {
   DCHECK(capture_loop_);
   DCHECK(encode_loop_);
   DCHECK(network_loop_);
+
+  // If the system has only one processor there's no use to have simutaneous

[Wez, 2011/10/20 00:52:15]

typo: "simutaneous"

[Alpha Left Google, 2011/10/31 18:47:24]

Done.

[Alpha Left Google, 2011/10/31 18:47:24]

Done.

+  // recording sessions, it will just create excessive system load.

[Wez, 2011/10/20 00:52:15]

The rate-limiting should result in this, surely?

[Alpha Left Google, 2011/10/31 18:47:24]

Actually max_recordings_ should be 1 here. My bad here.

+  if (base::SysInfo::NumberOfProcessors() < 2)
+    max_recordings_ = 2;

[Sergey Ulanov, 2011/10/20 00:17:00]

Don't get it. kMaxRecording = 2, so max_recordings_ is set to 2 ether way.

[Alpha Left Google, 2011/10/31 18:47:24]

I set it to 1, mistake here.

 }
 
 ScreenRecorder::~ScreenRecorder() {
 }
 
 // Public methods --------------------------------------------------------------
 
 void ScreenRecorder::Start() {
   capture_loop_->PostTask(
       FROM_HERE, NewRunnableMethod(this, &ScreenRecorder::DoStart));
 }
 
 void ScreenRecorder::Stop(const base::Closure& done_task) {
   if (MessageLoop::current() != capture_loop_) {
     capture_loop_->PostTask(FROM_HERE, base::Bind(
         &ScreenRecorder::Stop, this, done_task));
     return;
   }
 
   DCHECK(!done_task.is_null());
 
   capture_timer_.Stop();
   is_recording_ = false;
 
   network_loop_->PostTask(FROM_HERE, base::Bind(
       &ScreenRecorder::DoStopOnNetworkThread, this, done_task));
 }
 
-void ScreenRecorder::SetMaxRate(double rate) {
-  capture_loop_->PostTask(
-      FROM_HERE, NewRunnableMethod(this, &ScreenRecorder::DoSetMaxRate, rate));
-}
-
 void ScreenRecorder::AddConnection(
     scoped_refptr<ConnectionToClient> connection) {
   capture_loop_->PostTask(
       FROM_HERE,
       NewRunnableMethod(this, &ScreenRecorder::DoInvalidateFullScreen));
 
   // Add the client to the list so it can receive update stream.
   network_loop_->PostTask(
       FROM_HERE,
       NewRunnableMethod(this, &ScreenRecorder::DoAddConnection, connection));
@@ skipping 43 matching lines @@
 
 void ScreenRecorder::DoStart() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
   if (is_recording_) {
     NOTREACHED() << "Record session already started.";
     return;
   }
 
   is_recording_ = true;
   StartCaptureTimer();

[Sergey Ulanov, 2011/10/20 00:17:00]

DoCapture() call below starts the timer, so I think we can remove this now.

[Wez, 2011/10/20 00:52:15]

StartCaptureTimer() doesn't seem to be required at all?

[Alpha Left Google, 2011/10/31 18:47:24]

Done.

 
   // Capture first frame immedately.
   DoCapture();
 }
 
-void ScreenRecorder::DoSetMaxRate(double max_rate) {
-  DCHECK_EQ(capture_loop_, MessageLoop::current());
-
-  // TODO(hclam): Should also check for small epsilon.
-  DCHECK_GT(max_rate, 0.0) << "Rate is too small.";
-
-  max_rate_ = max_rate;
-
-  // Restart the timer with the new rate.
-  if (is_recording_) {
-    capture_timer_.Stop();
-    StartCaptureTimer();
-  }
-}
-
 void ScreenRecorder::StartCaptureTimer() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
-  base::TimeDelta interval = base::TimeDelta::FromMilliseconds(
-      static_cast<int>(base::Time::kMillisecondsPerSecond / max_rate_));
-  capture_timer_.Start(FROM_HERE, interval, this, &ScreenRecorder::DoCapture);
+  capture_timer_.Start(FROM_HERE,
+                       regulator_.NextRecordingDelay(),
+                       this,
+                       &ScreenRecorder::DoCapture);
 }
 
 void ScreenRecorder::DoCapture() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
   // Make sure we have at most two oustanding recordings. We can simply return
   // if we can't make a capture now, the next capture will be started by the
   // end of an encode operation.
-  if (recordings_ >= kMaxRecordings || !is_recording_) {
+  if (recordings_ >= max_recordings_ || !is_recording_) {
     frame_skipped_ = true;
     return;
   }
 
-  if (frame_skipped_) {
+  if (frame_skipped_)
     frame_skipped_ = false;
-    capture_timer_.Reset();
-  }
 
   // At this point we are going to perform one capture so save the current time.
   ++recordings_;
-  DCHECK_LE(recordings_, kMaxRecordings);
+  DCHECK_LE(recordings_, max_recordings_);
+
+  // Before doing a capture schedule for the next one.
+  capture_timer_.Stop();

[Sergey Ulanov, 2011/10/20 00:17:00]

Do we need this? DoCapture() is supposed to be called by the timer, so the timer
should not be running at this point because it is OneShotTimer.

[Alpha Left Google, 2011/10/31 18:47:24]

We shouldn't hit this case but there's no harm doing this.

+  capture_timer_.Start(FROM_HERE,
+                       regulator_.NextRecordingDelay(),
+                       this,
+                       &ScreenRecorder::DoCapture);
 
   // And finally perform one capture.
   capture_start_time_ = base::Time::Now();
   capturer()->CaptureInvalidRegion(
       NewCallback(this, &ScreenRecorder::CaptureDoneCallback));
 }
 
 void ScreenRecorder::CaptureDoneCallback(
     scoped_refptr<CaptureData> capture_data) {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
   if (!is_recording_)
     return;
 
   if (capture_data) {
     int capture_time = static_cast<int>(
         (base::Time::Now() - capture_start_time_).InMilliseconds());
     capture_data->set_capture_time_ms(capture_time);
+    regulator_.RecordCaptureTime(capture_time);
 
     // The best way to get this value is by binding the sequence number to
     // the callback when calling CaptureInvalidRects(). However the callback
     // system doesn't allow this. Reading from the member variable is
     // accurate as long as capture is synchronous as the following statement
     // will obtain the most recent sequence number received.
     capture_data->set_client_sequence_number(sequence_number_);
   }
 
   encode_loop_->PostTask(
@@ skipping 147 matching lines @@
   DCHECK_EQ(encode_loop_, MessageLoop::current());
 
   if (encoder_stopped_)
     return;
 
   bool last = (packet->flags() & VideoPacket::LAST_PACKET) != 0;
   if (last) {
     int encode_time = static_cast<int>(
         (base::Time::Now() - encode_start_time_).InMilliseconds());
     packet->set_encode_time_ms(encode_time);
+    regulator_.RecordEncodeTime(encode_time);
   }
 
   network_loop_->PostTask(
       FROM_HERE,
       NewRunnableMethod(this, &ScreenRecorder::DoSendVideoPacket, packet));
 }
 
 }  // namespace remoting