diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc index 4673707cd9..7852a05913 100644 --- a/webrtc/video/receive_statistics_proxy.cc +++ b/webrtc/video/receive_statistics_proxy.cc @@ -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_) diff --git a/webrtc/video/receive_statistics_proxy_unittest.cc b/webrtc/video/receive_statistics_proxy_unittest.cc index 6112dd7fd4..d37d203548 100644 --- a/webrtc/video/receive_statistics_proxy_unittest.cc +++ b/webrtc/video/receive_statistics_proxy_unittest.cc @@ -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 { 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(), content_type); + fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs); + } + // One extra with with double the interval. + fake_clock_.AdvanceTimeMilliseconds(kInterFrameDelayMs); + statistics_proxy_->OnDecodedFrame(rtc::Optional(), 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(), 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