Use std::array<> consistently for reusable audio buffers.
This is a minor change for places where we use
AudioFrame::kMaxDataSizeSamples sized intermediary buffers. The change
uses `std::array<>` instead of C style arrays which allows for use
of utility templates that incorporate type based buffer size checking.
Also adding `ClearSamples()` method, which complements CopySamples.
Bug: chromium:335805780
Change-Id: I813feb32937e020ceb9ca4b00632dc90907c93fb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/351681
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Per Ã…hgren <peah@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42400}
diff --git a/api/audio/audio_frame.cc b/api/audio/audio_frame.cc
index b7fcede..bee4be6 100644
--- a/api/audio/audio_frame.cc
+++ b/api/audio/audio_frame.cc
@@ -78,9 +78,9 @@
}
const size_t length = samples_per_channel * num_channels;
- RTC_CHECK_LE(length, kMaxDataSizeSamples);
+ RTC_CHECK_LE(length, data_.size());
if (data != nullptr) {
- memcpy(data_, data, sizeof(int16_t) * length);
+ memcpy(data_.data(), data, sizeof(int16_t) * length);
muted_ = false;
} else {
muted_ = true;
@@ -98,7 +98,7 @@
// copying over new values. If we don't, msan might complain in some tests.
// Consider locking down construction, avoiding the default constructor and
// prefering construction that initializes all state.
- memset(data_, 0, kMaxDataSizeBytes);
+ ClearSamples(data_);
}
timestamp_ = src.timestamp_;
@@ -115,7 +115,7 @@
absolute_capture_timestamp_ms_ = src.absolute_capture_timestamp_ms();
auto data = src.data_view();
- RTC_CHECK_LE(data.size(), kMaxDataSizeSamples);
+ RTC_CHECK_LE(data.size(), data_.size());
if (!muted_ && !data.empty()) {
memcpy(&data_[0], &data[0], sizeof(int16_t) * data.size());
}
@@ -134,7 +134,7 @@
}
const int16_t* AudioFrame::data() const {
- return muted_ ? zeroed_data().begin() : data_;
+ return muted_ ? zeroed_data().begin() : data_.data();
}
InterleavedView<const int16_t> AudioFrame::data_view() const {
@@ -155,16 +155,16 @@
// Consider instead if we should rather zero the buffer when `muted_` is set
// to `true`.
if (muted_) {
- memset(data_, 0, kMaxDataSizeBytes);
+ ClearSamples(data_);
muted_ = false;
}
- return data_;
+ return &data_[0];
}
InterleavedView<int16_t> AudioFrame::mutable_data(size_t samples_per_channel,
size_t num_channels) {
const size_t total_samples = samples_per_channel * num_channels;
- RTC_CHECK_LE(total_samples, kMaxDataSizeSamples);
+ RTC_CHECK_LE(total_samples, data_.size());
RTC_CHECK_LE(num_channels, kMaxConcurrentChannels);
// Sanity check for valid argument values during development.
// If `samples_per_channel` is < `num_channels` but larger than 0,
@@ -178,7 +178,7 @@
// Consider instead if we should rather zero the whole buffer when `muted_` is
// set to `true`.
if (muted_) {
- memset(data_, 0, total_samples * sizeof(int16_t));
+ ClearSamples(data_, total_samples);
muted_ = false;
}
samples_per_channel_ = samples_per_channel;
@@ -206,7 +206,7 @@
RTC_DCHECK_EQ(expected_num_channels, num_channels_);
}
#endif
- RTC_CHECK_LE(samples_per_channel_ * num_channels_, kMaxDataSizeSamples);
+ RTC_CHECK_LE(samples_per_channel_ * num_channels_, data_.size());
}
void AudioFrame::SetSampleRateAndChannelSize(int sample_rate) {
diff --git a/api/audio/audio_frame.h b/api/audio/audio_frame.h
index fa4c96c..40475c7 100644
--- a/api/audio/audio_frame.h
+++ b/api/audio/audio_frame.h
@@ -14,6 +14,8 @@
#include <stddef.h>
#include <stdint.h>
+#include <array>
+
#include "api/array_view.h"
#include "api/audio/audio_view.h"
#include "api/audio/channel_layout.h"
@@ -146,7 +148,7 @@
// Frame is muted by default.
bool muted() const;
- size_t max_16bit_samples() const { return kMaxDataSizeSamples; }
+ size_t max_16bit_samples() const { return data_.size(); }
size_t samples_per_channel() const { return samples_per_channel_; }
size_t num_channels() const { return num_channels_; }
@@ -211,7 +213,7 @@
// buffer per translation unit is to wrap a static in an inline function.
static rtc::ArrayView<const int16_t> zeroed_data();
- int16_t data_[kMaxDataSizeSamples];
+ std::array<int16_t, kMaxDataSizeSamples> data_;
bool muted_ = true;
ChannelLayout channel_layout_ = CHANNEL_LAYOUT_NONE;
diff --git a/api/audio/audio_view.h b/api/audio/audio_view.h
index ba5682b..c877ee4 100644
--- a/api/audio/audio_view.h
+++ b/api/audio/audio_view.h
@@ -248,6 +248,22 @@
source.size() * sizeof(typename S::value_type));
}
+// Sets all the samples in a view to 0. This template function is a simple
+// wrapper around `memset()` but adds the benefit of automatically calculating
+// the byte size from the number of samples and sample type.
+template <typename T>
+void ClearSamples(T& view) {
+ memset(&view[0], 0, view.size() * sizeof(typename T::value_type));
+}
+
+// Same as `ClearSamples()` above but allows for clearing only the first
+// `sample_count` number of samples.
+template <typename T>
+void ClearSamples(T& view, size_t sample_count) {
+ RTC_DCHECK_LE(sample_count, view.size());
+ memset(&view[0], 0, sample_count * sizeof(typename T::value_type));
+}
+
} // namespace webrtc
#endif // API_AUDIO_AUDIO_VIEW_H_
diff --git a/api/audio/test/audio_view_unittest.cc b/api/audio/test/audio_view_unittest.cc
index 62749ab..156fb39 100644
--- a/api/audio/test/audio_view_unittest.cc
+++ b/api/audio/test/audio_view_unittest.cc
@@ -10,6 +10,8 @@
#include "api/audio/audio_view.h"
+#include <array>
+
#include "test/gtest.h"
namespace webrtc {
@@ -155,4 +157,34 @@
ASSERT_EQ(dest_arr[i], source_arr[i]) << "i == " << i;
}
}
+
+TEST(AudioViewTest, ClearSamples) {
+ std::array<int16_t, 100u> samples = {};
+ FillBuffer(rtc::ArrayView<int16_t>(samples));
+ ASSERT_NE(samples[0], 0);
+ ClearSamples(samples);
+ for (const auto s : samples) {
+ ASSERT_EQ(s, 0);
+ }
+
+ std::array<float, 100u> samples_f = {};
+ FillBuffer(rtc::ArrayView<float>(samples_f));
+ ASSERT_NE(samples_f[0], 0.0);
+ ClearSamples(samples_f);
+ for (const auto s : samples_f) {
+ ASSERT_EQ(s, 0.0);
+ }
+
+ // Clear only half of the buffer
+ FillBuffer(rtc::ArrayView<int16_t>(samples));
+ const auto half_way = samples.size() / 2;
+ ClearSamples(samples, half_way);
+ for (size_t i = 0u; i < samples.size(); ++i) {
+ if (i < half_way) {
+ ASSERT_EQ(samples[i], 0);
+ } else {
+ ASSERT_NE(samples[i], 0);
+ }
+ }
+}
} // namespace webrtc
diff --git a/audio/remix_resample.cc b/audio/remix_resample.cc
index 26d0c85..fdea627 100644
--- a/audio/remix_resample.cc
+++ b/audio/remix_resample.cc
@@ -10,6 +10,8 @@
#include "audio/remix_resample.h"
+#include <array>
+
#include "api/audio/audio_frame.h"
#include "audio/utility/audio_frame_operations.h"
#include "common_audio/resampler/include/push_resampler.h"
@@ -40,7 +42,7 @@
AudioFrame* dst_frame) {
const int16_t* audio_ptr = src_data;
size_t audio_ptr_num_channels = num_channels;
- int16_t downmixed_audio[AudioFrame::kMaxDataSizeSamples];
+ std::array<int16_t, AudioFrame::kMaxDataSizeSamples> downmixed_audio;
// Downmix before resampling.
if (num_channels > dst_frame->num_channels_) {
@@ -54,7 +56,7 @@
num_channels),
InterleavedView<int16_t>(&downmixed_audio[0], samples_per_channel,
dst_frame->num_channels_));
- audio_ptr = downmixed_audio;
+ audio_ptr = downmixed_audio.data();
audio_ptr_num_channels = dst_frame->num_channels_;
}
diff --git a/modules/audio_coding/acm2/acm_receiver.cc b/modules/audio_coding/acm2/acm_receiver.cc
index 3474229f..ab11358 100644
--- a/modules/audio_coding/acm2/acm_receiver.cc
+++ b/modules/audio_coding/acm2/acm_receiver.cc
@@ -56,15 +56,13 @@
AcmReceiver::Config::~Config() = default;
AcmReceiver::AcmReceiver(const Config& config)
- : last_audio_buffer_(new int16_t[AudioFrame::kMaxDataSizeSamples]),
- neteq_(CreateNetEq(config.neteq_factory,
+ : neteq_(CreateNetEq(config.neteq_factory,
config.neteq_config,
&config.clock,
config.decoder_factory)),
clock_(config.clock),
resampled_last_output_frame_(true) {
- memset(last_audio_buffer_.get(), 0,
- sizeof(int16_t) * AudioFrame::kMaxDataSizeSamples);
+ ClearSamples(last_audio_buffer_);
}
AcmReceiver::~AcmReceiver() = default;
@@ -170,7 +168,7 @@
// Prime the resampler with the last frame.
int16_t temp_output[AudioFrame::kMaxDataSizeSamples];
int samples_per_channel_int = resampler_.Resample10Msec(
- last_audio_buffer_.get(), current_sample_rate_hz, desired_freq_hz,
+ last_audio_buffer_.data(), current_sample_rate_hz, desired_freq_hz,
audio_frame->num_channels_, AudioFrame::kMaxDataSizeSamples,
temp_output);
if (samples_per_channel_int < 0) {
@@ -206,7 +204,8 @@
}
// Store current audio in `last_audio_buffer_` for next time.
- memcpy(last_audio_buffer_.get(), audio_frame->data(),
+ // TODO: b/335805780 - Use CopySamples().
+ memcpy(last_audio_buffer_.data(), audio_frame->data(),
sizeof(int16_t) * audio_frame->samples_per_channel_ *
audio_frame->num_channels_);
diff --git a/modules/audio_coding/acm2/acm_receiver.h b/modules/audio_coding/acm2/acm_receiver.h
index 6393a86..21bee16 100644
--- a/modules/audio_coding/acm2/acm_receiver.h
+++ b/modules/audio_coding/acm2/acm_receiver.h
@@ -13,6 +13,7 @@
#include <stdint.h>
+#include <array>
#include <map>
#include <memory>
#include <string>
@@ -21,6 +22,7 @@
#include "absl/types/optional.h"
#include "api/array_view.h"
+#include "api/audio/audio_frame.h"
#include "api/audio_codecs/audio_decoder.h"
#include "api/audio_codecs/audio_decoder_factory.h"
#include "api/audio_codecs/audio_format.h"
@@ -233,11 +235,12 @@
mutable Mutex mutex_;
absl::optional<DecoderInfo> last_decoder_ RTC_GUARDED_BY(mutex_);
ACMResampler resampler_ RTC_GUARDED_BY(mutex_);
- std::unique_ptr<int16_t[]> last_audio_buffer_ RTC_GUARDED_BY(mutex_);
CallStatistics call_stats_ RTC_GUARDED_BY(mutex_);
const std::unique_ptr<NetEq> neteq_; // NetEq is thread-safe; no lock needed.
Clock& clock_;
bool resampled_last_output_frame_ RTC_GUARDED_BY(mutex_);
+ std::array<int16_t, AudioFrame::kMaxDataSizeSamples> last_audio_buffer_
+ RTC_GUARDED_BY(mutex_);
};
} // namespace acm2