From 51ad7c1277fbed64ca6cd8ff88fc48c9ef6cb4f7 Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 2 Jul 2024 17:18:25 +0200 Subject: [PATCH] Update FrameCombiner et al to use DeinterleavedView * FrameCombiner is simpler. No additional channel pointers for buffers. * Improve consistency in using views in downstream classes. * Deprecate older methods (some have upstream dependencies). * Use samples per channel instead of sample rate where the former is really what's needed. Bug: chromium:335805780 Change-Id: I0dde8ed7a5a187bbddd18d3b6c649aa0865e6d4a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/352582 Commit-Queue: Tomas Gunnarsson Reviewed-by: Sam Zackrisson Cr-Commit-Position: refs/heads/main@{#42575} --- modules/audio_mixer/frame_combiner.cc | 28 +++++-------- .../agc2/fixed_digital_level_estimator.cc | 8 ++-- .../agc2/fixed_digital_level_estimator.h | 10 ++++- .../fixed_digital_level_estimator_unittest.cc | 12 +++--- modules/audio_processing/agc2/limiter.cc | 35 ++++++++--------- modules/audio_processing/agc2/limiter.h | 7 +++- .../audio_processing/agc2/limiter_unittest.cc | 39 ++++++++++--------- modules/audio_processing/gain_controller2.cc | 2 +- 8 files changed, 70 insertions(+), 71 deletions(-) diff --git a/modules/audio_mixer/frame_combiner.cc b/modules/audio_mixer/frame_combiner.cc index ca79ecd243..dfe9511f5a 100644 --- a/modules/audio_mixer/frame_combiner.cc +++ b/modules/audio_mixer/frame_combiner.cc @@ -106,22 +106,22 @@ void MixToFloatFrame(rtc::ArrayView mix_list, } } -void RunLimiter(AudioFrameView mixing_buffer_view, Limiter* limiter) { - limiter->SetSamplesPerChannel(mixing_buffer_view.samples_per_channel()); - limiter->Process(mixing_buffer_view); +void RunLimiter(DeinterleavedView deinterleaved, Limiter* limiter) { + limiter->SetSamplesPerChannel(deinterleaved.samples_per_channel()); + limiter->Process(deinterleaved); } // Both interleaves and rounds. -void InterleaveToAudioFrame(AudioFrameView mixing_buffer_view, +void InterleaveToAudioFrame(DeinterleavedView deinterleaved, AudioFrame* audio_frame_for_mixing) { InterleavedView mixing_data = audio_frame_for_mixing->mutable_data( - mixing_buffer_view.samples_per_channel(), - mixing_buffer_view.num_channels()); + deinterleaved.samples_per_channel(), deinterleaved.num_channels()); // Put data in the result frame. for (size_t i = 0; i < mixing_data.num_channels(); ++i) { + auto channel = deinterleaved[i]; for (size_t j = 0; j < mixing_data.samples_per_channel(); ++j) { mixing_data[mixing_data.num_channels() * j + i] = - FloatS16ToS16(mixing_buffer_view.channel(i)[j]); + FloatS16ToS16(channel[j]); } } } @@ -191,21 +191,11 @@ void FrameCombiner::Combine(rtc::ArrayView mix_list, mixing_buffer_.data(), samples_per_channel, number_of_channels); MixToFloatFrame(mix_list, deinterleaved); - // Put float data in an AudioFrameView. - // TODO(tommi): We should be able to just use `deinterleaved` without an - // additional array of pointers. - std::array channel_pointers{}; - for (size_t i = 0; i < number_of_channels; ++i) { - channel_pointers[i] = deinterleaved[i].data(); - } - AudioFrameView mixing_buffer_view( - channel_pointers.data(), number_of_channels, samples_per_channel); - if (use_limiter_) { - RunLimiter(mixing_buffer_view, &limiter_); + RunLimiter(deinterleaved, &limiter_); } - InterleaveToAudioFrame(mixing_buffer_view, audio_frame_for_mixing); + InterleaveToAudioFrame(deinterleaved, audio_frame_for_mixing); } } // namespace webrtc diff --git a/modules/audio_processing/agc2/fixed_digital_level_estimator.cc b/modules/audio_processing/agc2/fixed_digital_level_estimator.cc index a927b9f7ca..73edcf0523 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator.cc +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator.cc @@ -56,15 +56,15 @@ void FixedDigitalLevelEstimator::CheckParameterCombination() { } std::array FixedDigitalLevelEstimator::ComputeLevel( - const AudioFrameView& float_frame) { + DeinterleavedView float_frame) { RTC_DCHECK_GT(float_frame.num_channels(), 0); RTC_DCHECK_EQ(float_frame.samples_per_channel(), samples_in_frame_); // Compute max envelope without smoothing. std::array envelope{}; - for (int channel_idx = 0; channel_idx < float_frame.num_channels(); + for (size_t channel_idx = 0; channel_idx < float_frame.num_channels(); ++channel_idx) { - const auto channel = float_frame.channel(channel_idx); + const auto channel = float_frame[channel_idx]; for (int sub_frame = 0; sub_frame < kSubFramesInFrame; ++sub_frame) { for (int sample_in_sub_frame = 0; sample_in_sub_frame < samples_in_sub_frame_; ++sample_in_sub_frame) { @@ -99,7 +99,7 @@ std::array FixedDigitalLevelEstimator::ComputeLevel( // Dump data for debug. RTC_DCHECK(apm_data_dumper_); - const auto channel = float_frame.channel(0); + const auto channel = float_frame[0]; apm_data_dumper_->DumpRaw("agc2_level_estimator_samples", samples_in_sub_frame_, &channel[sub_frame * samples_in_sub_frame_]); diff --git a/modules/audio_processing/agc2/fixed_digital_level_estimator.h b/modules/audio_processing/agc2/fixed_digital_level_estimator.h index eee6428d30..af102508e2 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator.h +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator.h @@ -27,7 +27,7 @@ class FixedDigitalLevelEstimator { public: // `samples_per_channel` is expected to be derived from this formula: // sample_rate_hz * kFrameDurationMs / 1000 - // or + // or, for a 10ms duration: // 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 @@ -46,7 +46,13 @@ class FixedDigitalLevelEstimator { // ms of audio produces a level estimates in the same scale. The // level estimate contains kSubFramesInFrame values. std::array ComputeLevel( - const AudioFrameView& float_frame); + DeinterleavedView float_frame); + + [[deprecated( + "Use DeinterleavedView variant")]] std::array + ComputeLevel(const AudioFrameView& float_frame) { + return ComputeLevel(float_frame.view()); + } // Rate may be changed at any time (but not concurrently) from the // value passed to the constructor. The class is not thread safe. 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 d83f38056b..c76db85a5c 100644 --- a/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc +++ b/modules/audio_processing/agc2/fixed_digital_level_estimator_unittest.cc @@ -40,8 +40,8 @@ void TestLevelEstimator(size_t samples_per_channel, num_channels, samples_per_channel, input_level_linear_scale); for (int i = 0; i < 500; ++i) { - const auto level = level_estimator.ComputeLevel( - vectors_with_float_frame.float_frame_view()); + const auto level = + level_estimator.ComputeLevel(vectors_with_float_frame.view()); // Give the estimator some time to ramp up. if (i < 50) { @@ -74,8 +74,8 @@ float TimeMsToDecreaseLevel(size_t samples_per_channel, // Give the LevelEstimator plenty of time to ramp up and stabilize float last_level = 0.f; for (int i = 0; i < 500; ++i) { - const auto level_envelope = level_estimator.ComputeLevel( - vectors_with_float_frame.float_frame_view()); + const auto level_envelope = + level_estimator.ComputeLevel(vectors_with_float_frame.view()); last_level = *level_envelope.rbegin(); } @@ -87,8 +87,8 @@ float TimeMsToDecreaseLevel(size_t samples_per_channel, DbfsToFloatS16(input_level_db - level_reduction_db); int sub_frames_until_level_reduction = 0; while (last_level > reduced_level_linear) { - const auto level_envelope = level_estimator.ComputeLevel( - vectors_with_zero_float_frame.float_frame_view()); + const auto level_envelope = + level_estimator.ComputeLevel(vectors_with_zero_float_frame.view()); for (const auto& v : level_envelope) { EXPECT_LT(v, last_level); sub_frames_until_level_reduction++; diff --git a/modules/audio_processing/agc2/limiter.cc b/modules/audio_processing/agc2/limiter.cc index 41912980fe..270dc13ad8 100644 --- a/modules/audio_processing/agc2/limiter.cc +++ b/modules/audio_processing/agc2/limiter.cc @@ -46,22 +46,20 @@ void InterpolateFirstSubframe(float last_factor, void ComputePerSampleSubframeFactors( const std::array& scaling_factors, - int samples_per_channel, - rtc::ArrayView per_sample_scaling_factors) { - const int num_subframes = scaling_factors.size() - 1; - const int subframe_size = - rtc::CheckedDivExact(samples_per_channel, num_subframes); + MonoView per_sample_scaling_factors) { + const size_t num_subframes = scaling_factors.size() - 1; + const int subframe_size = rtc::CheckedDivExact( + SamplesPerChannel(per_sample_scaling_factors), num_subframes); // Handle first sub-frame differently in case of attack. const bool is_attack = scaling_factors[0] > scaling_factors[1]; if (is_attack) { InterpolateFirstSubframe( scaling_factors[0], scaling_factors[1], - rtc::ArrayView( - per_sample_scaling_factors.subview(0, subframe_size))); + per_sample_scaling_factors.subview(0, subframe_size)); } - for (int i = is_attack ? 1 : 0; i < num_subframes; ++i) { + for (size_t i = is_attack ? 1 : 0; i < num_subframes; ++i) { const int subframe_start = i * subframe_size; const float scaling_start = scaling_factors[i]; const float scaling_end = scaling_factors[i + 1]; @@ -74,12 +72,12 @@ void ComputePerSampleSubframeFactors( } void ScaleSamples(MonoView per_sample_scaling_factors, - AudioFrameView signal) { + DeinterleavedView signal) { const int samples_per_channel = signal.samples_per_channel(); RTC_DCHECK_EQ(samples_per_channel, SamplesPerChannel(per_sample_scaling_factors)); - for (int i = 0; i < signal.num_channels(); ++i) { - MonoView channel = signal.channel(i); + for (size_t i = 0; i < signal.num_channels(); ++i) { + MonoView channel = signal[i]; for (int j = 0; j < samples_per_channel; ++j) { channel[j] = rtc::SafeClamp(channel[j] * per_sample_scaling_factors[j], kMinFloatS16Value, kMaxFloatS16Value); @@ -107,7 +105,10 @@ Limiter::Limiter(int sample_rate_hz, Limiter::~Limiter() = default; -void Limiter::Process(AudioFrameView signal) { +void Limiter::Process(DeinterleavedView signal) { + RTC_DCHECK_LE(signal.samples_per_channel(), + kMaximalNumberOfSamplesPerChannel); + const std::array level_estimate = level_estimator_.ComputeLevel(signal); @@ -118,13 +119,9 @@ void Limiter::Process(AudioFrameView signal) { return interp_gain_curve_.LookUpGainToApply(x); }); - const int samples_per_channel = signal.samples_per_channel(); - RTC_DCHECK_LE(samples_per_channel, kMaximalNumberOfSamplesPerChannel); - - auto per_sample_scaling_factors = - MonoView(&per_sample_scaling_factors_[0], samples_per_channel); - ComputePerSampleSubframeFactors(scaling_factors_, samples_per_channel, - per_sample_scaling_factors); + MonoView per_sample_scaling_factors(&per_sample_scaling_factors_[0], + signal.samples_per_channel()); + ComputePerSampleSubframeFactors(scaling_factors_, per_sample_scaling_factors); ScaleSamples(per_sample_scaling_factors, signal); last_scaling_factor_ = scaling_factors_.back(); diff --git a/modules/audio_processing/agc2/limiter.h b/modules/audio_processing/agc2/limiter.h index e2db1381ce..f5244be0a8 100644 --- a/modules/audio_processing/agc2/limiter.h +++ b/modules/audio_processing/agc2/limiter.h @@ -39,7 +39,12 @@ class Limiter { ~Limiter(); // Applies limiter and hard-clipping to `signal`. - void Process(AudioFrameView signal); + void Process(DeinterleavedView signal); + + [[deprecated("Use DeinterleavedView version")]] void Process( + AudioFrameView signal) { + return Process(signal.view()); + } InterpolatedGainCurve::Stats GetGainCurveStats() const; // Supported values must be diff --git a/modules/audio_processing/agc2/limiter_unittest.cc b/modules/audio_processing/agc2/limiter_unittest.cc index 905b6b081f..6c72e729ee 100644 --- a/modules/audio_processing/agc2/limiter_unittest.cc +++ b/modules/audio_processing/agc2/limiter_unittest.cc @@ -10,7 +10,8 @@ #include "modules/audio_processing/agc2/limiter.h" -#include "api/audio/audio_frame.h" +#include + #include "common_audio/include/audio_util.h" #include "modules/audio_processing/agc2/agc2_common.h" #include "modules/audio_processing/agc2/agc2_testing_common.h" @@ -21,40 +22,40 @@ namespace webrtc { TEST(Limiter, LimiterShouldConstructAndRun) { - const size_t samples_per_channel = SampleRateToDefaultChannelSize(48000); + constexpr size_t kSamplesPerChannel = 480; ApmDataDumper apm_data_dumper(0); - Limiter limiter(&apm_data_dumper, samples_per_channel, ""); + Limiter limiter(&apm_data_dumper, kSamplesPerChannel, ""); - VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, - kMaxAbsFloatS16Value); - limiter.Process(vectors_with_float_frame.float_frame_view()); + std::array buffer; + buffer.fill(kMaxAbsFloatS16Value); + limiter.Process( + DeinterleavedView(buffer.data(), kSamplesPerChannel, 1)); } TEST(Limiter, OutputVolumeAboveThreshold) { - const size_t samples_per_channel = SampleRateToDefaultChannelSize(48000); + constexpr size_t kSamplesPerChannel = 480; const float input_level = (kMaxAbsFloatS16Value + DbfsToFloatS16(test::kLimiterMaxInputLevelDbFs)) / 2.f; ApmDataDumper apm_data_dumper(0); - Limiter limiter(&apm_data_dumper, samples_per_channel, ""); + Limiter limiter(&apm_data_dumper, kSamplesPerChannel, ""); + + std::array buffer; // Give the level estimator time to adapt. for (int i = 0; i < 5; ++i) { - VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, - input_level); - limiter.Process(vectors_with_float_frame.float_frame_view()); + std::fill(buffer.begin(), buffer.end(), input_level); + limiter.Process( + DeinterleavedView(buffer.data(), kSamplesPerChannel, 1)); } - VectorFloatFrame vectors_with_float_frame(1, samples_per_channel, - input_level); - limiter.Process(vectors_with_float_frame.float_frame_view()); - rtc::ArrayView channel = - vectors_with_float_frame.float_frame_view().channel(0); - - for (const auto& sample : channel) { - EXPECT_LT(0.9f * kMaxAbsFloatS16Value, sample); + std::fill(buffer.begin(), buffer.end(), input_level); + limiter.Process( + DeinterleavedView(buffer.data(), kSamplesPerChannel, 1)); + for (const auto& sample : buffer) { + ASSERT_LT(0.9f * kMaxAbsFloatS16Value, sample); } } diff --git a/modules/audio_processing/gain_controller2.cc b/modules/audio_processing/gain_controller2.cc index 181c401a65..9b19bc8517 100644 --- a/modules/audio_processing/gain_controller2.cc +++ b/modules/audio_processing/gain_controller2.cc @@ -258,7 +258,7 @@ void GainController2::Process(absl::optional speech_probability, // computation in `limiter_`. fixed_gain_applier_.ApplyGain(float_frame); - limiter_.Process(float_frame); + limiter_.Process(float_frame.view()); // Periodically log limiter stats. if (++calls_since_last_limiter_log_ == kLogLimiterStatsPeriodNumFrames) {