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

Fix incorrect InterframeDelayMaxInMs histogram metrics

Browse Source 
Two bugs:

1) The max value should only be reported if the average is also
   reported. Otherwise the max might become lower than average.
   (On average).

2) When reporting that max value, actually use the max value.

BUG=webrtc:7420

Review-Url: https://codereview.webrtc.org/3002593002
Cr-Commit-Position: refs/heads/master@{#19352}
This commit is contained in:
sprang 2017-08-15 05:00:33 -07:00 committed by Commit Bot
parent b5ed905ce7
commit 892dab52b6
2 changed files with 70 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
webrtc/video/receive_statistics_proxy.cc
View File

@ -216,6 +216,11 @@ void ReceiveStatisticsProxy::UpdateHistograms() {
if (interframe_delay_ms_screenshare != -1) {
RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.Screenshare.InterframeDelayInMs",
interframe_delay_ms_screenshare);
RTC_DCHECK_GE(interframe_delay_max_ms_screenshare_,
interframe_delay_ms_screenshare);
RTC_HISTOGRAM_COUNTS_10000(
"WebRTC.Video.Screenshare.InterframeDelayMaxInMs",
interframe_delay_max_ms_screenshare_);
}
int interframe_delay_ms_video =
@ -223,24 +228,11 @@ void ReceiveStatisticsProxy::UpdateHistograms() {
if (interframe_delay_ms_video != -1) {
RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.InterframeDelayInMs",
interframe_delay_ms_video);
RTC_DCHECK_GE(interframe_delay_max_ms_video_, interframe_delay_ms_video);
RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.InterframeDelayMaxInMs",
interframe_delay_max_ms_video_);
}
int interframe_delay_max_ms_screenshare =
interframe_delay_max_ms_screenshare_;
if (interframe_delay_max_ms_screenshare != -1) {
RTC_HISTOGRAM_COUNTS_10000(
"WebRTC.Video.Screenshare.InterframeDelayMaxInMs",
interframe_delay_ms_screenshare);
}
int interframe_delay_max_ms_video = interframe_delay_max_ms_video_;
if (interframe_delay_max_ms_video != -1) {
RTC_HISTOGRAM_COUNTS_10000(
"WebRTC.Video.InterframeDelayMaxInMs",
interframe_delay_ms_video);
}
StreamDataCounters rtp = stats_.rtp_stats;
StreamDataCounters rtx;
for (auto it : rtx_stats_)

63
webrtc/video/receive_statistics_proxy_unittest.cc
View File

@ -30,7 +30,8 @@ const int kMinRequiredSamples = 200;
} // namespace
// TODO(sakal): ReceiveStatisticsProxy is lacking unittesting.
class ReceiveStatisticsProxyTest : public ::testing::Test {
class ReceiveStatisticsProxyTest
: public ::testing::TestWithParam<webrtc::VideoContentType> {
public:
ReceiveStatisticsProxyTest() : fake_clock_(1234), config_(GetTestConfig()) {}
virtual ~ReceiveStatisticsProxyTest() {}
@ -718,4 +719,64 @@ TEST_F(ReceiveStatisticsProxyTest, RtcpHistogramsAreUpdated) {
kNackPackets * 60 / metrics::kMinRunTimeInSeconds));
}
INSTANTIATE_TEST_CASE_P(ContentTypes,
ReceiveStatisticsProxyTest,
::testing::Values(VideoContentType::UNSPECIFIED,
VideoContentType::SCREENSHARE));
TEST_P(ReceiveStatisticsProxyTest, InterFrameDelaysAreReported) {
const VideoContentType content_type = GetParam();
const int kInterFrameDelayMs = 33;
for (int i = 0; i < kMinRequiredSamples; ++i) {
statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs);
}
// One extra with with double the interval.
fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs);
statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
statistics_proxy_.reset();
const int kExpectedInterFrame =
(kInterFrameDelayMs * (kMinRequiredSamples - 1) +
kInterFrameDelayMs * 2) /
kMinRequiredSamples;
switch (content_type) {
case VideoContentType::UNSPECIFIED:
EXPECT_EQ(kExpectedInterFrame,
metrics::MinSample("WebRTC.Video.InterframeDelayInMs"));
EXPECT_EQ(kInterFrameDelayMs * 2,
metrics::MinSample("WebRTC.Video.InterframeDelayMaxInMs"));
break;
case VideoContentType::SCREENSHARE:
EXPECT_EQ(
kExpectedInterFrame,
metrics::MinSample("WebRTC.Video.Screenshare.InterframeDelayInMs"));
EXPECT_EQ(kInterFrameDelayMs * 2,
metrics::MinSample(
"WebRTC.Video.Screenshare.InterframeDelayMaxInMs"));
break;
default:
RTC_NOTREACHED();
}
}
TEST_P(ReceiveStatisticsProxyTest, MaxInterFrameDelayOnlyWithValidAverage) {
const VideoContentType content_type = GetParam();
const int kInterFrameDelayMs = 33;
for (int i = 0; i < kMinRequiredSamples; ++i) {
statistics_proxy_->OnDecodedFrame(rtc::Optional<uint8_t>(), content_type);
fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs);
}
// |kMinRequiredSamples| samples, and thereby intervals, is required. That
// means we're one frame short of having a valid data set.
statistics_proxy_.reset();
EXPECT_EQ(0, metrics::NumSamples("WebRTC.Video.InterframeDelayInMs"));
EXPECT_EQ(0, metrics::NumSamples("WebRTC.Video.InterframeDelayMaxInMs"));
EXPECT_EQ(
0, metrics::NumSamples("WebRTC.Video.Screenshare.InterframeDelayInMs"));
EXPECT_EQ(0, metrics::NumSamples(
"WebRTC.Video.Screenshare.InterframeDelayMaxInMs"));
}
} // namespace webrtc