Simplified frame rate control in the chromoting host.

Insted of keeping semi-fixed frame rate, now capturing rate is controlled
by how fast we can send data to the client. Capturing of frame n is
started only after frame n-2 is sent (while n-1 is being encoded). This
guarantees that we don't clog the video channel buffers, and that we start
capturing only if we know that the frame will not need to wait for too long
in the buffer.

TEST=None
BUG=None

Review URL: http://codereview.chromium.org/5634002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68688 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 64 insertions, 173 deletions

diff --git a/remoting/host/screen_recorder.cc b/remoting/host/screen_recorder.cc
index a972c1a..cf3417c 100644
--- a/remoting/host/screen_recorder.cc
+++ b/remoting/host/screen_recorder.cc
@@ -10,6 +10,7 @@
 #include "base/scoped_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/task.h"
+#include "base/time.h"
 #include "remoting/base/capture_data.h"
 #include "remoting/base/tracer.h"
 #include "remoting/proto/control.pb.h"
@@ -27,17 +28,11 @@ namespace remoting {
 // experiment to provide good latency.
 static const double kDefaultCaptureRate = 20.0;
 
-// Interval that we perform rate regulation.
-static const base::TimeDelta kRateControlInterval =
-    base::TimeDelta::FromSeconds(1);
-
-// We divide the pending update stream number by this value to determine the
-// rate divider.
-static const int kSlowDownFactor = 10;
-
-// A list of dividers used to divide the max rate to determine the current
-// capture rate.
-static const int kRateDividers[] = {1, 2, 4, 8, 16};
+// 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;
 
 ScreenRecorder::ScreenRecorder(
     MessageLoop* capture_loop,
@@ -50,11 +45,10 @@ ScreenRecorder::ScreenRecorder(
       network_loop_(network_loop),
       capturer_(capturer),
       encoder_(encoder),
-      rate_(kDefaultCaptureRate),
       started_(false),
       recordings_(0),
-      max_rate_(kDefaultCaptureRate),
-      rate_control_started_(false) {
+      frame_skipped_(false),
+      max_rate_(kDefaultCaptureRate) {
   DCHECK(capture_loop_);
   DCHECK(encode_loop_);
   DCHECK(network_loop_);
@@ -83,10 +77,12 @@ void ScreenRecorder::SetMaxRate(double rate) {
 
 void ScreenRecorder::AddConnection(
     scoped_refptr<ConnectionToClient> connection) {
-  // Gets the init information for the connection.
-  capture_loop_->PostTask(
+  ScopedTracer tracer("AddConnection");
+
+  // Add the client to the list so it can receive update stream.
+  network_loop_->PostTask(
       FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoGetInitInfo, connection));
+      NewTracedMethod(this, &ScreenRecorder::DoAddConnection, connection));
 }
 
 void ScreenRecorder::RemoveConnection(
@@ -106,11 +102,13 @@ void ScreenRecorder::RemoveAllConnections() {
 
 Capturer* ScreenRecorder::capturer() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
+  DCHECK(capturer_.get());
   return capturer_.get();
 }
 
 Encoder* ScreenRecorder::encoder() {
   DCHECK_EQ(encode_loop_, MessageLoop::current());
+  DCHECK(encoder_.get());
   return encoder_.get();
 }
 
@@ -118,6 +116,7 @@ Encoder* ScreenRecorder::encoder() {
 
 void ScreenRecorder::DoStart() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
+  DCHECK(!started_);
 
   if (started_) {
     NOTREACHED() << "Record session already started.";
@@ -125,12 +124,10 @@ void ScreenRecorder::DoStart() {
   }
 
   started_ = true;
-  DoCapture();
+  StartCaptureTimer();
 
-  // Starts the rate regulation.
-  network_loop_->PostTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoStartRateControl));
+  // Capture first frame immedately.
+  DoCapture();
 }
 
 void ScreenRecorder::DoPause() {
@@ -141,56 +138,31 @@ void ScreenRecorder::DoPause() {
     return;
   }
 
+  capture_timer_.Stop();
   started_ = false;
-
-  // Pause the rate regulation.
-  network_loop_->PostTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoPauseRateControl));
-}
-
-void ScreenRecorder::DoSetRate(double rate) {
-  DCHECK_EQ(capture_loop_, MessageLoop::current());
-  if (rate == rate_)
-    return;
-
-  // Change the current capture rate.
-  rate_ = rate;
-
-  // If we have already started then schedule the next capture with the new
-  // rate.
-  if (started_)
-    ScheduleNextCapture();
 }
 
 void ScreenRecorder::DoSetMaxRate(double max_rate) {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
   // TODO(hclam): Should also check for small epsilon.
-  if (max_rate != 0) {
-    max_rate_ = max_rate;
-    DoSetRate(max_rate);
-  } else {
-    NOTREACHED() << "Rate is too small.";
+  DCHECK_GT(max_rate, 0.0) << "Rate is too small.";
+
+  max_rate_ = max_rate;
+
+  // Restart the timer with the new rate.
+  if (started_) {
+    capture_timer_.Stop();
+    StartCaptureTimer();
   }
 }
 
-void ScreenRecorder::ScheduleNextCapture() {
+void ScreenRecorder::StartCaptureTimer() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
-  ScopedTracer tracer("capture");
-
-  TraceContext::tracer()->PrintString("Capture Scheduled");
-
-  if (rate_ == 0)
-    return;
-
   base::TimeDelta interval = base::TimeDelta::FromMilliseconds(
-      static_cast<int>(base::Time::kMillisecondsPerSecond / rate_));
-  capture_loop_->PostDelayedTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoCapture),
-      interval.InMilliseconds());
+      static_cast<int>(base::Time::kMillisecondsPerSecond / max_rate_));
+  capture_timer_.Start(interval, this, &ScreenRecorder::DoCapture);
 }
 
 void ScreenRecorder::DoCapture() {
@@ -198,32 +170,22 @@ void ScreenRecorder::DoCapture() {
   // 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_ >= 2 || !started_) {
+  if (recordings_ >= kMaxRecordings || !started_) {
+    frame_skipped_ = true;
     return;
   }
-  TraceContext::tracer()->PrintString("Capture Started");
-
-  base::Time now = base::Time::Now();
-  base::TimeDelta interval = base::TimeDelta::FromMilliseconds(
-      static_cast<int>(base::Time::kMillisecondsPerSecond / rate_));
-  base::TimeDelta elapsed = now - last_capture_time_;
 
-  // If this method is called sooner than the required interval we return
-  // immediately
-  if (elapsed < interval) {
-    return;
+  if (frame_skipped_) {
+    frame_skipped_ = false;
+    capture_timer_.Reset();
   }
 
+  TraceContext::tracer()->PrintString("Capture Started");
+
   // At this point we are going to perform one capture so save the current time.
-  last_capture_time_ = now;
   ++recordings_;
 
-  // Before we actually do a capture, schedule the next one.
-  ScheduleNextCapture();
-
   // And finally perform one capture.
-  DCHECK(capturer());
-
   capturer()->CaptureInvalidRects(
       NewCallback(this, &ScreenRecorder::CaptureDoneCallback));
 }
@@ -239,7 +201,7 @@ void ScreenRecorder::CaptureDoneCallback(
       NewTracedMethod(this, &ScreenRecorder::DoEncode, capture_data));
 }
 
-void ScreenRecorder::DoFinishEncode() {
+void ScreenRecorder::DoFinishSend() {
   DCHECK_EQ(capture_loop_, MessageLoop::current());
 
   // Decrement the number of recording in process since we have completed
@@ -248,103 +210,43 @@ void ScreenRecorder::DoFinishEncode() {
 
   // Try to do a capture again. Note that the following method may do nothing
   // if it is too early to perform a capture.
-  if (rate_ > 0)
-    DoCapture();
-}
-
-void ScreenRecorder::DoGetInitInfo(
-    scoped_refptr<ConnectionToClient> connection) {
-  DCHECK_EQ(capture_loop_, MessageLoop::current());
-
-  ScopedTracer tracer("init");
-
-  // Add the client to the list so it can receive update stream.
-  network_loop_->PostTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoAddClient, connection));
+  DoCapture();
 }
 
 // Network thread --------------------------------------------------------------
 
-void ScreenRecorder::DoStartRateControl() {
-  DCHECK_EQ(network_loop_, MessageLoop::current());
-
-  if (rate_control_started_) {
-    NOTREACHED() << "Rate regulation already started";
-    return;
-  }
-  rate_control_started_ = true;
-  ScheduleNextRateControl();
-}
-
-void ScreenRecorder::DoPauseRateControl() {
+void ScreenRecorder::DoSendVideoPacket(VideoPacket* packet) {
   DCHECK_EQ(network_loop_, MessageLoop::current());
 
-  if (!rate_control_started_) {
-    NOTREACHED() << "Rate regulation not started";
-    return;
-  }
-  rate_control_started_ = false;
-}
-
-void ScreenRecorder::ScheduleNextRateControl() {
-  ScopedTracer tracer("Rate Control");
-  network_loop_->PostDelayedTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoRateControl),
-      kRateControlInterval.InMilliseconds());
-}
+  TraceContext::tracer()->PrintString("DoSendVideoPacket");
 
-void ScreenRecorder::DoRateControl() {
-  DCHECK_EQ(network_loop_, MessageLoop::current());
+  bool last = (packet->flags() & VideoPacket::LAST_PARTITION) != 0;
 
-  // If we have been paused then shutdown the rate regulation loop.
-  if (!rate_control_started_)
-    return;
+  for (ConnectionToClientList::const_iterator i = connections_.begin();
+       i < connections_.end(); ++i) {
+    Task* done_task = NULL;
 
-  int max_pending_update_streams = 0;
-  for (size_t i = 0; i < connections_.size(); ++i) {
-    max_pending_update_streams =
-        std::max(max_pending_update_streams,
-                 connections_[i]->video_stub()->GetPendingPackets());
-  }
+    // Call OnFrameSent() only for the last packet in the first connection.
+    if (last && i == connections_.begin()) {
+      done_task = NewTracedMethod(this, &ScreenRecorder::OnFrameSent, packet);
+    } else {
+      done_task = new DeleteTask<VideoPacket>(packet);
+    }
 
-  // If |slow_down| equals zero, we have no slow down.
-  size_t slow_down = max_pending_update_streams / kSlowDownFactor;
-  // Set new_rate to -1 for checking later.
-  double new_rate = -1;
-  // If the slow down is too large.
-  if (slow_down >= arraysize(kRateDividers)) {
-    // Then we stop the capture completely.
-    new_rate = 0;
-  } else {
-    // Slow down the capture rate using the divider.
-    new_rate = max_rate_ / kRateDividers[slow_down];
+    (*i)->video_stub()->ProcessVideoPacket(packet, done_task);
   }
-  DCHECK_NE(new_rate, -1.0);
 
-  // Then set the rate.
-  capture_loop_->PostTask(
-      FROM_HERE,
-      NewTracedMethod(this, &ScreenRecorder::DoSetRate, new_rate));
-  ScheduleNextRateControl();
+  TraceContext::tracer()->PrintString("DoSendVideoPacket done");
 }
 
-void ScreenRecorder::DoSendVideoPacket(VideoPacket* packet) {
-  DCHECK_EQ(network_loop_, MessageLoop::current());
-
-  TraceContext::tracer()->PrintString("DoSendUpdate");
-
-  for (ConnectionToClientList::const_iterator i = connections_.begin();
-       i < connections_.end(); ++i) {
-    (*i)->video_stub()->ProcessVideoPacket(
-        packet, new DeleteTask<VideoPacket>(packet));
-  }
-
-  TraceContext::tracer()->PrintString("DoSendUpdate done");
+void ScreenRecorder::OnFrameSent(VideoPacket* packet) {
+  delete packet;
+  capture_loop_->PostTask(
+      FROM_HERE, NewTracedMethod(this, &ScreenRecorder::DoFinishSend));
 }
 
-void ScreenRecorder::DoAddClient(scoped_refptr<ConnectionToClient> connection) {
+void ScreenRecorder::DoAddConnection(
+    scoped_refptr<ConnectionToClient> connection) {
   DCHECK_EQ(network_loop_, MessageLoop::current());
 
   // TODO(hclam): Force a full frame for next encode.
@@ -356,8 +258,8 @@ void ScreenRecorder::DoRemoveClient(
   DCHECK_EQ(network_loop_, MessageLoop::current());
 
   // TODO(hclam): Is it correct to do to a scoped_refptr?
-  ConnectionToClientList::iterator it
-      = std::find(connections_.begin(), connections_.end(), connection);
+  ConnectionToClientList::iterator it =
+      std::find(connections_.begin(), connections_.end(), connection);
   if (it != connections_.end()) {
     connections_.erase(it);
   }
@@ -380,14 +282,14 @@ void ScreenRecorder::DoEncode(
   // Early out if there's nothing to encode.
   if (!capture_data->dirty_rects().size()) {
     capture_loop_->PostTask(
-        FROM_HERE, NewTracedMethod(this, &ScreenRecorder::DoFinishEncode));
+        FROM_HERE, NewTracedMethod(this, &ScreenRecorder::DoFinishSend));
     return;
   }
 
   // TODO(hclam): Enable |force_refresh| if a new connection was
   // added.
   TraceContext::tracer()->PrintString("Encode start");
-  encoder_->Encode(capture_data, false,
+  encoder()->Encode(capture_data, false,
                    NewCallback(this, &ScreenRecorder::EncodeDataAvailableTask));
   TraceContext::tracer()->PrintString("Encode Done");
 }
@@ -395,20 +297,9 @@ void ScreenRecorder::DoEncode(
 void ScreenRecorder::EncodeDataAvailableTask(VideoPacket* packet) {
   DCHECK_EQ(encode_loop_, MessageLoop::current());
 
-  bool last = (packet->flags() & VideoPacket::LAST_PACKET) != 0;
-
-  // Before a new encode task starts, notify connected clients a new update
-  // stream is coming.
-  // Notify this will keep a reference to the DataBuffer in the
-  // task. The ownership will eventually pass to the ConnectionToClients.
   network_loop_->PostTask(
       FROM_HERE,
       NewTracedMethod(this, &ScreenRecorder::DoSendVideoPacket, packet));
-
-  if (last) {
-    capture_loop_->PostTask(
-        FROM_HERE, NewTracedMethod(this, &ScreenRecorder::DoFinishEncode));
-  }
 }
 
 }  // namespace remoting