Replace av_malloc with AlignedAlloc for memory allocation in VideoFrame.

media/base/video_frame.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/video_frame.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/logging.h"
+#include "base/memory/aligned_memory.h"
 #include "base/string_piece.h"
 #include "media/base/limits.h"
 #include "media/base/video_util.h"
-#if !defined(OS_ANDROID)
-#include "media/ffmpeg/ffmpeg_common.h"
-#endif
 
 namespace media {
 
 // static
 scoped_refptr<VideoFrame> VideoFrame::CreateFrame(
     VideoFrame::Format format,
     const gfx::Size& coded_size,
     const gfx::Rect& visible_rect,
     const gfx::Size& natural_size,
     base::TimeDelta timestamp) {
@@ skipping 109 matching lines @@
   const base::TimeDelta kZero;
   return CreateColorFrame(size, kBlackY, kBlackUV, kBlackUV, kZero);
 }
 
 static inline size_t RoundUp(size_t value, size_t alignment) {
   // Check that |alignment| is a power of 2.
   DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));
   return ((value + (alignment - 1)) & ~(alignment-1));
 }
 
+static const int kFrameAddressAlignment = 32;

[DaleCurtis, 2012/12/04 18:54:24]

Hmm, I'm worried about the proliferation of these constants and FFmpeg updates. 
Would it be possible to use the constants from DecoderBuffer:: which are
compile_assert checked against ffmpeg's?

[xhwang, 2012/12/04 19:40:50]

I am not sure if VideoFrame should depend on DecoderBuffer at all. How about
adding a separate set of enums VideoFrame and doing compile_assert for them too?

The reason I didn't do this in the first place is that I thought
FF_INPUT_BUFFER_PADDING_SIZE is only for input (not output). Is there one for
output too? The same question applies to kFFmpegInputBufferAlignmentSize, does
the output share the same values as the input?

[DaleCurtis, 2012/12/04 19:46:53]

Can we add a header file with the constants and use that header here and in
decoderbuffer?

The padding is traditionally for INPUT yes, but it seems odd to have two padding
constants with different values. 

[rbultje1, 2012/12/04 19:53:28]

FFmpeg has no constants for output padding, it is simply "15" (see utils.c).
Alignment constant can be shared, it is always 32.

[xhwang, 2012/12/04 20:54:56]

Hmm, I feel we should keep input and output buffer alignment/padding constants
separate. For example, what if later some codec needs to add some special
alignment/padding requirement on the input and/or output buffer? If we do this,
it's probably a better idea to keep those constants close to where it's used,
i.e. in decoder_buffer.h and video_frame.h separately. 

I am worrying about this also because fbarchard@ proposed some other
requirements (http://code.google.com/p/chromium/issues/detail?id=150920#c2; not
included in this CL though) that we may need to implement in the future.

WDYT?

 static const int kFrameSizeAlignment = 16;
 // Allows faster SIMD YUV convert. Also, FFmpeg overreads/-writes occasionally.
 static const int kFramePadBytes = 15;

[DaleCurtis, 2012/12/04 18:54:24]

This should be 16 and just set to DecoderBuffer::kPaddingSize.

[xhwang, 2012/12/04 19:40:50]

Will do.

 
 // Release data allocated by AllocateRGB() or AllocateYUV().
 static void ReleaseData(uint8* data) {
   DCHECK(data);
-  if (data) {
-#if !defined(OS_ANDROID)
-    av_free(data);
-#else
-    delete[] data;
-#endif
-  }
+  if (data)
+    base::AlignedFree(data);
 }
 
 void VideoFrame::AllocateRGB(size_t bytes_per_pixel) {
   // Round up to align at least at a 16-byte boundary for each row.
   // This is sufficient for MMX and SSE2 reads (movq/movdqa).
   size_t bytes_per_row = RoundUp(coded_size_.width(),
                                  kFrameSizeAlignment) * bytes_per_pixel;
   size_t aligned_height = RoundUp(coded_size_.height(), kFrameSizeAlignment);
   strides_[VideoFrame::kRGBPlane] = bytes_per_row;
-#if !defined(OS_ANDROID)
-  // TODO(dalecurtis): use DataAligned or so, so this #ifdef hackery
-  // doesn't need to be repeated in every single user of aligned data.
   data_[VideoFrame::kRGBPlane] = reinterpret_cast<uint8*>(
-      av_malloc(bytes_per_row * aligned_height + kFramePadBytes));
-#else
-  data_[VideoFrame::kRGBPlane] = new uint8_t[bytes_per_row * aligned_height];
-#endif
+      base::AlignedAlloc(bytes_per_row * aligned_height + kFramePadBytes,
+                         kFrameAddressAlignment));
   no_longer_needed_cb_ = base::Bind(&ReleaseData, data_[VideoFrame::kRGBPlane]);
   DCHECK(!(reinterpret_cast<intptr_t>(data_[VideoFrame::kRGBPlane]) & 7));
   COMPILE_ASSERT(0 == VideoFrame::kRGBPlane, RGB_data_must_be_index_0);
 }
 
 void VideoFrame::AllocateYUV() {
   DCHECK(format_ == VideoFrame::YV12 || format_ == VideoFrame::YV16);
   // Align Y rows at least at 16 byte boundaries.  The stride for both
   // YV12 and YV16 is 1/2 of the stride of Y.  For YV12, every row of bytes for
   // U and V applies to two rows of Y (one byte of UV for 4 bytes of Y), so in
   // the case of YV12 the strides are identical for the same width surface, but
   // the number of bytes allocated for YV12 is 1/2 the amount for U & V as
   // YV16. We also round the height of the surface allocated to be an even
   // number to avoid any potential of faulting by code that attempts to access
   // the Y values of the final row, but assumes that the last row of U & V
   // applies to a full two rows of Y.
   size_t y_stride = RoundUp(row_bytes(VideoFrame::kYPlane),
                             kFrameSizeAlignment);
   size_t uv_stride = RoundUp(row_bytes(VideoFrame::kUPlane),
                              kFrameSizeAlignment);
   // The *2 here is because some formats (e.g. h264) allow interlaced coding,
   // and then the size needs to be a multiple of two macroblocks (vertically).
   // See libavcodec/utils.c:avcodec_align_dimensions2().
   size_t y_height = RoundUp(coded_size_.height(), kFrameSizeAlignment * 2);
   size_t uv_height = format_ == VideoFrame::YV12 ? y_height / 2 : y_height;
   size_t y_bytes = y_height * y_stride;
   size_t uv_bytes = uv_height * uv_stride;
 
-#if !defined(OS_ANDROID)
-  // TODO(dalecurtis): use DataAligned or so, so this #ifdef hackery
-  // doesn't need to be repeated in every single user of aligned data.
   // The extra line of UV being allocated is because h264 chroma MC
   // overreads by one line in some cases, see libavcodec/utils.c:
   // avcodec_align_dimensions2() and libavcodec/x86/h264_chromamc.asm:
   // put_h264_chroma_mc4_ssse3().
   uint8* data = reinterpret_cast<uint8*>(
-      av_malloc(y_bytes + (uv_bytes * 2 + uv_stride) + kFramePadBytes));
-#else
-  uint8* data = new uint8_t[y_bytes + (uv_bytes * 2)];
-#endif
+      base::AlignedAlloc(y_bytes + (uv_bytes * 2 + uv_stride) + kFramePadBytes,
+                         kFrameAddressAlignment));
   no_longer_needed_cb_ = base::Bind(&ReleaseData, data);
   COMPILE_ASSERT(0 == VideoFrame::kYPlane, y_plane_data_must_be_index_0);
   data_[VideoFrame::kYPlane] = data;
   data_[VideoFrame::kUPlane] = data + y_bytes;
   data_[VideoFrame::kVPlane] = data + y_bytes + uv_bytes;
   strides_[VideoFrame::kYPlane] = y_stride;
   strides_[VideoFrame::kUPlane] = uv_stride;
   strides_[VideoFrame::kVPlane] = uv_stride;
 }
 
@@ skipping 116 matching lines @@
       break;
     for (int row = 0; row < rows(plane); ++row) {
       base::MD5Update(context, base::StringPiece(
           reinterpret_cast<char*>(data(plane) + stride(plane) * row),
           row_bytes(plane)));
     }
   }
 }
 
 }  // namespace media