From 8d7273357d92fab881561d886ce8dfe94e6e2238 Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Mon, 24 Oct 2022 22:32:24 +0200 Subject: [PATCH] APM: log both applied and recommended input volume stats This CL replaces the existing `WebRTC.Audio.ApmAnalogGain.*` stats with `WebRTC.Audio.Apm.AppliedInputVolume.*` and adds the `WebRTC.Audio.Apm.RecommendedInputVolume.*` stats. Bug: webrtc:7494 Change-Id: I70be710d20b1589fc814cbce3d3329ac1500686f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280220 Reviewed-by: Hanna Silen Commit-Queue: Alessio Bazzica Cr-Commit-Position: refs/heads/main@{#38468} --- modules/audio_processing/agc2/BUILD.gn | 2 + .../agc2/input_volume_stats_reporter.cc | 141 +++++++++++++----- .../agc2/input_volume_stats_reporter.h | 16 +- .../input_volume_stats_reporter_unittest.cc | 119 ++++++++++----- .../audio_processing/audio_processing_impl.cc | 10 +- .../audio_processing/audio_processing_impl.h | 2 + 6 files changed, 216 insertions(+), 74 deletions(-) diff --git a/modules/audio_processing/agc2/BUILD.gn b/modules/audio_processing/agc2/BUILD.gn index 976f96ec6b..b7a4030448 100644 --- a/modules/audio_processing/agc2/BUILD.gn +++ b/modules/audio_processing/agc2/BUILD.gn @@ -401,7 +401,9 @@ rtc_library("input_volume_stats_reporter_unittests") { sources = [ "input_volume_stats_reporter_unittest.cc" ] deps = [ ":input_volume_stats_reporter", + "../../../rtc_base:stringutils", "../../../system_wrappers:metrics", "../../../test:test_support", ] + absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] } diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter.cc b/modules/audio_processing/agc2/input_volume_stats_reporter.cc index 4a8b016f45..6d70186697 100644 --- a/modules/audio_processing/agc2/input_volume_stats_reporter.cc +++ b/modules/audio_processing/agc2/input_volume_stats_reporter.cc @@ -19,6 +19,8 @@ namespace webrtc { namespace { +using InputVolumeType = InputVolumeStatsReporter::InputVolumeType; + constexpr int kFramesIn60Seconds = 6000; constexpr int kMinInputVolume = 0; constexpr int kMaxInputVolume = 255; @@ -35,9 +37,102 @@ float ComputeAverageUpdate(int sum_updates, int num_updates) { return std::round(static_cast(sum_updates) / static_cast(num_updates)); } + +metrics::Histogram* CreateRateHistogram(absl::string_view name) { + return metrics::HistogramFactoryGetCounts( + name, /*min=*/1, /*max=*/kFramesIn60Seconds, /*bucket_count=*/50); +} + +metrics::Histogram* CreateAverageHistogram(absl::string_view name) { + return metrics::HistogramFactoryGetCounts(name, /*min=*/1, /*max=*/kMaxUpdate, + /*bucket_count=*/50); +} + +metrics::Histogram* CreateDecreaseRateHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateRateHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.DecreaseRate"); + case InputVolumeType::kRecommended: + return CreateRateHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.DecreaseRate"); + } +} + +metrics::Histogram* CreateDecreaseAverageHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.DecreaseAverage"); + case InputVolumeType::kRecommended: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.DecreaseAverage"); + } +} + +metrics::Histogram* CreateIncreaseRateHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateRateHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.IncreaseRate"); + case InputVolumeType::kRecommended: + return CreateRateHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.IncreaseRate"); + } +} + +metrics::Histogram* CreateIncreaseAverageHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.IncreaseAverage"); + case InputVolumeType::kRecommended: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.IncreaseAverage"); + } +} + +metrics::Histogram* CreateUpdateRateHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateRateHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.UpdateRate"); + case InputVolumeType::kRecommended: + return CreateRateHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.UpdateRate"); + } +} + +metrics::Histogram* CreateUpdateAverageHistogram( + InputVolumeType input_volume_type) { + switch (input_volume_type) { + case InputVolumeType::kApplied: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.AppliedInputVolume.UpdateAverage"); + case InputVolumeType::kRecommended: + return CreateAverageHistogram( + "WebRTC.Audio.Apm.RecommendedInputVolume.UpdateAverage"); + } +} + } // namespace -InputVolumeStatsReporter::InputVolumeStatsReporter() = default; +InputVolumeStatsReporter::InputVolumeStatsReporter( + InputVolumeType input_volume_type) + : decrease_rate_histogram_(CreateDecreaseRateHistogram(input_volume_type)), + decrease_average_histogram_( + CreateDecreaseAverageHistogram(input_volume_type)), + increase_rate_histogram_(CreateIncreaseRateHistogram(input_volume_type)), + increase_average_histogram_( + CreateIncreaseAverageHistogram(input_volume_type)), + update_rate_histogram_(CreateUpdateRateHistogram(input_volume_type)), + update_average_histogram_( + CreateUpdateAverageHistogram(input_volume_type)) {} InputVolumeStatsReporter::~InputVolumeStatsReporter() = default; @@ -74,47 +169,19 @@ void InputVolumeStatsReporter::LogVolumeUpdateStats() const { const float average_update = ComputeAverageUpdate( volume_update_stats_.sum_decreases + volume_update_stats_.sum_increases, num_updates); - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainDecreaseRate", - /*sample=*/volume_update_stats_.num_decreases, - /*min=*/1, - /*max=*/kFramesIn60Seconds, - /*bucket_count=*/50); + metrics::HistogramAdd(decrease_rate_histogram_, + volume_update_stats_.num_decreases); if (volume_update_stats_.num_decreases > 0) { - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainDecreaseAverage", - /*sample=*/average_decrease, - /*min=*/1, - /*max=*/kMaxUpdate, - /*bucket_count=*/50); + metrics::HistogramAdd(decrease_average_histogram_, average_decrease); } - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainIncreaseRate", - /*sample=*/volume_update_stats_.num_increases, - /*min=*/1, - /*max=*/kFramesIn60Seconds, - /*bucket_count=*/50); + metrics::HistogramAdd(increase_rate_histogram_, + volume_update_stats_.num_increases); if (volume_update_stats_.num_increases > 0) { - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainIncreaseAverage", - /*sample=*/average_increase, - /*min=*/1, - /*max=*/kMaxUpdate, - /*bucket_count=*/50); + metrics::HistogramAdd(increase_average_histogram_, average_increase); } - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainUpdateRate", - /*sample=*/num_updates, - /*min=*/1, - /*max=*/kFramesIn60Seconds, - /*bucket_count=*/50); + metrics::HistogramAdd(update_rate_histogram_, num_updates); if (num_updates > 0) { - RTC_HISTOGRAM_COUNTS_LINEAR( - /*name=*/"WebRTC.Audio.ApmAnalogGainUpdateAverage", - /*sample=*/average_update, - /*min=*/1, - /*max=*/kMaxUpdate, - /*bucket_count=*/50); + metrics::HistogramAdd(update_average_histogram_, average_update); } } diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter.h b/modules/audio_processing/agc2/input_volume_stats_reporter.h index 0040754209..757e6ff141 100644 --- a/modules/audio_processing/agc2/input_volume_stats_reporter.h +++ b/modules/audio_processing/agc2/input_volume_stats_reporter.h @@ -13,6 +13,7 @@ #include "absl/types/optional.h" #include "rtc_base/gtest_prod_util.h" +#include "system_wrappers/include/metrics.h" namespace webrtc { @@ -21,7 +22,12 @@ namespace webrtc { // the statistics into a histogram. class InputVolumeStatsReporter { public: - InputVolumeStatsReporter(); + enum class InputVolumeType { + kApplied = 0, + kRecommended = 1, + }; + + explicit InputVolumeStatsReporter(InputVolumeType input_volume_type); InputVolumeStatsReporter(const InputVolumeStatsReporter&) = delete; InputVolumeStatsReporter operator=(const InputVolumeStatsReporter&) = delete; ~InputVolumeStatsReporter(); @@ -57,6 +63,14 @@ class InputVolumeStatsReporter { // Computes aggregate stat and logs them into a histogram. void LogVolumeUpdateStats() const; + // Histograms. + metrics::Histogram* decrease_rate_histogram_; + metrics::Histogram* decrease_average_histogram_; + metrics::Histogram* increase_rate_histogram_; + metrics::Histogram* increase_average_histogram_; + metrics::Histogram* update_rate_histogram_; + metrics::Histogram* update_average_histogram_; + int log_volume_update_stats_counter_ = 0; absl::optional previous_input_volume_ = absl::nullopt; }; diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc b/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc index f8cd010784..a3e2ccaf91 100644 --- a/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc +++ b/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc @@ -10,24 +10,71 @@ #include "modules/audio_processing/agc2/input_volume_stats_reporter.h" +#include "absl/strings/string_view.h" +#include "rtc_base/strings/string_builder.h" #include "system_wrappers/include/metrics.h" #include "test/gmock.h" namespace webrtc { namespace { +using InputVolumeType = InputVolumeStatsReporter::InputVolumeType; + constexpr int kFramesIn60Seconds = 6000; -class InputVolumeStatsReporterTest : public ::testing::Test { +constexpr absl::string_view kLabelPrefix = "WebRTC.Audio.Apm."; + +class InputVolumeStatsReporterTest + : public ::testing::TestWithParam { public: - InputVolumeStatsReporterTest() {} + InputVolumeStatsReporterTest() { metrics::Reset(); } protected: - void SetUp() override { metrics::Reset(); } + InputVolumeType InputVolumeType() const { return GetParam(); } + std::string DecreaseRateLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "DecreaseRate") + .str(); + } + std::string DecreaseAverageLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "DecreaseAverage") + .str(); + } + std::string IncreaseRateLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "IncreaseRate") + .str(); + } + std::string IncreaseAverageLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "IncreaseAverage") + .str(); + } + std::string UpdateRateLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "UpdateRate") + .str(); + } + std::string UpdateAverageLabel() const { + return (rtc::StringBuilder(kLabelPrefix) + << VolumeTypeLabel() << "UpdateAverage") + .str(); + } + + private: + absl::string_view VolumeTypeLabel() const { + switch (InputVolumeType()) { + case InputVolumeType::kApplied: + return "AppliedInputVolume."; + case InputVolumeType::kRecommended: + return "RecommendedInputVolume."; + } + } }; -TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsEmpty) { - InputVolumeStatsReporter stats_reporter; +TEST_P(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsEmpty) { + InputVolumeStatsReporter stats_reporter(InputVolumeType()); constexpr int kInputVolume = 10; stats_reporter.UpdateStatistics(kInputVolume); // Update almost until the periodic logging and reset. @@ -35,25 +82,22 @@ TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsEmpty) { stats_reporter.UpdateStatistics(kInputVolume + 2); stats_reporter.UpdateStatistics(kInputVolume); } - EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateRate"), + EXPECT_METRIC_THAT(metrics::Samples(UpdateRateLabel()), ::testing::ElementsAre()); - EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseRate"), + EXPECT_METRIC_THAT(metrics::Samples(DecreaseRateLabel()), ::testing::ElementsAre()); - EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseRate"), + EXPECT_METRIC_THAT(metrics::Samples(IncreaseRateLabel()), + ::testing::ElementsAre()); + EXPECT_METRIC_THAT(metrics::Samples(UpdateAverageLabel()), + ::testing::ElementsAre()); + EXPECT_METRIC_THAT(metrics::Samples(DecreaseAverageLabel()), + ::testing::ElementsAre()); + EXPECT_METRIC_THAT(metrics::Samples(IncreaseAverageLabel()), ::testing::ElementsAre()); - EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateAverage"), - ::testing::ElementsAre()); - EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseAverage"), - ::testing::ElementsAre()); - EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseAverage"), - ::testing::ElementsAre()); } -TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsNotEmpty) { - InputVolumeStatsReporter stats_reporter; +TEST_P(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsNotEmpty) { + InputVolumeStatsReporter stats_reporter(InputVolumeType()); constexpr int kInputVolume = 10; stats_reporter.UpdateStatistics(kInputVolume); // Update until periodic logging. @@ -67,30 +111,30 @@ TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsNotEmpty) { stats_reporter.UpdateStatistics(kInputVolume); } EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateRate"), + metrics::Samples(UpdateRateLabel()), ::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds - 1, 1), ::testing::Pair(kFramesIn60Seconds, 1))); EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseRate"), + metrics::Samples(DecreaseRateLabel()), ::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds / 2 - 1, 1), ::testing::Pair(kFramesIn60Seconds / 2, 1))); EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseRate"), + metrics::Samples(IncreaseRateLabel()), ::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds / 2, 2))); EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateAverage"), + metrics::Samples(UpdateAverageLabel()), ::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1))); EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseAverage"), + metrics::Samples(DecreaseAverageLabel()), ::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1))); EXPECT_METRIC_THAT( - metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseAverage"), + metrics::Samples(IncreaseAverageLabel()), ::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1))); } } // namespace -TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsForEmptyStats) { - InputVolumeStatsReporter stats_reporter; +TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsForEmptyStats) { + InputVolumeStatsReporter stats_reporter(InputVolumeType()); const auto& update_stats = stats_reporter.volume_update_stats(); EXPECT_EQ(update_stats.num_decreases, 0); EXPECT_EQ(update_stats.sum_decreases, 0); @@ -98,10 +142,10 @@ TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsForEmptyStats) { EXPECT_EQ(update_stats.sum_increases, 0); } -TEST_F(InputVolumeStatsReporterTest, +TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterNoVolumeChange) { constexpr int kInputVolume = 10; - InputVolumeStatsReporter stats_reporter; + InputVolumeStatsReporter stats_reporter(InputVolumeType()); stats_reporter.UpdateStatistics(kInputVolume); stats_reporter.UpdateStatistics(kInputVolume); stats_reporter.UpdateStatistics(kInputVolume); @@ -112,10 +156,10 @@ TEST_F(InputVolumeStatsReporterTest, EXPECT_EQ(update_stats.sum_increases, 0); } -TEST_F(InputVolumeStatsReporterTest, +TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterVolumeIncrease) { constexpr int kInputVolume = 10; - InputVolumeStatsReporter stats_reporter; + InputVolumeStatsReporter stats_reporter(InputVolumeType()); stats_reporter.UpdateStatistics(kInputVolume); stats_reporter.UpdateStatistics(kInputVolume + 4); stats_reporter.UpdateStatistics(kInputVolume + 5); @@ -126,10 +170,10 @@ TEST_F(InputVolumeStatsReporterTest, EXPECT_EQ(update_stats.sum_increases, 5); } -TEST_F(InputVolumeStatsReporterTest, +TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterVolumeDecrease) { constexpr int kInputVolume = 10; - InputVolumeStatsReporter stats_reporter; + InputVolumeStatsReporter stats_reporter(InputVolumeType()); stats_reporter.UpdateStatistics(kInputVolume); stats_reporter.UpdateStatistics(kInputVolume - 4); stats_reporter.UpdateStatistics(kInputVolume - 5); @@ -140,8 +184,8 @@ TEST_F(InputVolumeStatsReporterTest, EXPECT_EQ(stats_update.sum_increases, 0); } -TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterReset) { - InputVolumeStatsReporter stats_reporter; +TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterReset) { + InputVolumeStatsReporter stats_reporter(InputVolumeType()); constexpr int kInputVolume = 10; stats_reporter.UpdateStatistics(kInputVolume); // Update until the periodic reset. @@ -169,4 +213,9 @@ TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterReset) { EXPECT_EQ(stats_after_reset.sum_increases, 3); } +INSTANTIATE_TEST_SUITE_P(, + InputVolumeStatsReporterTest, + ::testing::Values(InputVolumeType::kApplied, + InputVolumeType::kRecommended)); + } // namespace webrtc diff --git a/modules/audio_processing/audio_processing_impl.cc b/modules/audio_processing/audio_processing_impl.cc index 76e380426f..a0415e2bc3 100644 --- a/modules/audio_processing/audio_processing_impl.cc +++ b/modules/audio_processing/audio_processing_impl.cc @@ -291,7 +291,11 @@ AudioProcessingImpl::AudioProcessingImpl( MinimizeProcessingForUnusedOutput(), field_trial::IsEnabled("WebRTC-TransientSuppressorForcedOff")), capture_(), - capture_nonlocked_() { + capture_nonlocked_(), + applied_input_volume_stats_reporter_( + InputVolumeStatsReporter::InputVolumeType::kApplied), + recommended_input_volume_stats_reporter_( + InputVolumeStatsReporter::InputVolumeType::kRecommended) { RTC_LOG(LS_INFO) << "Injected APM submodules:" "\nEcho control factory: " << !!echo_control_factory_ @@ -1361,6 +1365,10 @@ int AudioProcessingImpl::ProcessCaptureStreamLocked() { stats_reporter_.UpdateStatistics(capture_.stats); UpdateRecommendedInputVolumeLocked(); + if (capture_.recommended_input_volume.has_value()) { + recommended_input_volume_stats_reporter_.UpdateStatistics( + *capture_.recommended_input_volume); + } if (submodules_.capture_levels_adjuster) { submodules_.capture_levels_adjuster->ApplyPostLevelAdjustment( diff --git a/modules/audio_processing/audio_processing_impl.h b/modules/audio_processing/audio_processing_impl.h index bc582b542d..5daea9088a 100644 --- a/modules/audio_processing/audio_processing_impl.h +++ b/modules/audio_processing/audio_processing_impl.h @@ -541,6 +541,8 @@ class AudioProcessingImpl : public AudioProcessing { InputVolumeStatsReporter applied_input_volume_stats_reporter_ RTC_GUARDED_BY(mutex_capture_); + InputVolumeStatsReporter recommended_input_volume_stats_reporter_ + RTC_GUARDED_BY(mutex_capture_); // Lock protection not needed. std::unique_ptr<