Add method for incrementing RtpPacketCounter. Removes duplicate code.

Correction to check if rtx is enabled on send-side (and not receive) when updating rtx send bitrate stat.

Remove unneeded guarded by annotations.

BUG=
R=mflodman@webrtc.org, stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/41729004

Cr-Commit-Position: refs/heads/master@{#8239}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8239 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/common_types.h

@@ -241,73 +241,6 @@ struct RtcpPacketTypeCounter {
   uint32_t unique_nack_requests;  // Number of unique NACKed RTP packets.
 };
 
-struct RtpPacketCounter {
-  RtpPacketCounter()
-    : header_bytes(0),
-      payload_bytes(0),
-      padding_bytes(0),
-      packets(0) {}
-
-  void Add(const RtpPacketCounter& other) {
-    header_bytes += other.header_bytes;
-    payload_bytes += other.payload_bytes;
-    padding_bytes += other.padding_bytes;
-    packets += other.packets;
-  }
-
-  size_t TotalBytes() const {
-    return header_bytes + payload_bytes + padding_bytes;
-  }
-
-  size_t header_bytes;   // Number of bytes used by RTP headers.
-  size_t payload_bytes;  // Payload bytes, excluding RTP headers and padding.
-  size_t padding_bytes;  // Number of padding bytes.
-  uint32_t packets;      // Number of packets.
-};
-
-// Data usage statistics for a (rtp) stream.
-struct StreamDataCounters {
-  StreamDataCounters() : first_packet_time_ms(-1) {}
-
-  void Add(const StreamDataCounters& other) {
-    transmitted.Add(other.transmitted);
-    retransmitted.Add(other.retransmitted);
-    fec.Add(other.fec);
-    if (other.first_packet_time_ms != -1 &&
-       (other.first_packet_time_ms < first_packet_time_ms ||
-        first_packet_time_ms == -1)) {
-      // Use oldest time.
-      first_packet_time_ms = other.first_packet_time_ms;
-    }
-  }
-
-  int64_t TimeSinceFirstPacketInMs(int64_t now_ms) const {
-    return (first_packet_time_ms == -1) ? -1 : (now_ms - first_packet_time_ms);
-  }
-
-  // Returns the number of bytes corresponding to the actual media payload (i.e.
-  // RTP headers, padding, retransmissions and fec packets are excluded).
-  // Note this function does not have meaning for an RTX stream.
-  size_t MediaPayloadBytes() const {
-    return transmitted.payload_bytes - retransmitted.payload_bytes -
-           fec.payload_bytes;
-  }
-
-  int64_t first_packet_time_ms;  // Time when first packet is sent/received.
-  RtpPacketCounter transmitted;  // Number of transmitted packets/bytes.
-  RtpPacketCounter retransmitted;  // Number of retransmitted packets/bytes.
-  RtpPacketCounter fec;  // Number of redundancy packets/bytes.
-};
-
-// Callback, called whenever byte/packet counts have been updated.
-class StreamDataCountersCallback {
- public:
-  virtual ~StreamDataCountersCallback() {}
-
-  virtual void DataCountersUpdated(const StreamDataCounters& counters,
-                                   uint32_t ssrc) = 0;
-};
-
 // Rate statistics for a stream.
 struct BitrateStatistics {
   BitrateStatistics() : bitrate_bps(0), packet_rate(0), timestamp_ms(0) {}
@@ -890,6 +823,80 @@ struct RTPHeader {
   RTPHeaderExtension extension;
 };
 
+struct RtpPacketCounter {
+  RtpPacketCounter()
+    : header_bytes(0),
+      payload_bytes(0),
+      padding_bytes(0),
+      packets(0) {}
+
+  void Add(const RtpPacketCounter& other) {
+    header_bytes += other.header_bytes;
+    payload_bytes += other.payload_bytes;
+    padding_bytes += other.padding_bytes;
+    packets += other.packets;
+  }
+
+  void AddPacket(size_t packet_length, const RTPHeader& header) {
+    ++packets;
+    header_bytes += header.headerLength;
+    padding_bytes += header.paddingLength;
+    payload_bytes +=
+        packet_length - (header.headerLength + header.paddingLength);
+  }
+
+  size_t TotalBytes() const {
+    return header_bytes + payload_bytes + padding_bytes;
+  }
+
+  size_t header_bytes;   // Number of bytes used by RTP headers.
+  size_t payload_bytes;  // Payload bytes, excluding RTP headers and padding.
+  size_t padding_bytes;  // Number of padding bytes.
+  uint32_t packets;      // Number of packets.
+};
+
+// Data usage statistics for a (rtp) stream.
+struct StreamDataCounters {
+  StreamDataCounters() : first_packet_time_ms(-1) {}
+
+  void Add(const StreamDataCounters& other) {
+    transmitted.Add(other.transmitted);
+    retransmitted.Add(other.retransmitted);
+    fec.Add(other.fec);
+    if (other.first_packet_time_ms != -1 &&
+       (other.first_packet_time_ms < first_packet_time_ms ||
+        first_packet_time_ms == -1)) {
+      // Use oldest time.
+      first_packet_time_ms = other.first_packet_time_ms;
+    }
+  }
+
+  int64_t TimeSinceFirstPacketInMs(int64_t now_ms) const {
+    return (first_packet_time_ms == -1) ? -1 : (now_ms - first_packet_time_ms);
+  }
+
+  // Returns the number of bytes corresponding to the actual media payload (i.e.
+  // RTP headers, padding, retransmissions and fec packets are excluded).
+  // Note this function does not have meaning for an RTX stream.
+  size_t MediaPayloadBytes() const {
+    return transmitted.payload_bytes - retransmitted.payload_bytes -
+           fec.payload_bytes;
+  }
+
+  int64_t first_packet_time_ms;  // Time when first packet is sent/received.
+  RtpPacketCounter transmitted;  // Number of transmitted packets/bytes.
+  RtpPacketCounter retransmitted;  // Number of retransmitted packets/bytes.
+  RtpPacketCounter fec;  // Number of redundancy packets/bytes.
+};
+
+// Callback, called whenever byte/packet counts have been updated.
+class StreamDataCountersCallback {
+ public:
+  virtual ~StreamDataCountersCallback() {}
+
+  virtual void DataCountersUpdated(const StreamDataCounters& counters,
+                                   uint32_t ssrc) = 0;
+};
 }  // namespace webrtc
 
 #endif  // WEBRTC_COMMON_TYPES_H_

webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc

@@ -81,17 +81,9 @@ void StreamStatisticianImpl::UpdateCounters(const RTPHeader& header,
   bool in_order = InOrderPacketInternal(header.sequenceNumber);
   ssrc_ = header.ssrc;
   incoming_bitrate_.Update(packet_length);
-  receive_counters_.transmitted.payload_bytes +=
+  receive_counters_.transmitted.AddPacket(packet_length, header);
-      packet_length - (header.paddingLength + header.headerLength);
-  receive_counters_.transmitted.header_bytes += header.headerLength;
-  receive_counters_.transmitted.padding_bytes += header.paddingLength;
-  ++receive_counters_.transmitted.packets;
   if (!in_order && retransmitted) {
-    ++receive_counters_.retransmitted.packets;
+    receive_counters_.retransmitted.AddPacket(packet_length, header);
-    receive_counters_.retransmitted.payload_bytes +=
-        packet_length - (header.paddingLength + header.headerLength);
-    receive_counters_.retransmitted.header_bytes += header.headerLength;
-    receive_counters_.retransmitted.padding_bytes += header.paddingLength;
   }
 
   if (receive_counters_.transmitted.packets == 1) {
@@ -204,11 +196,7 @@ void StreamStatisticianImpl::FecPacketReceived(const RTPHeader& header,
                                                size_t packet_length) {
   {
     CriticalSectionScoped cs(stream_lock_.get());
-    ++receive_counters_.fec.packets;
+    receive_counters_.fec.AddPacket(packet_length, header);
-    receive_counters_.fec.payload_bytes +=
-        packet_length - (header.headerLength + header.paddingLength);
-    receive_counters_.fec.header_bytes += header.headerLength;
-    receive_counters_.fec.padding_bytes += header.paddingLength;
   }
   NotifyRtpCallback();
 }

webrtc/modules/rtp_rtcp/source/receive_statistics_unittest.cc

@@ -257,29 +257,22 @@ class RtpTestCallback : public StreamDataCountersCallback {
     ++num_calls_;
   }
 
+  void MatchPacketCounter(const RtpPacketCounter& expected,
+                          const RtpPacketCounter& actual) {
+    EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
+    EXPECT_EQ(expected.header_bytes, actual.header_bytes);
+    EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
+    EXPECT_EQ(expected.packets, actual.packets);
+  }
+
   void Matches(uint32_t num_calls,
                uint32_t ssrc,
                const StreamDataCounters& expected) {
     EXPECT_EQ(num_calls, num_calls_);
     EXPECT_EQ(ssrc, ssrc_);
-    EXPECT_EQ(expected.transmitted.payload_bytes,
+    MatchPacketCounter(expected.transmitted, stats_.transmitted);
-              stats_.transmitted.payload_bytes);
+    MatchPacketCounter(expected.retransmitted, stats_.retransmitted);
-    EXPECT_EQ(expected.transmitted.header_bytes,
+    MatchPacketCounter(expected.fec, stats_.fec);
-              stats_.transmitted.header_bytes);
-    EXPECT_EQ(expected.transmitted.padding_bytes,
-              stats_.transmitted.padding_bytes);
-    EXPECT_EQ(expected.transmitted.packets, stats_.transmitted.packets);
-    EXPECT_EQ(expected.retransmitted.payload_bytes,
-              stats_.retransmitted.payload_bytes);
-    EXPECT_EQ(expected.retransmitted.header_bytes,
-              stats_.retransmitted.header_bytes);
-    EXPECT_EQ(expected.retransmitted.padding_bytes,
-              stats_.retransmitted.padding_bytes);
-    EXPECT_EQ(expected.retransmitted.packets, stats_.retransmitted.packets);
-    EXPECT_EQ(expected.fec.payload_bytes, stats_.fec.payload_bytes);
-    EXPECT_EQ(expected.fec.header_bytes, stats_.fec.header_bytes);
-    EXPECT_EQ(expected.fec.padding_bytes, stats_.fec.padding_bytes);
-    EXPECT_EQ(expected.fec.packets, stats_.fec.packets);
   }
 
   uint32_t num_calls_;

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

@@ -894,29 +894,17 @@ void RTPSender::UpdateRtpStats(const uint8_t* buffer,
   }
 
   total_bitrate_sent_.Update(packet_length);
-  ++counters->transmitted.packets;
+
   if (counters->first_packet_time_ms == -1) {
     counters->first_packet_time_ms = clock_->TimeInMilliseconds();
   }
   if (IsFecPacket(buffer, header)) {
-    ++counters->fec.packets;
+    counters->fec.AddPacket(packet_length, header);
-    counters->fec.payload_bytes +=
-        packet_length - (header.headerLength + header.paddingLength);
-    counters->fec.header_bytes += header.headerLength;
-    counters->fec.padding_bytes += header.paddingLength;
   }
-
   if (is_retransmit) {
-    ++counters->retransmitted.packets;
+    counters->retransmitted.AddPacket(packet_length, header);
-    counters->retransmitted.payload_bytes +=
-        packet_length - (header.headerLength + header.paddingLength);
-    counters->retransmitted.header_bytes += header.headerLength;
-    counters->retransmitted.padding_bytes += header.paddingLength;
   }
-  counters->transmitted.payload_bytes +=
+  counters->transmitted.AddPacket(packet_length, header);
-      packet_length - (header.headerLength + header.paddingLength);
-  counters->transmitted.header_bytes += header.headerLength;
-  counters->transmitted.padding_bytes += header.paddingLength;
 
   if (rtp_stats_callback_) {
     rtp_stats_callback_->DataCountersUpdated(*counters, ssrc);

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -914,24 +914,19 @@ TEST_F(RtpSenderTest, StreamDataCountersCallbacks) {
 
     uint32_t ssrc_;
     StreamDataCounters counters_;
+
+    void MatchPacketCounter(const RtpPacketCounter& expected,
+                            const RtpPacketCounter& actual) {
+      EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
+      EXPECT_EQ(expected.header_bytes, actual.header_bytes);
+      EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
+      EXPECT_EQ(expected.packets, actual.packets);
+    }
+
     void Matches(uint32_t ssrc, const StreamDataCounters& counters) {
       EXPECT_EQ(ssrc, ssrc_);
-      EXPECT_EQ(counters.transmitted.payload_bytes,
+      MatchPacketCounter(counters.transmitted, counters_.transmitted);
-                counters_.transmitted.payload_bytes);
+      MatchPacketCounter(counters.retransmitted, counters_.retransmitted);
-      EXPECT_EQ(counters.transmitted.header_bytes,
-                counters_.transmitted.header_bytes);
-      EXPECT_EQ(counters.transmitted.padding_bytes,
-                counters_.transmitted.padding_bytes);
-      EXPECT_EQ(counters.transmitted.packets,
-                counters_.transmitted.packets);
-      EXPECT_EQ(counters.retransmitted.payload_bytes,
-                counters_.retransmitted.payload_bytes);
-      EXPECT_EQ(counters.retransmitted.header_bytes,
-                counters_.retransmitted.header_bytes);
-      EXPECT_EQ(counters.retransmitted.padding_bytes,
-                counters_.retransmitted.padding_bytes);
-      EXPECT_EQ(counters.retransmitted.packets,
-                counters_.retransmitted.packets);
       EXPECT_EQ(counters.fec.packets, counters_.fec.packets);
     }
 

webrtc/video_engine/overuse_frame_detector.h

@@ -144,11 +144,13 @@ class OveruseFrameDetector : public Module {
   // Number of pixels of last captured frame.
   int num_pixels_ GUARDED_BY(crit_);
 
-  int64_t last_encode_sample_ms_ GUARDED_BY(crit_);
+  int64_t last_encode_sample_ms_;  // Only accessed by one thread.
+
   scoped_ptr<EncodeTimeAvg> encode_time_ GUARDED_BY(crit_);
   scoped_ptr<SendProcessingUsage> usage_ GUARDED_BY(crit_);
   scoped_ptr<FrameQueue> frame_queue_ GUARDED_BY(crit_);
-  int64_t last_sample_time_ms_ GUARDED_BY(crit_);
+
+  int64_t last_sample_time_ms_;  // Only accessed by one thread.
 
   scoped_ptr<CaptureQueueDelay> capture_queue_delay_ GUARDED_BY(crit_);
 

webrtc/video_engine/vie_channel.cc

@@ -289,8 +289,7 @@ void ViEChannel::UpdateHistogramsAtStopSend() {
       rtp_rtx.transmitted.padding_bytes * 8 / elapsed_sec / 1000);
   RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.RetransmittedBitrateSentInKbps",
       rtp_rtx.retransmitted.TotalBytes() * 8 / elapsed_sec / 1000);
-  uint32_t ssrc = 0;
+  if (rtp_rtcp_->RtxSendStatus() != kRtxOff) {
-  if (vie_receiver_.GetRtxSsrc(&ssrc)) {
     RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.RtxBitrateSentInKbps",
         rtx.transmitted.TotalBytes() * 8 / elapsed_sec / 1000);
   }