[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}

2 files changed, 69 insertions, 3 deletions

diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 2c6067ed00..434a758cee 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -2431,9 +2431,16 @@ VideoSenderInfo WebRtcVideoChannel::WebRtcVideoSendStream::GetVideoSenderInfo(
         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 2fdfb04fc6..563e3f337e 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -5270,6 +5270,65 @@ TEST_F(WebRtcVideoChannelTest, GetStatsReportsAdaptationAndBandwidthStats) {
 }
 
 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;