Chromoting to report roundtrip latency

remoting/client/chromoting_client.cc

 // Copyright (c) 2010 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/client/chromoting_client.h"
 
 #include "base/message_loop.h"
 #include "remoting/base/tracer.h"
 #include "remoting/client/chromoting_view.h"
 #include "remoting/client/client_context.h"
@@ skipping 119 matching lines @@
   }
 
   // Record size of the packet for statistics.
   stats_.video_bandwidth()->Record(packet->data().size());
 
   // Record statistics received from host.
   if (packet->has_capture_time_ms())
     stats_.video_capture_ms()->Record(packet->capture_time_ms());
   if (packet->has_encode_time_ms())
     stats_.video_encode_ms()->Record(packet->encode_time_ms());
+  if (packet->has_client_sequence_number() &&
+      packet->client_sequence_number()) {

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

Checking for presence of client_sequence_number is for backwards compatibility
with older Hosts.
Checking for non-zero value is because zero means that the Host has already sent
us a VideoPacket corresponding to the most recent input event.
This isn't clear without looking at the ScreenRecorder implementation, though.

It would be better to have the client keep a record of the most-recent sequence
number it received a VideoPacket for, and only log the round-trip if it receives
one with a higher sequence number, I think.

If you'd prefer to keep the arrangement where the Host zeroes its internal idea
of the client_sequence_number then better to have that propagate to the client
as an absence of a client_sequence_number field at all, and to clarify in this
the proto that zero is not a valid sequence number for a client to send.

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

Sounds good to me. The reason I didn't want to not put client_sequence_number is
because I needed to add two places in two different encoder. Your scheme seems
better.

+    base::TimeDelta round_trip_latency =
+        base::Time::Now() -
+        base::Time::FromInternalValue(packet->client_sequence_number());
+    stats_.round_trip_ms()->Record(round_trip_latency.InMilliseconds());
+  }
 
   received_packets_.push_back(QueuedVideoPacket(packet, done));
   if (!packet_being_processed_)
     DispatchPacket();
 }
 
 int ChromotingClient::GetPendingPackets() {
   return received_packets_.size();
 }
 
@@ skipping 140 matching lines @@
   if (msg->success()) {
     connection_->OnClientAuthenticated();
   }
 
   view_->UpdateLoginStatus(msg->success(), msg->error_info());
   done->Run();
   delete done;
 }
 
 }  // namespace remoting