Replace av_malloc with AlignedAlloc for memory allocation in VideoFrame.

media/base/video_frame.cc

Index: media/base/video_frame.cc
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 28b4f9efe02d102812229d36345c024562d2731a..30c6484695f372f72dc000a01cc99d42b916a10f 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -9,12 +9,10 @@
 #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 {
 
@@ -144,6 +142,7 @@ static inline size_t RoundUp(size_t value, size_t alignment) {
   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.

@@ -151,13 +150,8 @@ static const int kFramePadBytes = 15;
 // 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) {
@@ -167,14 +161,9 @@ void VideoFrame::AllocateRGB(size_t bytes_per_pixel) {
                                  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);
@@ -203,18 +192,13 @@ void VideoFrame::AllocateYUV() {
   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;