admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Stats] Include RTX retransmissions in the VideoSenderInfo.

Browse Source 
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}
This commit is contained in:
Henrik Boström 2020-03-18 16:52:17 +01:00 committed by Commit Bot
parent ebf739be7b
commit fc29b0ad46
2 changed files with 69 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
media/engine/webrtc_video_engine.cc
View File

@ -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 +=

59
media/engine/webrtc_video_engine_unittest.cc
View File

@ -5269,6 +5269,65 @@ TEST_F(WebRtcVideoChannelTest, GetStatsReportsAdaptationAndBandwidthStats) {
EXPECT_EQ(stats.number_of_cpu_adapt_changes, info.senders[0].adapt_changes);
}
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();