Cast: Simplify code path for RTCP sender report

media/cast/cast_defines.h

 // Copyright 2013 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.
 
 #ifndef MEDIA_CAST_CAST_DEFINES_H_
 #define MEDIA_CAST_CAST_DEFINES_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 158 matching lines @@
   return base::TimeTicks::UnixEpoch() + elapsed_since_unix_epoch;
 }
 
 inline uint32 GetVideoRtpTimestamp(const base::TimeTicks& time_ticks) {
   base::TimeTicks zero_time;
   base::TimeDelta recorded_delta = time_ticks - zero_time;
   // Timestamp is in 90 KHz for video.
   return static_cast<uint32>(recorded_delta.InMilliseconds() * 90);
 }
 
-class RtpSenderStatistics {
+class RtpTimestampHelper {

[miu, 2014/05/13 01:11:56]

Please make this class Chromium-style compliant, and implement methods in a .cc
file, rather than in-line.

[Alpha Left Google, 2014/05/13 21:45:39]

Done.

  public:
-  explicit RtpSenderStatistics(int frequency)
+  explicit RtpTimestampHelper(int frequency)
       : frequency_(frequency),
-        rtp_timestamp_(0) {
-    memset(&sender_info_, 0, sizeof(sender_info_));
+        last_rtp_timestamp_(0),
+        stored_(false) {
   }
 
-  ~RtpSenderStatistics() {}
+  ~RtpTimestampHelper() {}
 
-  void UpdateInfo(const base::TimeTicks& now) {
-    // Update RTP timestamp and return last stored statistics.
-    uint32 ntp_seconds = 0;
-    uint32 ntp_fraction = 0;
-    uint32 rtp_timestamp = 0;
-    if (rtp_timestamp_ > 0) {
-      base::TimeDelta time_since_last_send = now - time_sent_;
-      rtp_timestamp = rtp_timestamp_ + time_since_last_send.InMilliseconds() *
-                                           (frequency_ / 1000);
-      // Update NTP time to current time.
-      ConvertTimeTicksToNtp(now, &ntp_seconds, &ntp_fraction);
-    }
-    // Populate sender info.
-    sender_info_.rtp_timestamp = rtp_timestamp;
-    sender_info_.ntp_seconds = ntp_seconds;
-    sender_info_.ntp_fraction = ntp_fraction;
+  // Compute a RTP timestamp using current time, last encoded time and
+  // last encoded RTP timestamp.
+  // Return true if |rtp_timestamp| is computed.
+  bool GetCurrentTimeAsRtpTimestamp(const base::TimeTicks& now,
+                                    uint32* rtp_timestamp) {

[miu, 2014/05/13 01:11:56]

Method should be const.

[Alpha Left Google, 2014/05/13 21:45:39]

Done.

+    if (!stored_)
+      return false;
+    base::TimeDelta elapsed_time = now - last_capture_time_;
+    *rtp_timestamp = last_rtp_timestamp_ + elapsed_time.InMilliseconds() *

[hubbe, 2014/05/12 19:46:04]

Is Millisecond accurate enough here?

[Alpha Left Google, 2014/05/12 19:49:33]

I tried not to change the algorithm used here so I preserve the usage of
milliseconds. I think in practice this is good enough.

+        frequency_ / base::Time::kMillisecondsPerSecond;
+    return true;
   }
 
-  transport::RtcpSenderInfo sender_info() const {
-    return sender_info_;
+  // Store the capture time and the corresponding RTP timestamp for the
+  // last encoded frame.
+  void StoreLatestTime(base::TimeTicks capture_time, uint32 rtp_timestamp) {
+    last_capture_time_ = capture_time;
+    last_rtp_timestamp_ = rtp_timestamp;
+    stored_ = true;
   }
 
-  void Store(transport::RtcpSenderInfo sender_info,
-             base::TimeTicks time_sent,
-             uint32 rtp_timestamp) {
-    sender_info_ = sender_info;
-    time_sent_ = time_sent;
-    rtp_timestamp_ = rtp_timestamp;
-}
-
  private:
   int frequency_;
-  transport::RtcpSenderInfo sender_info_;
-  base::TimeTicks time_sent_;
-  uint32 rtp_timestamp_;
+  base::TimeTicks last_capture_time_;
+  uint32 last_rtp_timestamp_;
+  bool stored_;

[miu, 2014/05/13 01:11:56]

You don't need this extra boolean member.  Just check for
last_capture_time.is_null().

[Alpha Left Google, 2014/05/13 21:45:39]

Done.

 
-  DISALLOW_COPY_AND_ASSIGN(RtpSenderStatistics);
+  DISALLOW_COPY_AND_ASSIGN(RtpTimestampHelper);
 };
 
 }  // namespace cast
 }  // namespace media
 
 #endif  // MEDIA_CAST_CAST_DEFINES_H_