Add CDM video decoder.

webkit/media/crypto/ppapi/ffmpeg_cdm_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 "webkit/media/crypto/ppapi/ffmpeg_cdm_video_frame.h"
+
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/base/limits.h"
+#include "webkit/media/crypto/ppapi/ffmpeg_cdm_video_decoder.h"
+
+

[xhwang, 2012/10/17 18:43:00]

remove extra empty line

[Tom Finegan, 2012/10/17 19:55:44]

Done.

+namespace webkit_media {
+
+namespace {
+
+inline size_t RoundUp(size_t value, size_t alignment) {

[xhwang, 2012/10/17 18:43:00]

Can we use "int" ?

[Tom Finegan, 2012/10/17 19:55:44]

Done.

+  // Check that |alignment| is a power of 2.
+  DCHECK((alignment + (alignment - 1)) == (alignment | (alignment - 1)));
+  return ((value + (alignment - 1)) & ~(alignment-1));
+}
+
+int RowBytes(cdm::VideoFormat format,
+             cdm::VideoFrame::VideoPlane plane,
+             int width) {
+  switch (format) {
+    case cdm::kYv12:
+    case cdm::kI420:
+      if (plane == cdm::VideoFrame::kYPlane)
+        return width;
+      return RoundUp(width, 2) / 2;
+
+    default:
+      NOTREACHED() << "Unsupported video frame format: " << format;
+      return 0;
+  }
+}
+
+}  // namespace

[xhwang, 2012/10/17 18:43:00]

media code prefer using static to anonymous namespace for historical reason. The
so only rule you are violating is "be consistent with existing code". :) 

[Tom Finegan, 2012/10/17 19:55:44]

Removed anonymous namespace, and made things static.

+
+// static
+FFmpegCdmVideoFrame* FFmpegCdmVideoFrame::Create(cdm::Allocator* allocator,
+                                                 cdm::VideoFormat format,
+                                                 const cdm::Size& size) {
+  if (!IsValidConfig(format, size)) {
+    LOG(ERROR) << "Create() invalid config.";
+    return NULL;
+  }
+
+  scoped_ptr<FFmpegCdmVideoFrame> frame(new FFmpegCdmVideoFrame(format, size));
+
+  if (!frame->AllocateYuv(allocator))
+    return NULL;
+
+  return frame.release();
+}
+
+FFmpegCdmVideoFrame::FFmpegCdmVideoFrame(cdm::VideoFormat format,
+                                         const cdm::Size& size)
+    : format_(format),
+      size_(size),
+      frame_buffer_(NULL) {
+  for (int32_t i = 0; i < cdm::VideoFrame::kMaxPlanes; ++i) {
+    plane_offsets_[i] = 0;
+    strides_[i] = 0;
+  }
+}
+
+FFmpegCdmVideoFrame::~FFmpegCdmVideoFrame() {
+  if (frame_buffer_)
+    frame_buffer_->Destroy();
+}
+
+int32_t FFmpegCdmVideoFrame::plane_offset(
+    cdm::VideoFrame::VideoPlane plane) const {
+  DCHECK(0 <= plane && plane < cdm::VideoFrame::kMaxPlanes);
+  return plane_offsets_[plane];
+}
+
+int32_t FFmpegCdmVideoFrame::stride(
+    cdm::VideoFrame::VideoPlane plane) const {
+  DCHECK(0 <= plane && plane < cdm::VideoFrame::kMaxPlanes);
+  return strides_[plane];
+}
+
+// static
+bool FFmpegCdmVideoFrame::IsValidConfig(cdm::VideoFormat format,

[xhwang, 2012/10/17 18:43:00]

This function is not using any class members. Make is a file scope static
function in this file instead of a class static function.

[Tom Finegan, 2012/10/17 19:55:44]

Can't do that-- this method is called from in this file, and from in
ffmpeg_cdm_video_decoder.cc.

Well, I can if you want me to make two copies of the function and declare it in
ffmpeg_cdm_video_frame.h, but that doesn't seem right.

[xhwang, 2012/10/17 20:23:10]

Ok, did't notice it's used in another file. In that case, make it a standalone
non-class non-static function?

[Tom Finegan, 2012/10/17 21:32:22]

I made it a static member function of FFmpegCdmVideoFrame for two reasons:

- This is what media::VideoFrame does (and it has the same characteristic of not
touching class members, but I think that's true for a lot of static member
functions)
- I don't want to put a method named IsValidConfig() in the webkit_media
namespace

The latter could be solved with naming it IsValidVideoFrameConfig(), but then
we're back to it looking like it should be a member of FFmpegCdmVideoFrame.

I guess all that I really want to know about this is why you want it changed.
Can we just leave this as-is?

+                                        const cdm::Size& data_size) {
+  return ((format == cdm::kYv12 || format == cdm::kI420) &&
+          data_size.width > 0 && data_size.height > 0 &&
+          data_size.width <= media::limits::kMaxDimension &&
+          data_size.height <= media::limits::kMaxDimension &&
+          data_size.width * data_size.height <= media::limits::kMaxCanvas);
+}
+
+void FFmpegCdmVideoFrame::TransferVideoFrame(cdm::VideoFrame* target) {

[ddorwin, 2012/10/17 06:10:19]

Should we clear this frame to invalid values so it doesn't get used? Or is that
overkill? Are there DCHECKs on frame_buffer_ that would cover this?

[Tom Finegan, 2012/10/17 19:55:44]

There's a DCHECK on frame_buffer() in FFmpegCdmVideoDecoder::DecodeFrame() when
ffmpeg says gives us a non-zero framed_decoded.

Invalidating the values doesn't seem like overkill. Done.

Also, added code setting the missing values from cdm::VideoFrame.

+  DCHECK(frame_buffer_);
+  DCHECK(target);
+  target->set_format(format_);
+  target->set_frame_buffer(frame_buffer_);
+  frame_buffer_ = NULL;
+
+  const int kMaxPlanes = cdm::VideoFrame::kMaxPlanes;
+  for (int i = 0; i < kMaxPlanes; ++i) {
+    cdm::VideoFrame::VideoPlane plane =
+        static_cast<cdm::VideoFrame::VideoPlane>(i);
+    target->set_plane_offset(plane, plane_offsets_[i]);
+    target->set_stride(plane, strides_[i]);
+  }
+}
+
+bool FFmpegCdmVideoFrame::AllocateYuv(cdm::Allocator* allocator) {
+  DCHECK(allocator);
+  DCHECK(format_ == cdm::kYv12 || format_ == cdm::kI420);
+
+  // Align Y rows at least at 32 byte boundaries for safety on all platforms.
+  DVLOG(3) << "AllocateYuv() width=" << size_.width << " height="
+           << size_.height;
+  size_t y_stride = RoundUp(
+      RowBytes(format_, cdm::VideoFrame::kYPlane, size_.width),
+      kBufferAlignment);
+  size_t uv_stride = RoundUp(
+      RowBytes(format_, cdm::VideoFrame::kUPlane, size_.width),
+      kBufferAlignment);

[xhwang, 2012/10/17 18:43:00]

So for Y, stride is RoundUp(width, kBufferAlignment).
For U and V, stride is 
RoundUp(RoundUp(width, 2) / 2, kBufferAlignment)
Why do we need to round-up twice for UV plane? If we do need it, add comments
here.

[Tom Finegan, 2012/10/17 19:55:44]

I think it's in case width is an odd number, which isn't strictly valid for
YV12/I420, but can happen. 

This is copied from media, and there's no explanation there. I'm not certain of
the problem it originally solved beyond what I've said above. I prefer keeping
this as it is in media.

[xhwang, 2012/10/17 20:23:10]

Since kBufferAlignment is even, I don't think it's necessary. But I agree we can
just keep it as it is in media now.

[Tom Finegan, 2012/10/17 21:32:22]

sgtm

+
+  size_t y_height = RoundUp(size_.height, kBufferAlignment);

[xhwang, 2012/10/17 18:43:00]

What exactly is kBufferAlignment (==32 for now)? Why do we need to round up
height? This could add up to 31 empty rows. It seems to me all rows are properly
aligned so the whole Y/U/V planes are all aligned already.

[Tom Finegan, 2012/10/17 19:55:44]

Added comment from media that explains this.

+  size_t uv_height = y_height / 2;
+  size_t y_bytes = y_height * y_stride;
+  size_t uv_bytes = uv_height * uv_stride;
+
+  frame_buffer_ = allocator->Allocate(y_bytes + (uv_bytes * 2));
+  if (!frame_buffer_) {
+    LOG(ERROR) << "AllocateYuv() cdm::Allocator::Allocate failed.";
+    return false;
+  }
+
+  COMPILE_ASSERT(0 == cdm::VideoFrame::kYPlane, y_plane_data_must_be_index_0);
+  uint8_t* data = frame_buffer_->data();
+  DCHECK_EQ(0U, reinterpret_cast<uintptr_t>(data) & (kBufferAlignment - 1));
+
+  plane_offsets_[cdm::VideoFrame::kYPlane] =  0;
+
+  if (format_ == cdm::kYv12) {
+    plane_offsets_[cdm::VideoFrame::kVPlane] = y_bytes;
+    plane_offsets_[cdm::VideoFrame::kUPlane] = y_bytes + uv_bytes;
+  } else {
+    plane_offsets_[cdm::VideoFrame::kUPlane] = y_bytes;
+    plane_offsets_[cdm::VideoFrame::kVPlane] = y_bytes + uv_bytes;
+  }
+
+  strides_[cdm::VideoFrame::kYPlane] = y_stride;
+  strides_[cdm::VideoFrame::kUPlane] = uv_stride;
+  strides_[cdm::VideoFrame::kVPlane] = uv_stride;
+
+  return true;
+}
+
+}  // namespace webkit_media