Add AudioMultiVector::ReadInterleavedFromIndex()
The new method differs from the existing method with the same name in
two respects:
* It reads into an InterleavedView<> instead of pointer.
* It either reads exactly the requested number of samples or fails
After examining the behavior in NetEq, where this method is called from,
it seems that what we need is a method that reads exactly the required
number of samples, not partially.
A follow-up change to NetEqImpl and SyncBuffer will change
SyncBuffer::GetNextAudioInterleaved() to read the requested buffer size
or fail, as well as update NetEqImpl::GetAudioInternal accordingly.
Bug: chromium:335805780
Change-Id: Ia78f888d52c9acb4b53b7042b31b8b0c53199e7b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/392143
Reviewed-by: Per Åhgren <peah@webrtc.org>
Reviewed-by: Jakob Ivarsson <jakobi@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44787}
diff --git a/modules/audio_coding/neteq/audio_multi_vector.cc b/modules/audio_coding/neteq/audio_multi_vector.cc
index baad772..6ba4bc3 100644
--- a/modules/audio_coding/neteq/audio_multi_vector.cc
+++ b/modules/audio_coding/neteq/audio_multi_vector.cc
@@ -13,6 +13,7 @@
#include <algorithm>
#include <cstddef>
#include <cstdint>
+#include <memory>
#include <vector>
#include "api/array_view.h"
@@ -21,23 +22,26 @@
#include "rtc_base/checks.h"
namespace webrtc {
-
-AudioMultiVector::AudioMultiVector(size_t N) : channels_(N) {
- RTC_DCHECK_GT(N, 0);
- RTC_DCHECK_LE(N, kMaxNumberOfAudioChannels);
- for (auto& c : channels_) {
- c.reset(new AudioVector());
+namespace {
+std::vector<std::unique_ptr<AudioVector>> InitializeChannelVector(
+ size_t num_channels,
+ size_t channel_size = 0u) {
+ RTC_DCHECK_GT(num_channels, 0u);
+ RTC_CHECK_LE(num_channels, kMaxNumberOfAudioChannels);
+ std::vector<std::unique_ptr<AudioVector>> channels(num_channels);
+ for (auto& c : channels) {
+ c = channel_size ? std::make_unique<AudioVector>(channel_size)
+ : std::make_unique<AudioVector>();
}
+ return channels;
}
+} // namespace
+
+AudioMultiVector::AudioMultiVector(size_t N)
+ : channels_(InitializeChannelVector(N)) {}
AudioMultiVector::AudioMultiVector(size_t N, size_t initial_size)
- : channels_(N) {
- RTC_DCHECK_GT(N, 0);
- RTC_DCHECK_LE(N, kMaxNumberOfAudioChannels);
- for (auto& c : channels_) {
- c.reset(new AudioVector(initial_size));
- }
-}
+ : channels_(InitializeChannelVector(N, initial_size)) {}
AudioMultiVector::~AudioMultiVector() = default;
@@ -148,6 +152,27 @@
return index;
}
+bool AudioMultiVector::ReadInterleavedFromIndex(
+ const size_t start_index,
+ InterleavedView<int16_t> dst) const {
+ RTC_DCHECK_EQ(dst.num_channels(), Channels());
+ if (start_index + dst.samples_per_channel() > Size()) {
+ return false;
+ }
+ if (Channels() == 1) {
+ // Special case to avoid the nested for loop below.
+ return channels_[0]->CopyTo(start_index, dst.AsMono());
+ }
+ size_t index = 0;
+ for (size_t i = 0; i < dst.samples_per_channel(); ++i) {
+ for (const auto& ch : channels_) {
+ dst[index] = (*ch)[i + start_index];
+ ++index;
+ }
+ }
+ return true;
+}
+
size_t AudioMultiVector::ReadInterleavedFromEnd(size_t length,
int16_t* destination) const {
length = std::min(length, Size()); // Cannot read more than Size() elements.
diff --git a/modules/audio_coding/neteq/audio_multi_vector.h b/modules/audio_coding/neteq/audio_multi_vector.h
index b98e870..486c13a 100644
--- a/modules/audio_coding/neteq/audio_multi_vector.h
+++ b/modules/audio_coding/neteq/audio_multi_vector.h
@@ -18,6 +18,7 @@
#include <vector>
#include "api/array_view.h"
+#include "api/audio/audio_view.h"
#include "modules/audio_coding/neteq/audio_vector.h"
namespace webrtc {
@@ -86,6 +87,19 @@
size_t length,
int16_t* destination) const;
+ // Reads `dst.samples_per_channel()` from each channel into `dst`, a total of
+ // `dst.size()` samples, starting from the position provided by `start_index`.
+ //
+ // If not enough samples are available to read, then *none* will be read and
+ // the function returns false. If enough samples could be read, the return
+ // value will be true.
+ //
+ // This behavior is currently different from the pointer based
+ // `ReadInterleaved*` methods, but intentionally so in order to simplify the
+ // logic at the caller site.
+ bool ReadInterleavedFromIndex(const size_t start_index,
+ InterleavedView<int16_t> dst) const;
+
// Like ReadInterleaved() above, but reads from the end instead of from
// the beginning.
size_t ReadInterleavedFromEnd(size_t length, int16_t* destination) const;
@@ -129,7 +143,7 @@
AudioVector& operator[](size_t index);
protected:
- std::vector<std::unique_ptr<AudioVector>> channels_;
+ const std::vector<std::unique_ptr<AudioVector>> channels_;
};
} // namespace webrtc
diff --git a/modules/audio_coding/neteq/audio_multi_vector_unittest.cc b/modules/audio_coding/neteq/audio_multi_vector_unittest.cc
index 6193847..c9fe230 100644
--- a/modules/audio_coding/neteq/audio_multi_vector_unittest.cc
+++ b/modules/audio_coding/neteq/audio_multi_vector_unittest.cc
@@ -14,8 +14,10 @@
#include <cstdint>
#include <cstring>
+#include <memory>
#include <vector>
+#include "api/audio/audio_view.h"
#include "modules/audio_coding/neteq/audio_vector.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "test/gtest.h"
@@ -187,21 +189,37 @@
TEST_P(AudioMultiVectorTest, ReadInterleaved) {
AudioMultiVector vec(num_channels_);
vec.PushBackInterleaved(array_interleaved_);
- int16_t* output = new int16_t[array_interleaved_.size()];
+ std::unique_ptr<int16_t[]> output(new int16_t[array_interleaved_.size()]);
// Read 5 samples.
size_t read_samples = 5;
EXPECT_EQ(num_channels_ * read_samples,
- vec.ReadInterleaved(read_samples, output));
- EXPECT_EQ(0, memcmp(array_interleaved_.data(), output,
+ vec.ReadInterleaved(read_samples, output.get()));
+ EXPECT_EQ(0, memcmp(array_interleaved_.data(), output.get(),
read_samples * sizeof(int16_t)));
// Read too many samples. Expect to get all samples from the vector.
EXPECT_EQ(array_interleaved_.size(),
- vec.ReadInterleaved(array_length() + 1, output));
- EXPECT_EQ(0, memcmp(array_interleaved_.data(), output,
+ vec.ReadInterleaved(array_length() + 1, output.get()));
+ EXPECT_EQ(0, memcmp(array_interleaved_.data(), output.get(),
read_samples * sizeof(int16_t)));
+}
- delete[] output;
+TEST_P(AudioMultiVectorTest, ReadInterleavedView) {
+ AudioMultiVector vec(num_channels_);
+ vec.PushBackInterleaved(array_interleaved_);
+
+ // Read 5 samples.
+ size_t samples_per_channel = 5;
+ ASSERT_GT(array_length(), samples_per_channel);
+ std::unique_ptr<int16_t[]> buffer(new int16_t[array_interleaved_.size()]);
+ InterleavedView<int16_t> view(buffer.get(), samples_per_channel,
+ num_channels_);
+ EXPECT_TRUE(vec.ReadInterleavedFromIndex(0u, view));
+ EXPECT_EQ(0, memcmp(array_interleaved_.data(), &view[0],
+ view.size() * sizeof(int16_t)));
+ // Trying to read too much should result in failure.
+ // Attempt to read 5 samples when only 4 can be read.
+ EXPECT_FALSE(vec.ReadInterleavedFromIndex(vec.Size() - 4u, view));
}
// Test the PopFront method.
diff --git a/modules/audio_coding/neteq/audio_vector.cc b/modules/audio_coding/neteq/audio_vector.cc
index afbd6d4..096c8c4 100644
--- a/modules/audio_coding/neteq/audio_vector.cc
+++ b/modules/audio_coding/neteq/audio_vector.cc
@@ -16,6 +16,7 @@
#include <cstring>
#include <memory>
+#include "api/audio/audio_view.h"
#include "rtc_base/checks.h"
namespace webrtc {
@@ -62,6 +63,24 @@
}
}
+bool AudioVector::CopyTo(size_t position, MonoView<int16_t> dst) const {
+ if ((Size() - position) < dst.size()) {
+ return false;
+ }
+ const size_t copy_index = (begin_index_ + position) % capacity_;
+ const size_t first_chunk_length =
+ std::min(dst.size(), capacity_ - copy_index);
+ auto first_chunk = dst.subview(0, first_chunk_length);
+ CopySamples(first_chunk,
+ MonoView<const int16_t>(&array_[copy_index], first_chunk_length));
+ const size_t remaining_length = dst.size() - first_chunk_length;
+ if (remaining_length > 0) {
+ auto second_chunk = dst.subview(first_chunk_length, remaining_length);
+ CopySamples(second_chunk, MonoView<int16_t>(&array_[0], remaining_length));
+ }
+ return true;
+}
+
void AudioVector::PushFront(const AudioVector& prepend_this) {
const size_t length = prepend_this.Size();
if (length == 0)
diff --git a/modules/audio_coding/neteq/audio_vector.h b/modules/audio_coding/neteq/audio_vector.h
index 6b37aed..b8e7d49 100644
--- a/modules/audio_coding/neteq/audio_vector.h
+++ b/modules/audio_coding/neteq/audio_vector.h
@@ -16,6 +16,7 @@
#include <cstdint>
#include <memory>
+#include "api/audio/audio_view.h"
#include "rtc_base/checks.h"
namespace webrtc {
@@ -44,6 +45,11 @@
// Copies `length` values from `position` in this vector to `copy_to`.
void CopyTo(size_t length, size_t position, int16_t* copy_to) const;
+ // Copies `dst.size()` samples from a given position of the vector.
+ // If not enough samples are available, the function will return
+ // false and not do any copying.
+ bool CopyTo(size_t position, MonoView<int16_t> dst) const;
+
// Prepends the contents of AudioVector `prepend_this` to this object. The
// length of this object is increased with the length of `prepend_this`.
void PushFront(const AudioVector& prepend_this);
diff --git a/modules/audio_coding/neteq/audio_vector_unittest.cc b/modules/audio_coding/neteq/audio_vector_unittest.cc
index 97c1b7e..b215c4a 100644
--- a/modules/audio_coding/neteq/audio_vector_unittest.cc
+++ b/modules/audio_coding/neteq/audio_vector_unittest.cc
@@ -12,8 +12,10 @@
#include <stdlib.h>
+#include <array>
#include <cstdint>
+#include "api/audio/audio_view.h"
#include "rtc_base/numerics/safe_conversions.h"
#include "test/gtest.h"
@@ -28,7 +30,9 @@
}
}
- size_t array_length() const { return sizeof(array_) / sizeof(array_[0]); }
+ constexpr size_t array_length() const {
+ return sizeof(array_) / sizeof(array_[0]);
+ }
int16_t array_[10];
};
@@ -79,6 +83,34 @@
EXPECT_TRUE(vec_copy.Empty());
}
+TEST_F(AudioVectorTest, CopyTo) {
+ AudioVector vec;
+ vec.PushBack(array_, array_length());
+ ASSERT_EQ(vec.Size(), 10u);
+
+ std::array<int16_t, 10> buffer;
+ MonoView<int16_t> view(&buffer[0], buffer.size());
+ // Try to read 10 samples from position 1, which should fail.
+ EXPECT_FALSE(vec.CopyTo(1, view));
+ // Reading 10 samples from position 0, should succeed.
+ EXPECT_TRUE(vec.CopyTo(0, view));
+ EXPECT_EQ(view[5], 5); // sanity check.
+
+ // Free up space at the front of the buffer. This changes
+ // the internal start position without reallocating memory.
+ vec.PopFront(2);
+
+ std::array<int16_t, 2> new_values = {20, 21};
+ vec.PushBack(&new_values[0], new_values.size());
+
+ // Now the CopyTo() operation will need to wrap around
+ // the end of the internal buffer to fill the view.
+ EXPECT_TRUE(vec.CopyTo(0, view));
+ EXPECT_EQ(view[0], 2);
+ EXPECT_EQ(view[8], 20);
+ EXPECT_EQ(view[9], 21);
+}
+
// Test the PushBack method with another AudioVector as input argument.
TEST_F(AudioVectorTest, PushBackVector) {
static const size_t kLength = 10;
