aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBjorn Mellem <mellem@webrtc.org>2019-07-29 22:09:00 +0000
committerCommit Bot <commit-bot@chromium.org>2019-07-29 23:39:49 +0000
commitbcd068d0453a52f20ad15d4efb1ecd695f25ce50 (patch)
tree887df3beae6509649850ba913948f68e978327d5
parent0a88ea050cda58de81d624cf2764d46929447ed5 (diff)
downloadwebrtc-bcd068d0453a52f20ad15d4efb1ecd695f25ce50.tar.gz
Revert "Only include payload in bytes sent/received."
This reverts commit 74a1b4b1321b426392d4c32e4a02361226ad5358. Reason for revert: requested by chromium Original change's description: > Only include payload in bytes sent/received. > > According to https://www.w3.org/TR/webrtc-stats/#sentrtpstats-dict* and > https://tools.ietf.org/html/rfc3550#section-6.4.1, the bytes sent > statistic should not include headers or padding. > > Similarly, according to > https://www.w3.org/TR/webrtc-stats/#inboundrtpstats-dict*, bytes > received are calculated the same way as bytes sent (eg. not including > padding or headers). > > This change stops adding padding and headers to these statistics. > > Bug: webrtc:8516,webrtc:10525 > Change-Id: I891ad5a11a493cc3212afe93e13f62795bf4031f > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146180 > Reviewed-by: Stefan Holmer <stefan@webrtc.org> > Reviewed-by: Erik Språng <sprang@webrtc.org> > Reviewed-by: Steve Anton <steveanton@webrtc.org> > Reviewed-by: Henrik Boström <hbos@webrtc.org> > Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> > Reviewed-by: Oskar Sundbom <ossu@webrtc.org> > Commit-Queue: Bjorn Mellem <mellem@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#28647} TBR=steveanton@webrtc.org,ilnik@webrtc.org,hbos@webrtc.org,ossu@webrtc.org,sprang@webrtc.org,stefan@webrtc.org,mellem@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:8516, webrtc:10525 Change-Id: Ibd31a8264c19f0c6f57d8deb3974593d198046ab Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147400 Reviewed-by: Bjorn Mellem <mellem@webrtc.org> Commit-Queue: Bjorn Mellem <mellem@webrtc.org> Cr-Commit-Position: refs/heads/master@{#28701}
-rw-r--r--audio/channel_receive.cc6
-rw-r--r--audio/channel_send.cc7
-rw-r--r--media/engine/webrtc_video_engine.cc10
-rw-r--r--media/engine/webrtc_video_engine_unittest.cc18
4 files changed, 26 insertions, 15 deletions
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index 20aa217144..f248c99c6d 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -767,7 +767,11 @@ CallReceiveStatistics ChannelReceive::GetRTCPStatistics() const {
if (statistician) {
StreamDataCounters data_counters;
statistician->GetReceiveStreamDataCounters(&data_counters);
- stats.bytesReceived = data_counters.transmitted.payload_bytes;
+ // TODO(http://crbug.com/webrtc/10525): Bytes received should only include
+ // payload bytes, not header and padding bytes.
+ stats.bytesReceived = data_counters.transmitted.payload_bytes +
+ data_counters.transmitted.header_bytes +
+ data_counters.transmitted.padding_bytes;
stats.packetsReceived = data_counters.transmitted.packets;
stats.last_packet_received_timestamp_ms =
data_counters.last_packet_received_timestamp_ms;
diff --git a/audio/channel_send.cc b/audio/channel_send.cc
index f00e0dcd8d..8ce33a46c1 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -1078,8 +1078,13 @@ CallSendStatistics ChannelSend::GetRTCPStatistics() const {
StreamDataCounters rtp_stats;
StreamDataCounters rtx_stats;
_rtpRtcpModule->GetSendStreamDataCounters(&rtp_stats, &rtx_stats);
+ // TODO(https://crbug.com/webrtc/10525): Bytes sent should only include
+ // payload bytes, not header and padding bytes.
stats.bytesSent =
- rtp_stats.transmitted.payload_bytes + rtx_stats.transmitted.payload_bytes;
+ rtp_stats.transmitted.payload_bytes +
+ rtp_stats.transmitted.padding_bytes + rtp_stats.transmitted.header_bytes +
+ rtx_stats.transmitted.payload_bytes +
+ rtx_stats.transmitted.padding_bytes + rtx_stats.transmitted.header_bytes;
// TODO(https://crbug.com/webrtc/10555): RTX retransmissions should show up in
// separate outbound-rtp stream objects.
stats.retransmitted_bytes_sent = rtp_stats.retransmitted.payload_bytes;
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 7fa02487f5..9658ade62d 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2362,7 +2362,11 @@ VideoSenderInfo WebRtcVideoChannel::WebRtcVideoSendStream::GetVideoSenderInfo(
it != stats.substreams.end(); ++it) {
// TODO(pbos): Wire up additional stats, such as padding bytes.
webrtc::VideoSendStream::StreamStats stream_stats = it->second;
- info.bytes_sent += stream_stats.rtp_stats.transmitted.payload_bytes;
+ // TODO(http://crbug.com/webrtc/10525): Bytes sent should only include
+ // payload bytes, not header and padding bytes.
+ info.bytes_sent += stream_stats.rtp_stats.transmitted.payload_bytes +
+ stream_stats.rtp_stats.transmitted.header_bytes +
+ stream_stats.rtp_stats.transmitted.padding_bytes;
info.packets_sent += stream_stats.rtp_stats.transmitted.packets;
info.total_packet_send_delay_ms += stream_stats.total_packet_send_delay_ms;
// TODO(https://crbug.com/webrtc/10555): RTX retransmissions should show up
@@ -2779,7 +2783,9 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo(
if (stats.current_payload_type != -1) {
info.codec_payload_type = stats.current_payload_type;
}
- info.bytes_rcvd = stats.rtp_stats.transmitted.payload_bytes;
+ info.bytes_rcvd = stats.rtp_stats.transmitted.payload_bytes +
+ stats.rtp_stats.transmitted.header_bytes +
+ stats.rtp_stats.transmitted.padding_bytes;
info.packets_rcvd = stats.rtp_stats.transmitted.packets;
info.packets_lost = stats.rtcp_stats.packets_lost;
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 68c25be414..4874cf6200 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -87,8 +87,6 @@ static const uint32_t kFlexfecSsrc = 5;
static const uint32_t kIncomingUnsignalledSsrc = 0xC0FFEE;
static const uint32_t kDefaultRecvSsrc = 0;
-constexpr uint32_t kRtpHeaderSize = 12;
-
static const char kUnsupportedExtensionName[] =
"urn:ietf:params:rtp-hdrext:unsupported";
@@ -1605,8 +1603,7 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
ASSERT_EQ(1U, info.senders.size());
// TODO(whyuan): bytes_sent and bytes_rcvd are different. Are both payload?
// For webrtc, bytes_sent does not include the RTP header length.
- EXPECT_EQ(info.senders[0].bytes_sent,
- NumRtpBytes() - kRtpHeaderSize * NumRtpPackets());
+ EXPECT_GT(info.senders[0].bytes_sent, 0);
EXPECT_EQ(NumRtpPackets(), info.senders[0].packets_sent);
EXPECT_EQ(0.0, info.senders[0].fraction_lost);
ASSERT_TRUE(info.senders[0].codec_payload_type);
@@ -1629,8 +1626,7 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
EXPECT_EQ(info.senders[0].ssrcs()[0], info.receivers[0].ssrcs()[0]);
ASSERT_TRUE(info.receivers[0].codec_payload_type);
EXPECT_EQ(DefaultCodec().id, *info.receivers[0].codec_payload_type);
- EXPECT_EQ(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
- info.receivers[0].bytes_rcvd);
+ EXPECT_EQ(NumRtpBytes(), info.receivers[0].bytes_rcvd);
EXPECT_EQ(NumRtpPackets(), info.receivers[0].packets_rcvd);
EXPECT_EQ(0, info.receivers[0].packets_lost);
// TODO(asapersson): Not set for webrtc. Handle missing stats.
@@ -1681,8 +1677,7 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStatsMultipleRecvStreams) {
ASSERT_EQ(1U, info.senders.size());
// TODO(whyuan): bytes_sent and bytes_rcvd are different. Are both payload?
// For webrtc, bytes_sent does not include the RTP header length.
- EXPECT_EQ_WAIT(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
- GetSenderStats(0).bytes_sent, kTimeout);
+ EXPECT_GT(GetSenderStats(0).bytes_sent, 0);
EXPECT_EQ_WAIT(NumRtpPackets(), GetSenderStats(0).packets_sent, kTimeout);
EXPECT_EQ(kVideoWidth, GetSenderStats(0).send_frame_width);
EXPECT_EQ(kVideoHeight, GetSenderStats(0).send_frame_height);
@@ -1691,8 +1686,7 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStatsMultipleRecvStreams) {
for (size_t i = 0; i < info.receivers.size(); ++i) {
EXPECT_EQ(1U, GetReceiverStats(i).ssrcs().size());
EXPECT_EQ(i + 1, GetReceiverStats(i).ssrcs()[0]);
- EXPECT_EQ_WAIT(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
- GetReceiverStats(i).bytes_rcvd, kTimeout);
+ EXPECT_EQ_WAIT(NumRtpBytes(), GetReceiverStats(i).bytes_rcvd, kTimeout);
EXPECT_EQ_WAIT(NumRtpPackets(), GetReceiverStats(i).packets_rcvd, kTimeout);
EXPECT_EQ_WAIT(kVideoWidth, GetReceiverStats(i).frame_width, kTimeout);
EXPECT_EQ_WAIT(kVideoHeight, GetReceiverStats(i).frame_height, kTimeout);
@@ -5176,7 +5170,9 @@ TEST_F(WebRtcVideoChannelTest, GetStatsTranslatesReceivePacketStatsCorrectly) {
cricket::VideoMediaInfo info;
ASSERT_TRUE(channel_->GetStats(&info));
- EXPECT_EQ(stats.rtp_stats.transmitted.payload_bytes,
+ EXPECT_EQ(stats.rtp_stats.transmitted.payload_bytes +
+ stats.rtp_stats.transmitted.header_bytes +
+ stats.rtp_stats.transmitted.padding_bytes,
rtc::checked_cast<size_t>(info.receivers[0].bytes_rcvd));
EXPECT_EQ(stats.rtp_stats.transmitted.packets,
rtc::checked_cast<unsigned int>(info.receivers[0].packets_rcvd));