Make Resample10Msec with views return void and update PushResampler.
Since we're using views, there's liberal checking that the dimensions
are correct when calling methods such as CopySamples().
This allows us to remove error checks when calling Resample methods.
Bug: chromium:335805780
Change-Id: Ib7af879d3e7e7a84e4f10148ccc264b95134ef19
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390922
Reviewed-by: Per Åhgren <peah@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44669}
diff --git a/audio/audio_transport_impl.cc b/audio/audio_transport_impl.cc
index ef8d3ba..cd83fd9 100644
--- a/audio/audio_transport_impl.cc
+++ b/audio/audio_transport_impl.cc
@@ -11,15 +11,24 @@
#include "audio/audio_transport_impl.h"
#include <algorithm>
+#include <cstddef>
+#include <cstdint>
#include <memory>
+#include <optional>
#include <utility>
+#include <vector>
+#include "api/audio/audio_frame.h"
+#include "api/audio/audio_mixer.h"
+#include "api/audio/audio_view.h"
#include "audio/remix_resample.h"
#include "audio/utility/audio_frame_operations.h"
#include "call/audio_sender.h"
+#include "common_audio/resampler/include/push_resampler.h"
#include "modules/async_audio_processing/async_audio_processing.h"
#include "modules/audio_processing/include/audio_frame_proxies.h"
#include "rtc_base/checks.h"
+#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/trace_event.h"
namespace webrtc {
@@ -67,10 +76,10 @@
// Resample audio in `frame` to given sample rate preserving the
// channel count and place the result in `destination`.
-int Resample(const AudioFrame& frame,
- const int destination_sample_rate,
- PushResampler<int16_t>* resampler,
- InterleavedView<int16_t> destination) {
+void Resample(const AudioFrame& frame,
+ const int destination_sample_rate,
+ PushResampler<int16_t>* resampler,
+ InterleavedView<int16_t> destination) {
TRACE_EVENT2("webrtc", "Resample", "frame sample rate", frame.sample_rate_hz_,
"destination_sample_rate", destination_sample_rate);
const size_t target_number_of_samples_per_channel =
@@ -82,7 +91,7 @@
frame.num_channels_ * target_number_of_samples_per_channel);
// TODO(yujo): Add special case handling of muted frames.
- return resampler->Resample(frame.data_view(), destination);
+ resampler->Resample(frame.data_view(), destination);
}
} // namespace
@@ -233,11 +242,10 @@
RTC_DCHECK_EQ(error, AudioProcessing::kNoError);
}
- nSamplesOut =
- Resample(mixed_frame_, samplesPerSec, &render_resampler_,
- InterleavedView<int16_t>(static_cast<int16_t*>(audioSamples),
- nSamples, nChannels));
- RTC_DCHECK_EQ(nSamplesOut, nChannels * nSamples);
+ InterleavedView<int16_t> resampled(static_cast<int16_t*>(audioSamples),
+ nSamples, nChannels);
+ Resample(mixed_frame_, samplesPerSec, &render_resampler_, resampled);
+ nSamplesOut = resampled.size();
return 0;
}
@@ -266,11 +274,9 @@
*elapsed_time_ms = mixed_frame_.elapsed_time_ms_;
*ntp_time_ms = mixed_frame_.ntp_time_ms_;
- int output_samples =
- Resample(mixed_frame_, sample_rate, &render_resampler_,
- InterleavedView<int16_t>(static_cast<int16_t*>(audio_data),
- number_of_frames, number_of_channels));
- RTC_DCHECK_EQ(output_samples, number_of_channels * number_of_frames);
+ Resample(mixed_frame_, sample_rate, &render_resampler_,
+ InterleavedView<int16_t>(static_cast<int16_t*>(audio_data),
+ number_of_frames, number_of_channels));
}
void AudioTransportImpl::UpdateAudioSenders(std::vector<AudioSender*> senders,
diff --git a/audio/remix_resample.cc b/audio/remix_resample.cc
index 93e8651..0f87803 100644
--- a/audio/remix_resample.cc
+++ b/audio/remix_resample.cc
@@ -11,12 +11,13 @@
#include "audio/remix_resample.h"
#include <array>
+#include <cstdint>
#include "api/audio/audio_frame.h"
+#include "api/audio/audio_view.h"
#include "audio/utility/audio_frame_operations.h"
#include "common_audio/resampler/include/push_resampler.h"
#include "rtc_base/checks.h"
-#include "rtc_base/logging.h"
namespace webrtc {
namespace voe {
@@ -70,12 +71,9 @@
// `dst_frame` as a target buffer with the same number of channels as the
// source.
auto original_dst_number_of_channels = dst_frame->num_channels_;
- int out_length = resampler->Resample(
- src_data, dst_frame->mutable_data(dst_frame->samples_per_channel_,
- src_data.num_channels()));
- RTC_CHECK_NE(out_length, -1) << "src_data.size=" << src_data.size();
- RTC_DCHECK_EQ(dst_frame->samples_per_channel(),
- out_length / src_data.num_channels());
+ resampler->Resample(src_data,
+ dst_frame->mutable_data(dst_frame->samples_per_channel_,
+ src_data.num_channels()));
// Upmix after resampling.
if (src_data.num_channels() == 1 && original_dst_number_of_channels == 2) {
diff --git a/common_audio/resampler/include/push_resampler.h b/common_audio/resampler/include/push_resampler.h
index 20d4042..fe33d56 100644
--- a/common_audio/resampler/include/push_resampler.h
+++ b/common_audio/resampler/include/push_resampler.h
@@ -32,13 +32,10 @@
size_t num_channels);
~PushResampler();
- // Returns the total number of samples provided in destination (e.g. 32 kHz,
- // 2 channel audio gives 640 samples) - never a negative value.
- int Resample(InterleavedView<const T> src, InterleavedView<T> dst);
- // For when a deinterleaved/mono channel already exists and we can skip the
- // deinterleaved operation. The return value is the number of resampled
- // samples.
- int Resample(MonoView<const T> src, MonoView<T> dst);
+ void Resample(InterleavedView<const T> src, InterleavedView<T> dst);
+ // For when a deinterleaved (or mono) channel already exists and we can skip
+ // the deinterleaved operation.
+ void Resample(MonoView<const T> src, MonoView<T> dst);
private:
// Ensures that source and destination buffers for deinterleaving are
diff --git a/common_audio/resampler/push_resampler.cc b/common_audio/resampler/push_resampler.cc
index 2e75679..d35b9af 100644
--- a/common_audio/resampler/push_resampler.cc
+++ b/common_audio/resampler/push_resampler.cc
@@ -15,7 +15,7 @@
#include <memory>
-#include "api/audio/audio_frame.h"
+#include "api/audio/audio_view.h"
#include "common_audio/include/audio_util.h"
#include "common_audio/resampler/push_sinc_resampler.h"
#include "rtc_base/checks.h"
@@ -77,8 +77,8 @@
}
template <typename T>
-int PushResampler<T>::Resample(InterleavedView<const T> src,
- InterleavedView<T> dst) {
+void PushResampler<T>::Resample(InterleavedView<const T> src,
+ InterleavedView<T> dst) {
EnsureInitialized(SamplesPerChannel(src), SamplesPerChannel(dst),
NumChannels(src));
@@ -91,7 +91,7 @@
// 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<int>(src.data().size());
+ return;
}
Deinterleave(src, source_view_);
@@ -103,21 +103,19 @@
}
Interleave<T>(destination_view_, dst);
- return static_cast<int>(dst.size());
}
template <typename T>
-int PushResampler<T>::Resample(MonoView<const T> src, MonoView<T> dst) {
+void PushResampler<T>::Resample(MonoView<const T> src, MonoView<T> dst) {
RTC_DCHECK_EQ(resamplers_.size(), 1);
RTC_DCHECK_EQ(SamplesPerChannel(src), SamplesPerChannel(source_view_));
RTC_DCHECK_EQ(SamplesPerChannel(dst), SamplesPerChannel(destination_view_));
if (SamplesPerChannel(src) == SamplesPerChannel(dst)) {
CopySamples(dst, src);
- return static_cast<int>(src.size());
+ } else {
+ resamplers_[0]->Resample(src, dst);
}
-
- return resamplers_[0]->Resample(src, dst);
}
// Explictly generate required instantiations.
diff --git a/modules/audio_coding/acm2/acm_resampler.cc b/modules/audio_coding/acm2/acm_resampler.cc
index 346ec68..1587ecc 100644
--- a/modules/audio_coding/acm2/acm_resampler.cc
+++ b/modules/audio_coding/acm2/acm_resampler.cc
@@ -39,34 +39,20 @@
SampleRateToDefaultChannelSize(out_freq_hz),
num_audio_channels);
RTC_DCHECK_GE(out_capacity_samples, dst.size());
- return Resample10Msec(src, in_freq_hz, dst, out_freq_hz);
+ Resample10Msec(src, in_freq_hz, dst, out_freq_hz);
+ return dst.samples_per_channel();
}
-int ACMResampler::Resample10Msec(InterleavedView<const int16_t> src,
- int in_freq_hz,
- InterleavedView<int16_t> dst,
- int out_freq_hz) {
+void ACMResampler::Resample10Msec(InterleavedView<const int16_t> src,
+ int in_freq_hz,
+ InterleavedView<int16_t> dst,
+ int out_freq_hz) {
RTC_DCHECK_EQ(src.num_channels(), dst.num_channels());
if (in_freq_hz == out_freq_hz) {
- // TODO(tommi): Could this `if()` be replaced with a stricter check?
- // RTC_CHECK_EQ(dst.size(), src.size());
- // Eventually change the method to never return -1 and not have to
- // expect that at higher level call sites. The Resample() method below
- // can never return -1
- if (dst.size() < src.size()) {
- RTC_DCHECK_NOTREACHED();
- return -1;
- }
CopySamples(dst, src);
- RTC_DCHECK_EQ(dst.samples_per_channel(), src.samples_per_channel());
- return static_cast<int>(dst.samples_per_channel());
+ } else {
+ resampler_.Resample(src, dst);
}
-
- int out_length = resampler_.Resample(src, dst);
- RTC_CHECK_GE(out_length, 0);
- RTC_DCHECK_EQ(out_length, dst.size());
- RTC_DCHECK_EQ(out_length / dst.num_channels(), dst.samples_per_channel());
- return static_cast<int>(dst.samples_per_channel());
}
ResamplerHelper::ResamplerHelper() {
diff --git a/modules/audio_coding/acm2/acm_resampler.h b/modules/audio_coding/acm2/acm_resampler.h
index abf08f0..256884b 100644
--- a/modules/audio_coding/acm2/acm_resampler.h
+++ b/modules/audio_coding/acm2/acm_resampler.h
@@ -36,10 +36,10 @@
size_t out_capacity_samples,
int16_t* out_audio);
- int Resample10Msec(InterleavedView<const int16_t> src,
- int src_freq_hz,
- InterleavedView<int16_t> dst,
- int dst_freq_hz);
+ void Resample10Msec(InterleavedView<const int16_t> src,
+ int src_freq_hz,
+ InterleavedView<int16_t> dst,
+ int dst_freq_hz);
private:
PushResampler<int16_t> resampler_;
diff --git a/modules/audio_coding/acm2/audio_coding_module.cc b/modules/audio_coding/acm2/audio_coding_module.cc
index 373e3a3..32a36a7 100644
--- a/modules/audio_coding/acm2/audio_coding_module.cc
+++ b/modules/audio_coding/acm2/audio_coding_module.cc
@@ -153,12 +153,6 @@
const AudioFrame* AddDataNoPreProcess(const AudioFrame& in_frame)
RTC_EXCLUSIVE_LOCKS_REQUIRED(acm_mutex_);
- // Resamples `source` into the `preprocess_frame_` buffer.
- // `sample_rate` refers to the sample rate of `source`.
- bool ResampleToPreProcessFrame(InterleavedView<const int16_t> source,
- int sample_rate)
- RTC_EXCLUSIVE_LOCKS_REQUIRED(acm_mutex_);
-
// Change required states after starting to receive the codec corresponding
// to `index`.
int UpdateUponReceivingCodec(int index);
@@ -512,10 +506,11 @@
preprocess_frame_.SetSampleRateAndChannelSize(encoder_stack_->SampleRateHz());
if (resample) {
- if (!ResampleToPreProcessFrame(resample_src_audio,
- in_frame.sample_rate_hz())) {
- return -1;
- }
+ resampler_.Resample10Msec(
+ resample_src_audio, in_frame.sample_rate_hz(),
+ preprocess_frame_.mutable_data(preprocess_frame_.samples_per_channel(),
+ preprocess_frame_.num_channels()),
+ preprocess_frame_.sample_rate_hz());
}
expected_codec_ts_ +=
@@ -546,24 +541,6 @@
return ret;
}
-bool AudioCodingModuleImpl::ResampleToPreProcessFrame(
- InterleavedView<const int16_t> source,
- int sample_rate) {
- int samples_per_channel = resampler_.Resample10Msec(
- source, sample_rate,
- preprocess_frame_.mutable_data(preprocess_frame_.samples_per_channel(),
- preprocess_frame_.num_channels()),
- preprocess_frame_.sample_rate_hz());
-
- // TODO(tommi): Resample10Msec() should never return an error. Remove this
- // check and change this method to return void.
- if (samples_per_channel < 0) {
- RTC_LOG(LS_ERROR) << "Cannot add 10 ms audio, resampling failed";
- }
-
- return samples_per_channel >= 0;
-}
-
/////////////////////////////////////////
// (FEC) Forward Error Correction (codec internal)
//
diff --git a/modules/audio_coding/test/opus_test.cc b/modules/audio_coding/test/opus_test.cc
index 1f31e3c..da44f26 100644
--- a/modules/audio_coding/test/opus_test.cc
+++ b/modules/audio_coding/test/opus_test.cc
@@ -267,9 +267,8 @@
// If input audio is sampled at 32 kHz, resampling to 48 kHz is required.
InterleavedView<int16_t> dst(&audio[written_samples], 480, channels);
- EXPECT_EQ(480, resampler_.Resample10Msec(audio_frame.data_view(),
- audio_frame.sample_rate_hz_, dst,
- 48000));
+ resampler_.Resample10Msec(audio_frame.data_view(),
+ audio_frame.sample_rate_hz_, dst, 48000);
written_samples += 480 * channels;
// Sometimes we need to loop over the audio vector to produce the right
diff --git a/modules/audio_processing/BUILD.gn b/modules/audio_processing/BUILD.gn
index 2a7e72d..b42ded6 100644
--- a/modules/audio_processing/BUILD.gn
+++ b/modules/audio_processing/BUILD.gn
@@ -341,6 +341,7 @@
"../../api/audio:aec3_factory",
"../../api/audio:audio_frame_api",
"../../api/audio:audio_processing",
+ "../../api/audio:audio_processing_statistics",
"../../api/audio:builtin_audio_processing_builder",
"../../api/audio:echo_detector_creator",
"../../api/environment",
diff --git a/modules/audio_processing/audio_processing_unittest.cc b/modules/audio_processing/audio_processing_unittest.cc
index 373b574..0961036 100644
--- a/modules/audio_processing/audio_processing_unittest.cc
+++ b/modules/audio_processing/audio_processing_unittest.cc
@@ -13,41 +13,54 @@
#include <stdio.h>
#include <algorithm>
+#include <array>
#include <cmath>
+#include <cstdint>
+#include <cstring>
+#include <iostream>
#include <limits>
+#include <map>
#include <memory>
#include <numeric>
+#include <ostream>
#include <queue>
#include <string>
+#include <tuple>
+#include <utility>
+#include <vector>
#include "absl/flags/flag.h"
#include "absl/strings/string_view.h"
+#include "api/array_view.h"
+#include "api/audio/audio_processing_statistics.h"
+#include "api/audio/audio_view.h"
#include "api/audio/builtin_audio_processing_builder.h"
+#include "api/audio/echo_control.h"
#include "api/audio/echo_detector_creator.h"
+#include "api/environment/environment.h"
#include "api/environment/environment_factory.h"
#include "api/make_ref_counted.h"
+#include "api/scoped_refptr.h"
+#include "common_audio/channel_buffer.h"
#include "common_audio/include/audio_util.h"
#include "common_audio/resampler/include/push_resampler.h"
#include "common_audio/resampler/push_sinc_resampler.h"
-#include "common_audio/signal_processing/include/signal_processing_library.h"
#include "modules/audio_processing/aec_dump/aec_dump_factory.h"
-#include "modules/audio_processing/audio_processing_impl.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
#include "modules/audio_processing/test/protobuf_utils.h"
#include "modules/audio_processing/test/test_utils.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/checks.h"
#include "rtc_base/fake_clock.h"
-#include "rtc_base/gtest_prod_util.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "rtc_base/numerics/safe_minmax.h"
#include "rtc_base/protobuf_utils.h"
#include "rtc_base/strings/string_builder.h"
#include "rtc_base/swap_queue.h"
-#include "rtc_base/system/arch.h"
+#include "rtc_base/system/file_wrapper.h"
#include "rtc_base/task_queue_for_test.h"
-#include "rtc_base/thread.h"
#include "system_wrappers/include/cpu_features_wrapper.h"
+#include "test/gmock.h"
#include "test/gtest.h"
#include "test/testsupport/file_utils.h"
@@ -1669,7 +1682,6 @@
// of enabled components.
TEST_F(ApmTest, Process) {
- GOOGLE_PROTOBUF_VERIFY_VERSION;
audioproc::OutputData ref_data;
if (!absl::GetFlag(FLAGS_write_apm_ref_data)) {
@@ -2166,8 +2178,7 @@
out_num);
InterleavedView<float> dst(cmp_data.get(), ref_samples_per_channel,
out_num);
- ASSERT_EQ(ref_length,
- static_cast<size_t>(resampler.Resample(src, dst)));
+ resampler.Resample(src, dst);
out_ptr = cmp_data.get();
}
