Chromoting to report roundtrip latency

remoting/host/screen_recorder.cc

Index: remoting/host/screen_recorder.cc
diff --git a/remoting/host/screen_recorder.cc b/remoting/host/screen_recorder.cc
index 8faaf3b07ff4752ebc5dca6b90fb70ab26ed562d..4147d98e0f1512aa79fe8952336dc1550adbfb93 100644
--- a/remoting/host/screen_recorder.cc
+++ b/remoting/host/screen_recorder.cc
@@ -49,7 +49,8 @@ ScreenRecorder::ScreenRecorder(
       network_stopped_(false),
       recordings_(0),
       frame_skipped_(false),
-      max_rate_(kDefaultCaptureRate) {
+      max_rate_(kDefaultCaptureRate),
+      sequence_number_(0) {
   DCHECK(capture_loop_);
   DCHECK(encode_loop_);
   DCHECK(network_loop_);
@@ -102,6 +103,18 @@ void ScreenRecorder::RemoveAllConnections() {
       NewTracedMethod(this, &ScreenRecorder::DoRemoveAllClients));
 }
 
+void ScreenRecorder::UpdateSequenceNumber(int64 sequence_number) {
+  if (MessageLoop::current() != capture_loop_) {

[simonmorris, 2011/04/05 10:47:06]

Maybe a comment to explain why this should be run on the
capture loop? (If this is because of the "capture is
synchronous" comment below).

[Alpha Left Google, 2011/04/08 00:07:20]

Done.

+    capture_loop_->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(this, &ScreenRecorder::UpdateSequenceNumber,
+                          sequence_number));
+    return;
+  }
+
+  sequence_number_ = sequence_number;
+}
+
 // Private accessors -----------------------------------------------------------
 
 Capturer* ScreenRecorder::capturer() {
@@ -228,6 +241,15 @@ void ScreenRecorder::CaptureDoneCallback(
   int capture_time =
       (base::Time::Now() - capture_start_time_).InMilliseconds();
   capture_data->set_capture_time_ms(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

[simonmorris, 2011/04/05 10:47:06]

Maybe "as long as sequence_number_ is set in the capture loop
thread", to tie into UpdateSequenceNumber() above?

[Alpha Left Google, 2011/04/08 00:07:20]

Well that's not true.. capture being synchronous is important for this to be
accurate.

[simonmorris, 2011/04/08 08:18:25]

OK.

+  // will obtain the last sequence number received.

[Wez, 2011/04/05 13:46:20]

"last" -> "most recent"

[Alpha Left Google, 2011/04/08 00:07:20]

Done.

+  capture_data->set_client_sequence_number(sequence_number_);
+  sequence_number_ = 0;
+
   encode_loop_->PostTask(
       FROM_HERE,
       NewTracedMethod(this, &ScreenRecorder::DoEncode, capture_data));