[Stats] Include RTX retransmissions in the VideoSenderInfo.
Ignoring retransmissions carried over the RTX stream was a bug. This CL
fixes the bug, so that all retransmissions are accounted for. It also
adds test coverage for this.
This resolves https://crbug.com/webrtc/11440 but does not resolve
https://crbug.com/webrtc/11439.
Bug: webrtc:11440, webrtc:11439
Change-Id: Ifb10aa60a0f453738aaa30de90eaa5b31f9ec265
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170639
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30822}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 2c6067e..434a758 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2431,9 +2431,16 @@
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
- // in separate outbound-rtp stream objects.
- if (!stream_stats.is_rtx && !stream_stats.is_flexfec) {
+ if (!stream_stats.is_flexfec) {
+ // Retransmissions can happen over the same SSRC that media is sent over,
+ // or a separate RTX stream is negotiated per SSRC, in which case there
+ // will be a |stream_stats| with "is_rtx == true". Since we are currently
+ // aggregating all substreams' counters into a single "info" we do not
+ // need to know the relationship between RTX streams and RTP streams here.
+ // TODO(https://crbug.com/webrtc/11439): To unblock simulcast-aware stats,
+ // where substreams are not aggregated, we need to know the relationship
+ // between RTX streams and RTP streams so that the correct "info" object
+ // accounts for the correct RTX retransmissions.
info.retransmitted_bytes_sent +=
stream_stats.rtp_stats.retransmitted.payload_bytes;
info.retransmitted_packets_sent +=
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 2fdfb04..563e3f3 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -5270,6 +5270,65 @@
}
TEST_F(WebRtcVideoChannelTest,
+ GetStatsReportsTransmittedAndRetransmittedBytesAndPacketsCorrectly) {
+ FakeVideoSendStream* stream = AddSendStream();
+ webrtc::VideoSendStream::Stats stats;
+ // Simulcast layer 1, RTP stream. header+padding=10, payload=20, packets=3.
+ stats.substreams[101].is_rtx = false;
+ stats.substreams[101].rtp_stats.transmitted.header_bytes = 5;
+ stats.substreams[101].rtp_stats.transmitted.padding_bytes = 5;
+ stats.substreams[101].rtp_stats.transmitted.payload_bytes = 20;
+ stats.substreams[101].rtp_stats.transmitted.packets = 3;
+ stats.substreams[101].rtp_stats.retransmitted.header_bytes = 0;
+ stats.substreams[101].rtp_stats.retransmitted.padding_bytes = 0;
+ stats.substreams[101].rtp_stats.retransmitted.payload_bytes = 0;
+ stats.substreams[101].rtp_stats.retransmitted.packets = 0;
+ // Simulcast layer 1, RTX stream. header+padding=5, payload=10, packets=1.
+ stats.substreams[102].is_rtx = true;
+ stats.substreams[102].rtp_stats.retransmitted.header_bytes = 3;
+ stats.substreams[102].rtp_stats.retransmitted.padding_bytes = 2;
+ stats.substreams[102].rtp_stats.retransmitted.payload_bytes = 10;
+ stats.substreams[102].rtp_stats.retransmitted.packets = 1;
+ stats.substreams[102].rtp_stats.transmitted =
+ stats.substreams[102].rtp_stats.retransmitted;
+ // Simulcast layer 2, RTP stream. header+padding=20, payload=40, packets=7.
+ stats.substreams[201].is_rtx = false;
+ stats.substreams[201].rtp_stats.transmitted.header_bytes = 10;
+ stats.substreams[201].rtp_stats.transmitted.padding_bytes = 10;
+ stats.substreams[201].rtp_stats.transmitted.payload_bytes = 40;
+ stats.substreams[201].rtp_stats.transmitted.packets = 7;
+ stats.substreams[201].rtp_stats.retransmitted.header_bytes = 0;
+ stats.substreams[201].rtp_stats.retransmitted.padding_bytes = 0;
+ stats.substreams[201].rtp_stats.retransmitted.payload_bytes = 0;
+ stats.substreams[201].rtp_stats.retransmitted.packets = 0;
+ // Simulcast layer 2, RTX stream. header+padding=10, payload=20, packets=4.
+ stats.substreams[202].is_rtx = true;
+ stats.substreams[202].rtp_stats.retransmitted.header_bytes = 6;
+ stats.substreams[202].rtp_stats.retransmitted.padding_bytes = 4;
+ stats.substreams[202].rtp_stats.retransmitted.payload_bytes = 20;
+ stats.substreams[202].rtp_stats.retransmitted.packets = 4;
+ stats.substreams[202].rtp_stats.transmitted =
+ stats.substreams[202].rtp_stats.retransmitted;
+ stream->SetStats(stats);
+
+ cricket::VideoMediaInfo info;
+ ASSERT_TRUE(channel_->GetStats(&info));
+ // TODO(https://crbug.com/webrtc/9547): Populate individual VideoSenderInfo
+ // objects for each simulcast stream, instead of accumulating all layers into
+ // a single VideoSenderInfo. When this is fixed, this test should expect that
+ // there are two VideoSenderInfo, where the first info accounts for the first
+ // RTX and the second info accounts for the second RTX. In order for the test
+ // to be set up correctly, it may need to be updated such that the
+ // relationship between RTP and RTX streams are known. See also
+ // https://crbug.com/webrtc/11439.
+ EXPECT_EQ(45u, info.senders[0].header_and_padding_bytes_sent);
+ EXPECT_EQ(90u, info.senders[0].payload_bytes_sent);
+ EXPECT_EQ(15, info.senders[0].packets_sent);
+ EXPECT_EQ(30u, info.senders[0].retransmitted_bytes_sent);
+ EXPECT_EQ(5u, info.senders[0].retransmitted_packets_sent);
+}
+
+TEST_F(WebRtcVideoChannelTest,
GetStatsTranslatesBandwidthLimitedResolutionCorrectly) {
FakeVideoSendStream* stream = AddSendStream();
webrtc::VideoSendStream::Stats stats;
