Report video and network stats averaged over 1s, and create corresponding UMA metrics.

remoting/client/chromoting_stats.h

 // 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.
 
 // ChromotingStats defines a bundle of performance counters and statistics
 // for chromoting.
 
 #ifndef REMOTING_CLIENT_CHROMOTING_STATS_H_
 #define REMOTING_CLIENT_CHROMOTING_STATS_H_
 
 #include "remoting/base/rate_counter.h"
 #include "remoting/base/running_average.h"
 
 namespace remoting {
 
 class ChromotingStats {
  public:
   ChromotingStats();
   virtual ~ChromotingStats();
+  // Constant used to calculate the average for rate metrics and used by the
+  // plugin for the frequency at which stats should be updated.
+  static const int kStatsUpdateFrequencyInSeconds = 1;
 
+  // The video and packet rate metrics below are updated per video packet
+  // received and then, for reporting, averaged over a 1s time-window.
+  // Bytes/sec for non-empty video-packets.

[Wez, 2015/07/14 20:24:16]

nit: Bytes per second

[anandc, 2015/07/15 03:59:54]

Done.

   RateCounter* video_bandwidth() { return &video_bandwidth_; }
+  // frames/sec for non-empty video-packets.

[Wez, 2015/07/14 20:24:15]

nit: Frames per second

[anandc, 2015/07/15 03:59:54]

Done.

   RateCounter* video_frame_rate() { return &video_frame_rate_; }
+  // packets/sec, including empty video-packets.

[Wez, 2015/07/14 20:24:16]

nit: Video packets per second

You might also clarify that this is >= frames per second, since frames are each
contained in a single packet.

[anandc, 2015/07/15 03:59:54]

Done.

+  RateCounter* video_packet_rate() { return &video_packet_rate_; }
+
+  // The latency metrics below are recorded per video packet received and, for
+  // reporting, averaged over the N most recent samples.
+  // N is defined by kLatencySampleSize.

[Wez, 2015/07/14 20:24:15]

Why not fix these to average over a fixed time, e.g. 1s, rather than a fixed
number of samples?

[anandc, 2015/07/15 03:59:54]

Good suggestion. Here are 2 ways to do that: 
change these to be rate counters, in which case the RateCounter class should be
updated to include a way to return an average for the number of samples recorded
over its specific time-window.
Or, keep this is a running-average, with a large enough sample size, say 60 or
75, so that we deal with cases where there is a burst of packets.

Option #1 sounds better.
Would you be OK with doing that in the follow-up CL planned for the refactoring,
https://code.google.com/p/chromium/issues/detail?id=508602?

[Wez, 2015/07/16 19:03:35]

Doing it in the follow-up SGTM.

I'd prefer that we just update the RunningAverage class to average over a time
window rather than over a fixed number of samples.

Note that the RunningAverage class could be made to average over the entire
period between samples, rather than averaging over a fixed window, so that each
call to sample it is implicitly looking at the time period since the preceding
sample.

[anandc, 2015/07/16 20:18:47]

Yes, that would work. Thanks.

   RunningAverage* video_capture_ms() { return &video_capture_ms_; }
   RunningAverage* video_encode_ms() { return &video_encode_ms_; }
   RunningAverage* video_decode_ms() { return &video_decode_ms_; }
   RunningAverage* video_paint_ms() { return &video_paint_ms_; }
   RunningAverage* round_trip_ms() { return &round_trip_ms_; }
 
  private:
   RateCounter video_bandwidth_;
   RateCounter video_frame_rate_;
+  RateCounter video_packet_rate_;
   RunningAverage video_capture_ms_;
   RunningAverage video_encode_ms_;
   RunningAverage video_decode_ms_;
   RunningAverage video_paint_ms_;
   RunningAverage round_trip_ms_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromotingStats);
 };
 
 }  // namespace remoting
 
 #endif  // REMOTING_CLIENT_CHROMOTING_STATS_H_