From 4414939954fd908b6490ce1bb88271e161219aa3 Mon Sep 17 00:00:00 2001 From: "asapersson@webrtc.org" Date: Wed, 4 Feb 2015 08:34:47 +0000 Subject: [PATCH] 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 | 141 +++++++++--------- .../source/receive_statistics_impl.cc | 18 +-- .../source/receive_statistics_unittest.cc | 29 ++-- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 20 +-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 27 ++-- webrtc/video_engine/overuse_frame_detector.h | 6 +- webrtc/video_engine/vie_channel.cc | 3 +- 7 files changed, 108 insertions(+), 136 deletions(-) diff --git a/webrtc/common_types.h b/webrtc/common_types.h index f347ec3dae..17f50fa672 100644 --- a/webrtc/common_types.h +++ b/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_ diff --git a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc b/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc index 916ef54de2..4a7da029ca 100644 --- a/webrtc/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/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 += - packet_length - (header.paddingLength + header.headerLength); - receive_counters_.transmitted.header_bytes += header.headerLength; - receive_counters_.transmitted.padding_bytes += header.paddingLength; - ++receive_counters_.transmitted.packets; + receive_counters_.transmitted.AddPacket(packet_length, header); if (!in_order && retransmitted) { - ++receive_counters_.retransmitted.packets; - 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; + receive_counters_.retransmitted.AddPacket(packet_length, header); } 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.payload_bytes += - packet_length - (header.headerLength + header.paddingLength); - receive_counters_.fec.header_bytes += header.headerLength; - receive_counters_.fec.padding_bytes += header.paddingLength; + receive_counters_.fec.AddPacket(packet_length, header); } NotifyRtpCallback(); } diff --git a/webrtc/modules/rtp_rtcp/source/receive_statistics_unittest.cc b/webrtc/modules/rtp_rtcp/source/receive_statistics_unittest.cc index 7f3f564b7f..ccba6083e5 100644 --- a/webrtc/modules/rtp_rtcp/source/receive_statistics_unittest.cc +++ b/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, - stats_.transmitted.payload_bytes); - EXPECT_EQ(expected.transmitted.header_bytes, - 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); + MatchPacketCounter(expected.transmitted, stats_.transmitted); + MatchPacketCounter(expected.retransmitted, stats_.retransmitted); + MatchPacketCounter(expected.fec, stats_.fec); } uint32_t num_calls_; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 71077c56b5..f325984008 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/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.payload_bytes += - packet_length - (header.headerLength + header.paddingLength); - counters->fec.header_bytes += header.headerLength; - counters->fec.padding_bytes += header.paddingLength; + counters->fec.AddPacket(packet_length, header); } - if (is_retransmit) { - ++counters->retransmitted.packets; - counters->retransmitted.payload_bytes += - packet_length - (header.headerLength + header.paddingLength); - counters->retransmitted.header_bytes += header.headerLength; - counters->retransmitted.padding_bytes += header.paddingLength; + counters->retransmitted.AddPacket(packet_length, header); } - counters->transmitted.payload_bytes += - packet_length - (header.headerLength + header.paddingLength); - counters->transmitted.header_bytes += header.headerLength; - counters->transmitted.padding_bytes += header.paddingLength; + counters->transmitted.AddPacket(packet_length, header); if (rtp_stats_callback_) { rtp_stats_callback_->DataCountersUpdated(*counters, ssrc); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index f7a9184ff6..cd4b747cd3 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/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, - counters_.transmitted.payload_bytes); - 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); + MatchPacketCounter(counters.transmitted, counters_.transmitted); + MatchPacketCounter(counters.retransmitted, counters_.retransmitted); EXPECT_EQ(counters.fec.packets, counters_.fec.packets); } diff --git a/webrtc/video_engine/overuse_frame_detector.h b/webrtc/video_engine/overuse_frame_detector.h index 2c1cd1395e..8da22bde0d 100644 --- a/webrtc/video_engine/overuse_frame_detector.h +++ b/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 encode_time_ GUARDED_BY(crit_); scoped_ptr usage_ GUARDED_BY(crit_); scoped_ptr 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 capture_queue_delay_ GUARDED_BY(crit_); diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 1ec8a104d2..12d8ac2156 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/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 (vie_receiver_.GetRtxSsrc(&ssrc)) { + if (rtp_rtcp_->RtxSendStatus() != kRtxOff) { RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.RtxBitrateSentInKbps", rtx.transmitted.TotalBytes() * 8 / elapsed_sec / 1000); }