From 19c51ea5370d5e78b384e6ef38cbebc48f23a425 Mon Sep 17 00:00:00 2001 From: Tommi Date: Wed, 29 May 2024 09:52:55 +0200 Subject: [PATCH] Use std::array<> consistently for reusable audio buffers. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Per Åhgren Cr-Commit-Position: refs/heads/main@{#42400} --- api/audio/audio_frame.cc | 20 +++++++------- api/audio/audio_frame.h | 6 +++-- api/audio/audio_view.h | 16 ++++++++++++ api/audio/test/audio_view_unittest.cc | 32 +++++++++++++++++++++++ audio/remix_resample.cc | 6 +++-- modules/audio_coding/acm2/acm_receiver.cc | 11 ++++---- modules/audio_coding/acm2/acm_receiver.h | 5 +++- 7 files changed, 75 insertions(+), 21 deletions(-) diff --git a/api/audio/audio_frame.cc b/api/audio/audio_frame.cc index b7fcedea00..bee4be67ec 100644 --- a/api/audio/audio_frame.cc +++ b/api/audio/audio_frame.cc @@ -78,9 +78,9 @@ void AudioFrame::UpdateFrame(uint32_t timestamp, } 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 @@ void AudioFrame::CopyFrom(const AudioFrame& src) { // 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 @@ void AudioFrame::CopyFrom(const AudioFrame& src) { 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 @@ int64_t AudioFrame::ElapsedProfileTimeMs() const { } const int16_t* AudioFrame::data() const { - return muted_ ? zeroed_data().begin() : data_; + return muted_ ? zeroed_data().begin() : data_.data(); } InterleavedView AudioFrame::data_view() const { @@ -155,16 +155,16 @@ int16_t* AudioFrame::mutable_data() { // 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 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 @@ InterleavedView AudioFrame::mutable_data(size_t samples_per_channel, // 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 @@ void AudioFrame::SetLayoutAndNumChannels(ChannelLayout layout, 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 fa4c96c676..40475c771d 100644 --- a/api/audio/audio_frame.h +++ b/api/audio/audio_frame.h @@ -14,6 +14,8 @@ #include #include +#include + #include "api/array_view.h" #include "api/audio/audio_view.h" #include "api/audio/channel_layout.h" @@ -146,7 +148,7 @@ class AudioFrame { // 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 @@ class AudioFrame { // buffer per translation unit is to wrap a static in an inline function. static rtc::ArrayView zeroed_data(); - int16_t data_[kMaxDataSizeSamples]; + std::array 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 ba5682bce3..c877ee4b37 100644 --- a/api/audio/audio_view.h +++ b/api/audio/audio_view.h @@ -248,6 +248,22 @@ void CopySamples(D& destination, const S& source) { 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 +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 +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 62749ab579..156fb397b1 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 + #include "test/gtest.h" namespace webrtc { @@ -155,4 +157,34 @@ TEST(AudioViewTest, CopySamples) { ASSERT_EQ(dest_arr[i], source_arr[i]) << "i == " << i; } } + +TEST(AudioViewTest, ClearSamples) { + std::array samples = {}; + FillBuffer(rtc::ArrayView(samples)); + ASSERT_NE(samples[0], 0); + ClearSamples(samples); + for (const auto s : samples) { + ASSERT_EQ(s, 0); + } + + std::array samples_f = {}; + FillBuffer(rtc::ArrayView(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(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 26d0c85b37..fdea627675 100644 --- a/audio/remix_resample.cc +++ b/audio/remix_resample.cc @@ -10,6 +10,8 @@ #include "audio/remix_resample.h" +#include + #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 @@ void RemixAndResample(const int16_t* src_data, 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 downmixed_audio; // Downmix before resampling. if (num_channels > dst_frame->num_channels_) { @@ -54,7 +56,7 @@ void RemixAndResample(const int16_t* src_data, num_channels), InterleavedView(&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 3474229f7f..ab113586cb 100644 --- a/modules/audio_coding/acm2/acm_receiver.cc +++ b/modules/audio_coding/acm2/acm_receiver.cc @@ -56,15 +56,13 @@ AcmReceiver::Config::Config(const Config&) = default; 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 @@ int AcmReceiver::GetAudio(int desired_freq_hz, // 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 @@ int AcmReceiver::GetAudio(int desired_freq_hz, } // 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 6393a866f6..21bee16d6a 100644 --- a/modules/audio_coding/acm2/acm_receiver.h +++ b/modules/audio_coding/acm2/acm_receiver.h @@ -13,6 +13,7 @@ #include +#include #include #include #include @@ -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 @@ class AcmReceiver { mutable Mutex mutex_; absl::optional last_decoder_ RTC_GUARDED_BY(mutex_); ACMResampler resampler_ RTC_GUARDED_BY(mutex_); - std::unique_ptr last_audio_buffer_ RTC_GUARDED_BY(mutex_); CallStatistics call_stats_ RTC_GUARDED_BY(mutex_); const std::unique_ptr neteq_; // NetEq is thread-safe; no lock needed. Clock& clock_; bool resampled_last_output_frame_ RTC_GUARDED_BY(mutex_); + std::array last_audio_buffer_ + RTC_GUARDED_BY(mutex_); }; } // namespace acm2