diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc index 05b6c45dd..cd914be8f 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc @@ -171,7 +171,7 @@ VCMJitterBuffer::~VCMJitterBuffer() { } void VCMJitterBuffer::UpdateHistograms() { - if (num_packets_ <= 0) { + if (num_packets_ <= 0 || !running_) { return; } int64_t elapsed_sec = diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc index 42523e9aa..d70973ef0 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc @@ -20,6 +20,8 @@ #include "webrtc/modules/video_coding/main/source/test/stream_generator.h" #include "webrtc/modules/video_coding/main/test/test_util.h" #include "webrtc/system_wrappers/interface/clock.h" +#include "webrtc/system_wrappers/interface/metrics.h" +#include "webrtc/test/histogram.h" namespace webrtc { @@ -275,6 +277,48 @@ TEST_F(TestBasicJitterBuffer, SinglePacketFrame) { jitter_buffer_->ReleaseFrame(frame_out); } +TEST_F(TestBasicJitterBuffer, VerifyHistogramStats) { + test::ClearHistograms(); + // Always start with a complete key frame when not allowing errors. + jitter_buffer_->SetDecodeErrorMode(kNoErrors); + packet_->frameType = kVideoFrameKey; + packet_->isFirstPacket = true; + packet_->markerBit = true; + packet_->timestamp += 123 * 90; + + // Insert single packet frame to the jitter buffer and get a frame. + bool retransmitted = false; + EXPECT_EQ(kCompleteSession, jitter_buffer_->InsertPacket(*packet_, + &retransmitted)); + VCMEncodedFrame* frame_out = DecodeCompleteFrame(); + CheckOutFrame(frame_out, size_, false); + EXPECT_EQ(kVideoFrameKey, frame_out->FrameType()); + jitter_buffer_->ReleaseFrame(frame_out); + + // Verify that histograms are updated when the jitter buffer is stopped. + clock_->AdvanceTimeMilliseconds(metrics::kMinRunTimeInSeconds * 1000); + jitter_buffer_->Stop(); + EXPECT_EQ(0, test::LastHistogramSample( + "WebRTC.Video.DiscardedPacketsInPercent")); + EXPECT_EQ(0, test::LastHistogramSample( + "WebRTC.Video.DuplicatedPacketsInPercent")); + EXPECT_NE(-1, test::LastHistogramSample( + "WebRTC.Video.CompleteFramesReceivedPerSecond")); + EXPECT_EQ(1000, test::LastHistogramSample( + "WebRTC.Video.KeyFramesReceivedInPermille")); + + // Verify that histograms are not updated if stop is called again. + jitter_buffer_->Stop(); + EXPECT_EQ(1, test::NumHistogramSamples( + "WebRTC.Video.DiscardedPacketsInPercent")); + EXPECT_EQ(1, test::NumHistogramSamples( + "WebRTC.Video.DuplicatedPacketsInPercent")); + EXPECT_EQ(1, test::NumHistogramSamples( + "WebRTC.Video.CompleteFramesReceivedPerSecond")); + EXPECT_EQ(1, test::NumHistogramSamples( + "WebRTC.Video.KeyFramesReceivedInPermille")); +} + TEST_F(TestBasicJitterBuffer, DualPacketFrame) { packet_->frameType = kVideoFrameKey; packet_->isFirstPacket = true; diff --git a/webrtc/test/histogram.cc b/webrtc/test/histogram.cc index 42307151f..0e6e9b2c0 100644 --- a/webrtc/test/histogram.cc +++ b/webrtc/test/histogram.cc @@ -12,6 +12,8 @@ #include +#include "webrtc/base/criticalsection.h" +#include "webrtc/base/thread_annotations.h" #include "webrtc/system_wrappers/interface/metrics.h" // Test implementation of histogram methods in @@ -19,8 +21,17 @@ namespace webrtc { namespace { -// Map holding the last added sample to a histogram (mapped by histogram name). -std::map histograms_; +struct SampleInfo { + SampleInfo(int sample) + : last(sample), total(1) {} + int last; // Last added sample. + int total; // Total number of added samples. +}; + +rtc::CriticalSection histogram_crit_; +// Map holding info about added samples to a histogram (mapped by the histogram +// name). +std::map histograms_ GUARDED_BY(histogram_crit_); } // namespace namespace metrics { @@ -32,17 +43,39 @@ Histogram* HistogramFactoryGetEnumeration(const std::string& name, void HistogramAdd( Histogram* histogram_pointer, const std::string& name, int sample) { - histograms_[name] = sample; + rtc::CritScope cs(&histogram_crit_); + auto it = histograms_.find(name); + if (it == histograms_.end()) { + histograms_.insert(std::make_pair(name, SampleInfo(sample))); + return; + } + it->second.last = sample; + ++it->second.total; } } // namespace metrics namespace test { int LastHistogramSample(const std::string& name) { - std::map::const_iterator it = histograms_.find(name); + rtc::CritScope cs(&histogram_crit_); + const auto it = histograms_.find(name); if (it == histograms_.end()) { return -1; } - return it->second; + return it->second.last; +} + +int NumHistogramSamples(const std::string& name) { + rtc::CritScope cs(&histogram_crit_); + const auto it = histograms_.find(name); + if (it == histograms_.end()) { + return 0; + } + return it->second.total; +} + +void ClearHistograms() { + rtc::CritScope cs(&histogram_crit_); + histograms_.clear(); } } // namespace test } // namespace webrtc diff --git a/webrtc/test/histogram.h b/webrtc/test/histogram.h index 213fc8c22..44ce32b4f 100644 --- a/webrtc/test/histogram.h +++ b/webrtc/test/histogram.h @@ -20,6 +20,12 @@ namespace test { // found). int LastHistogramSample(const std::string& name); +// Returns the number of added samples to a histogram. +int NumHistogramSamples(const std::string& name); + +// Removes all histograms. +void ClearHistograms(); + } // namespace test } // namespace webrtc