From a4ac4786a8ca503ac3cb15dfb0a7f6a62a9bdd52 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Fri, 29 Apr 2016 08:00:22 -0700 Subject: [PATCH] Define rtc::BufferT, like rtc::Buffer but for any trivial type And redefine rtc::Buffer as using Buffer = BufferT; (In the long run, I'd like to remove the type alias and rename the template to just rtc::Buffer, but that requires all current users of Buffer to start saying Buffer instead, and since Buffer is used in the API, we can't do that in one step.) The immediate reason for the new template is that we'd like to use BufferT in the AudioDecoder interface. BUG=webrtc:5801 Review-Url: https://codereview.webrtc.org/1929903002 Cr-Commit-Position: refs/heads/master@{#12564} --- webrtc/base/BUILD.gn | 1 - webrtc/base/base.gyp | 1 - webrtc/base/buffer.cc | 43 ------ webrtc/base/buffer.h | 252 +++++++++++++++++++------------ webrtc/base/buffer_unittest.cc | 60 +++++++- webrtc/base/copyonwritebuffer.h | 49 ++++-- webrtc/media/base/mediachannel.h | 2 +- 7 files changed, 252 insertions(+), 156 deletions(-) delete mode 100644 webrtc/base/buffer.cc diff --git a/webrtc/base/BUILD.gn b/webrtc/base/BUILD.gn index 11e886ccc0..63c2da533e 100644 --- a/webrtc/base/BUILD.gn +++ b/webrtc/base/BUILD.gn @@ -103,7 +103,6 @@ static_library("rtc_base_approved") { "bind.h", "bitbuffer.cc", "bitbuffer.h", - "buffer.cc", "buffer.h", "bufferqueue.cc", "bufferqueue.h", diff --git a/webrtc/base/base.gyp b/webrtc/base/base.gyp index 99ca457276..74b029c1f7 100644 --- a/webrtc/base/base.gyp +++ b/webrtc/base/base.gyp @@ -34,7 +34,6 @@ 'bind.h', 'bitbuffer.cc', 'bitbuffer.h', - 'buffer.cc', 'buffer.h', 'bufferqueue.cc', 'bufferqueue.h', diff --git a/webrtc/base/buffer.cc b/webrtc/base/buffer.cc deleted file mode 100644 index 6051e6db95..0000000000 --- a/webrtc/base/buffer.cc +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2015 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "webrtc/base/buffer.h" - -#include -#include - -namespace rtc { - -Buffer::Buffer() : size_(0), capacity_(0), data_(nullptr) { - RTC_DCHECK(IsConsistent()); -} - -Buffer::Buffer(Buffer&& buf) - : size_(buf.size()), - capacity_(buf.capacity()), - data_(std::move(buf.data_)) { - RTC_DCHECK(IsConsistent()); - buf.OnMovedFrom(); -} - -Buffer::Buffer(size_t size) : Buffer(size, size) { -} - -Buffer::Buffer(size_t size, size_t capacity) - : size_(size), - capacity_(std::max(size, capacity)), - data_(new uint8_t[capacity_]) { - RTC_DCHECK(IsConsistent()); -} - -// Note: The destructor works even if the buffer has been moved from. -Buffer::~Buffer() = default; - -}; // namespace rtc diff --git a/webrtc/base/buffer.h b/webrtc/base/buffer.h index f007929a29..b8b8fc0e16 100644 --- a/webrtc/base/buffer.h +++ b/webrtc/base/buffer.h @@ -13,79 +13,114 @@ #include #include +#include #include #include "webrtc/base/array_view.h" #include "webrtc/base/checks.h" -#include "webrtc/base/constructormagic.h" namespace rtc { namespace internal { -// (Internal; please don't use outside this file.) ByteType::t is int if T -// is uint8_t, int8_t, or char; otherwise, it's a compilation error. Use like -// this: -// -// template ::t = 0> -// void foo(T* x); -// -// to let foo be defined only for byte-sized integers. -template -struct ByteType { - private: - static int F(uint8_t*); - static int F(int8_t*); - static int F(char*); - - public: - using t = decltype(F(static_cast(nullptr))); +// (Internal; please don't use outside this file.) Determines if elements of +// type U are compatible with a BufferT. For most types, we just ignore +// top-level const and forbid top-level volatile and require T and U to be +// otherwise equal, but all byte-sized integers (notably char, int8_t, and +// uint8_t) are compatible with each other. (Note: We aim to get rid of this +// behavior, and treat all types the same.) +template +struct BufferCompat { + static constexpr bool value = + !std::is_volatile::value && + ((std::is_integral::value && sizeof(T) == 1) + ? (std::is_integral::value && sizeof(U) == 1) + : (std::is_same::type>::value)); }; } // namespace internal // Basic buffer class, can be grown and shrunk dynamically. -// Unlike std::string/vector, does not initialize data when expanding capacity. -class Buffer { +// Unlike std::string/vector, does not initialize data when increasing size. +template +class BufferT { + // We want T's destructor and default constructor to be trivial, i.e. perform + // no action, so that we don't have to touch the memory we allocate and + // deallocate. And we want T to be trivially copyable, so that we can copy T + // instances with std::memcpy. This is precisely the definition of a trivial + // type. + static_assert(std::is_trivial::value, "T must be a trivial type."); + + // This class relies heavily on being able to mutate its data. + static_assert(!std::is_const::value, "T may not be const"); + public: - Buffer(); // An empty buffer. - Buffer(Buffer&& buf); // Move contents from an existing buffer. + // An empty BufferT. + BufferT() : size_(0), capacity_(0), data_(nullptr) { + RTC_DCHECK(IsConsistent()); + } - // Construct a buffer with the specified number of uninitialized bytes. - explicit Buffer(size_t size); - Buffer(size_t size, size_t capacity); + // Disable copy construction and copy assignment, since copying a buffer is + // expensive enough that we want to force the user to be explicit about it. + BufferT(const BufferT&) = delete; + BufferT& operator=(const BufferT&) = delete; - // Construct a buffer and copy the specified number of bytes into it. The - // source array may be (const) uint8_t*, int8_t*, or char*. - template ::t = 0> - Buffer(const T* data, size_t size) - : Buffer(data, size, size) {} + BufferT(BufferT&& buf) + : size_(buf.size()), + capacity_(buf.capacity()), + data_(std::move(buf.data_)) { + RTC_DCHECK(IsConsistent()); + buf.OnMovedFrom(); + } - template ::t = 0> - Buffer(const T* data, size_t size, size_t capacity) - : Buffer(size, capacity) { - std::memcpy(data_.get(), data, size); + // Construct a buffer with the specified number of uninitialized elements. + explicit BufferT(size_t size) : BufferT(size, size) {} + + BufferT(size_t size, size_t capacity) + : size_(size), + capacity_(std::max(size, capacity)), + data_(new T[capacity_]) { + RTC_DCHECK(IsConsistent()); + } + + // Construct a buffer and copy the specified number of elements into it. + template ::value>::type* = nullptr> + BufferT(const U* data, size_t size) : BufferT(data, size, size) {} + + template ::value>::type* = nullptr> + BufferT(U* data, size_t size, size_t capacity) : BufferT(size, capacity) { + static_assert(sizeof(T) == sizeof(U), ""); + std::memcpy(data_.get(), data, size * sizeof(U)); } // Construct a buffer from the contents of an array. - template ::t = 0> - Buffer(const T(&array)[N]) - : Buffer(array, N) {} + template ::value>::type* = nullptr> + BufferT(U (&array)[N]) : BufferT(array, N) {} - ~Buffer(); - - // Get a pointer to the data. Just .data() will give you a (const) uint8_t*, - // but you may also use .data() and .data(). - template ::t = 0> - const T* data() const { + // Get a pointer to the data. Just .data() will give you a (const) T*, but if + // T is a byte-sized integer, you may also use .data() for any other + // byte-sized integer U. + template ::value>::type* = nullptr> + const U* data() const { RTC_DCHECK(IsConsistent()); - return reinterpret_cast(data_.get()); + return reinterpret_cast(data_.get()); } - template ::t = 0> - T* data() { + template ::value>::type* = nullptr> + U* data() { RTC_DCHECK(IsConsistent()); - return reinterpret_cast(data_.get()); + return reinterpret_cast(data_.get()); } size_t size() const { @@ -98,7 +133,7 @@ class Buffer { return capacity_; } - Buffer& operator=(Buffer&& buf) { + BufferT& operator=(BufferT&& buf) { RTC_DCHECK(IsConsistent()); RTC_DCHECK(buf.IsConsistent()); size_ = buf.size_; @@ -108,94 +143,120 @@ class Buffer { return *this; } - bool operator==(const Buffer& buf) const { + bool operator==(const BufferT& buf) const { RTC_DCHECK(IsConsistent()); - return size_ == buf.size() && memcmp(data_.get(), buf.data(), size_) == 0; + if (size_ != buf.size_) { + return false; + } + if (std::is_integral::value) { + // Optimization. + return std::memcmp(data_.get(), buf.data_.get(), size_ * sizeof(T)) == 0; + } + for (size_t i = 0; i < size_; ++i) { + if (data_[i] != buf.data_[i]) { + return false; + } + } + return true; } - bool operator!=(const Buffer& buf) const { return !(*this == buf); } + bool operator!=(const BufferT& buf) const { return !(*this == buf); } - uint8_t& operator[](size_t index) { + T& operator[](size_t index) { RTC_DCHECK_LT(index, size_); return data()[index]; } - uint8_t operator[](size_t index) const { + T operator[](size_t index) const { RTC_DCHECK_LT(index, size_); return data()[index]; } // The SetData functions replace the contents of the buffer. They accept the // same input types as the constructors. - template ::t = 0> - void SetData(const T* data, size_t size) { + template ::value>::type* = nullptr> + void SetData(const U* data, size_t size) { RTC_DCHECK(IsConsistent()); size_ = 0; AppendData(data, size); } - template ::t = 0> - void SetData(const T(&array)[N]) { + template ::value>::type* = nullptr> + void SetData(const U (&array)[N]) { SetData(array, N); } - void SetData(const Buffer& buf) { SetData(buf.data(), buf.size()); } + void SetData(const BufferT& buf) { SetData(buf.data(), buf.size()); } - // Replace the data in the buffer with at most |max_bytes| of data, using the - // function |setter|, which should have the following signature: - // size_t setter(ArrayView view) + // Replace the data in the buffer with at most |max_elements| of data, using + // the function |setter|, which should have the following signature: + // size_t setter(ArrayView view) // |setter| is given an appropriately typed ArrayView of the area in which to // write the data (i.e. starting at the beginning of the buffer) and should - // return the number of bytes actually written. This number must be <= - // |max_bytes|. - template ::t = 0> - size_t SetData(size_t max_bytes, F&& setter) { + // return the number of elements actually written. This number must be <= + // |max_elements|. + template ::value>::type* = nullptr> + size_t SetData(size_t max_elements, F&& setter) { RTC_DCHECK(IsConsistent()); size_ = 0; - return AppendData(max_bytes, std::forward(setter)); + return AppendData(max_elements, std::forward(setter)); } - // The AppendData functions adds data to the end of the buffer. They accept + // The AppendData functions add data to the end of the buffer. They accept // the same input types as the constructors. - template ::t = 0> - void AppendData(const T* data, size_t size) { + template ::value>::type* = nullptr> + void AppendData(const U* data, size_t size) { RTC_DCHECK(IsConsistent()); const size_t new_size = size_ + size; EnsureCapacity(new_size); - std::memcpy(data_.get() + size_, data, size); + static_assert(sizeof(T) == sizeof(U), ""); + std::memcpy(data_.get() + size_, data, size * sizeof(U)); size_ = new_size; RTC_DCHECK(IsConsistent()); } - template ::t = 0> - void AppendData(const T(&array)[N]) { + template ::value>::type* = nullptr> + void AppendData(const U (&array)[N]) { AppendData(array, N); } - void AppendData(const Buffer& buf) { AppendData(buf.data(), buf.size()); } + void AppendData(const BufferT& buf) { AppendData(buf.data(), buf.size()); } - // Append at most |max_bytes| of data to the end of the buffer, using the - // function |setter|, which should have the following signature: - // size_t setter(ArrayView view) + // Append at most |max_elements| to the end of the buffer, using the function + // |setter|, which should have the following signature: + // size_t setter(ArrayView view) // |setter| is given an appropriately typed ArrayView of the area in which to // write the data (i.e. starting at the former end of the buffer) and should - // return the number of bytes actually written. This number must be <= - // |max_bytes|. - template ::t = 0> - size_t AppendData(size_t max_bytes, F&& setter) { + // return the number of elements actually written. This number must be <= + // |max_elements|. + template ::value>::type* = nullptr> + size_t AppendData(size_t max_elements, F&& setter) { RTC_DCHECK(IsConsistent()); const size_t old_size = size_; - SetSize(old_size + max_bytes); - T *base_ptr = data() + old_size; - size_t written_bytes = - setter(rtc::ArrayView(base_ptr, max_bytes)); + SetSize(old_size + max_elements); + U* base_ptr = data() + old_size; + size_t written_elements = setter(rtc::ArrayView(base_ptr, max_elements)); - RTC_CHECK_LE(written_bytes, max_bytes); - size_ = old_size + written_bytes; + RTC_CHECK_LE(written_elements, max_elements); + size_ = old_size + written_elements; RTC_DCHECK(IsConsistent()); - return written_bytes; + return written_elements; } // Sets the size of the buffer. If the new size is smaller than the old, the @@ -214,8 +275,8 @@ class Buffer { RTC_DCHECK(IsConsistent()); if (capacity <= capacity_) return; - std::unique_ptr new_data(new uint8_t[capacity]); - std::memcpy(new_data.get(), data_.get(), size_); + std::unique_ptr new_data(new T[capacity]); + std::memcpy(new_data.get(), data_.get(), size_ * sizeof(T)); data_ = std::move(new_data); capacity_ = capacity; RTC_DCHECK(IsConsistent()); @@ -229,7 +290,7 @@ class Buffer { } // Swaps two buffers. Also works for buffers that have been moved from. - friend void swap(Buffer& a, Buffer& b) { + friend void swap(BufferT& a, BufferT& b) { using std::swap; swap(a.size_, b.size_); swap(a.capacity_, b.capacity_); @@ -262,11 +323,12 @@ class Buffer { size_t size_; size_t capacity_; - std::unique_ptr data_; - - RTC_DISALLOW_COPY_AND_ASSIGN(Buffer); + std::unique_ptr data_; }; +// By far the most common sort of buffer. +using Buffer = BufferT; + } // namespace rtc #endif // WEBRTC_BASE_BUFFER_H_ diff --git a/webrtc/base/buffer_unittest.cc b/webrtc/base/buffer_unittest.cc index 2f3bcfd606..e9a853c615 100644 --- a/webrtc/base/buffer_unittest.cc +++ b/webrtc/base/buffer_unittest.cc @@ -11,8 +11,8 @@ #include "webrtc/base/buffer.h" #include "webrtc/base/gunit.h" -#include // std::swap (pre-C++11) -#include // std::swap (C++11 and later) +#include +#include namespace rtc { @@ -301,4 +301,60 @@ TEST(BufferTest, TestBracketWrite) { } } +TEST(BufferTest, TestInt16) { + static constexpr int16_t test_data[] = {14, 15, 16, 17, 18}; + BufferT buf(test_data); + EXPECT_EQ(buf.size(), 5u); + EXPECT_EQ(buf.capacity(), 5u); + EXPECT_NE(buf.data(), nullptr); + for (size_t i = 0; i != buf.size(); ++i) { + EXPECT_EQ(test_data[i], buf[i]); + } + BufferT buf2(test_data); + EXPECT_EQ(buf, buf2); + buf2[0] = 9; + EXPECT_NE(buf, buf2); +} + +TEST(BufferTest, TestFloat) { + static constexpr float test_data[] = {14, 15, 16, 17, 18}; + BufferT buf; + EXPECT_EQ(buf.size(), 0u); + EXPECT_EQ(buf.capacity(), 0u); + EXPECT_EQ(buf.data(), nullptr); + buf.SetData(test_data); + EXPECT_EQ(buf.size(), 5u); + EXPECT_EQ(buf.capacity(), 5u); + EXPECT_NE(buf.data(), nullptr); + float* p1 = buf.data(); + while (buf.data() == p1) { + buf.AppendData(test_data); + } + EXPECT_EQ(buf.size(), buf.capacity()); + EXPECT_GT(buf.size(), 5u); + EXPECT_EQ(buf.size() % 5, 0u); + EXPECT_NE(buf.data(), nullptr); + for (size_t i = 0; i != buf.size(); ++i) { + EXPECT_EQ(test_data[i % 5], buf[i]); + } +} + +TEST(BufferTest, TestStruct) { + struct BloodStone { + bool blood; + const char* stone; + }; + BufferT buf(4); + EXPECT_EQ(buf.size(), 4u); + EXPECT_EQ(buf.capacity(), 4u); + EXPECT_NE(buf.data(), nullptr); + BufferT buf2(4); + for (size_t i = 0; i < buf2.size(); ++i) { + buf2[i] = &buf[i]; + } + static const char kObsidian[] = "obsidian"; + buf2[2]->stone = kObsidian; + EXPECT_EQ(kObsidian, buf[2].stone); +} + } // namespace rtc diff --git a/webrtc/base/copyonwritebuffer.h b/webrtc/base/copyonwritebuffer.h index 87f24bf51d..a7e52beea5 100644 --- a/webrtc/base/copyonwritebuffer.h +++ b/webrtc/base/copyonwritebuffer.h @@ -36,10 +36,14 @@ class CopyOnWriteBuffer { // Construct a buffer and copy the specified number of bytes into it. The // source array may be (const) uint8_t*, int8_t*, or char*. - template ::t = 0> + template ::value>::type* = nullptr> CopyOnWriteBuffer(const T* data, size_t size) : CopyOnWriteBuffer(data, size, size) {} - template ::t = 0> + template ::value>::type* = nullptr> CopyOnWriteBuffer(const T* data, size_t size, size_t capacity) : CopyOnWriteBuffer(size, capacity) { if (buffer_) { @@ -48,22 +52,29 @@ class CopyOnWriteBuffer { } // Construct a buffer from the contents of an array. - template ::t = 0> - CopyOnWriteBuffer(const T(&array)[N]) // NOLINT: runtime/explicit + template ::value>::type* = nullptr> + CopyOnWriteBuffer(const T (&array)[N]) // NOLINT: runtime/explicit : CopyOnWriteBuffer(array, N) {} ~CopyOnWriteBuffer(); // Get a pointer to the data. Just .data() will give you a (const) uint8_t*, // but you may also use .data() and .data(). - template ::t = 0> + template ::value>::type* = nullptr> const T* data() const { return cdata(); } // Get writable pointer to the data. This will create a copy of the underlying // data if it is shared with other buffers. - template ::t = 0> + template ::value>::type* = nullptr> T* data() { RTC_DCHECK(IsConsistent()); if (!buffer_) { @@ -75,7 +86,9 @@ class CopyOnWriteBuffer { // Get const pointer to the data. This will not create a copy of the // underlying data if it is shared with other buffers. - template ::t = 0> + template ::value>::type* = nullptr> T* cdata() const { RTC_DCHECK(IsConsistent()); if (!buffer_) { @@ -137,7 +150,9 @@ class CopyOnWriteBuffer { // Replace the contents of the buffer. Accepts the same types as the // constructors. - template ::t = 0> + template ::value>::type* = nullptr> void SetData(const T* data, size_t size) { RTC_DCHECK(IsConsistent()); if (!buffer_ || !buffer_->HasOneRef()) { @@ -149,8 +164,11 @@ class CopyOnWriteBuffer { RTC_DCHECK(IsConsistent()); } - template ::t = 0> - void SetData(const T(&array)[N]) { + template ::value>::type* = nullptr> + void SetData(const T (&array)[N]) { SetData(array, N); } @@ -163,7 +181,9 @@ class CopyOnWriteBuffer { } // Append data to the buffer. Accepts the same types as the constructors. - template ::t = 0> + template ::value>::type* = nullptr> void AppendData(const T* data, size_t size) { RTC_DCHECK(IsConsistent()); if (!buffer_) { @@ -178,8 +198,11 @@ class CopyOnWriteBuffer { RTC_DCHECK(IsConsistent()); } - template ::t = 0> - void AppendData(const T(&array)[N]) { + template ::value>::type* = nullptr> + void AppendData(const T (&array)[N]) { AppendData(array, N); } diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index bd1185f722..7c054b6f3a 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -17,6 +17,7 @@ #include "webrtc/api/rtpparameters.h" #include "webrtc/base/basictypes.h" +#include "webrtc/base/buffer.h" #include "webrtc/base/copyonwritebuffer.h" #include "webrtc/base/dscp.h" #include "webrtc/base/logging.h" @@ -34,7 +35,6 @@ #include "webrtc/pc/audiomonitor.h" namespace rtc { -class Buffer; class RateLimiter; class Timing; }