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