From a106095333cc0ecb0405dbc2199445878c197c16 Mon Sep 17 00:00:00 2001 From: Alessio Bazzica Date: Mon, 12 Dec 2022 17:07:12 +0100 Subject: [PATCH] Fix WebRTC.Audio.Apm.RecommendedInputVolume.OnChangeToMatchTarget tests - Reset the tested metrics to avoid interactions between tests that depend on the execution order - Address the comment in [1] by adding a function to log the same histogram in two different places [1] https://chromium-review.googlesource.com/c/chromium/src/+/4087426/4/tools/metrics/histograms/metadata/web_rtc/histograms.xml#179 Bug: webrtc:7494 Change-Id: Ia4d339b03c8078eb63626c91579f8a9547f087f7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/287681 Reviewed-by: Hanna Silen Commit-Queue: Alessio Bazzica Cr-Commit-Position: refs/heads/main@{#38873} --- modules/audio_processing/agc/BUILD.gn | 1 + modules/audio_processing/agc/agc_manager_direct.cc | 6 +++--- modules/audio_processing/agc2/BUILD.gn | 1 + modules/audio_processing/agc2/input_volume_controller.cc | 6 +++--- .../agc2/input_volume_controller_unittest.cc | 6 ++++++ .../audio_processing/agc2/input_volume_stats_reporter.cc | 6 ++++++ modules/audio_processing/agc2/input_volume_stats_reporter.h | 6 ++++++ 7 files changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/audio_processing/agc/BUILD.gn b/modules/audio_processing/agc/BUILD.gn index 24402e559a..508f901b08 100644 --- a/modules/audio_processing/agc/BUILD.gn +++ b/modules/audio_processing/agc/BUILD.gn @@ -36,6 +36,7 @@ rtc_library("agc") { "../../../system_wrappers:metrics", "../agc2:clipping_predictor", "../agc2:gain_map", + "../agc2:input_volume_stats_reporter", "../vad", ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] diff --git a/modules/audio_processing/agc/agc_manager_direct.cc b/modules/audio_processing/agc/agc_manager_direct.cc index 67c9c6ddf9..b8ad4a8bb9 100644 --- a/modules/audio_processing/agc/agc_manager_direct.cc +++ b/modules/audio_processing/agc/agc_manager_direct.cc @@ -17,6 +17,7 @@ #include "common_audio/include/audio_util.h" #include "modules/audio_processing/agc/gain_control.h" #include "modules/audio_processing/agc2/gain_map_internal.h" +#include "modules/audio_processing/agc2/input_volume_stats_reporter.h" #include "modules/audio_processing/include/audio_frame_view.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -650,9 +651,8 @@ void AgcManagerDirect::Process(const AudioBuffer& audio_buffer, if (volume_after_clipping_handling != recommended_input_volume_) { // The recommended input volume was adjusted in order to match the target // level. - RTC_HISTOGRAM_COUNTS_LINEAR( - "WebRTC.Audio.Apm.RecommendedInputVolume.OnChangeToMatchTarget", - recommended_input_volume_, 1, kMaxMicLevel, 50); + UpdateHistogramOnRecommendedInputVolumeChangeToMatchTarget( + recommended_input_volume_); } } diff --git a/modules/audio_processing/agc2/BUILD.gn b/modules/audio_processing/agc2/BUILD.gn index 220f4d0e9a..bd59ad3dae 100644 --- a/modules/audio_processing/agc2/BUILD.gn +++ b/modules/audio_processing/agc2/BUILD.gn @@ -208,6 +208,7 @@ rtc_library("input_volume_controller") { deps = [ ":clipping_predictor", ":gain_map", + ":input_volume_stats_reporter", "..:api", "..:audio_buffer", "..:audio_frame_view", diff --git a/modules/audio_processing/agc2/input_volume_controller.cc b/modules/audio_processing/agc2/input_volume_controller.cc index 549fb3e48e..46962ae220 100644 --- a/modules/audio_processing/agc2/input_volume_controller.cc +++ b/modules/audio_processing/agc2/input_volume_controller.cc @@ -15,6 +15,7 @@ #include "api/array_view.h" #include "modules/audio_processing/agc2/gain_map_internal.h" +#include "modules/audio_processing/agc2/input_volume_stats_reporter.h" #include "modules/audio_processing/include/audio_frame_view.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -536,9 +537,8 @@ void InputVolumeController::Process(float speech_probability, if (volume_after_clipping_handling != recommended_input_volume_) { // The recommended input volume was adjusted in order to match the target // level. - RTC_HISTOGRAM_COUNTS_LINEAR( - "WebRTC.Audio.Apm.RecommendedInputVolume.OnChangeToMatchTarget", - recommended_input_volume_, 1, kMaxInputVolume, 50); + UpdateHistogramOnRecommendedInputVolumeChangeToMatchTarget( + recommended_input_volume_); } } diff --git a/modules/audio_processing/agc2/input_volume_controller_unittest.cc b/modules/audio_processing/agc2/input_volume_controller_unittest.cc index cb1c7de230..29349872a4 100644 --- a/modules/audio_processing/agc2/input_volume_controller_unittest.cc +++ b/modules/audio_processing/agc2/input_volume_controller_unittest.cc @@ -1420,6 +1420,8 @@ TEST(InputVolumeControllerTest, SpeechProbabilityThresholdIsEffective) { TEST(InputVolumeControllerTest, DoNotLogRecommendedInputVolumeOnChangeToMatchTarget) { + metrics::Reset(); + SpeechSamplesReader reader; auto controller = CreateInputVolumeController(); controller->Initialize(); @@ -1439,6 +1441,8 @@ TEST(InputVolumeControllerTest, TEST(InputVolumeControllerTest, LogRecommendedInputVolumeOnUpwardChangeToMatchTarget) { + metrics::Reset(); + SpeechSamplesReader reader; auto controller = CreateInputVolumeController(); controller->Initialize(); @@ -1457,6 +1461,8 @@ TEST(InputVolumeControllerTest, TEST(InputVolumeControllerTest, LogRecommendedInputVolumeOnDownwardChangeToMatchTarget) { + metrics::Reset(); + SpeechSamplesReader reader; auto controller = CreateInputVolumeController(); controller->Initialize(); diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter.cc b/modules/audio_processing/agc2/input_volume_stats_reporter.cc index dfeb2185a9..05624b1f92 100644 --- a/modules/audio_processing/agc2/input_volume_stats_reporter.cc +++ b/modules/audio_processing/agc2/input_volume_stats_reporter.cc @@ -162,4 +162,10 @@ void InputVolumeStatsReporter::LogVolumeUpdateStats() const { } } +void UpdateHistogramOnRecommendedInputVolumeChangeToMatchTarget(int volume) { + RTC_HISTOGRAM_COUNTS_LINEAR( + "WebRTC.Audio.Apm.RecommendedInputVolume.OnChangeToMatchTarget", volume, + 1, kMaxInputVolume, 50); +} + } // namespace webrtc diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter.h b/modules/audio_processing/agc2/input_volume_stats_reporter.h index 0037a9a5ab..31b110031c 100644 --- a/modules/audio_processing/agc2/input_volume_stats_reporter.h +++ b/modules/audio_processing/agc2/input_volume_stats_reporter.h @@ -85,6 +85,12 @@ class InputVolumeStatsReporter { int log_volume_update_stats_counter_ = 0; absl::optional previous_input_volume_ = absl::nullopt; }; + +// Updates the histogram that keeps track of recommended input volume changes +// required in order to match the target level in the input volume adaptation +// process. +void UpdateHistogramOnRecommendedInputVolumeChangeToMatchTarget(int volume); + } // namespace webrtc #endif // MODULES_AUDIO_PROCESSING_AGC2_INPUT_VOLUME_STATS_REPORTER_H_