Construct DecoderBuffer by transferring ownership of byte array.

Changed the data type of underlying buffer to unique_ptr<uint8_t[]>.
Removed the alignment and padding logic because ffmpeg copies the data
in av_packet_ref().

Bug: 834088
Change-Id: I984963b081ff24afb2f05aa0b7aeb718be7fa899
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090303Reviewed-by: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Qingwen Pi <qwp@google.com>
Cr-Commit-Position: refs/heads/master@{#747785}

media/base/decoder_buffer.cc

@@ -8,15 +8,6 @@
 
 namespace media {
 
-// Allocates a block of memory which is padded for use with the SIMD
-// optimizations used by FFmpeg.
-static uint8_t* AllocateFFmpegSafeBlock(size_t size) {
-  uint8_t* const block = reinterpret_cast<uint8_t*>(base::AlignedAlloc(
-      size + DecoderBuffer::kPaddingSize, DecoderBuffer::kAlignmentSize));
-  memset(block + size, 0, DecoderBuffer::kPaddingSize);
-  return block;
-}
-
 DecoderBuffer::DecoderBuffer(size_t size)
     : size_(size), side_data_size_(0), is_key_frame_(false) {
   Initialize();
@@ -46,6 +37,12 @@ DecoderBuffer::DecoderBuffer(const uint8_t* data,
   memcpy(side_data_.get(), side_data, side_data_size_);
 }
 
+DecoderBuffer::DecoderBuffer(std::unique_ptr<uint8_t[]> data, size_t size)
+    : size_(size),
+      data_(std::move(data)),
+      side_data_size_(0),
+      is_key_frame_(false) {}
+
 DecoderBuffer::DecoderBuffer(std::unique_ptr<UnalignedSharedMemory> shm,
                              size_t size)
     : size_(size),
@@ -67,9 +64,9 @@ DecoderBuffer::~DecoderBuffer() {
 }
 
 void DecoderBuffer::Initialize() {
-  data_.reset(AllocateFFmpegSafeBlock(size_));
+  data_.reset(new uint8_t[size_]);
   if (side_data_size_ > 0)
-    side_data_.reset(AllocateFFmpegSafeBlock(side_data_size_));
+    side_data_.reset(new uint8_t[side_data_size_]);
 }
 
 // static
@@ -92,6 +89,14 @@ scoped_refptr<DecoderBuffer> DecoderBuffer::CopyFrom(const uint8_t* data,
       new DecoderBuffer(data, data_size, side_data, side_data_size));
 }
 
+// static
+scoped_refptr<DecoderBuffer> DecoderBuffer::FromArray(
+    std::unique_ptr<uint8_t[]> data,
+    size_t size) {
+  CHECK(data);
+  return base::WrapRefCounted(new DecoderBuffer(std::move(data), size));
+}
+
 // static
 scoped_refptr<DecoderBuffer> DecoderBuffer::FromSharedMemoryRegion(
     base::subtle::PlatformSharedMemoryRegion region,
@@ -185,7 +190,7 @@ void DecoderBuffer::CopySideDataFrom(const uint8_t* side_data,
                                      size_t side_data_size) {
   if (side_data_size > 0) {
     side_data_size_ = side_data_size;
-    side_data_.reset(AllocateFFmpegSafeBlock(side_data_size_));
+    side_data_.reset(new uint8_t[side_data_size_]);
     memcpy(side_data_.get(), side_data, side_data_size_);
   } else {
     side_data_.reset();

media/base/decoder_buffer.h

@@ -28,10 +28,6 @@ namespace media {
 
 // A specialized buffer for interfacing with audio / video decoders.
 //
-// Specifically ensures that data is aligned and padded as necessary by the
-// underlying decoding framework.  On desktop platforms this means memory is
-// allocated using FFmpeg with particular alignment and padding requirements.
-//
 // Also includes decoder specific functionality for decryption.
 //
 // NOTE: It is illegal to call any method when end_of_stream() is true.
@@ -47,25 +43,30 @@ class MEDIA_EXPORT DecoderBuffer
 #endif
   };
 
-  // Allocates buffer with |size| >= 0.  Buffer will be padded and aligned
-  // as necessary, and |is_key_frame_| will default to false.
+  // Allocates buffer with |size| >= 0. |is_key_frame_| will default to false.
   explicit DecoderBuffer(size_t size);
 
-  // Create a DecoderBuffer whose |data_| is copied from |data|.  Buffer will be
-  // padded and aligned as necessary.  |data| must not be NULL and |size| >= 0.
-  // The buffer's |is_key_frame_| will default to false.
+  // Create a DecoderBuffer whose |data_| is copied from |data|. |data| must not
+  // be NULL and |size| >= 0. The buffer's |is_key_frame_| will default to
+  // false.
   static scoped_refptr<DecoderBuffer> CopyFrom(const uint8_t* data,
                                                size_t size);
 
   // Create a DecoderBuffer whose |data_| is copied from |data| and |side_data_|
-  // is copied from |side_data|. Buffers will be padded and aligned as necessary
-  // Data pointers must not be NULL and sizes must be >= 0. The buffer's
-  // |is_key_frame_| will default to false.
+  // is copied from |side_data|. Data pointers must not be NULL and sizes must
+  // be >= 0. The buffer's |is_key_frame_| will default to false.
   static scoped_refptr<DecoderBuffer> CopyFrom(const uint8_t* data,
                                                size_t size,
                                                const uint8_t* side_data,
                                                size_t side_data_size);
 
+  // Create a DecoderBuffer where data() of |size| bytes resides within the heap
+  // as byte array. The buffer's |is_key_frame_| will default to false.
+  //
+  // Ownership of |data| is transferred to the buffer.
+  static scoped_refptr<DecoderBuffer> FromArray(std::unique_ptr<uint8_t[]> data,
+                                                size_t size);
+
   // Create a DecoderBuffer where data() of |size| bytes resides within the
   // memory referred to by |region| at non-negative offset |offset|. The
   // buffer's |is_key_frame_| will default to false.
@@ -197,19 +198,21 @@ class MEDIA_EXPORT DecoderBuffer
  protected:
   friend class base::RefCountedThreadSafe<DecoderBuffer>;
 
-  // Allocates a buffer of size |size| >= 0 and copies |data| into it.  Buffer
-  // will be padded and aligned as necessary.  If |data| is NULL then |data_| is
-  // set to NULL and |buffer_size_| to 0.  |is_key_frame_| will default to
-  // false.
+  // Allocates a buffer of size |size| >= 0 and copies |data| into it. If |data|
+  // is NULL then |data_| is set to NULL and |buffer_size_| to 0.
+  // |is_key_frame_| will default to false.
   DecoderBuffer(const uint8_t* data,
                 size_t size,
                 const uint8_t* side_data,
                 size_t side_data_size);
 
+  DecoderBuffer(std::unique_ptr<uint8_t[]> data, size_t size);
+
   DecoderBuffer(std::unique_ptr<UnalignedSharedMemory> shm, size_t size);
 
   DecoderBuffer(std::unique_ptr<ReadOnlyUnalignedMapping> shared_mem_mapping,
                 size_t size);
+
   virtual ~DecoderBuffer();
 
  private:
@@ -221,11 +224,11 @@ class MEDIA_EXPORT DecoderBuffer
   // Size of the encoded data.
   size_t size_;
   // Encoded data, if it is stored on the heap.
-  std::unique_ptr<uint8_t, base::AlignedFreeDeleter> data_;
+  std::unique_ptr<uint8_t[]> data_;
 
   // Side data. Used for alpha channel in VPx, and for text cues.
   size_t side_data_size_;
-  std::unique_ptr<uint8_t, base::AlignedFreeDeleter> side_data_;
+  std::unique_ptr<uint8_t[]> side_data_;
 
   // Encoded data, if it is stored in a shared memory mapping.
   std::unique_ptr<ReadOnlyUnalignedMapping> shared_mem_mapping_;

media/base/decoder_buffer_unittest.cc

@@ -63,6 +63,21 @@ TEST(DecoderBufferTest, CopyFrom) {
   EXPECT_FALSE(buffer3->is_key_frame());
 }
 
+TEST(DecoderBufferTest, FromArray) {
+  const uint8_t kData[] = "hello";
+  const size_t kDataSize = base::size(kData);
+  std::unique_ptr<uint8_t[]> ptr(new uint8_t[kDataSize]);
+  memcpy(ptr.get(), kData, kDataSize);
+
+  scoped_refptr<DecoderBuffer> buffer(
+      DecoderBuffer::FromArray(std::move(ptr), kDataSize));
+  ASSERT_TRUE(buffer.get());
+  EXPECT_EQ(buffer->data_size(), kDataSize);
+  EXPECT_EQ(0, memcmp(buffer->data(), kData, kDataSize));
+  EXPECT_FALSE(buffer->end_of_stream());
+  EXPECT_FALSE(buffer->is_key_frame());
+}
+
 TEST(DecoderBufferTest, FromPlatformSharedMemoryRegion) {
   const uint8_t kData[] = "hello";
   const size_t kDataSize = base::size(kData);
@@ -171,35 +186,6 @@ TEST(DecoderBufferTest, FromSharedMemoryRegion_ZeroSize) {
   ASSERT_FALSE(buffer.get());
 }
 
-#if !defined(OS_ANDROID)
-TEST(DecoderBufferTest, PaddingAlignment) {
-  const uint8_t kData[] = "hello";
-  const size_t kDataSize = base::size(kData);
-  scoped_refptr<DecoderBuffer> buffer2(DecoderBuffer::CopyFrom(
-      reinterpret_cast<const uint8_t*>(&kData), kDataSize));
-  ASSERT_TRUE(buffer2.get());
-
-  // Padding data should always be zeroed.
-  for(int i = 0; i < DecoderBuffer::kPaddingSize; i++)
-    EXPECT_EQ((buffer2->data() + kDataSize)[i], 0);
-
-  // If the data is padded correctly we should be able to read and write past
-  // the end of the data by DecoderBuffer::kPaddingSize bytes without crashing
-  // or Valgrind/ASAN throwing errors.
-  const uint8_t kFillChar = 0xFF;
-  memset(
-      buffer2->writable_data() + kDataSize, kFillChar,
-      DecoderBuffer::kPaddingSize);
-  for(int i = 0; i < DecoderBuffer::kPaddingSize; i++)
-    EXPECT_EQ((buffer2->data() + kDataSize)[i], kFillChar);
-
-  EXPECT_EQ(0u, reinterpret_cast<uintptr_t>(
-      buffer2->data()) & (DecoderBuffer::kAlignmentSize - 1));
-
-  EXPECT_FALSE(buffer2->is_key_frame());
-}
-#endif
-
 TEST(DecoderBufferTest, ReadingWriting) {
   const char kData[] = "hello";
   const size_t kDataSize = base::size(kData);