Clean up VideoFrame::CreateXXX and VideoFrame::AllocXXX methods.

No need to confuse people with potentially-NULL pointers due to out-of-memory conditions.

Review URL: http://codereview.chromium.org/7396007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@92917 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 126 insertions, 151 deletions

diff --git a/media/base/pts_stream_unittest.cc b/media/base/pts_stream_unittest.cc
index abab227..22f3807 100644
--- a/media/base/pts_stream_unittest.cc
+++ b/media/base/pts_stream_unittest.cc
@@ -11,7 +11,7 @@ namespace media {
 class PtsStreamTest : public testing::Test {
  public:
   PtsStreamTest() {
-    VideoFrame::CreateBlackFrame(16, 16, &video_frame_);
+    video_frame_ = VideoFrame::CreateBlackFrame(16, 16);
 
     // Use typical frame rate of 25 fps.
     base::TimeDelta frame_duration = base::TimeDelta::FromMicroseconds(40000);
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 332c3c5..c78b5a4 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -29,117 +29,103 @@ size_t VideoFrame::GetNumberOfPlanes(VideoFrame::Format format) {
 }
 
 // static
-void VideoFrame::CreateFrame(VideoFrame::Format format,
-                             size_t width,
-                             size_t height,
-                             base::TimeDelta timestamp,
-                             base::TimeDelta duration,
-                             scoped_refptr<VideoFrame>* frame_out) {
+scoped_refptr<VideoFrame> VideoFrame::CreateFrame(
+    VideoFrame::Format format,
+    size_t width,
+    size_t height,
+    base::TimeDelta timestamp,
+    base::TimeDelta duration) {
   DCHECK(width > 0 && height > 0);
   DCHECK(width * height < 100000000);
-  DCHECK(frame_out);
-  bool alloc_worked = false;
   scoped_refptr<VideoFrame> frame(
       new VideoFrame(VideoFrame::TYPE_SYSTEM_MEMORY, format, width, height));
-  if (frame) {
-    frame->SetTimestamp(timestamp);
-    frame->SetDuration(duration);
-    switch (format) {
-      case VideoFrame::RGB555:
-      case VideoFrame::RGB565:
-        alloc_worked = frame->AllocateRGB(2u);
-        break;
-      case VideoFrame::RGB24:
-        alloc_worked = frame->AllocateRGB(3u);
-        break;
-      case VideoFrame::RGB32:
-      case VideoFrame::RGBA:
-        alloc_worked = frame->AllocateRGB(4u);
-        break;
-      case VideoFrame::YV12:
-      case VideoFrame::YV16:
-        alloc_worked = frame->AllocateYUV();
-        break;
-      case VideoFrame::ASCII:
-        alloc_worked = frame->AllocateRGB(1u);
-        break;
-      default:
-        NOTREACHED();
-        alloc_worked = false;
-        break;
-    }
+  frame->SetTimestamp(timestamp);
+  frame->SetDuration(duration);
+  switch (format) {
+    case VideoFrame::RGB555:
+    case VideoFrame::RGB565:
+      frame->AllocateRGB(2u);
+      break;
+    case VideoFrame::RGB24:
+      frame->AllocateRGB(3u);
+      break;
+    case VideoFrame::RGB32:
+    case VideoFrame::RGBA:
+      frame->AllocateRGB(4u);
+      break;
+    case VideoFrame::YV12:
+    case VideoFrame::YV16:
+      frame->AllocateYUV();
+      break;
+    case VideoFrame::ASCII:
+      frame->AllocateRGB(1u);
+      break;
+    default:
+      NOTREACHED();
+      return NULL;
   }
-  *frame_out = alloc_worked ? frame : NULL;
+  return frame;
 }
 
 // static
-void VideoFrame::CreateFrameExternal(SurfaceType type,
-                                     Format format,
-                                     size_t width,
-                                     size_t height,
-                                     size_t planes,
-                                     uint8* const data[kMaxPlanes],
-                                     const int32 strides[kMaxPlanes],
-                                     base::TimeDelta timestamp,
-                                     base::TimeDelta duration,
-                                     void* private_buffer,
-                                     scoped_refptr<VideoFrame>* frame_out) {
-  DCHECK(frame_out);
+scoped_refptr<VideoFrame> VideoFrame::CreateFrameExternal(
+    SurfaceType type,
+    Format format,
+    size_t width,
+    size_t height,
+    size_t planes,
+    uint8* const data[kMaxPlanes],
+    const int32 strides[kMaxPlanes],
+    base::TimeDelta timestamp,
+    base::TimeDelta duration,
+    void* private_buffer) {
   scoped_refptr<VideoFrame> frame(
       new VideoFrame(type, format, width, height));
-  if (frame) {
-    frame->SetTimestamp(timestamp);
-    frame->SetDuration(duration);
-    frame->external_memory_ = true;
-    frame->planes_ = planes;
-    frame->private_buffer_ = private_buffer;
-    for (size_t i = 0; i < kMaxPlanes; ++i) {
-      frame->data_[i] = data[i];
-      frame->strides_[i] = strides[i];
-    }
+  frame->SetTimestamp(timestamp);
+  frame->SetDuration(duration);
+  frame->external_memory_ = true;
+  frame->planes_ = planes;
+  frame->private_buffer_ = private_buffer;
+  for (size_t i = 0; i < kMaxPlanes; ++i) {
+    frame->data_[i] = data[i];
+    frame->strides_[i] = strides[i];
   }
-  *frame_out = frame;
+  return frame;
 }
 
 // static
-void VideoFrame::CreateFrameGlTexture(Format format,
-                                      size_t width,
-                                      size_t height,
-                                      GlTexture const textures[kMaxPlanes],
-                                      scoped_refptr<VideoFrame>* frame_out) {
-  DCHECK(frame_out);
+scoped_refptr<VideoFrame> VideoFrame::CreateFrameGlTexture(
+    Format format,
+    size_t width,
+    size_t height,
+    GlTexture const textures[kMaxPlanes]) {
   scoped_refptr<VideoFrame> frame(
       new VideoFrame(TYPE_GL_TEXTURE, format, width, height));
-  if (frame) {
-    frame->external_memory_ = true;
-    frame->planes_ = GetNumberOfPlanes(format);
-    for (size_t i = 0; i < kMaxPlanes; ++i) {
-      frame->gl_textures_[i] = textures[i];
-      // TODO(hclam): Fix me for color format other than RGBA.
-      frame->strides_[i] = width;
-    }
+  frame->external_memory_ = true;
+  frame->planes_ = GetNumberOfPlanes(format);
+  for (size_t i = 0; i < kMaxPlanes; ++i) {
+    frame->gl_textures_[i] = textures[i];
+    // TODO(hclam): Fix me for color format other than RGBA.
+    frame->strides_[i] = width;
   }
-  *frame_out = frame;
+  return frame;
 }
 
 // static
-void VideoFrame::CreateEmptyFrame(scoped_refptr<VideoFrame>* frame_out) {
-  *frame_out = new VideoFrame(VideoFrame::TYPE_SYSTEM_MEMORY,
-                              VideoFrame::EMPTY, 0, 0);
+scoped_refptr<VideoFrame> VideoFrame::CreateEmptyFrame() {
+  return new VideoFrame(VideoFrame::TYPE_SYSTEM_MEMORY,
+                        VideoFrame::EMPTY, 0, 0);
 }
 
 // static
-void VideoFrame::CreateBlackFrame(int width, int height,
-                                  scoped_refptr<VideoFrame>* frame_out) {
+scoped_refptr<VideoFrame> VideoFrame::CreateBlackFrame(int width, int height) {
   DCHECK_GT(width, 0);
   DCHECK_GT(height, 0);
 
   // Create our frame.
-  scoped_refptr<VideoFrame> frame;
   const base::TimeDelta kZero;
-  VideoFrame::CreateFrame(VideoFrame::YV12, width, height, kZero, kZero,
-                          &frame);
-  DCHECK(frame);
+  scoped_refptr<VideoFrame> frame =
+      VideoFrame::CreateFrame(VideoFrame::YV12, width, height, kZero, kZero);
 
   // Now set the data to YUV(0,128,128).
   const uint8 kBlackY = 0x00;
@@ -162,8 +148,7 @@ void VideoFrame::CreateBlackFrame(int width, int height,
     v_plane += frame->stride(VideoFrame::kVPlane);
   }
 
-  // Success!
-  *frame_out = frame;
+  return frame;
 }
 
 static inline size_t RoundUp(size_t value, size_t alignment) {
@@ -172,24 +157,21 @@ static inline size_t RoundUp(size_t value, size_t alignment) {
   return ((value + (alignment - 1)) & ~(alignment-1));
 }
 
-bool VideoFrame::AllocateRGB(size_t bytes_per_pixel) {
+void VideoFrame::AllocateRGB(size_t bytes_per_pixel) {
   // Round up to align at a 64-bit (8 byte) boundary for each row.  This
   // is sufficient for MMX reads (movq).
   size_t bytes_per_row = RoundUp(width_ * bytes_per_pixel, 8);
   planes_ = VideoFrame::kNumRGBPlanes;
   strides_[VideoFrame::kRGBPlane] = bytes_per_row;
   data_[VideoFrame::kRGBPlane] = new uint8[bytes_per_row * height_];
-  DCHECK(data_[VideoFrame::kRGBPlane]);
   DCHECK(!(reinterpret_cast<intptr_t>(data_[VideoFrame::kRGBPlane]) & 7));
   COMPILE_ASSERT(0 == VideoFrame::kRGBPlane, RGB_data_must_be_index_0);
-  return (NULL != data_[VideoFrame::kRGBPlane]);
 }
 
-static const int kFramePadBytes = 15;  // allows faster SIMD YUV convert
+static const int kFramePadBytes = 15;  // Allows faster SIMD YUV convert.
 
-bool VideoFrame::AllocateYUV() {
-  DCHECK(format_ == VideoFrame::YV12 ||
-         format_ == VideoFrame::YV16);
+void VideoFrame::AllocateYUV() {
+  DCHECK(format_ == VideoFrame::YV12 || format_ == VideoFrame::YV16);
   // Align Y rows at 32-bit (4 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
@@ -208,19 +190,14 @@ bool VideoFrame::AllocateYUV() {
     uv_bytes /= 2;
   }
   uint8* data = new uint8[y_bytes + (uv_bytes * 2) + kFramePadBytes];
-  if (data) {
-    planes_ = VideoFrame::kNumYUVPlanes;
-    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_bytes_per_row;
-    strides_[VideoFrame::kUPlane] = uv_stride;
-    strides_[VideoFrame::kVPlane] = uv_stride;
-    return true;
-  }
-  NOTREACHED();
-  return false;
+  planes_ = VideoFrame::kNumYUVPlanes;
+  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_bytes_per_row;
+  strides_[VideoFrame::kUPlane] = uv_stride;
+  strides_[VideoFrame::kVPlane] = uv_stride;
 }
 
 VideoFrame::VideoFrame(VideoFrame::SurfaceType type,
diff --git a/media/base/video_frame.h b/media/base/video_frame.h
index ab552895..a694c5f 100644
--- a/media/base/video_frame.h
+++ b/media/base/video_frame.h
@@ -57,43 +57,42 @@ class VideoFrame : public StreamSample {
 
   // Creates a new frame in system memory with given parameters. Buffers for
   // the frame are allocated but not initialized.
-  static void CreateFrame(Format format,
-                          size_t width,
-                          size_t height,
-                          base::TimeDelta timestamp,
-                          base::TimeDelta duration,
-                          scoped_refptr<VideoFrame>* frame_out);
+  static scoped_refptr<VideoFrame> CreateFrame(
+      Format format,
+      size_t width,
+      size_t height,
+      base::TimeDelta timestamp,
+      base::TimeDelta duration);
 
   // Creates a new frame with given parameters. Buffers for the frame are
   // provided externally. Reference to the buffers and strides are copied
   // from |data| and |strides| respectively.
-  static void CreateFrameExternal(SurfaceType type,
-                                  Format format,
-                                  size_t width,
-                                  size_t height,
-                                  size_t planes,
-                                  uint8* const data[kMaxPlanes],
-                                  const int32 strides[kMaxPlanes],
-                                  base::TimeDelta timestamp,
-                                  base::TimeDelta duration,
-                                  void* private_buffer,
-                                  scoped_refptr<VideoFrame>* frame_out);
+  static scoped_refptr<VideoFrame> CreateFrameExternal(
+      SurfaceType type,
+      Format format,
+      size_t width,
+      size_t height,
+      size_t planes,
+      uint8* const data[kMaxPlanes],
+      const int32 strides[kMaxPlanes],
+      base::TimeDelta timestamp,
+      base::TimeDelta duration,
+      void* private_buffer);
 
   // Creates a new frame with GL textures.
-  static void CreateFrameGlTexture(Format format,
-                                   size_t width,
-                                   size_t height,
-                                   GlTexture const textures[kMaxPlanes],
-                                   scoped_refptr<VideoFrame>* frame_out);
+  static scoped_refptr<VideoFrame> CreateFrameGlTexture(
+      Format format,
+      size_t width,
+      size_t height,
+      GlTexture const textures[kMaxPlanes]);
 
   // Creates a frame with format equals to VideoFrame::EMPTY, width, height
   // timestamp and duration are all 0.
-  static void CreateEmptyFrame(scoped_refptr<VideoFrame>* frame_out);
+  static scoped_refptr<VideoFrame> CreateEmptyFrame();
 
   // Allocates YV12 frame based on |width| and |height|, and sets its data to
   // the YUV equivalent of RGB(0,0,0).
-  static void CreateBlackFrame(int width, int height,
-                               scoped_refptr<VideoFrame>* frame_out);
+  static scoped_refptr<VideoFrame> CreateBlackFrame(int width, int height);
 
   SurfaceType type() const { return type_; }
 
@@ -130,8 +129,8 @@ class VideoFrame : public StreamSample {
   virtual ~VideoFrame();
 
   // Used internally by CreateFrame().
-  bool AllocateRGB(size_t bytes_per_pixel);
-  bool AllocateYUV();
+  void AllocateRGB(size_t bytes_per_pixel);
+  void AllocateYUV();
 
   // Frame format.
   Format format_;
diff --git a/media/base/video_frame_unittest.cc b/media/base/video_frame_unittest.cc
index a889271..8f6b4e7 100644
--- a/media/base/video_frame_unittest.cc
+++ b/media/base/video_frame_unittest.cc
@@ -52,12 +52,11 @@ void ExpectFrameColor(media::VideoFrame* yv12_frame, uint32 expect_rgb_color) {
             yv12_frame->stride(VideoFrame::kVPlane));
 
   scoped_refptr<media::VideoFrame> rgb_frame;
-  media::VideoFrame::CreateFrame(VideoFrame::RGBA,
-                                 yv12_frame->width(),
-                                 yv12_frame->height(),
-                                 yv12_frame->GetTimestamp(),
-                                 yv12_frame->GetDuration(),
-                                 &rgb_frame);
+  rgb_frame = media::VideoFrame::CreateFrame(VideoFrame::RGBA,
+                                             yv12_frame->width(),
+                                             yv12_frame->height(),
+                                             yv12_frame->GetTimestamp(),
+                                             yv12_frame->GetDuration());
 
   ASSERT_EQ(yv12_frame->width(), rgb_frame->width());
   ASSERT_EQ(yv12_frame->height(), rgb_frame->height());
@@ -95,9 +94,9 @@ TEST(VideoFrame, CreateFrame) {
   const base::TimeDelta kDurationB = base::TimeDelta::FromMicroseconds(5678);
 
   // Create a YV12 Video Frame.
-  scoped_refptr<media::VideoFrame> frame;
-  VideoFrame::CreateFrame(media::VideoFrame::YV12, kWidth, kHeight,
-                          kTimestampA, kDurationA, &frame);
+  scoped_refptr<media::VideoFrame> frame =
+      VideoFrame::CreateFrame(media::VideoFrame::YV12, kWidth, kHeight,
+                              kTimestampA, kDurationA);
   ASSERT_TRUE(frame);
 
   // Test StreamSample implementation.
@@ -129,7 +128,7 @@ TEST(VideoFrame, CreateFrame) {
   }
 
   // Test an empty frame.
-  VideoFrame::CreateEmptyFrame(&frame);
+  frame = VideoFrame::CreateEmptyFrame();
   EXPECT_TRUE(frame->IsEndOfStream());
 }
 
@@ -139,8 +138,8 @@ TEST(VideoFrame, CreateBlackFrame) {
   const uint8 kExpectedYRow[] = { 0, 0 };
   const uint8 kExpectedUVRow[] = { 128 };
 
-  scoped_refptr<media::VideoFrame> frame;
-  VideoFrame::CreateBlackFrame(kWidth, kHeight, &frame);
+  scoped_refptr<media::VideoFrame> frame =
+      VideoFrame::CreateBlackFrame(kWidth, kHeight);
   ASSERT_TRUE(frame);
 
   // Test basic properties.
@@ -174,14 +173,14 @@ TEST(VideoFrame, CreateBlackFrame) {
 TEST(VideoFram, CreateExternalFrame) {
   scoped_array<uint8> memory(new uint8[1]);
 
-  scoped_refptr<media::VideoFrame> frame;
   uint8* data[3] = {memory.get(), NULL, NULL};
   int strides[3] = {1, 0, 0};
-  VideoFrame::CreateFrameExternal(media::VideoFrame::TYPE_SYSTEM_MEMORY,
-                                  media::VideoFrame::RGB32, 0, 0, 3,
-                                  data, strides,
-                                  base::TimeDelta(), base::TimeDelta(),
-                                  NULL, &frame);
+  scoped_refptr<media::VideoFrame> frame =
+      VideoFrame::CreateFrameExternal(media::VideoFrame::TYPE_SYSTEM_MEMORY,
+                                      media::VideoFrame::RGB32, 0, 0, 3,
+                                      data, strides,
+                                      base::TimeDelta(), base::TimeDelta(),
+                                      NULL);
   ASSERT_TRUE(frame);
 
   // Test frame properties.