Implement media::VideoDecodeAccelerator on Mac

content/common/gpu/media/mac_video_decode_accelerator.mm

Index: content/common/gpu/media/mac_video_decode_accelerator.mm
diff --git a/content/common/gpu/media/mac_video_decode_accelerator.mm b/content/common/gpu/media/mac_video_decode_accelerator.mm
new file mode 100644
index 0000000000000000000000000000000000000000..8d572c8af1cd7954fa4051b09f9edd8733f13c72
--- /dev/null
+++ b/content/common/gpu/media/mac_video_decode_accelerator.mm
@@ -0,0 +1,310 @@
+// 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 "content/common/gpu/media/mac_video_decode_accelerator.h"
+
+#include "base/bind.h"
+#include "base/file_path.h"
+#import "base/mac/foundation_util.h"
+#import "base/memory/ref_counted_memory.h"
+#import "base/message_loop.h"
+#include "base/location.h"
+#include "base/native_library.h"
+#include "ui/surface/io_surface_support_mac.h"
+#include "ui/gfx/video_decode_acceleration_support_mac.h"
+
+namespace {
+
+enum { kNumPictureBuffers = 4 };
+
+class ScopedContextSetter {
+ public:
+  ScopedContextSetter(CGLContextObj context)
+      : old_context_(NULL),
+        did_succeed_(false) {
+    old_context_ = CGLGetCurrentContext();
+    did_succeed_ = CGLSetCurrentContext(context) == kCGLNoError;
+  }
+
+  ~ScopedContextSetter() {
+    if (did_succeed_)
+      CGLSetCurrentContext(old_context_);
+  }
+
+  bool did_succeed() const {
+    return did_succeed_;
+  }
+
+ private:
+  CGLContextObj old_context_;
+  bool did_succeed_;
+};
+
+}  // namespace
+
+static bool BindImageToTexture(CGLContextObj context,
+                               CVImageBufferRef image,
+                               uint32 texture_id) {
+  ScopedContextSetter scoped_context_setter(context);
+  if (!scoped_context_setter.did_succeed()) {
+    DLOG(ERROR) << "Unable to set OpenGL context.";

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

s/DLOG(ERROR)/DVLOG(1)/
here and everywhere below (see chromium-dev traffic about why).

[sail, 2012/05/29 03:45:01]

Done.

+    return false;
+  }
+
+  IOSurfaceSupport* io_surface_support = IOSurfaceSupport::Initialize();
+  DCHECK(io_surface_support);
+
+  CFTypeRef io_surface =
+      io_surface_support->CVPixelBufferGetIOSurface(image);
+  if (!io_surface) {
+    DLOG(ERROR) << "Unable to get IOSurface for CVPixelBuffer.";

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

I'm afraid I wasn't clear before.  The problem with the unhappy codepaths was
only half missing logging.  The bigger half of the problem is whether the
failure should make the MacVDA instance unusable going forward, and a secondary
concern is whether a particular failure should trigger client notification of
error.
Wrapping these three things (logging, setting error status, and notifying
client) in a macro helps to make sure you never forget one or more of them.  

[sail, 2012/05/29 03:45:01]

Done.

To mark the MacVDA instance unusable I'm just clearing the vda_support_ member
variable. This is simpler than adding a new state.

[sail, 2012/05/29 23:22:57]

Actually, I changed this to check for a client_ instead. This way it's
compatible with my later CL that moves the vda_support_ creation into Decode().

+    return false;
+  }
+
+  glEnable(GL_TEXTURE_RECTANGLE_ARB);
+  glBindTexture(GL_TEXTURE_RECTANGLE_ARB, texture_id);
+  if (io_surface_support->CGLTexImageIOSurface2D(
+          context,
+          GL_TEXTURE_RECTANGLE_ARB,
+          GL_RGB,
+          io_surface_support->IOSurfaceGetWidth(io_surface),
+          io_surface_support->IOSurfaceGetHeight(io_surface),
+          GL_YCBCR_422_APPLE,
+          GL_UNSIGNED_SHORT_8_8_APPLE,
+          io_surface,
+          0) != kCGLNoError) {
+    DLOG(ERROR) << "Failed to bind image to texture.";
+    return false;
+  }
+  return glGetError() == GL_NO_ERROR;
+}
+
+MacVideoDecodeAccelerator::MacVideoDecodeAccelerator(
+    media::VideoDecodeAccelerator::Client* client)
+    : client_(client),
+      cgl_context_(NULL),
+      nalu_len_field_size_(0),
+      did_request_pictures_(false) {
+}
+
+void MacVideoDecodeAccelerator::SetGLContext(void* gl_context) {
+  DCHECK(CalledOnValidThread());
+  cgl_context_ = static_cast<CGLContextObj>(gl_context);
+}
+
+bool MacVideoDecodeAccelerator::SetConfigInfo(
+    uint32_t frame_width,

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

This config info includes width & height, and the mid-stream frame size changes
I'm talking about are changes in width & height.  How can they be unrelated?

[sail, 2012/05/29 03:45:01]

So taking a step back.

All the Mac video API client code that I've seen do this:
  VDACreate(extra_data)
  while (!end of data) {
    VDADecode();
  }
  VDADestroy();

So if there's a frame size change in the middle of the stream I assume
VDADecode() will either handle it or not handle it. Either way it doesn't seem
like there's much I should be doing.

Also, frame size is just one example of the information encoded in the AVC
configuration record.

If you can point me at a sample video I can test the Mac VDA API and see what it
does. I've found that getting sample videos that the Mac video decoder can
handle is tricky. It's very picky about what it supports.

[Ami GONE FROM CHROMIUM, 2012/05/30 00:29:58]

Presumably you need to request different textures, no?
There's a mid-stream frame-size-changing .webm checked into
third_party/WebKit/LayoutTests/platform/chromium/media/resources/frame_size_change.webm
but that's webm.  You'll have to transcode it to h264 to test.

[sail, 2012/05/30 20:13:46]

Sounds good. I've filed bug 130352 to track this. Will work on it ASAP.

+    uint32_t frame_height,

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

Is your plan to land this first and then immediately land
https://chromiumcodereview.appspot.com/10411085/ top of it, or what?

[sail, 2012/05/29 03:45:01]

I'm planning to land the various CLs one after another but as long as the
ordering is correct they won't break the build.

I'll hold off on hooking up the Mac VDA code to GpuVideoDecodeAccelerator till
the very last CL.

+    const std::vector<uint8_t>& avc_data) {
+  DCHECK(CalledOnValidThread());
+  frame_size_ = gfx::Size(frame_width, frame_height);
+  nalu_len_field_size_ = (avc_data[4] & 0x03) + 1;
+
+  DCHECK(!vda_support_.get());
+  vda_support_ = new gfx::VideoDecodeAccelerationSupport();
+  return vda_support_->Create(frame_size_.width(), frame_size_.height(),
+      kCVPixelFormatType_422YpCbCr8, &avc_data.front(), avc_data.size()) ==
+      gfx::VideoDecodeAccelerationSupport::SUCCESS;
+}
+
+bool MacVideoDecodeAccelerator::Initialize(media::VideoCodecProfile profile) {
+  DCHECK(CalledOnValidThread());
+
+  IOSurfaceSupport* io_surface_support = IOSurfaceSupport::Initialize();
+  if (!io_surface_support)
+    return false;
+
+  MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+      &MacVideoDecodeAccelerator::NotifyInitializeDone, this));
+  return true;
+}
+
+void MacVideoDecodeAccelerator::Decode(
+    const media::BitstreamBuffer& bitstream_buffer) {
+  DCHECK(CalledOnValidThread());
+  base::SharedMemory memory(bitstream_buffer.handle(), true);
+  if (!memory.Map(bitstream_buffer.size())) {
+    LOG(ERROR) << "Failed to SharedMemory::Map().";
+    if (client_)
+      client_->NotifyError(UNREADABLE_INPUT);
+    return;
+  }
+
+  size_t buffer_size = bitstream_buffer.size();
+  if (buffer_size < nalu_len_field_size_ + 1) {
+    LOG(ERROR) << "Bitstream contains invalid data.";
+    if (client_)
+      client_->NotifyError(INVALID_ARGUMENT);
+    return;
+  }
+
+  // The decoder can only handle slice types 1-5.
+  const uint8_t* buffer = static_cast<const uint8_t*>(memory.memory());
+  uint8_t nalu_type = buffer[nalu_len_field_size_] & 0x1f;
+  if (nalu_type < 1 || nalu_type > 5) {
+    if (client_)
+      client_->NotifyEndOfBitstreamBuffer(bitstream_buffer.id());
+    return;
+  }
+
+  // Keep a ref counted copy of the buffer.
+  std::vector<uint8_t> vbuffer(buffer, buffer + buffer_size);
+  scoped_refptr<base::RefCountedBytes> bytes(
+      base::RefCountedBytes::TakeVector(&vbuffer));
+
+  // Store the buffer size at the beginning of the buffer as the decoder
+  // expects.
+  size_t frame_buffer_size = buffer_size - nalu_len_field_size_;
+  DCHECK(nalu_len_field_size_ <= 4);

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

DCHECK_LE

[sail, 2012/05/29 03:45:01]

Done.

+  uint64_t max_frame_buffer_size = (1llu << (nalu_len_field_size_ * 8)) - 1;

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

s/uint64/int64/
s/llu/LL/

[sail, 2012/05/29 03:45:01]

Done.

+  if (frame_buffer_size > max_frame_buffer_size) {

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

FWIW, writing this as 
if (frame_buffer_size >> (nalu_len_field_size_ * 8))
might be clearer.

(if you keep m_f_b_s, make it const)

[sail, 2012/05/29 03:45:01]

Unfortunately that overflows when nalu_len_field_size_ is 4 and chrome is built
as a 32 bit app.

Added const.

[Ami GONE FROM CHROMIUM, 2012/05/30 00:29:58]

WDYM "overflows"?

[sail, 2012/05/30 20:13:46]

Sorry, not sure what the correct term is. If I do:
  uint32_t a = 1024;
  uint32_t b = a >> 32;
Then GCC gives the the following warning:
  warning: right shift count >= width of type
And the value of b is 1024, not 0.

+    LOG(ERROR) << "Bitstream is too large.";

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

s/is/buffer is/

[sail, 2012/05/29 03:45:01]

Done.

+    if (client_)
+      client_->NotifyError(INVALID_ARGUMENT);
+    return;
+  }
+  for (size_t i = 0; i < nalu_len_field_size_; ++i) {
+    size_t shift = nalu_len_field_size_ * 8 - (i + 1) * 8;
+    bytes->data()[i] = (frame_buffer_size >> shift) & 0xff;
+  }
+
+  if (vda_support_) {
+    vda_support_->Decode(bytes->front(), bytes->size(),
+                         base::Bind(&MacVideoDecodeAccelerator::OnFrameReady,
+                                    this, bitstream_buffer.id(), bytes));
+  }
+
+  if (!did_request_pictures_) {
+    did_request_pictures_ = true;
+    if (client_)
+      client_->ProvidePictureBuffers(kNumPictureBuffers, frame_size_);

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

Again needs to be out-of-line to avoid re-entrancy.

[sail, 2012/05/29 03:45:01]

Done.

+  }
+}
+
+void MacVideoDecodeAccelerator::AssignPictureBuffers(
+    const std::vector<media::PictureBuffer>& buffers) {
+  DCHECK(CalledOnValidThread());
+  available_pictures_.insert(
+      available_pictures_.end(), buffers.begin(), buffers.end());
+  SendImages();
+}
+
+void MacVideoDecodeAccelerator::ReusePictureBuffer(int32 picture_buffer_id) {
+  DCHECK(CalledOnValidThread());
+  std::map<int32, UsedPictureInfo>::iterator it =
+      used_pictures_.find(picture_buffer_id);
+  if (it == used_pictures_.end()) {
+    LOG(ERROR) << "Missing picture buffer id: " << picture_buffer_id;
+    if (client_)
+      client_->NotifyError(INVALID_ARGUMENT);
+    return;
+  }
+  UsedPictureInfo info = it->second;
+  used_pictures_.erase(it);
+  available_pictures_.push_back(info.picture_buffer);
+  SendImages();
+}
+
+void MacVideoDecodeAccelerator::Flush() {
+  DCHECK(CalledOnValidThread());
+  if (vda_support_)
+    vda_support_->Flush(true);

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

I'm surprised this can be done synchronously. 
In particular it means that in the same stack where the client called Flush() it
might receive PictureReady callbacks.  I think this needs to be moved
out-of-line.
(basically replace NotifyFlushDone with DoFlush).

[sail, 2012/05/29 03:45:01]

Even though the flush is synchronous the decoded images are queued up in the
thread's message loop.

The NotifyFlushDone is added to the end of the message loop so it gets called
once all the images are sent back.

+  MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+      &MacVideoDecodeAccelerator::NotifyFlushDone, this));
+}
+
+void MacVideoDecodeAccelerator::Reset() {
+  DCHECK(CalledOnValidThread());
+  if (vda_support_)
+    vda_support_->Flush(false);
+  MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
+      &MacVideoDecodeAccelerator::NotifyResetDone, this));
+}
+
+void MacVideoDecodeAccelerator::Destroy() {
+  DCHECK(CalledOnValidThread());
+  if (vda_support_) {
+    vda_support_->Destroy();
+    vda_support_ = NULL;
+  }
+  client_ = NULL;
+}
+
+MacVideoDecodeAccelerator::~MacVideoDecodeAccelerator() {
+  DCHECK(CalledOnValidThread());
+  Destroy();
+}
+
+void MacVideoDecodeAccelerator::OnFrameReady(
+    int32 bitstream_buffer_id,
+    scoped_refptr<base::RefCountedBytes> bytes,
+    CVImageBufferRef image,
+    int status) {
+  DCHECK(CalledOnValidThread());
+  if (image) {

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

Is this failing not a cause for notifying the client of something?

[sail, 2012/05/29 03:45:01]

The image can be NULL if Reset() is called.
I added a check for the status code though.

+    DecodedImageInfo info;
+    info.image.reset(image, base::mac::RETAIN);
+    info.bitstream_buffer_id = bitstream_buffer_id;
+    decoded_images_.push_back(info);
+    SendImages();
+  }
+  if (client_)
+    client_->NotifyEndOfBitstreamBuffer(bitstream_buffer_id);

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

Add
// TODO(sail): this assumes Decode() is handed a single NALU at a time.  Make
that assumption go away.

[sail, 2012/05/29 03:45:01]

Done.

+}
+
+void MacVideoDecodeAccelerator::SendImages() {
+  if (!client_)

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

In this case do you not want to clear the queues?

[sail, 2012/05/29 03:45:01]

I added code to clear the decoded image queue in Destroy() (which gets called
when ever client is set to NULL).

I also added a DCHECK here that if a client doesn't exist then the decoded image
queue must be empty.

+    return;
+
+  while (available_pictures_.size() && decoded_images_.size()) {
+    DecodedImageInfo info = decoded_images_.front();
+    decoded_images_.pop_front();
+    media::PictureBuffer picture_buffer = available_pictures_.front();
+    available_pictures_.pop_front();
+
+    if (!BindImageToTexture(cgl_context_, info.image,
+                            picture_buffer.texture_id())) {
+      LOG(ERROR) << "Error binding image to texture.";
+      client_->NotifyError(PLATFORM_FAILURE);
+      return;
+    }
+
+    used_pictures_.insert(std::pair<int, UsedPictureInfo>(

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

what's the problem if you replace
std::pair<int, UsedPictureInfo>(
with
std::make_pair(
?

[sail, 2012/05/29 03:45:01]

Done.

+        picture_buffer.id(), UsedPictureInfo(picture_buffer, info.image)));
+    client_->PictureReady(
+        media::Picture(picture_buffer.id(), info.bitstream_buffer_id));
+  }
+}
+
+void MacVideoDecodeAccelerator::NotifyInitializeDone() {
+  if (client_)
+    client_->NotifyInitializeDone();
+}
+
+void MacVideoDecodeAccelerator::NotifyFlushDone() {
+  if (client_)
+    client_->NotifyFlushDone();

[Ami GONE FROM CHROMIUM, 2012/05/24 21:30:30]

In that case in what sense is the flush "done"???

[sail, 2012/05/29 03:45:01]

So it looks like there are a few solutions:
   #1 wait for the decoded images to be sent to the client (as new pictures
become available).
   #2 give up and empty the decoded images queue
   #3 do nothing an send these images even after NotifyFlushDone is called.
Do you have a preference for one vs the other?

Also, it looks like I'd have to implement this for all three decoders. If you
want me to do #1 can I do it in a separate CL?

[Ami GONE FROM CHROMIUM, 2012/05/30 00:29:58]

This is the only option that's compatible with the API as described in
video_decode_accelerator.h.
That's a partial-Reset(), not a Flush().
This is terrible ;)
A correct client would be within its rights to CHECK-fail.
Are you saying you think OVDA has this bug?  Can you repro it in a unittest? 
(it shouldn't)
As long as it gets done in the short term, sure.

[sail, 2012/05/30 20:13:46]

Sounds good. I've filed bug 130357 to track this work. I'll also add a unit test
for this.

+}
+
+void MacVideoDecodeAccelerator::NotifyResetDone() {
+  decoded_images_.clear();
+  if (client_)
+    client_->NotifyResetDone();
+}
+
+MacVideoDecodeAccelerator::UsedPictureInfo::UsedPictureInfo(
+    const media::PictureBuffer& pic,
+    const base::mac::ScopedCFTypeRef<CVImageBufferRef>& image)
+    : picture_buffer(pic),
+      image(image, base::mac::RETAIN) {
+}
+
+MacVideoDecodeAccelerator::UsedPictureInfo::~UsedPictureInfo() {
+}
+
+MacVideoDecodeAccelerator::DecodedImageInfo::DecodedImageInfo() {
+}
+
+MacVideoDecodeAccelerator::DecodedImageInfo::~DecodedImageInfo() {
+}