Revert "APM: log both applied and recommended input volume stats"
This reverts commit 8d7273357d92fab881561d886ce8dfe94e6e2238.
Reason for revert: revert needed to land https://webrtc-review.googlesource.com/c/src/+/280600
Original change's description:
> 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 <silen@webrtc.org>
> Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38468}
Bug: webrtc:7494
Change-Id: I4a2acfd5a983d9397932b2879cfa057deaf0eb2b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280581
Commit-Queue: Alessio Bazzica <alessiob@webrtc.org>
Auto-Submit: Alessio Bazzica <alessiob@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#38476}
diff --git a/modules/audio_processing/agc2/BUILD.gn b/modules/audio_processing/agc2/BUILD.gn
index 0e20ae6..bc15076 100644
--- a/modules/audio_processing/agc2/BUILD.gn
+++ b/modules/audio_processing/agc2/BUILD.gn
@@ -436,9 +436,7 @@
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 6d70186..4a8b016 100644
--- a/modules/audio_processing/agc2/input_volume_stats_reporter.cc
+++ b/modules/audio_processing/agc2/input_volume_stats_reporter.cc
@@ -19,8 +19,6 @@
namespace webrtc {
namespace {
-using InputVolumeType = InputVolumeStatsReporter::InputVolumeType;
-
constexpr int kFramesIn60Seconds = 6000;
constexpr int kMinInputVolume = 0;
constexpr int kMaxInputVolume = 255;
@@ -37,102 +35,9 @@
return std::round(static_cast<float>(sum_updates) /
static_cast<float>(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(
- 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;
InputVolumeStatsReporter::~InputVolumeStatsReporter() = default;
@@ -169,19 +74,47 @@
const float average_update = ComputeAverageUpdate(
volume_update_stats_.sum_decreases + volume_update_stats_.sum_increases,
num_updates);
- metrics::HistogramAdd(decrease_rate_histogram_,
- volume_update_stats_.num_decreases);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainDecreaseRate",
+ /*sample=*/volume_update_stats_.num_decreases,
+ /*min=*/1,
+ /*max=*/kFramesIn60Seconds,
+ /*bucket_count=*/50);
if (volume_update_stats_.num_decreases > 0) {
- metrics::HistogramAdd(decrease_average_histogram_, average_decrease);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainDecreaseAverage",
+ /*sample=*/average_decrease,
+ /*min=*/1,
+ /*max=*/kMaxUpdate,
+ /*bucket_count=*/50);
}
- metrics::HistogramAdd(increase_rate_histogram_,
- volume_update_stats_.num_increases);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainIncreaseRate",
+ /*sample=*/volume_update_stats_.num_increases,
+ /*min=*/1,
+ /*max=*/kFramesIn60Seconds,
+ /*bucket_count=*/50);
if (volume_update_stats_.num_increases > 0) {
- metrics::HistogramAdd(increase_average_histogram_, average_increase);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainIncreaseAverage",
+ /*sample=*/average_increase,
+ /*min=*/1,
+ /*max=*/kMaxUpdate,
+ /*bucket_count=*/50);
}
- metrics::HistogramAdd(update_rate_histogram_, num_updates);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainUpdateRate",
+ /*sample=*/num_updates,
+ /*min=*/1,
+ /*max=*/kFramesIn60Seconds,
+ /*bucket_count=*/50);
if (num_updates > 0) {
- metrics::HistogramAdd(update_average_histogram_, average_update);
+ RTC_HISTOGRAM_COUNTS_LINEAR(
+ /*name=*/"WebRTC.Audio.ApmAnalogGainUpdateAverage",
+ /*sample=*/average_update,
+ /*min=*/1,
+ /*max=*/kMaxUpdate,
+ /*bucket_count=*/50);
}
}
diff --git a/modules/audio_processing/agc2/input_volume_stats_reporter.h b/modules/audio_processing/agc2/input_volume_stats_reporter.h
index 757e6ff..0040754 100644
--- a/modules/audio_processing/agc2/input_volume_stats_reporter.h
+++ b/modules/audio_processing/agc2/input_volume_stats_reporter.h
@@ -13,7 +13,6 @@
#include "absl/types/optional.h"
#include "rtc_base/gtest_prod_util.h"
-#include "system_wrappers/include/metrics.h"
namespace webrtc {
@@ -22,12 +21,7 @@
// the statistics into a histogram.
class InputVolumeStatsReporter {
public:
- enum class InputVolumeType {
- kApplied = 0,
- kRecommended = 1,
- };
-
- explicit InputVolumeStatsReporter(InputVolumeType input_volume_type);
+ InputVolumeStatsReporter();
InputVolumeStatsReporter(const InputVolumeStatsReporter&) = delete;
InputVolumeStatsReporter operator=(const InputVolumeStatsReporter&) = delete;
~InputVolumeStatsReporter();
@@ -63,14 +57,6 @@
// 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<int> 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 a3e2cca..f8cd010 100644
--- a/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc
+++ b/modules/audio_processing/agc2/input_volume_stats_reporter_unittest.cc
@@ -10,71 +10,24 @@
#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;
-constexpr absl::string_view kLabelPrefix = "WebRTC.Audio.Apm.";
-
-class InputVolumeStatsReporterTest
- : public ::testing::TestWithParam<InputVolumeType> {
+class InputVolumeStatsReporterTest : public ::testing::Test {
public:
- InputVolumeStatsReporterTest() { metrics::Reset(); }
+ InputVolumeStatsReporterTest() {}
protected:
- 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.";
- }
- }
+ void SetUp() override { metrics::Reset(); }
};
-TEST_P(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsEmpty) {
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsEmpty) {
+ InputVolumeStatsReporter stats_reporter;
constexpr int kInputVolume = 10;
stats_reporter.UpdateStatistics(kInputVolume);
// Update almost until the periodic logging and reset.
@@ -82,22 +35,25 @@
stats_reporter.UpdateStatistics(kInputVolume + 2);
stats_reporter.UpdateStatistics(kInputVolume);
}
- EXPECT_METRIC_THAT(metrics::Samples(UpdateRateLabel()),
+ EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateRate"),
::testing::ElementsAre());
- EXPECT_METRIC_THAT(metrics::Samples(DecreaseRateLabel()),
+ EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseRate"),
::testing::ElementsAre());
- EXPECT_METRIC_THAT(metrics::Samples(IncreaseRateLabel()),
+ EXPECT_METRIC_THAT(metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseRate"),
::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_P(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsNotEmpty) {
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+TEST_F(InputVolumeStatsReporterTest, CheckLogVolumeUpdateStatsNotEmpty) {
+ InputVolumeStatsReporter stats_reporter;
constexpr int kInputVolume = 10;
stats_reporter.UpdateStatistics(kInputVolume);
// Update until periodic logging.
@@ -111,30 +67,30 @@
stats_reporter.UpdateStatistics(kInputVolume);
}
EXPECT_METRIC_THAT(
- metrics::Samples(UpdateRateLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateRate"),
::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds - 1, 1),
::testing::Pair(kFramesIn60Seconds, 1)));
EXPECT_METRIC_THAT(
- metrics::Samples(DecreaseRateLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseRate"),
::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds / 2 - 1, 1),
::testing::Pair(kFramesIn60Seconds / 2, 1)));
EXPECT_METRIC_THAT(
- metrics::Samples(IncreaseRateLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseRate"),
::testing::ElementsAre(::testing::Pair(kFramesIn60Seconds / 2, 2)));
EXPECT_METRIC_THAT(
- metrics::Samples(UpdateAverageLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainUpdateAverage"),
::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1)));
EXPECT_METRIC_THAT(
- metrics::Samples(DecreaseAverageLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainDecreaseAverage"),
::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1)));
EXPECT_METRIC_THAT(
- metrics::Samples(IncreaseAverageLabel()),
+ metrics::Samples("WebRTC.Audio.ApmAnalogGainIncreaseAverage"),
::testing::ElementsAre(::testing::Pair(2, 1), ::testing::Pair(3, 1)));
}
} // namespace
-TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsForEmptyStats) {
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsForEmptyStats) {
+ InputVolumeStatsReporter stats_reporter;
const auto& update_stats = stats_reporter.volume_update_stats();
EXPECT_EQ(update_stats.num_decreases, 0);
EXPECT_EQ(update_stats.sum_decreases, 0);
@@ -142,10 +98,10 @@
EXPECT_EQ(update_stats.sum_increases, 0);
}
-TEST_P(InputVolumeStatsReporterTest,
+TEST_F(InputVolumeStatsReporterTest,
CheckVolumeUpdateStatsAfterNoVolumeChange) {
constexpr int kInputVolume = 10;
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+ InputVolumeStatsReporter stats_reporter;
stats_reporter.UpdateStatistics(kInputVolume);
stats_reporter.UpdateStatistics(kInputVolume);
stats_reporter.UpdateStatistics(kInputVolume);
@@ -156,10 +112,10 @@
EXPECT_EQ(update_stats.sum_increases, 0);
}
-TEST_P(InputVolumeStatsReporterTest,
+TEST_F(InputVolumeStatsReporterTest,
CheckVolumeUpdateStatsAfterVolumeIncrease) {
constexpr int kInputVolume = 10;
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+ InputVolumeStatsReporter stats_reporter;
stats_reporter.UpdateStatistics(kInputVolume);
stats_reporter.UpdateStatistics(kInputVolume + 4);
stats_reporter.UpdateStatistics(kInputVolume + 5);
@@ -170,10 +126,10 @@
EXPECT_EQ(update_stats.sum_increases, 5);
}
-TEST_P(InputVolumeStatsReporterTest,
+TEST_F(InputVolumeStatsReporterTest,
CheckVolumeUpdateStatsAfterVolumeDecrease) {
constexpr int kInputVolume = 10;
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+ InputVolumeStatsReporter stats_reporter;
stats_reporter.UpdateStatistics(kInputVolume);
stats_reporter.UpdateStatistics(kInputVolume - 4);
stats_reporter.UpdateStatistics(kInputVolume - 5);
@@ -184,8 +140,8 @@
EXPECT_EQ(stats_update.sum_increases, 0);
}
-TEST_P(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterReset) {
- InputVolumeStatsReporter stats_reporter(InputVolumeType());
+TEST_F(InputVolumeStatsReporterTest, CheckVolumeUpdateStatsAfterReset) {
+ InputVolumeStatsReporter stats_reporter;
constexpr int kInputVolume = 10;
stats_reporter.UpdateStatistics(kInputVolume);
// Update until the periodic reset.
@@ -213,9 +169,4 @@
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 a0415e2..76e3804 100644
--- a/modules/audio_processing/audio_processing_impl.cc
+++ b/modules/audio_processing/audio_processing_impl.cc
@@ -291,11 +291,7 @@
MinimizeProcessingForUnusedOutput(),
field_trial::IsEnabled("WebRTC-TransientSuppressorForcedOff")),
capture_(),
- capture_nonlocked_(),
- applied_input_volume_stats_reporter_(
- InputVolumeStatsReporter::InputVolumeType::kApplied),
- recommended_input_volume_stats_reporter_(
- InputVolumeStatsReporter::InputVolumeType::kRecommended) {
+ capture_nonlocked_() {
RTC_LOG(LS_INFO) << "Injected APM submodules:"
"\nEcho control factory: "
<< !!echo_control_factory_
@@ -1365,10 +1361,6 @@
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 5daea90..bc582b5 100644
--- a/modules/audio_processing/audio_processing_impl.h
+++ b/modules/audio_processing/audio_processing_impl.h
@@ -541,8 +541,6 @@
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<