From 03ad9b892c15a506119002c9a93bd7ae6a4f1bb4 Mon Sep 17 00:00:00 2001 From: Alex Loiko Date: Mon, 13 Aug 2018 17:40:43 +0200 Subject: [PATCH] Fine-grained limiter metrics. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FixedGainController is used in two places. One is the AudioMixer. There it's used to limit the audio level after adding streams. The other is GainController2, where it's placed after steps that could boost the audio level outside the allowed range. We log metrics from the FGC. To avoid confusion, this CL makes the two use cases log to different histograms. Chromium histogram CL is https://chromium-review.googlesource.com/c/chromium/src/+/1170833 Bug: webrtc:7494 Change-Id: I1abe60fd8e96556f144d2ee576254b15beca1174 Reviewed-on: https://webrtc-review.googlesource.com/93464 Commit-Queue: Alex Loiko Reviewed-by: Ivo Creusen Reviewed-by: Åsa Persson Cr-Commit-Position: refs/heads/master@{#24284} --- modules/audio_mixer/frame_combiner.cc | 2 +- modules/audio_processing/agc2/BUILD.gn | 1 + .../agc2/fixed_gain_controller.cc | 11 +- .../agc2/fixed_gain_controller.h | 2 + .../agc2/fixed_gain_controller_unittest.cc | 41 ++++++- .../agc2/gain_curve_applier.cc | 5 +- .../agc2/gain_curve_applier.h | 4 +- .../agc2/gain_curve_applier_unittest.cc | 4 +- .../agc2/interpolated_gain_curve.cc | 112 ++++++++++++------ .../agc2/interpolated_gain_curve.h | 23 +++- .../agc2/interpolated_gain_curve_unittest.cc | 20 ++-- 11 files changed, 167 insertions(+), 58 deletions(-) diff --git a/modules/audio_mixer/frame_combiner.cc b/modules/audio_mixer/frame_combiner.cc index 25e2194744..4759c3a40f 100644 --- a/modules/audio_mixer/frame_combiner.cc +++ b/modules/audio_mixer/frame_combiner.cc @@ -116,7 +116,7 @@ void InterleaveToAudioFrame(AudioFrameView mixing_buffer_view, FrameCombiner::FrameCombiner(bool use_limiter) : data_dumper_(new ApmDataDumper(0)), - limiter_(data_dumper_.get()), + limiter_(data_dumper_.get(), "AudioMixer"), use_limiter_(use_limiter) { limiter_.SetGain(0.f); } diff --git a/modules/audio_processing/agc2/BUILD.gn b/modules/audio_processing/agc2/BUILD.gn index d26d914146..c56689b2a6 100644 --- a/modules/audio_processing/agc2/BUILD.gn +++ b/modules/audio_processing/agc2/BUILD.gn @@ -231,6 +231,7 @@ rtc_source_set("fixed_digital_unittests") { "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", "../../../rtc_base:rtc_base_tests_utils", + "../../../system_wrappers:metrics_default", "//third_party/abseil-cpp/absl/memory", ] } diff --git a/modules/audio_processing/agc2/fixed_gain_controller.cc b/modules/audio_processing/agc2/fixed_gain_controller.cc index e59ae346fe..6aa390ea36 100644 --- a/modules/audio_processing/agc2/fixed_gain_controller.cc +++ b/modules/audio_processing/agc2/fixed_gain_controller.cc @@ -34,8 +34,17 @@ bool CloseToOne(float gain_factor) { } // namespace FixedGainController::FixedGainController(ApmDataDumper* apm_data_dumper) + : FixedGainController(apm_data_dumper, "Agc2") {} + +FixedGainController::FixedGainController(ApmDataDumper* apm_data_dumper, + std::string histogram_name_prefix) : apm_data_dumper_(apm_data_dumper), - gain_curve_applier_(48000, apm_data_dumper_) {} + gain_curve_applier_(48000, apm_data_dumper_, histogram_name_prefix) { + // Do update histograms.xml when adding name prefixes. + RTC_DCHECK(histogram_name_prefix == "" || histogram_name_prefix == "Test" || + histogram_name_prefix == "AudioMixer" || + histogram_name_prefix == "Agc2"); +} void FixedGainController::SetGain(float gain_to_apply_db) { // Changes in gain_to_apply_ cause discontinuities. We assume diff --git a/modules/audio_processing/agc2/fixed_gain_controller.h b/modules/audio_processing/agc2/fixed_gain_controller.h index 2b92cfcfca..a41a13f516 100644 --- a/modules/audio_processing/agc2/fixed_gain_controller.h +++ b/modules/audio_processing/agc2/fixed_gain_controller.h @@ -20,6 +20,8 @@ class ApmDataDumper; class FixedGainController { public: explicit FixedGainController(ApmDataDumper* apm_data_dumper); + FixedGainController(ApmDataDumper* apm_data_dumper, + std::string histogram_name_prefix); void Process(AudioFrameView signal); diff --git a/modules/audio_processing/agc2/fixed_gain_controller_unittest.cc b/modules/audio_processing/agc2/fixed_gain_controller_unittest.cc index 72bb409a3b..fa68a01a4b 100644 --- a/modules/audio_processing/agc2/fixed_gain_controller_unittest.cc +++ b/modules/audio_processing/agc2/fixed_gain_controller_unittest.cc @@ -16,6 +16,7 @@ #include "modules/audio_processing/agc2/vector_float_frame.h" #include "modules/audio_processing/logging/apm_data_dumper.h" #include "rtc_base/gunit.h" +#include "system_wrappers/include/metrics_default.h" namespace webrtc { namespace { @@ -49,14 +50,22 @@ ApmDataDumper test_data_dumper(0); std::unique_ptr CreateFixedGainController( float gain_to_apply, - size_t rate) { + size_t rate, + std::string histogram_name_prefix) { std::unique_ptr fgc = - absl::make_unique(&test_data_dumper); + absl::make_unique(&test_data_dumper, + histogram_name_prefix); fgc->SetGain(gain_to_apply); fgc->SetSampleRate(rate); return fgc; } +std::unique_ptr CreateFixedGainController( + float gain_to_apply, + size_t rate) { + return CreateFixedGainController(gain_to_apply, rate, ""); +} + } // namespace TEST(AutomaticGainController2FixedDigital, CreateUse) { @@ -173,4 +182,32 @@ TEST(AutomaticGainController2FixedDigital, GainShouldChangeOnSetGain) { kInputLevel * 10); } +TEST(AutomaticGainController2FixedDigital, RegionHistogramIsUpdated) { + constexpr size_t kSampleRate = 8000; + constexpr float kGainDb = 0.f; + constexpr float kInputLevel = 1000.f; + constexpr size_t kNumFrames = 5; + + metrics::Reset(); + + std::unique_ptr fixed_gc_no_saturation = + CreateFixedGainController(kGainDb, kSampleRate, "Test"); + + static_cast(RunFixedGainControllerWithConstantInput( + fixed_gc_no_saturation.get(), kInputLevel, kNumFrames, kSampleRate)); + + // Destroying FixedGainController should cause the last limiter region to be + // logged. + fixed_gc_no_saturation.reset(); + + EXPECT_EQ(1, metrics::NumSamples( + "WebRTC.Audio.Test.FixedDigitalGainCurveRegion.Identity")); + EXPECT_EQ(0, metrics::NumSamples( + "WebRTC.Audio.Test.FixedDigitalGainCurveRegion.Knee")); + EXPECT_EQ(0, metrics::NumSamples( + "WebRTC.Audio.Test.FixedDigitalGainCurveRegion.Limiter")); + EXPECT_EQ(0, metrics::NumSamples( + "WebRTC.Audio.Test.FixedDigitalGainCurveRegion.Saturation")); +} + } // namespace webrtc diff --git a/modules/audio_processing/agc2/gain_curve_applier.cc b/modules/audio_processing/agc2/gain_curve_applier.cc index 122839acc8..7c1410b232 100644 --- a/modules/audio_processing/agc2/gain_curve_applier.cc +++ b/modules/audio_processing/agc2/gain_curve_applier.cc @@ -84,8 +84,9 @@ void ScaleSamples(rtc::ArrayView per_sample_scaling_factors, } // namespace GainCurveApplier::GainCurveApplier(size_t sample_rate_hz, - ApmDataDumper* apm_data_dumper) - : interp_gain_curve_(apm_data_dumper), + ApmDataDumper* apm_data_dumper, + std::string histogram_name) + : interp_gain_curve_(apm_data_dumper, histogram_name), level_estimator_(sample_rate_hz, apm_data_dumper), apm_data_dumper_(apm_data_dumper) {} diff --git a/modules/audio_processing/agc2/gain_curve_applier.h b/modules/audio_processing/agc2/gain_curve_applier.h index 86ca251948..f546f6325d 100644 --- a/modules/audio_processing/agc2/gain_curve_applier.h +++ b/modules/audio_processing/agc2/gain_curve_applier.h @@ -23,7 +23,9 @@ class ApmDataDumper; class GainCurveApplier { public: - GainCurveApplier(size_t sample_rate_hz, ApmDataDumper* apm_data_dumper); + GainCurveApplier(size_t sample_rate_hz, + ApmDataDumper* apm_data_dumper, + std::string histogram_name_prefix); ~GainCurveApplier(); diff --git a/modules/audio_processing/agc2/gain_curve_applier_unittest.cc b/modules/audio_processing/agc2/gain_curve_applier_unittest.cc index d9179a4c43..0f75f62c40 100644 --- a/modules/audio_processing/agc2/gain_curve_applier_unittest.cc +++ b/modules/audio_processing/agc2/gain_curve_applier_unittest.cc @@ -23,7 +23,7 @@ TEST(GainCurveApplier, GainCurveApplierShouldConstructAndRun) { const int sample_rate_hz = 48000; ApmDataDumper apm_data_dumper(0); - GainCurveApplier gain_curve_applier(sample_rate_hz, &apm_data_dumper); + GainCurveApplier gain_curve_applier(sample_rate_hz, &apm_data_dumper, ""); VectorFloatFrame vectors_with_float_frame(1, sample_rate_hz / 100, kMaxAbsFloatS16Value); @@ -37,7 +37,7 @@ TEST(GainCurveApplier, OutputVolumeAboveThreshold) { 2.f; ApmDataDumper apm_data_dumper(0); - GainCurveApplier gain_curve_applier(sample_rate_hz, &apm_data_dumper); + GainCurveApplier gain_curve_applier(sample_rate_hz, &apm_data_dumper, ""); // Give the level estimator time to adapt. for (int i = 0; i < 5; ++i) { diff --git a/modules/audio_processing/agc2/interpolated_gain_curve.cc b/modules/audio_processing/agc2/interpolated_gain_curve.cc index 7f1f16cba4..73e6a8ecc3 100644 --- a/modules/audio_processing/agc2/interpolated_gain_curve.cc +++ b/modules/audio_processing/agc2/interpolated_gain_curve.cc @@ -14,41 +14,8 @@ #include "modules/audio_processing/logging/apm_data_dumper.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" -#include "system_wrappers/include/metrics.h" namespace webrtc { -namespace { -void LogRegionStats(const InterpolatedGainCurve::Stats& stats) { - using Region = InterpolatedGainCurve::GainCurveRegion; - const int duration_s = - stats.region_duration_frames / (1000 / kFrameDurationMs); - - switch (stats.region) { - case Region::kIdentity: { - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.Audio.Agc2.FixedDigitalGainCurveRegion.Identity", duration_s); - break; - } - case Region::kKnee: { - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.Audio.Agc2.FixedDigitalGainCurveRegion.Knee", duration_s); - break; - } - case Region::kLimiter: { - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.Audio.Agc2.FixedDigitalGainCurveRegion.Limiter", duration_s); - break; - } - case Region::kSaturation: { - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.Audio.Agc2.FixedDigitalGainCurveRegion.Saturation", - duration_s); - break; - } - default: { RTC_NOTREACHED(); } - } -} -} // namespace constexpr std::array InterpolatedGainCurve::approximation_params_x_; @@ -59,8 +26,17 @@ constexpr std::array constexpr std::array InterpolatedGainCurve::approximation_params_q_; -InterpolatedGainCurve::InterpolatedGainCurve(ApmDataDumper* apm_data_dumper) - : apm_data_dumper_(apm_data_dumper) {} +InterpolatedGainCurve::InterpolatedGainCurve(ApmDataDumper* apm_data_dumper, + std::string histogram_name_prefix) + : region_logger_("WebRTC.Audio." + histogram_name_prefix + + ".FixedDigitalGainCurveRegion.Identity", + "WebRTC.Audio." + histogram_name_prefix + + ".FixedDigitalGainCurveRegion.Knee", + "WebRTC.Audio." + histogram_name_prefix + + ".FixedDigitalGainCurveRegion.Limiter", + "WebRTC.Audio." + histogram_name_prefix + + ".FixedDigitalGainCurveRegion.Saturation"), + apm_data_dumper_(apm_data_dumper) {} InterpolatedGainCurve::~InterpolatedGainCurve() { if (stats_.available) { @@ -73,7 +49,69 @@ InterpolatedGainCurve::~InterpolatedGainCurve() { stats_.look_ups_limiter_region); apm_data_dumper_->DumpRaw("agc2_interp_gain_curve_lookups_saturation", stats_.look_ups_saturation_region); - LogRegionStats(stats_); + region_logger_.LogRegionStats(stats_); + } +} + +InterpolatedGainCurve::RegionLogger::RegionLogger( + std::string identity_histogram_name, + std::string knee_histogram_name, + std::string limiter_histogram_name, + std::string saturation_histogram_name) + : identity_histogram( + metrics::HistogramFactoryGetCounts(identity_histogram_name, + 1, + 10000, + 50)), + knee_histogram(metrics::HistogramFactoryGetCounts(knee_histogram_name, + 1, + 10000, + 50)), + limiter_histogram( + metrics::HistogramFactoryGetCounts(limiter_histogram_name, + 1, + 10000, + 50)), + saturation_histogram( + metrics::HistogramFactoryGetCounts(saturation_histogram_name, + 1, + 10000, + 50)) {} + +InterpolatedGainCurve::RegionLogger::~RegionLogger() = default; + +void InterpolatedGainCurve::RegionLogger::LogRegionStats( + const InterpolatedGainCurve::Stats& stats) const { + using Region = InterpolatedGainCurve::GainCurveRegion; + const int duration_s = + stats.region_duration_frames / (1000 / kFrameDurationMs); + + switch (stats.region) { + case Region::kIdentity: { + if (identity_histogram) { + metrics::HistogramAdd(identity_histogram, duration_s); + } + break; + } + case Region::kKnee: { + if (knee_histogram) { + metrics::HistogramAdd(knee_histogram, duration_s); + } + break; + } + case Region::kLimiter: { + if (limiter_histogram) { + metrics::HistogramAdd(limiter_histogram, duration_s); + } + break; + } + case Region::kSaturation: { + if (saturation_histogram) { + metrics::HistogramAdd(saturation_histogram, duration_s); + } + break; + } + default: { RTC_NOTREACHED(); } } } @@ -100,7 +138,7 @@ void InterpolatedGainCurve::UpdateStats(float input_level) const { if (region == stats_.region) { ++stats_.region_duration_frames; } else { - LogRegionStats(stats_); + region_logger_.LogRegionStats(stats_); stats_.region_duration_frames = 0; stats_.region = region; diff --git a/modules/audio_processing/agc2/interpolated_gain_curve.h b/modules/audio_processing/agc2/interpolated_gain_curve.h index fc5842bd2c..68d4532238 100644 --- a/modules/audio_processing/agc2/interpolated_gain_curve.h +++ b/modules/audio_processing/agc2/interpolated_gain_curve.h @@ -12,11 +12,13 @@ #define MODULES_AUDIO_PROCESSING_AGC2_INTERPOLATED_GAIN_CURVE_H_ #include +#include #include "modules/audio_processing/agc2/agc2_common.h" #include "rtc_base/constructormagic.h" #include "rtc_base/gtest_prod_util.h" +#include "system_wrappers/include/metrics.h" namespace webrtc { @@ -59,8 +61,8 @@ class InterpolatedGainCurve { int64_t region_duration_frames = 0; }; - // InterpolatedGainCurve(InterpolatedGainCurve&&); - explicit InterpolatedGainCurve(ApmDataDumper* apm_data_dumper); + InterpolatedGainCurve(ApmDataDumper* apm_data_dumper, + std::string histogram_name_prefix); ~InterpolatedGainCurve(); Stats get_stats() const { return stats_; } @@ -76,6 +78,23 @@ class InterpolatedGainCurve { // ComputeInterpolatedGainCurve. FRIEND_TEST_ALL_PREFIXES(AutomaticGainController2InterpolatedGainCurve, CheckApproximationParams); + + struct RegionLogger { + metrics::Histogram* identity_histogram; + metrics::Histogram* knee_histogram; + metrics::Histogram* limiter_histogram; + metrics::Histogram* saturation_histogram; + + RegionLogger(std::string identity_histogram_name, + std::string knee_histogram_name, + std::string limiter_histogram_name, + std::string saturation_histogram_name); + + ~RegionLogger(); + + void LogRegionStats(const InterpolatedGainCurve::Stats& stats) const; + } region_logger_; + void UpdateStats(float input_level) const; ApmDataDumper* const apm_data_dumper_; diff --git a/modules/audio_processing/agc2/interpolated_gain_curve_unittest.cc b/modules/audio_processing/agc2/interpolated_gain_curve_unittest.cc index d75ddbd7f7..dd696313ae 100644 --- a/modules/audio_processing/agc2/interpolated_gain_curve_unittest.cc +++ b/modules/audio_processing/agc2/interpolated_gain_curve_unittest.cc @@ -32,7 +32,7 @@ const Limiter limiter; } // namespace TEST(AutomaticGainController2InterpolatedGainCurve, CreateUse) { - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( kLevelEpsilon, DbfsToFloatS16(limiter.max_input_level_db() + 1), 500); @@ -42,7 +42,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CreateUse) { } TEST(AutomaticGainController2InterpolatedGainCurve, CheckValidOutput) { - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( kLevelEpsilon, limiter.max_input_level_linear() * 2.0, 500); @@ -55,7 +55,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckValidOutput) { } TEST(AutomaticGainController2InterpolatedGainCurve, CheckMonotonicity) { - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( kLevelEpsilon, limiter.max_input_level_linear() + kLevelEpsilon + 0.5, @@ -69,7 +69,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckMonotonicity) { } TEST(AutomaticGainController2InterpolatedGainCurve, CheckApproximation) { - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( kLevelEpsilon, limiter.max_input_level_linear() - kLevelEpsilon, 500); @@ -82,7 +82,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckApproximation) { } TEST(AutomaticGainController2InterpolatedGainCurve, CheckRegionBoundaries) { - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const std::vector levels{ {kLevelEpsilon, limiter.knee_start_linear() + kLevelEpsilon, @@ -101,7 +101,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckRegionBoundaries) { TEST(AutomaticGainController2InterpolatedGainCurve, CheckIdentityRegion) { constexpr size_t kNumSteps = 10; - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace(kLevelEpsilon, limiter.knee_start_linear(), kNumSteps); @@ -120,7 +120,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckIdentityRegion) { TEST(AutomaticGainController2InterpolatedGainCurve, CheckNoOverApproximationKnee) { constexpr size_t kNumSteps = 10; - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace(limiter.knee_start_linear() + kLevelEpsilon, @@ -142,7 +142,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, TEST(AutomaticGainController2InterpolatedGainCurve, CheckNoOverApproximationBeyondKnee) { constexpr size_t kNumSteps = 10; - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( limiter.limiter_start_linear() + kLevelEpsilon, @@ -164,7 +164,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, TEST(AutomaticGainController2InterpolatedGainCurve, CheckNoOverApproximationWithSaturation) { constexpr size_t kNumSteps = 3; - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); const auto levels = test::LinSpace( limiter.max_input_level_linear() + kLevelEpsilon, @@ -185,7 +185,7 @@ TEST(AutomaticGainController2InterpolatedGainCurve, CheckApproximationParams) { test::InterpolatedParameters parameters = test::ComputeInterpolatedGainCurveApproximationParams(); - InterpolatedGainCurve igc(&apm_data_dumper); + InterpolatedGainCurve igc(&apm_data_dumper, ""); for (size_t i = 0; i < kInterpolatedGainCurveTotalPoints; ++i) { // The tolerance levels are chosen to account for deviations due