From 79e9f4b9c128963eef2c7031dd9311f86fca5535 Mon Sep 17 00:00:00 2001 From: Yves Gerey Date: Sat, 13 Apr 2019 18:59:53 +0200 Subject: [PATCH] Replace test::Statistics by webrtc::RunningStatistics. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The former became redundant and didn't guarantee numerical stability for variance computation. Bug: webrtc:10412 Change-Id: Idc291abe9add41bde9e7734f179e5d6c65f2630b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132460 Commit-Queue: Yves Gerey Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/master@{#27605} --- modules/video_coding/BUILD.gn | 1 + .../codecs/test/videocodec_test_stats_impl.cc | 67 +++++++++++-------- test/BUILD.gn | 3 +- test/scenario/BUILD.gn | 1 + test/scenario/performance_stats.h | 8 +-- test/scenario/scenario_unittest.cc | 6 +- test/scenario/stats_collection_unittest.cc | 6 +- test/statistics.cc | 58 ---------------- test/statistics.h | 40 ----------- video/BUILD.gn | 1 + video/video_analyzer.cc | 12 ++-- video/video_analyzer.h | 52 +++++++------- 12 files changed, 86 insertions(+), 169 deletions(-) delete mode 100644 test/statistics.cc delete mode 100644 test/statistics.h diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 29a5d8c503..8249822a03 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -675,6 +675,7 @@ if (rtc_include_tests) { deps = [ "../../api:videocodec_test_fixture_api", "../../rtc_base:checks", + "../../rtc_base:rtc_numerics", "../../test:test_common", "../rtp_rtcp:rtp_rtcp_format", ] diff --git a/modules/video_coding/codecs/test/videocodec_test_stats_impl.cc b/modules/video_coding/codecs/test/videocodec_test_stats_impl.cc index 1e9db7ff9a..32cc124edc 100644 --- a/modules/video_coding/codecs/test/videocodec_test_stats_impl.cc +++ b/modules/video_coding/codecs/test/videocodec_test_stats_impl.cc @@ -18,7 +18,7 @@ #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" -#include "test/statistics.h" +#include "rtc_base/numerics/running_statistics.h" namespace webrtc { namespace test { @@ -187,20 +187,20 @@ VideoStatistics VideoCodecTestStatsImpl::SliceAndCalcVideoStatistic( VideoStatistics video_stat; float buffer_level_bits = 0.0f; - Statistics buffer_level_sec; + RunningStatistics buffer_level_sec; - Statistics key_frame_size_bytes; - Statistics delta_frame_size_bytes; + RunningStatistics key_frame_size_bytes; + RunningStatistics delta_frame_size_bytes; - Statistics frame_encoding_time_us; - Statistics frame_decoding_time_us; + RunningStatistics frame_encoding_time_us; + RunningStatistics frame_decoding_time_us; - Statistics psnr_y; - Statistics psnr_u; - Statistics psnr_v; - Statistics psnr; - Statistics ssim; - Statistics qp; + RunningStatistics psnr_y; + RunningStatistics psnr_u; + RunningStatistics psnr_v; + RunningStatistics psnr; + RunningStatistics ssim; + RunningStatistics qp; size_t rtp_timestamp_first_frame = 0; size_t rtp_timestamp_prev_frame = 0; @@ -326,32 +326,41 @@ VideoStatistics VideoCodecTestStatsImpl::SliceAndCalcVideoStatistic( // http://bugs.webrtc.org/10400: On Windows, we only get millisecond // granularity in the frame encode/decode timing measurements. // So we need to softly avoid a div-by-zero here. - const float mean_encode_time_us = frame_encoding_time_us.Mean(); + const float mean_encode_time_us = + frame_encoding_time_us.GetMean().value_or(0); video_stat.enc_speed_fps = mean_encode_time_us > 0.0f ? 1000000.0f / mean_encode_time_us : std::numeric_limits::max(); - const float mean_decode_time_us = frame_decoding_time_us.Mean(); + const float mean_decode_time_us = + frame_decoding_time_us.GetMean().value_or(0); video_stat.dec_speed_fps = mean_decode_time_us > 0.0f ? 1000000.0f / mean_decode_time_us : std::numeric_limits::max(); - video_stat.avg_delay_sec = buffer_level_sec.Mean(); - video_stat.max_key_frame_delay_sec = - 8 * key_frame_size_bytes.Max() / 1000 / target_bitrate_kbps; - video_stat.max_delta_frame_delay_sec = - 8 * delta_frame_size_bytes.Max() / 1000 / target_bitrate_kbps; + auto MaxDelaySec = + [target_bitrate_kbps](const RunningStatistics& stats) { + return 8 * stats.GetMax().value_or(0) / 1000 / target_bitrate_kbps; + }; - video_stat.avg_key_frame_size_bytes = key_frame_size_bytes.Mean(); - video_stat.avg_delta_frame_size_bytes = delta_frame_size_bytes.Mean(); - video_stat.avg_qp = qp.Mean(); + video_stat.avg_delay_sec = buffer_level_sec.GetMean().value_or(0); + video_stat.max_key_frame_delay_sec = MaxDelaySec(key_frame_size_bytes); + video_stat.max_delta_frame_delay_sec = MaxDelaySec(key_frame_size_bytes); - video_stat.avg_psnr_y = psnr_y.Mean(); - video_stat.avg_psnr_u = psnr_u.Mean(); - video_stat.avg_psnr_v = psnr_v.Mean(); - video_stat.avg_psnr = psnr.Mean(); - video_stat.min_psnr = psnr.Min(); - video_stat.avg_ssim = ssim.Mean(); - video_stat.min_ssim = ssim.Min(); + video_stat.avg_key_frame_size_bytes = + key_frame_size_bytes.GetMean().value_or(0); + video_stat.avg_delta_frame_size_bytes = + delta_frame_size_bytes.GetMean().value_or(0); + video_stat.avg_qp = qp.GetMean().value_or(0); + + video_stat.avg_psnr_y = psnr_y.GetMean().value_or(0); + video_stat.avg_psnr_u = psnr_u.GetMean().value_or(0); + video_stat.avg_psnr_v = psnr_v.GetMean().value_or(0); + video_stat.avg_psnr = psnr.GetMean().value_or(0); + video_stat.min_psnr = + psnr.GetMin().value_or(std::numeric_limits::max()); + video_stat.avg_ssim = ssim.GetMean().value_or(0); + video_stat.min_ssim = + ssim.GetMin().value_or(std::numeric_limits::max()); return video_stat; } diff --git a/test/BUILD.gn b/test/BUILD.gn index f16644372c..85baf98911 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -674,8 +674,6 @@ rtc_source_set("test_common") { "null_transport.cc", "null_transport.h", "rtp_rtcp_observer.h", - "statistics.cc", - "statistics.h", "video_decoder_proxy_factory.h", "video_encoder_proxy_factory.h", ] @@ -742,6 +740,7 @@ rtc_source_set("test_common") { "../modules/video_coding:webrtc_vp9", "../rtc_base:checks", "../rtc_base:rtc_base_approved", + "../rtc_base:rtc_numerics", "../system_wrappers", "../system_wrappers:field_trial", "../video", diff --git a/test/scenario/BUILD.gn b/test/scenario/BUILD.gn index aafafa54c4..eba344bf7b 100644 --- a/test/scenario/BUILD.gn +++ b/test/scenario/BUILD.gn @@ -128,6 +128,7 @@ if (rtc_include_tests) { "../../rtc_base:checks", "../../rtc_base:rtc_base_approved", "../../rtc_base:rtc_base_tests_utils", + "../../rtc_base:rtc_numerics", "../../rtc_base:rtc_task_queue", "../../rtc_base:safe_minmax", "../../rtc_base:task_queue_for_test", diff --git a/test/scenario/performance_stats.h b/test/scenario/performance_stats.h index e58dab37bf..b00c62c337 100644 --- a/test/scenario/performance_stats.h +++ b/test/scenario/performance_stats.h @@ -14,7 +14,7 @@ #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/video_frame_buffer.h" -#include "test/statistics.h" +#include "rtc_base/numerics/running_statistics.h" namespace webrtc { namespace test { @@ -39,9 +39,9 @@ struct VideoQualityStats { int captures_count = 0; int valid_count = 0; int lost_count = 0; - Statistics end_to_end_seconds; - Statistics frame_size; - Statistics psnr; + RunningStatistics end_to_end_seconds; + RunningStatistics frame_size; + RunningStatistics psnr; }; } // namespace test diff --git a/test/scenario/scenario_unittest.cc b/test/scenario/scenario_unittest.cc index cf7d28ee2a..aa65a12e68 100644 --- a/test/scenario/scenario_unittest.cc +++ b/test/scenario/scenario_unittest.cc @@ -101,8 +101,9 @@ TEST(ScenarioTest, MAYBE_SimTimeEncoding) { s.RunFor(TimeDelta::seconds(60)); } // Regression tests based on previous runs. - EXPECT_NEAR(analyzer.stats().psnr.Mean(), 38, 2); EXPECT_EQ(analyzer.stats().lost_count, 0); + ASSERT_TRUE(analyzer.stats().psnr.GetMean()); + EXPECT_NEAR(*analyzer.stats().psnr.GetMean(), 38, 2); } // TODO(bugs.webrtc.org/10515): Remove this when performance has been improved. @@ -121,8 +122,9 @@ TEST(ScenarioTest, MAYBE_RealTimeEncoding) { s.RunFor(TimeDelta::seconds(10)); } // Regression tests based on previous runs. - EXPECT_NEAR(analyzer.stats().psnr.Mean(), 38, 10); EXPECT_LT(analyzer.stats().lost_count, 2); + ASSERT_TRUE(analyzer.stats().psnr.GetMean()); + EXPECT_NEAR(*analyzer.stats().psnr.GetMean(), 38, 10); } TEST(ScenarioTest, SimTimeFakeing) { diff --git a/test/scenario/stats_collection_unittest.cc b/test/scenario/stats_collection_unittest.cc index ee01ad53c2..7e65ec1607 100644 --- a/test/scenario/stats_collection_unittest.cc +++ b/test/scenario/stats_collection_unittest.cc @@ -41,7 +41,8 @@ TEST(ScenarioAnalyzerTest, PsnrIsHighWhenNetworkIsGood) { } // This is mainty a regression test, the target is based on previous runs and // might change due to changes in configuration and encoder etc. - EXPECT_GT(analyzer.stats().psnr.Mean(), 40); + ASSERT_TRUE(analyzer.stats().psnr.GetMean()); + EXPECT_GT(*analyzer.stats().psnr.GetMean(), 40); } TEST(ScenarioAnalyzerTest, PsnrIsLowWhenNetworkIsBad) { @@ -56,7 +57,8 @@ TEST(ScenarioAnalyzerTest, PsnrIsLowWhenNetworkIsBad) { } // This is mainty a regression test, the target is based on previous runs and // might change due to changes in configuration and encoder etc. - EXPECT_LT(analyzer.stats().psnr.Mean(), 30); + ASSERT_TRUE(analyzer.stats().psnr.GetMean()); + EXPECT_LT(*analyzer.stats().psnr.GetMean(), 30); } } // namespace test } // namespace webrtc diff --git a/test/statistics.cc b/test/statistics.cc deleted file mode 100644 index 44803968f7..0000000000 --- a/test/statistics.cc +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ -#include "test/statistics.h" - -#include -#include -#include - -namespace webrtc { -namespace test { - -Statistics::Statistics() - : sum_(0.0), - sum_squared_(0.0), - max_(std::numeric_limits::min()), - min_(std::numeric_limits::max()), - count_(0) {} - -void Statistics::AddSample(double sample) { - sum_ += sample; - sum_squared_ += sample * sample; - max_ = std::max(max_, sample); - min_ = std::min(min_, sample); - ++count_; -} - -double Statistics::Max() const { - return max_; -} - -double Statistics::Mean() const { - if (count_ == 0) - return 0.0; - return sum_ / count_; -} - -double Statistics::Min() const { - return min_; -} - -double Statistics::Variance() const { - if (count_ == 0) - return 0.0; - return sum_squared_ / count_ - Mean() * Mean(); -} - -double Statistics::StandardDeviation() const { - return sqrt(Variance()); -} -} // namespace test -} // namespace webrtc diff --git a/test/statistics.h b/test/statistics.h deleted file mode 100644 index 2fa706f603..0000000000 --- a/test/statistics.h +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ -#ifndef TEST_STATISTICS_H_ -#define TEST_STATISTICS_H_ - -#include - -namespace webrtc { -namespace test { - -class Statistics { - public: - Statistics(); - - void AddSample(double sample); - - double Max() const; - double Mean() const; - double Min() const; - double Variance() const; - double StandardDeviation() const; - - private: - double sum_; - double sum_squared_; - double max_; - double min_; - uint64_t count_; -}; -} // namespace test -} // namespace webrtc - -#endif // TEST_STATISTICS_H_ diff --git a/video/BUILD.gn b/video/BUILD.gn index fffd58261c..907a96d914 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -274,6 +274,7 @@ if (rtc_include_tests) { "../modules/video_coding:webrtc_vp9", "../rtc_base:rtc_base_approved", "../rtc_base:rtc_base_tests_utils", + "../rtc_base:rtc_numerics", "../system_wrappers", "../test:fake_video_codecs", "../test:fileutils", diff --git a/video/video_analyzer.cc b/video/video_analyzer.cc index cb6c6f1aaf..11d2c7b648 100644 --- a/video/video_analyzer.cc +++ b/video/video_analyzer.cc @@ -652,8 +652,8 @@ void VideoAnalyzer::PrintResults() { // Disable quality check for quick test, as quality checks may fail // because too few samples were collected. if (!is_quick_test_enabled_) { - EXPECT_GT(psnr_.Mean(), avg_psnr_threshold_); - EXPECT_GT(ssim_.Mean(), avg_ssim_threshold_); + EXPECT_GT(*psnr_.GetMean(), avg_psnr_threshold_); + EXPECT_GT(*ssim_.GetMean(), avg_ssim_threshold_); } } @@ -727,11 +727,11 @@ void VideoAnalyzer::PerformFrameComparison( } void VideoAnalyzer::PrintResult(const char* result_type, - test::Statistics stats, + Statistics stats, const char* unit) { - test::PrintResultMeanAndError(result_type, "", test_label_.c_str(), - stats.Mean(), stats.StandardDeviation(), unit, - false); + test::PrintResultMeanAndError( + result_type, "", test_label_.c_str(), stats.GetMean().value_or(0), + stats.GetStandardDeviation().value_or(0), unit, false); } void VideoAnalyzer::PrintSamplesToFile() { diff --git a/video/video_analyzer.h b/video/video_analyzer.h index 8caaf1420a..36ed03e830 100644 --- a/video/video_analyzer.h +++ b/video/video_analyzer.h @@ -17,10 +17,10 @@ #include #include "api/video/video_source_interface.h" +#include "rtc_base/numerics/running_statistics.h" #include "rtc_base/time_utils.h" #include "test/layer_filtering_transport.h" #include "test/rtp_file_writer.h" -#include "test/statistics.h" namespace webrtc { @@ -28,6 +28,8 @@ class VideoAnalyzer : public PacketReceiver, public Transport, public rtc::VideoSinkInterface { public: + using Statistics = RunningStatistics; + VideoAnalyzer(test::LayerFilteringTransport* transport, const std::string& test_label, double avg_psnr_threshold, @@ -190,9 +192,7 @@ class VideoAnalyzer : public PacketReceiver, bool FrameProcessed(); void PrintResults(); void PerformFrameComparison(const FrameComparison& comparison); - void PrintResult(const char* result_type, - test::Statistics stats, - const char* unit); + void PrintResult(const char* result_type, Statistics stats, const char* unit); void PrintSamplesToFile(void); double GetAverageMediaBitrateBps(); void AddCapturedFrameForComparison(const VideoFrame& video_frame); @@ -213,28 +213,28 @@ class VideoAnalyzer : public PacketReceiver, rtc::CriticalSection comparison_lock_; std::vector samples_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics sender_time_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics receiver_time_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics network_time_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics psnr_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics ssim_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics end_to_end_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics rendered_delta_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics encoded_frame_size_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics encode_frame_rate_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics encode_time_ms_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics encode_usage_percent_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics decode_time_ms_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics decode_time_max_ms_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics media_bitrate_bps_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics fec_bitrate_bps_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics send_bandwidth_bps_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics memory_usage_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics time_between_freezes_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics audio_expand_rate_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics audio_accelerate_rate_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics audio_jitter_buffer_ms_ RTC_GUARDED_BY(comparison_lock_); - test::Statistics pixels_ RTC_GUARDED_BY(comparison_lock_); + Statistics sender_time_ RTC_GUARDED_BY(comparison_lock_); + Statistics receiver_time_ RTC_GUARDED_BY(comparison_lock_); + Statistics network_time_ RTC_GUARDED_BY(comparison_lock_); + Statistics psnr_ RTC_GUARDED_BY(comparison_lock_); + Statistics ssim_ RTC_GUARDED_BY(comparison_lock_); + Statistics end_to_end_ RTC_GUARDED_BY(comparison_lock_); + Statistics rendered_delta_ RTC_GUARDED_BY(comparison_lock_); + Statistics encoded_frame_size_ RTC_GUARDED_BY(comparison_lock_); + Statistics encode_frame_rate_ RTC_GUARDED_BY(comparison_lock_); + Statistics encode_time_ms_ RTC_GUARDED_BY(comparison_lock_); + Statistics encode_usage_percent_ RTC_GUARDED_BY(comparison_lock_); + Statistics decode_time_ms_ RTC_GUARDED_BY(comparison_lock_); + Statistics decode_time_max_ms_ RTC_GUARDED_BY(comparison_lock_); + Statistics media_bitrate_bps_ RTC_GUARDED_BY(comparison_lock_); + Statistics fec_bitrate_bps_ RTC_GUARDED_BY(comparison_lock_); + Statistics send_bandwidth_bps_ RTC_GUARDED_BY(comparison_lock_); + Statistics memory_usage_ RTC_GUARDED_BY(comparison_lock_); + Statistics time_between_freezes_ RTC_GUARDED_BY(comparison_lock_); + Statistics audio_expand_rate_ RTC_GUARDED_BY(comparison_lock_); + Statistics audio_accelerate_rate_ RTC_GUARDED_BY(comparison_lock_); + Statistics audio_jitter_buffer_ms_ RTC_GUARDED_BY(comparison_lock_); + Statistics pixels_ RTC_GUARDED_BY(comparison_lock_); // Rendered frame with worst PSNR is saved for further analysis. absl::optional worst_frame_ RTC_GUARDED_BY(comparison_lock_);