From 093824c4d25a25f9b403b97acc7883e908fa3e91 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 13 Jun 2024 15:28:34 +0200 Subject: [PATCH] Switch away from hz to samples per channel for FrameCombiner et al MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This simplifies the following steps: * FrameCombiner infers the sample rate from channel size * Sends the inferred sample rate to FixedDigitalLevelEstimator and Limiter. * Those classes then convert the sample rate to channel size. Along the way perform checks that the derived channel size value is a legal value (which has already been done by FrameCombiner). To: * FrameCombiner sends channel size to FixedDigitalLevelEstimator and Limiter. Bug: chromium:335805780 Change-Id: I6d2953ba5ee99771f3ff5bf4f4a049a8a29b5577 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/352581 Reviewed-by: Per Åhgren Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#42480} --- modules/audio_mixer/frame_combiner.cc | 7 +--- modules/audio_processing/BUILD.gn | 1 + modules/audio_processing/agc2/BUILD.gn | 2 + .../agc2/fixed_digital_level_estimator.cc | 16 +++++--- .../agc2/fixed_digital_level_estimator.h | 16 +++++--- .../fixed_digital_level_estimator_unittest.cc | 41 +++++++++++-------- modules/audio_processing/agc2/limiter.cc | 29 ++++++------- modules/audio_processing/agc2/limiter.h | 28 +++++++++---- .../audio_processing/agc2/limiter_unittest.cc | 15 +++---- modules/audio_processing/gain_controller2.cc | 5 ++- 10 files changed, 95 insertions(+), 65 deletions(-) diff --git a/modules/audio_mixer/frame_combiner.cc b/modules/audio_mixer/frame_combiner.cc index b8d897e34e..a458bb1745 100644 --- a/modules/audio_mixer/frame_combiner.cc +++ b/modules/audio_mixer/frame_combiner.cc @@ -106,10 +106,7 @@ void MixToFloatFrame(rtc::ArrayView mix_list, } void RunLimiter(AudioFrameView mixing_buffer_view, Limiter* limiter) { - const size_t sample_rate = mixing_buffer_view.samples_per_channel() * 1000 / - AudioMixerImpl::kFrameDurationInMs; - // TODO(alessiob): Avoid calling SetSampleRate every time. - limiter->SetSampleRate(sample_rate); + limiter->SetSamplesPerChannel(mixing_buffer_view.samples_per_channel()); limiter->Process(mixing_buffer_view); } @@ -134,7 +131,7 @@ constexpr size_t FrameCombiner::kMaximumChannelSize; FrameCombiner::FrameCombiner(bool use_limiter) : data_dumper_(new ApmDataDumper(0)), - limiter_(static_cast(48000), data_dumper_.get(), "AudioMixer"), + limiter_(data_dumper_.get(), kMaximumChannelSize, "AudioMixer"), use_limiter_(use_limiter) { static_assert(kMaximumChannelSize * kMaximumNumberOfChannels <= AudioFrame::kMaxDataSizeSamples, diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn index 2f32a4a6d8..7959fdbd0d 100644 --- a/modules/audio_processing/BUILD.gn +++ b/modules/audio_processing/BUILD.gn @@ -108,6 +108,7 @@ rtc_library("gain_controller2") { ":apm_logging", ":audio_buffer", ":audio_frame_view", + "../../api/audio:audio_frame_api", "../../api/audio:audio_processing", "../../common_audio", "../../rtc_base:checks", diff --git a/modules/audio_processing/agc2/BUILD.gn b/modules/audio_processing/agc2/BUILD.gn index 0635120c37..31cc110e9b 100644 --- a/modules/audio_processing/agc2/BUILD.gn +++ b/modules/audio_processing/agc2/BUILD.gn @@ -148,6 +148,7 @@ rtc_library("fixed_digital") { "..:apm_logging", "..:audio_frame_view", "../../../api:array_view", + "../../../api/audio:audio_frame_api", "../../../common_audio", "../../../rtc_base:checks", "../../../rtc_base:gtest_prod", @@ -388,6 +389,7 @@ rtc_library("fixed_digital_unittests") { "..:apm_logging", "..:audio_frame_view", "../../../api:array_view", + "../../../api/audio:audio_frame_api", "../../../common_audio", "../../../rtc_base:checks", "../../../rtc_base:gunit_helpers", diff --git a/modules/audio_processing/agc2/fixed_digital_level_estimator.cc b/modules/audio_processing/agc2/fixed_digital_level_estimator.cc index 1995b24913..a927b9f7ca 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator.cc +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator.cc @@ -14,6 +14,7 @@ #include #include "api/array_view.h" +#include "api/audio/audio_frame.h" #include "modules/audio_processing/logging/apm_data_dumper.h" #include "rtc_base/checks.h" @@ -34,14 +35,17 @@ constexpr float kDecayFilterConstant = 0.9971259f; } // namespace FixedDigitalLevelEstimator::FixedDigitalLevelEstimator( - int sample_rate_hz, + size_t samples_per_channel, ApmDataDumper* apm_data_dumper) : apm_data_dumper_(apm_data_dumper), filter_state_level_(kInitialFilterStateLevel) { - SetSampleRate(sample_rate_hz); + SetSamplesPerChannel(samples_per_channel); CheckParameterCombination(); RTC_DCHECK(apm_data_dumper_); - apm_data_dumper_->DumpRaw("agc2_level_estimator_samplerate", sample_rate_hz); + // Convert `samples_per_channel` to sample rate for + // `agc2_level_estimator_samplerate`. + apm_data_dumper_->DumpRaw("agc2_level_estimator_samplerate", + samples_per_channel * kDefaultAudioBuffersPerSec); } void FixedDigitalLevelEstimator::CheckParameterCombination() { @@ -106,9 +110,9 @@ std::array FixedDigitalLevelEstimator::ComputeLevel( return envelope; } -void FixedDigitalLevelEstimator::SetSampleRate(int sample_rate_hz) { - samples_in_frame_ = - rtc::CheckedDivExact(sample_rate_hz * kFrameDurationMs, 1000); +void FixedDigitalLevelEstimator::SetSamplesPerChannel( + size_t samples_per_channel) { + samples_in_frame_ = static_cast(samples_per_channel); samples_in_sub_frame_ = rtc::CheckedDivExact(samples_in_frame_, kSubFramesInFrame); CheckParameterCombination(); diff --git a/modules/audio_processing/agc2/fixed_digital_level_estimator.h b/modules/audio_processing/agc2/fixed_digital_level_estimator.h index d26b55950c..eee6428d30 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator.h +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator.h @@ -25,12 +25,16 @@ class ApmDataDumper; // filtering. class FixedDigitalLevelEstimator { public: - // Sample rates are allowed if the number of samples in a frame - // (sample_rate_hz * kFrameDurationMs / 1000) is divisible by + // `samples_per_channel` is expected to be derived from this formula: + // sample_rate_hz * kFrameDurationMs / 1000 + // or + // sample_rate_hz / 100 + // I.e. the number of samples for 10ms of the given sample rate. The + // expectation is that samples per channel is divisible by // kSubFramesInSample. For kFrameDurationMs=10 and - // kSubFramesInSample=20, this means that sample_rate_hz has to be - // divisible by 2000. - FixedDigitalLevelEstimator(int sample_rate_hz, + // kSubFramesInSample=20, this means that the original sample rate has to be + // divisible by 2000 and therefore `samples_per_channel` by 20. + FixedDigitalLevelEstimator(size_t samples_per_channel, ApmDataDumper* apm_data_dumper); FixedDigitalLevelEstimator(const FixedDigitalLevelEstimator&) = delete; @@ -46,7 +50,7 @@ class FixedDigitalLevelEstimator { // Rate may be changed at any time (but not concurrently) from the // value passed to the constructor. The class is not thread safe. - void SetSampleRate(int sample_rate_hz); + void SetSamplesPerChannel(size_t samples_per_channel); // Resets the level estimator internal state. void Reset(); diff --git a/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc b/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc index 97b421d04c..d83f38056b 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc @@ -12,6 +12,7 @@ #include +#include "api/audio/audio_frame.h" #include "common_audio/include/audio_util.h" #include "modules/audio_processing/agc2/agc2_common.h" #include "modules/audio_processing/agc2/agc2_testing_common.h" @@ -26,17 +27,17 @@ constexpr float kInputLevel = 10000.f; // Run audio at specified settings through the level estimator, and // verify that the output level falls within the bounds. -void TestLevelEstimator(int sample_rate_hz, +void TestLevelEstimator(size_t samples_per_channel, int num_channels, float input_level_linear_scale, float expected_min, float expected_max) { ApmDataDumper apm_data_dumper(0); - FixedDigitalLevelEstimator level_estimator(sample_rate_hz, &apm_data_dumper); + FixedDigitalLevelEstimator level_estimator(samples_per_channel, + &apm_data_dumper); const VectorFloatFrame vectors_with_float_frame( - num_channels, rtc::CheckedDivExact(sample_rate_hz, 100), - input_level_linear_scale); + num_channels, samples_per_channel, input_level_linear_scale); for (int i = 0; i < 500; ++i) { const auto level = level_estimator.ComputeLevel( @@ -56,7 +57,7 @@ void TestLevelEstimator(int sample_rate_hz, // Returns time it takes for the level estimator to decrease its level // estimate by 'level_reduction_db'. -float TimeMsToDecreaseLevel(int sample_rate_hz, +float TimeMsToDecreaseLevel(size_t samples_per_channel, int num_channels, float input_level_db, float level_reduction_db) { @@ -64,10 +65,11 @@ float TimeMsToDecreaseLevel(int sample_rate_hz, RTC_DCHECK_GT(level_reduction_db, 0); const VectorFloatFrame vectors_with_float_frame( - num_channels, rtc::CheckedDivExact(sample_rate_hz, 100), input_level); + num_channels, samples_per_channel, input_level); ApmDataDumper apm_data_dumper(0); - FixedDigitalLevelEstimator level_estimator(sample_rate_hz, &apm_data_dumper); + FixedDigitalLevelEstimator level_estimator(samples_per_channel, + &apm_data_dumper); // Give the LevelEstimator plenty of time to ramp up and stabilize float last_level = 0.f; @@ -78,8 +80,8 @@ float TimeMsToDecreaseLevel(int sample_rate_hz, } // Set input to 0. - VectorFloatFrame vectors_with_zero_float_frame( - num_channels, rtc::CheckedDivExact(sample_rate_hz, 100), 0); + VectorFloatFrame vectors_with_zero_float_frame(num_channels, + samples_per_channel, 0); const float reduced_level_linear = DbfsToFloatS16(input_level_db - level_reduction_db); @@ -102,21 +104,22 @@ float TimeMsToDecreaseLevel(int sample_rate_hz, } // namespace TEST(GainController2FixedDigitalLevelEstimator, EstimatorShouldNotCrash) { - TestLevelEstimator(8000, 1, 0, std::numeric_limits::lowest(), + TestLevelEstimator(SampleRateToDefaultChannelSize(8000u), 1, 0, + std::numeric_limits::lowest(), std::numeric_limits::max()); } TEST(GainController2FixedDigitalLevelEstimator, EstimatorShouldEstimateConstantLevel) { - TestLevelEstimator(10000, 1, kInputLevel, kInputLevel * 0.99, - kInputLevel * 1.01); + TestLevelEstimator(SampleRateToDefaultChannelSize(10000u), 1, kInputLevel, + kInputLevel * 0.99, kInputLevel * 1.01); } TEST(GainController2FixedDigitalLevelEstimator, EstimatorShouldEstimateConstantLevelForManyChannels) { constexpr size_t num_channels = 10; - TestLevelEstimator(20000, num_channels, kInputLevel, kInputLevel * 0.99, - kInputLevel * 1.01); + TestLevelEstimator(SampleRateToDefaultChannelSize(20000u), num_channels, + kInputLevel, kInputLevel * 0.99, kInputLevel * 1.01); } TEST(GainController2FixedDigitalLevelEstimator, TimeToDecreaseForLowLevel) { @@ -125,7 +128,8 @@ TEST(GainController2FixedDigitalLevelEstimator, TimeToDecreaseForLowLevel) { constexpr float kExpectedTime = kLevelReductionDb * test::kDecayMs; const float time_to_decrease = - TimeMsToDecreaseLevel(22000, 1, kInitialLowLevel, kLevelReductionDb); + TimeMsToDecreaseLevel(SampleRateToDefaultChannelSize(22000u), 1, + kInitialLowLevel, kLevelReductionDb); EXPECT_LE(kExpectedTime * 0.9, time_to_decrease); EXPECT_LE(time_to_decrease, kExpectedTime * 1.1); @@ -136,8 +140,8 @@ TEST(GainController2FixedDigitalLevelEstimator, constexpr float kLevelReductionDb = 25; constexpr float kExpectedTime = kLevelReductionDb * test::kDecayMs; - const float time_to_decrease = - TimeMsToDecreaseLevel(26000, 1, 0, kLevelReductionDb); + const float time_to_decrease = TimeMsToDecreaseLevel( + SampleRateToDefaultChannelSize(26000u), 1, 0, kLevelReductionDb); EXPECT_LE(kExpectedTime * 0.9, time_to_decrease); EXPECT_LE(time_to_decrease, kExpectedTime * 1.1); @@ -150,7 +154,8 @@ TEST(GainController2FixedDigitalLevelEstimator, constexpr size_t kNumChannels = 10; const float time_to_decrease = - TimeMsToDecreaseLevel(28000, kNumChannels, 0, kLevelReductionDb); + TimeMsToDecreaseLevel(SampleRateToDefaultChannelSize(28000u), + kNumChannels, 0, kLevelReductionDb); EXPECT_LE(kExpectedTime * 0.9, time_to_decrease); EXPECT_LE(time_to_decrease, kExpectedTime * 1.1); diff --git a/modules/audio_processing/agc2/limiter.cc b/modules/audio_processing/agc2/limiter.cc index 57731baddc..41912980fe 100644 --- a/modules/audio_processing/agc2/limiter.cc +++ b/modules/audio_processing/agc2/limiter.cc @@ -86,24 +86,25 @@ void ScaleSamples(MonoView per_sample_scaling_factors, } } } - -void CheckLimiterSampleRate(int sample_rate_hz) { - // Check that per_sample_scaling_factors_ is large enough. - RTC_DCHECK_LE(sample_rate_hz, - kMaximalNumberOfSamplesPerChannel * 1000 / kFrameDurationMs); -} - } // namespace -Limiter::Limiter(int sample_rate_hz, - ApmDataDumper* apm_data_dumper, +Limiter::Limiter(ApmDataDumper* apm_data_dumper, + size_t samples_per_channel, absl::string_view histogram_name) : interp_gain_curve_(apm_data_dumper, histogram_name), - level_estimator_(sample_rate_hz, apm_data_dumper), + level_estimator_(samples_per_channel, apm_data_dumper), apm_data_dumper_(apm_data_dumper) { - CheckLimiterSampleRate(sample_rate_hz); + RTC_DCHECK_LE(samples_per_channel, kMaximalNumberOfSamplesPerChannel); } +// Deprecated ctor. +Limiter::Limiter(int sample_rate_hz, + ApmDataDumper* apm_data_dumper, + absl::string_view histogram_name_prefix) + : Limiter(apm_data_dumper, + SampleRateToDefaultChannelSize(sample_rate_hz), + histogram_name_prefix) {} + Limiter::~Limiter() = default; void Limiter::Process(AudioFrameView signal) { @@ -140,9 +141,9 @@ InterpolatedGainCurve::Stats Limiter::GetGainCurveStats() const { return interp_gain_curve_.get_stats(); } -void Limiter::SetSampleRate(int sample_rate_hz) { - CheckLimiterSampleRate(sample_rate_hz); - level_estimator_.SetSampleRate(sample_rate_hz); +void Limiter::SetSamplesPerChannel(size_t samples_per_channel) { + RTC_DCHECK_LE(samples_per_channel, kMaximalNumberOfSamplesPerChannel); + level_estimator_.SetSamplesPerChannel(samples_per_channel); } void Limiter::Reset() { diff --git a/modules/audio_processing/agc2/limiter.h b/modules/audio_processing/agc2/limiter.h index d4d556349c..e2db1381ce 100644 --- a/modules/audio_processing/agc2/limiter.h +++ b/modules/audio_processing/agc2/limiter.h @@ -14,6 +14,7 @@ #include #include "absl/strings/string_view.h" +#include "api/audio/audio_frame.h" #include "modules/audio_processing/agc2/fixed_digital_level_estimator.h" #include "modules/audio_processing/agc2/interpolated_gain_curve.h" #include "modules/audio_processing/include/audio_frame_view.h" @@ -23,9 +24,16 @@ class ApmDataDumper; class Limiter { public: - Limiter(int sample_rate_hz, - ApmDataDumper* apm_data_dumper, + // See `SetSamplesPerChannel()` for valid values for `samples_per_channel`. + Limiter(ApmDataDumper* apm_data_dumper, + size_t samples_per_channel, absl::string_view histogram_name_prefix); + + [[deprecated("Use constructor that accepts samples_per_channel")]] Limiter( + int sample_rate_hz, + ApmDataDumper* apm_data_dumper, + absl::string_view histogram_name_prefix); + Limiter(const Limiter& limiter) = delete; Limiter& operator=(const Limiter& limiter) = delete; ~Limiter(); @@ -34,12 +42,16 @@ class Limiter { void Process(AudioFrameView signal); InterpolatedGainCurve::Stats GetGainCurveStats() const; - // Supported rates must be - // * supported by FixedDigitalLevelEstimator - // * below kMaximalNumberOfSamplesPerChannel*1000/kFrameDurationMs - // so that samples_per_channel fit in the - // per_sample_scaling_factors_ array. - void SetSampleRate(int sample_rate_hz); + // Supported values must be + // * Supported by FixedDigitalLevelEstimator + // * Below or equal to kMaximalNumberOfSamplesPerChannel so that samples + // fit in the per_sample_scaling_factors_ array. + void SetSamplesPerChannel(size_t samples_per_channel); + + [[deprecated("Use SetSamplesPerChannel")]] void SetSampleRate( + int sample_rate_hz) { + SetSamplesPerChannel(SampleRateToDefaultChannelSize(sample_rate_hz)); + } // Resets the internal state. void Reset(); diff --git a/modules/audio_processing/agc2/limiter_unittest.cc b/modules/audio_processing/agc2/limiter_unittest.cc index e662a7fc89..905b6b081f 100644 --- a/modules/audio_processing/agc2/limiter_unittest.cc +++ b/modules/audio_processing/agc2/limiter_unittest.cc @@ -10,6 +10,7 @@ #include "modules/audio_processing/agc2/limiter.h" +#include "api/audio/audio_frame.h" #include "common_audio/include/audio_util.h" #include "modules/audio_processing/agc2/agc2_common.h" #include "modules/audio_processing/agc2/agc2_testing_common.h" @@ -20,33 +21,33 @@ namespace webrtc { TEST(Limiter, LimiterShouldConstructAndRun) { - const int sample_rate_hz = 48000; + const size_t samples_per_channel = SampleRateToDefaultChannelSize(48000); ApmDataDumper apm_data_dumper(0); - Limiter limiter(sample_rate_hz, &apm_data_dumper, ""); + Limiter limiter(&apm_data_dumper, samples_per_channel, ""); - VectorFloatFrame vectors_with_float_frame(1, sample_rate_hz / 100, + VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, kMaxAbsFloatS16Value); limiter.Process(vectors_with_float_frame.float_frame_view()); } TEST(Limiter, OutputVolumeAboveThreshold) { - const int sample_rate_hz = 48000; + const size_t samples_per_channel = SampleRateToDefaultChannelSize(48000); const float input_level = (kMaxAbsFloatS16Value + DbfsToFloatS16(test::kLimiterMaxInputLevelDbFs)) / 2.f; ApmDataDumper apm_data_dumper(0); - Limiter limiter(sample_rate_hz, &apm_data_dumper, ""); + Limiter limiter(&apm_data_dumper, samples_per_channel, ""); // Give the level estimator time to adapt. for (int i = 0; i < 5; ++i) { - VectorFloatFrame vectors_with_float_frame(1, sample_rate_hz / 100, + VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, input_level); limiter.Process(vectors_with_float_frame.float_frame_view()); } - VectorFloatFrame vectors_with_float_frame(1, sample_rate_hz / 100, + VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, input_level); limiter.Process(vectors_with_float_frame.float_frame_view()); rtc::ArrayView channel = diff --git a/modules/audio_processing/gain_controller2.cc b/modules/audio_processing/gain_controller2.cc index dd3521268d..181c401a65 100644 --- a/modules/audio_processing/gain_controller2.cc +++ b/modules/audio_processing/gain_controller2.cc @@ -13,6 +13,7 @@ #include #include +#include "api/audio/audio_frame.h" #include "common_audio/include/audio_util.h" #include "modules/audio_processing/agc2/agc2_common.h" #include "modules/audio_processing/agc2/cpu_features.h" @@ -94,7 +95,9 @@ GainController2::GainController2( fixed_gain_applier_( /*hard_clip_samples=*/false, /*initial_gain_factor=*/DbToRatio(config.fixed_digital.gain_db)), - limiter_(sample_rate_hz, &data_dumper_, /*histogram_name_prefix=*/"Agc2"), + limiter_(&data_dumper_, + SampleRateToDefaultChannelSize(sample_rate_hz), + /*histogram_name_prefix=*/"Agc2"), calls_since_last_limiter_log_(0) { RTC_DCHECK(Validate(config)); data_dumper_.InitiateNewSetOfRecordings();