From 250155d0dbc55eacf122a99271126d52b609c306 Mon Sep 17 00:00:00 2001 From: Alex Loiko Date: Mon, 26 Mar 2018 14:36:46 +0200 Subject: [PATCH] Fix histogram logging in InterpolatedGainCurve. We had the following pattern: if (case_A) metric = METRIC_A; if (case_B) metric = METRIC_B; RTC_HISTOGRAM_COUNTS_10000(metric, value); That's wrong, because once the logging macro runs once, it will use the same histogram no matter what the first argument is. The macro expands into roughly static Histogram* histogram_ptr = nullptr; if (histogram_ptr == nullptr) { // Look up the histogram and put in histogram_ptr } // Add data through the histogram pointer. We change the logging to use macros with string literals. We add a macro for every of the 4 possible invocations. The macros will expand to one static pointer each. Bug: webrtc:8925 Change-Id: Ic7e4a6299eff31dd5988047edfcedce7d369e5ce Reviewed-on: https://webrtc-review.googlesource.com/64724 Reviewed-by: Sam Zackrisson Reviewed-by: Alessio Bazzica Commit-Queue: Alex Loiko Cr-Commit-Position: refs/heads/master@{#22606} --- .../agc2/interpolated_gain_curve.cc | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/modules/audio_processing/agc2/interpolated_gain_curve.cc b/modules/audio_processing/agc2/interpolated_gain_curve.cc index aac533d83f..7f1f16cba4 100644 --- a/modules/audio_processing/agc2/interpolated_gain_curve.cc +++ b/modules/audio_processing/agc2/interpolated_gain_curve.cc @@ -20,19 +20,33 @@ namespace webrtc { namespace { void LogRegionStats(const InterpolatedGainCurve::Stats& stats) { using Region = InterpolatedGainCurve::GainCurveRegion; + const int duration_s = + stats.region_duration_frames / (1000 / kFrameDurationMs); - std::string histogram_name = "WebRTC.Audio.Agc2.FixedDigitalGainCurveRegion."; - if (stats.region == Region::kIdentity) { - histogram_name += "Identity"; - } else if (stats.region == Region::kKnee) { - histogram_name += "Knee"; - } else if (stats.region == Region::kLimiter) { - histogram_name += "Limiter"; - } else { - histogram_name += "Saturation"; + 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(); } } - RTC_HISTOGRAM_COUNTS_10000(histogram_name, - stats.region_duration_frames / 100); } } // namespace