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

Sent bitrate stats are incorrect if FlexFEC is configured:

Browse Source 
WebRTC.Video.BitrateSentInKbps
WebRTC.Video.MediaBitrateSentInKbps
WebRTC.Video.PaddingBitrateSentInKbps
WebRTC.Video.RetransmittedBitrateSentInKbps
WebRTC.Video.FecBitrateSentInKbps

RtpSender has two StreamDataCounters: for the non-RTX and the RTX stream.
The same counter (for the non-RTX stream) is reported for both the media SSRC and the FlexFEC SSRC.
Bitrate stats are summed for all SSRCs, thus the counter for the non-RTX stream is counted twice.
Do not store the counter for the FlexFEC SSRC.

Do not include info from FlexFEC substreams in VideoSendStream::Stats::ToString (periodically logged during a call).

BUG=webrtc:6774

Review-Url: https://codereview.webrtc.org/2525293002
Cr-Commit-Position: refs/heads/master@{#15238}
This commit is contained in:
asapersson 2016-11-25 03:52:46 -08:00 committed by Commit bot
parent 6b272c5c37
commit a6a699a130
4 changed files with 130 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
webrtc/video/send_statistics_proxy.cc
View File

@ -432,6 +432,7 @@ VideoSendStream::StreamStats* SendStatisticsProxy::GetStatsEntry(
// Insert new entry and return ptr.
VideoSendStream::StreamStats* entry = &stats_.substreams[ssrc];
entry->is_rtx = is_rtx;
entry->is_flexfec = is_flexfec;
return entry;
}
@ -617,6 +618,12 @@ void SendStatisticsProxy::DataCountersUpdated(
RTC_DCHECK(stats) << "DataCountersUpdated reported for unknown ssrc: "
<< ssrc;
if (stats->is_flexfec) {
// The same counters are reported for both the media ssrc and flexfec ssrc.
// Bitrate stats are summed for all SSRCs. Use fec stats from media update.
return;
}
stats->rtp_stats = counters;
if (uma_container_->first_rtp_stats_time_ms_ == -1)
uma_container_->first_rtp_stats_time_ms_ = clock_->TimeInMilliseconds();

129
webrtc/video/send_statistics_proxy_unittest.cc
View File

@ -25,6 +25,7 @@ const uint32_t kFirstSsrc = 17;
const uint32_t kSecondSsrc = 42;
const uint32_t kFirstRtxSsrc = 18;
const uint32_t kSecondRtxSsrc = 43;
const uint32_t kFlexFecSsrc = 55;
const int kQpIdx0 = 21;
const int kQpIdx1 = 39;
@ -60,6 +61,25 @@ class SendStatisticsProxyTest : public ::testing::Test {
return config;
}
VideoSendStream::Config GetTestConfigWithFlexFec() {
VideoSendStream::Config config(nullptr);
config.rtp.ssrcs.push_back(kFirstSsrc);
config.rtp.ssrcs.push_back(kSecondSsrc);
config.rtp.rtx.ssrcs.push_back(kFirstRtxSsrc);
config.rtp.rtx.ssrcs.push_back(kSecondRtxSsrc);
config.rtp.flexfec.flexfec_payload_type = 50;
config.rtp.flexfec.flexfec_ssrc = kFlexFecSsrc;
return config;
}
VideoSendStream::StreamStats GetStreamStats(uint32_t ssrc) {
VideoSendStream::Stats stats = statistics_proxy_->GetStats();
std::map<uint32_t, VideoSendStream::StreamStats>::iterator it =
stats.substreams.find(ssrc);
EXPECT_NE(it, stats.substreams.end());
return it->second;
}
void ExpectEqual(VideoSendStream::Stats one, VideoSendStream::Stats other) {
EXPECT_EQ(one.input_frame_rate, other.input_frame_rate);
EXPECT_EQ(one.encode_frame_rate, other.encode_frame_rate);
@ -824,6 +844,99 @@ TEST_F(SendStatisticsProxyTest, ResetsRtcpCountersOnContentChange) {
4 * 100 / 5));
}
TEST_F(SendStatisticsProxyTest, GetStatsReportsIsFlexFec) {
statistics_proxy_.reset(
new SendStatisticsProxy(&fake_clock_, GetTestConfigWithFlexFec(),
VideoEncoderConfig::ContentType::kRealtimeVideo));
StreamDataCountersCallback* proxy =
static_cast<StreamDataCountersCallback*>(statistics_proxy_.get());
StreamDataCounters counters;
proxy->DataCountersUpdated(counters, kFirstSsrc);
proxy->DataCountersUpdated(counters, kFlexFecSsrc);
EXPECT_FALSE(GetStreamStats(kFirstSsrc).is_flexfec);
EXPECT_TRUE(GetStreamStats(kFlexFecSsrc).is_flexfec);
}
TEST_F(SendStatisticsProxyTest, SendBitratesAreReportedWithFlexFecEnabled) {
statistics_proxy_.reset(
new SendStatisticsProxy(&fake_clock_, GetTestConfigWithFlexFec(),
VideoEncoderConfig::ContentType::kRealtimeVideo));
StreamDataCountersCallback* proxy =
static_cast<StreamDataCountersCallback*>(statistics_proxy_.get());
StreamDataCounters counters;
StreamDataCounters rtx_counters;
proxy->DataCountersUpdated(counters, kFirstSsrc);
proxy->DataCountersUpdated(counters, kSecondSsrc);
proxy->DataCountersUpdated(rtx_counters, kFirstRtxSsrc);
proxy->DataCountersUpdated(rtx_counters, kSecondRtxSsrc);
proxy->DataCountersUpdated(counters, kFlexFecSsrc);
counters.transmitted.header_bytes = 5000;
counters.transmitted.packets = 20;
counters.transmitted.padding_bytes = 10000;
counters.transmitted.payload_bytes = 20000;
counters.retransmitted.header_bytes = 400;
counters.retransmitted.packets = 2;
counters.retransmitted.padding_bytes = 1000;
counters.retransmitted.payload_bytes = 2000;
counters.fec = counters.retransmitted;
rtx_counters.transmitted = counters.transmitted;
fake_clock_.AdvanceTimeMilliseconds(1000 * metrics::kMinRunTimeInSeconds);
proxy->DataCountersUpdated(counters, kFirstSsrc);
proxy->DataCountersUpdated(counters, kSecondSsrc);
proxy->DataCountersUpdated(rtx_counters, kFirstRtxSsrc);
proxy->DataCountersUpdated(rtx_counters, kSecondRtxSsrc);
proxy->DataCountersUpdated(counters, kFlexFecSsrc);
// Reset stats proxy causes histograms to be reported.
statistics_proxy_.reset();
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.BitrateSentInKbps"));
EXPECT_EQ(1,
metrics::NumEvents(
"WebRTC.Video.BitrateSentInKbps",
static_cast<int>((counters.transmitted.TotalBytes() * 4 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.MediaBitrateSentInKbps"));
EXPECT_EQ(1, metrics::NumEvents(
"WebRTC.Video.MediaBitrateSentInKbps",
static_cast<int>((counters.MediaPayloadBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.PaddingBitrateSentInKbps"));
EXPECT_EQ(1,
metrics::NumEvents(
"WebRTC.Video.PaddingBitrateSentInKbps",
static_cast<int>((counters.transmitted.padding_bytes * 4 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
EXPECT_EQ(1,
metrics::NumSamples("WebRTC.Video.RetransmittedBitrateSentInKbps"));
EXPECT_EQ(1,
metrics::NumEvents(
"WebRTC.Video.RetransmittedBitrateSentInKbps",
static_cast<int>((counters.retransmitted.TotalBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.RtxBitrateSentInKbps"));
EXPECT_EQ(
1, metrics::NumEvents(
"WebRTC.Video.RtxBitrateSentInKbps",
static_cast<int>((rtx_counters.transmitted.TotalBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.FecBitrateSentInKbps"));
EXPECT_EQ(1, metrics::NumEvents(
"WebRTC.Video.FecBitrateSentInKbps",
static_cast<int>((counters.fec.TotalBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
}
TEST_F(SendStatisticsProxyTest, ResetsRtpCountersOnContentChange) {
StreamDataCountersCallback* proxy =
static_cast<StreamDataCountersCallback*>(statistics_proxy_.get());
@ -835,15 +948,15 @@ TEST_F(SendStatisticsProxyTest, ResetsRtpCountersOnContentChange) {
proxy->DataCountersUpdated(rtx_counters, kFirstRtxSsrc);
proxy->DataCountersUpdated(rtx_counters, kSecondRtxSsrc);
counters.transmitted.header_bytes = 400;
counters.transmitted.header_bytes = 5000;
counters.transmitted.packets = 20;
counters.transmitted.padding_bytes = 1000;
counters.transmitted.payload_bytes = 2000;
counters.transmitted.padding_bytes = 10000;
counters.transmitted.payload_bytes = 20000;
counters.retransmitted.header_bytes = 40;
counters.retransmitted.header_bytes = 400;
counters.retransmitted.packets = 2;
counters.retransmitted.padding_bytes = 100;
counters.retransmitted.payload_bytes = 200;
counters.retransmitted.padding_bytes = 1000;
counters.retransmitted.payload_bytes = 2000;
counters.fec = counters.retransmitted;
@ -898,7 +1011,7 @@ TEST_F(SendStatisticsProxyTest, ResetsRtpCountersOnContentChange) {
EXPECT_EQ(1, metrics::NumSamples("WebRTC.Video.FecBitrateSentInKbps"));
EXPECT_EQ(1, metrics::NumEvents(
"WebRTC.Video.FecBitrateSentInKbps",
static_cast<int>((rtx_counters.fec.TotalBytes() * 2 * 8) /
static_cast<int>((counters.fec.TotalBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
// New start time but same counter values.
@ -965,7 +1078,7 @@ TEST_F(SendStatisticsProxyTest, ResetsRtpCountersOnContentChange) {
1, metrics::NumSamples("WebRTC.Video.Screenshare.FecBitrateSentInKbps"));
EXPECT_EQ(1, metrics::NumEvents(
"WebRTC.Video.Screenshare.FecBitrateSentInKbps",
static_cast<int>((rtx_counters.fec.TotalBytes() * 2 * 8) /
static_cast<int>((counters.fec.TotalBytes() * 2 * 8) /
metrics::kMinRunTimeInSeconds / 1000)));
}

2
webrtc/video/video_send_stream.cc
View File

@ -214,7 +214,7 @@ std::string VideoSendStream::Stats::ToString(int64_t time_ms) const {
ss << "bw_adapted: " << (bw_limited_resolution ? "true" : "false");
ss << '}';
for (const auto& substream : substreams) {
if (!substream.second.is_rtx) {
if (!substream.second.is_rtx && !substream.second.is_flexfec) {
ss << " {ssrc: " << substream.first << ", ";
ss << substream.second.ToString();
ss << '}';

1
webrtc/video_send_stream.h
View File

@ -36,6 +36,7 @@ class VideoSendStream {
FrameCounts frame_counts;
bool is_rtx = false;
bool is_flexfec = false;
int width = 0;
int height = 0;
// TODO(holmer): Move bitrate_bps out to the webrtc::Call layer.