From cfdbb0d371ce5e5f70c8f301637f10e92280cf77 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 30 May 2024 13:30:33 +0200 Subject: [PATCH] Update PushResampler to use a single buffer for source, destination. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PushResampler now uses a single buffer for the deinterleaved channel sources and another for the deinterleaved destinations. Before, there was a dedicated buffer per channel (source and dest). This reduces allocations and allows for using DeinterleavedView for both which simplifies some checks. Bug: chromium:335805780 Change-Id: I553a36164109127fa332ab17918d53832d442303 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351542 Reviewed-by: Per Åhgren Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#42415} --- common_audio/channel_buffer.h | 38 +++++++ common_audio/include/audio_util.h | 64 ++---------- .../resampler/include/push_resampler.h | 21 +--- common_audio/resampler/push_resampler.cc | 99 +++++++------------ common_audio/resampler/push_sinc_resampler.h | 7 ++ rtc_tools/unpack_aecdump/unpack.cc | 1 + 6 files changed, 94 insertions(+), 136 deletions(-) diff --git a/common_audio/channel_buffer.h b/common_audio/channel_buffer.h index 9f08d6089b..583f7d7171 100644 --- a/common_audio/channel_buffer.h +++ b/common_audio/channel_buffer.h @@ -23,6 +23,44 @@ namespace webrtc { +// TODO: b/335805780 - Remove this method. Instead, use Deinterleave() from +// audio_util.h which requires size checked buffer views. +template +void Deinterleave(const T* interleaved, + size_t samples_per_channel, + size_t num_channels, + T* const* deinterleaved) { + for (size_t i = 0; i < num_channels; ++i) { + T* channel = deinterleaved[i]; + size_t interleaved_idx = i; + for (size_t j = 0; j < samples_per_channel; ++j) { + channel[j] = interleaved[interleaved_idx]; + interleaved_idx += num_channels; + } + } +} + +// `Interleave()` variant for cases where the deinterleaved channels aren't +// represented by a `DeinterleavedView`. +// TODO: b/335805780 - Remove this method. Instead, use Deinterleave() from +// audio_util.h which requires size checked buffer views. +template +void Interleave(const T* const* deinterleaved, + size_t samples_per_channel, + size_t num_channels, + InterleavedView& interleaved) { + RTC_DCHECK_EQ(NumChannels(interleaved), num_channels); + RTC_DCHECK_EQ(SamplesPerChannel(interleaved), samples_per_channel); + for (size_t i = 0; i < num_channels; ++i) { + const T* channel = deinterleaved[i]; + size_t interleaved_idx = i; + for (size_t j = 0; j < samples_per_channel; ++j) { + interleaved[interleaved_idx] = channel[j]; + interleaved_idx += num_channels; + } + } +} + // Helper to encapsulate a contiguous data buffer, full or split into frequency // bands, with access to a pointer arrays of the deinterleaved channels and // bands. The buffer is zero initialized at creation. diff --git a/common_audio/include/audio_util.h b/common_audio/include/audio_util.h index e0042455d8..07ec2ecbcf 100644 --- a/common_audio/include/audio_util.h +++ b/common_audio/include/audio_util.h @@ -130,23 +130,6 @@ void Deinterleave(const InterleavedView& interleaved, } } -// TODO: b/335805780 - Move into test code where this is used once PushResampler -// has been changed to use a single allocation for deinterleaved audio buffers. -template -void Deinterleave(const T* interleaved, - size_t samples_per_channel, - size_t num_channels, - T* const* deinterleaved) { - for (size_t i = 0; i < num_channels; ++i) { - T* channel = deinterleaved[i]; - size_t interleaved_idx = i; - for (size_t j = 0; j < samples_per_channel; ++j) { - channel[j] = interleaved[interleaved_idx]; - interleaved_idx += num_channels; - } - } -} - // Interleave audio from the channel buffers pointed to by `deinterleaved` to // `interleaved`. There must be sufficient space allocated in `interleaved` // (`samples_per_channel` * `num_channels`). @@ -166,44 +149,8 @@ void Interleave(const DeinterleavedView& deinterleaved, } } -// `Interleave()` variant for cases where the deinterleaved channels aren't -// represented by a `DeinterleavedView`. -// TODO: b/335805780 - Move into test code where this is used. -template -void Interleave(const T* const* deinterleaved, - size_t samples_per_channel, - size_t num_channels, - InterleavedView& interleaved) { - RTC_DCHECK_EQ(NumChannels(interleaved), num_channels); - RTC_DCHECK_EQ(SamplesPerChannel(interleaved), samples_per_channel); - for (size_t i = 0; i < num_channels; ++i) { - const T* channel = deinterleaved[i]; - size_t interleaved_idx = i; - for (size_t j = 0; j < samples_per_channel; ++j) { - interleaved[interleaved_idx] = channel[j]; - interleaved_idx += num_channels; - } - } -} - -// Copies audio from a single channel buffer pointed to by `mono` to each -// channel of `interleaved`. There must be sufficient space allocated in -// `interleaved` (`samples_per_channel` * `num_channels`). -// TODO: b/335805780 - Accept ArrayView. -template -void UpmixMonoToInterleaved(const T* mono, - int num_frames, - int num_channels, - T* interleaved) { - int interleaved_idx = 0; - for (int i = 0; i < num_frames; ++i) { - for (int j = 0; j < num_channels; ++j) { - interleaved[interleaved_idx++] = mono[i]; - } - } -} - -// TODO: b/335805780 - Accept ArrayView. +// TODO: b/335805780 - Accept DeInterleavedView and MonoView. +// Possibly just delete this method if it isn't used. template void DownmixToMono(const T* const* input_channels, size_t num_frames, @@ -220,7 +167,8 @@ void DownmixToMono(const T* const* input_channels, // Downmixes an interleaved multichannel signal to a single channel by averaging // all channels. -// TODO: b/335805780 - Accept ArrayView. +// TODO: b/335805780 - Accept InterleavedView and DeinterleavedView. +// Possibly just delete this method if it isn't used. template void DownmixInterleavedToMonoImpl(const T* interleaved, size_t num_frames, @@ -243,14 +191,14 @@ void DownmixInterleavedToMonoImpl(const T* interleaved, } } -// TODO: b/335805780 - Accept ArrayView. +// TODO: b/335805780 - Accept InterleavedView and DeinterleavedView. template void DownmixInterleavedToMono(const T* interleaved, size_t num_frames, int num_channels, T* deinterleaved); -// TODO: b/335805780 - Accept ArrayView. +// TODO: b/335805780 - Accept InterleavedView and DeinterleavedView. template <> void DownmixInterleavedToMono(const int16_t* interleaved, size_t num_frames, diff --git a/common_audio/resampler/include/push_resampler.h b/common_audio/resampler/include/push_resampler.h index a899655f6d..0ed463bf5b 100644 --- a/common_audio/resampler/include/push_resampler.h +++ b/common_audio/resampler/include/push_resampler.h @@ -22,7 +22,6 @@ class PushSincResampler; // Wraps PushSincResampler to provide stereo support. // Note: This implementation assumes 10ms buffer sizes throughout. -// TODO(ajm): add support for an arbitrary number of channels. template class PushResampler { public: @@ -40,22 +39,12 @@ class PushResampler { int Resample(InterleavedView src, InterleavedView dst); private: - int src_sample_rate_hz_; - int dst_sample_rate_hz_; - size_t num_channels_; - // Vector that is needed to provide the proper inputs and outputs to the - // interleave/de-interleave methods used in Resample. This needs to be - // heap-allocated on the state to support an arbitrary number of channels - // without doing run-time heap-allocations in the Resample method. - std::vector channel_data_array_; + std::unique_ptr source_; + std::unique_ptr destination_; + DeinterleavedView source_view_; + DeinterleavedView destination_view_; - struct ChannelResampler { - std::unique_ptr resampler; - std::vector source; - std::vector destination; - }; - - std::vector channel_resamplers_; + std::vector> resamplers_; }; } // namespace webrtc diff --git a/common_audio/resampler/push_resampler.cc b/common_audio/resampler/push_resampler.cc index 53d759340c..5d732dddd4 100644 --- a/common_audio/resampler/push_resampler.cc +++ b/common_audio/resampler/push_resampler.cc @@ -23,11 +23,10 @@ namespace webrtc { template -PushResampler::PushResampler() - : src_sample_rate_hz_(0), dst_sample_rate_hz_(0), num_channels_(0) {} +PushResampler::PushResampler() = default; template -PushResampler::~PushResampler() {} +PushResampler::~PushResampler() = default; template int PushResampler::InitializeIfNeeded(int src_sample_rate_hz, @@ -35,45 +34,36 @@ int PushResampler::InitializeIfNeeded(int src_sample_rate_hz, size_t num_channels) { // These checks used to be factored out of this template function due to // Windows debug build issues with clang. http://crbug.com/615050 - RTC_DCHECK_GT(src_sample_rate_hz, 0); - RTC_DCHECK_GT(dst_sample_rate_hz, 0); - RTC_DCHECK_GT(num_channels, 0); - - if (src_sample_rate_hz == src_sample_rate_hz_ && - dst_sample_rate_hz == dst_sample_rate_hz_ && - num_channels == num_channels_) { - // No-op if settings haven't changed. - return 0; - } - - if (src_sample_rate_hz <= 0 || dst_sample_rate_hz <= 0 || num_channels <= 0) { - return -1; - } - - src_sample_rate_hz_ = src_sample_rate_hz; - dst_sample_rate_hz_ = dst_sample_rate_hz; - num_channels_ = num_channels; - - // TODO: b/335805780 - Change this to use a single buffer for source and - // destination and initialize each ChannelResampler() with a pointer to - // channels in each deinterleaved buffer. That way, DeinterleavedView can be - // used for the two buffers. + RTC_CHECK_GT(src_sample_rate_hz, 0); + RTC_CHECK_GT(dst_sample_rate_hz, 0); + RTC_CHECK_GT(num_channels, 0); const size_t src_size_10ms_mono = static_cast(src_sample_rate_hz / 100); const size_t dst_size_10ms_mono = static_cast(dst_sample_rate_hz / 100); - channel_resamplers_.clear(); - for (size_t i = 0; i < num_channels; ++i) { - channel_resamplers_.push_back(ChannelResampler()); - auto channel_resampler = channel_resamplers_.rbegin(); - channel_resampler->resampler = std::make_unique( - src_size_10ms_mono, dst_size_10ms_mono); - channel_resampler->source.resize(src_size_10ms_mono); - channel_resampler->destination.resize(dst_size_10ms_mono); + + if (src_size_10ms_mono == SamplesPerChannel(source_view_) && + dst_size_10ms_mono == SamplesPerChannel(destination_view_) && + num_channels == NumChannels(source_view_)) { + // No-op if settings haven't changed. + return 0; } - channel_data_array_.resize(num_channels_); + // Allocate two buffers for all source and destination channels. + // Then organize source and destination views together with an array of + // resamplers for each channel in the deinterlaved buffers. + source_.reset(new T[src_size_10ms_mono * num_channels]); + destination_.reset(new T[dst_size_10ms_mono * num_channels]); + source_view_ = + DeinterleavedView(source_.get(), src_size_10ms_mono, num_channels); + destination_view_ = DeinterleavedView(destination_.get(), + dst_size_10ms_mono, num_channels); + resamplers_.resize(num_channels); + for (size_t i = 0; i < num_channels; ++i) { + resamplers_[i] = std::make_unique(src_size_10ms_mono, + dst_size_10ms_mono); + } return 0; } @@ -81,42 +71,27 @@ int PushResampler::InitializeIfNeeded(int src_sample_rate_hz, template int PushResampler::Resample(InterleavedView src, InterleavedView dst) { - RTC_DCHECK_EQ(NumChannels(src), num_channels_); - RTC_DCHECK_EQ(NumChannels(dst), num_channels_); - RTC_DCHECK_EQ(SamplesPerChannel(src), - SampleRateToDefaultChannelSize(src_sample_rate_hz_)); - RTC_DCHECK_EQ(SamplesPerChannel(dst), - SampleRateToDefaultChannelSize(dst_sample_rate_hz_)); + RTC_DCHECK_EQ(NumChannels(src), NumChannels(source_view_)); + RTC_DCHECK_EQ(NumChannels(dst), NumChannels(destination_view_)); + RTC_DCHECK_EQ(SamplesPerChannel(src), SamplesPerChannel(source_view_)); + RTC_DCHECK_EQ(SamplesPerChannel(dst), SamplesPerChannel(destination_view_)); - if (src_sample_rate_hz_ == dst_sample_rate_hz_) { + if (SamplesPerChannel(src) == SamplesPerChannel(dst)) { // The old resampler provides this memcpy facility in the case of matching // sample rates, so reproduce it here for the sinc resampler. CopySamples(dst, src); return static_cast(src.data().size()); } - for (size_t ch = 0; ch < num_channels_; ++ch) { - channel_data_array_[ch] = channel_resamplers_[ch].source.data(); + Deinterleave(src, source_view_); + + for (size_t i = 0; i < resamplers_.size(); ++i) { + size_t dst_length_mono = + resamplers_[i]->Resample(source_view_[i], destination_view_[i]); + RTC_DCHECK_EQ(dst_length_mono, SamplesPerChannel(dst)); } - // TODO: b/335805780 - Deinterleave should accept InterleavedView<> as input. - Deinterleave(&src.data()[0], src.samples_per_channel(), src.num_channels(), - channel_data_array_.data()); - - for (auto& resampler : channel_resamplers_) { - size_t dst_length_mono = resampler.resampler->Resample( - resampler.source.data(), src.samples_per_channel(), - resampler.destination.data(), dst.samples_per_channel()); - RTC_DCHECK_EQ(dst_length_mono, dst.samples_per_channel()); - } - - for (size_t ch = 0; ch < num_channels_; ++ch) { - channel_data_array_[ch] = channel_resamplers_[ch].destination.data(); - } - - // TODO: b/335805780 - Interleave should accept DeInterleavedView<> as src. - Interleave(channel_data_array_.data(), dst.samples_per_channel(), - num_channels_, dst); + Interleave(destination_view_, dst); return static_cast(dst.size()); } diff --git a/common_audio/resampler/push_sinc_resampler.h b/common_audio/resampler/push_sinc_resampler.h index 7946ef8f82..b2c71555a9 100644 --- a/common_audio/resampler/push_sinc_resampler.h +++ b/common_audio/resampler/push_sinc_resampler.h @@ -16,6 +16,7 @@ #include +#include "api/audio/audio_view.h" #include "common_audio/resampler/sinc_resampler.h" namespace webrtc { @@ -40,6 +41,12 @@ class PushSincResampler : public SincResamplerCallback { // at least as large as `destination_frames`. Returns the number of samples // provided in destination (for convenience, since this will always be equal // to `destination_frames`). + template + size_t Resample(const MonoView& source, const MonoView& destination) { + return Resample(&source[0], SamplesPerChannel(source), &destination[0], + SamplesPerChannel(destination)); + } + size_t Resample(const int16_t* source, size_t source_frames, int16_t* destination, diff --git a/rtc_tools/unpack_aecdump/unpack.cc b/rtc_tools/unpack_aecdump/unpack.cc index 42c143c7a8..800e84c207 100644 --- a/rtc_tools/unpack_aecdump/unpack.cc +++ b/rtc_tools/unpack_aecdump/unpack.cc @@ -25,6 +25,7 @@ #include "absl/flags/flag.h" #include "absl/flags/parse.h" #include "api/function_view.h" +#include "common_audio/channel_buffer.h" #include "common_audio/include/audio_util.h" #include "common_audio/wav_file.h" #include "modules/audio_processing/test/protobuf_utils.h"