Implement Pepper PPB_VideoDecoder interface.

ppapi/proxy/video_decoder_resource.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 "ppapi/proxy/video_decoder_resource.h"
+
+#include "base/bind.h"
+#include "gpu/command_buffer/client/gles2_cmd_helper.h"
+#include "gpu/command_buffer/client/gles2_implementation.h"
+#include "gpu/command_buffer/common/mailbox.h"
+#include "ipc/ipc_message.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/c/ppb_opengles2.h"
+#include "ppapi/proxy/plugin_dispatcher.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/proxy/ppb_graphics_3d_proxy.h"
+#include "ppapi/shared_impl/ppapi_globals.h"
+#include "ppapi/shared_impl/ppb_graphics_3d_shared.h"
+#include "ppapi/shared_impl/proxy_lock.h"
+#include "ppapi/shared_impl/resource_tracker.h"
+#include "ppapi/thunk/enter.h"
+
+using ppapi::thunk::EnterResourceNoLock;
+using ppapi::thunk::PPB_Graphics3D_API;
+using ppapi::thunk::PPB_VideoDecoder_API;
+
+namespace {
+
+// Maximum number of concurrent decodes which can be pending.
+const uint32_t kMaximumPendingDecodes = 8;
+
+// Minimum size of shared-memory buffers we allocate. Make them large since
+// we try to reuse them.
+const uint32_t kMinimumBitstreamBufferSize = 100 << 10;
+
+}  // namespace
+
+namespace ppapi {
+namespace proxy {
+
+VideoDecoderResource::ShmBuffer::ShmBuffer(base::SharedMemory* shm,
+                                           uint32_t size,
+                                           uint32_t shm_id)
+    : shm_(shm), size_(size), addr_(NULL), shm_id_(shm_id) {
+  if (shm_->Map(size_))
+    addr_ = shm_->memory();
+  DCHECK(addr_);

[Tom Sepez, 2014/05/08 20:35:07]

might want to handle error cases here.  I'd suspect that being unable to map
memory could occur at any time based on system load, etc.

[bbudge, 2014/05/14 19:35:04]

Added a check that Map succeeded below, when we create the shm buffer.

+}
+
+VideoDecoderResource::ShmBuffer::~ShmBuffer() {
+  shm_->Unmap();

[piman, 2014/05/08 04:26:04]

nit: that's implicit with the destruction.

[bbudge, 2014/05/14 16:40:41]

Done.

+  addr_ = NULL;
+}
+
+VideoDecoderResource::Texture::Texture(uint32_t texture_target,
+                                       const PP_Size& size)
+    : texture_target_(texture_target), size_(size) {
+}
+
+VideoDecoderResource::Texture::~Texture() {
+}
+
+VideoDecoderResource::Picture::Picture(int32_t decode_id, uint32_t texture_id)
+    : decode_id_(decode_id), texture_id_(texture_id) {
+}
+
+VideoDecoderResource::Picture::~Picture() {
+}
+
+VideoDecoderResource::VideoDecoderResource(Connection connection,
+                                           PP_Instance instance)
+    : PluginResource(connection, instance),
+      decode_id_(0),
+      decode_size_(0),
+      decode_buffer_(NULL),
+      pending_decode_count_(0),
+      get_shm_buffer_pending_(false),
+      get_picture_(NULL),
+      gles2_impl_(NULL),
+      initialized_(false),
+      // Set |decoder_last_error_| to PP_OK after successful initialization.
+      // This makes error checking a little more concise, since we can check
+      // that the decoder is initialized and hasn't returned an error in one
+      // comparison.
+      decoder_last_error_(PP_ERROR_FAILED) {
+  SendCreate(RENDERER, PpapiHostMsg_VideoDecoder_Create());
+}
+
+VideoDecoderResource::~VideoDecoderResource() {
+  FlushCommandBuffer();
+
+  // Destroy any textures which haven't been dismissed.
+  TextureMap::iterator it = textures_.begin();
+  for (; it != textures_.end(); ++it)
+    DeleteGLTexture(it->first);
+  // Release our ref on the graphics resource.
+  graphics3d_ = NULL;
+  gles2_impl_ = NULL;
+}
+
+PPB_VideoDecoder_API* VideoDecoderResource::AsPPB_VideoDecoder_API() {
+  return this;
+}
+
+int32_t VideoDecoderResource::Initialize(
+    PP_Resource graphics_context,
+    PP_VideoProfile profile,
+    PP_Bool allow_software_fallback,
+    scoped_refptr<TrackedCallback> callback) {
+  if (initialized_)
+    return PP_ERROR_FAILED;
+  if (initialize_callback_)
+    return PP_ERROR_INPROGRESS;
+  if (!graphics_context)
+    return PP_ERROR_BADRESOURCE;
+  EnterResourceNoLock<PPB_Graphics3D_API> enter_context(graphics_context, true);
+  if (enter_context.failed())
+    return PP_ERROR_BADRESOURCE;
+
+  initialize_callback_ = callback;
+
+  // Take a reference to keep the graphics resource alive for the lifetime of
+  // this resource.
+  graphics3d_ = static_cast<PPB_Graphics3D_Shared*>(enter_context.object());
+  gles2_impl_ = graphics3d_->gles2_impl();
+
+  Call<PpapiPluginMsg_VideoDecoder_InitializeReply>(
+      RENDERER,
+      PpapiHostMsg_VideoDecoder_Initialize(graphics3d_->host_resource(),
+                                           profile,
+                                           PP_ToBool(allow_software_fallback)),
+      base::Bind(&VideoDecoderResource::OnPluginMsgInitializeComplete, this));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t VideoDecoderResource::Decode(uint32_t decode_id,
+                                     uint32_t size,
+                                     const void* buffer,
+                                     scoped_refptr<TrackedCallback> callback) {
+  if (decoder_last_error_)
+    return decoder_last_error_;
+  if (flush_callback_ || reset_callback_)
+    return PP_ERROR_FAILED;
+  if (decode_callback_)
+    return PP_ERROR_INPROGRESS;
+
+  return TryDecode(decode_id, size, buffer, callback);
+}
+
+int32_t VideoDecoderResource::GetPicture(
+    PP_VideoPicture* picture,
+    scoped_refptr<TrackedCallback> callback) {
+  if (decoder_last_error_)
+    return decoder_last_error_;
+  if (reset_callback_)
+    return PP_ERROR_FAILED;
+  if (get_picture_callback_)
+    return PP_ERROR_INPROGRESS;
+
+  // If the next picture is ready, return it synchronously.
+  if (!received_pictures_.empty()) {
+    WriteNextPicture(picture);
+    return PP_OK;
+  }
+
+  get_picture_callback_ = callback;
+  get_picture_ = picture;
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void VideoDecoderResource::RecyclePicture(const PP_VideoPicture* picture) {
+  if (decoder_last_error_)
+    return;
+  if (reset_callback_)
+    return;
+
+  Post(RENDERER, PpapiHostMsg_VideoDecoder_RecyclePicture(picture->texture_id));
+}
+
+int32_t VideoDecoderResource::Flush(scoped_refptr<TrackedCallback> callback) {
+  if (decoder_last_error_)
+    return decoder_last_error_;
+  if (reset_callback_)
+    return PP_ERROR_FAILED;
+  if (flush_callback_)
+    return PP_ERROR_INPROGRESS;
+  flush_callback_ = callback;
+
+  FlushCommandBuffer();
+  Call<PpapiPluginMsg_VideoDecoder_FlushReply>(
+      RENDERER,
+      PpapiHostMsg_VideoDecoder_Flush(),
+      base::Bind(&VideoDecoderResource::OnPluginMsgFlushComplete, this));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t VideoDecoderResource::Reset(scoped_refptr<TrackedCallback> callback) {
+  if (decoder_last_error_)
+    return decoder_last_error_;
+  if (flush_callback_)
+    return PP_ERROR_FAILED;
+  if (reset_callback_)
+    return PP_ERROR_INPROGRESS;
+  reset_callback_ = callback;
+
+  // Cause any pending Decode or GetPicture callbacks to abort immediately.
+  // Reentrancy isn't a problem, since all calls fail until Reset completes.
+  if (TrackedCallback::IsPending(decode_callback_))
+    decode_callback_->Abort();
+  decode_callback_ = NULL;
+  if (TrackedCallback::IsPending(get_picture_callback_))
+    get_picture_callback_->Abort();
+  get_picture_callback_ = NULL;
+  FlushCommandBuffer();
+  Call<PpapiPluginMsg_VideoDecoder_ResetReply>(
+      RENDERER,
+      PpapiHostMsg_VideoDecoder_Reset(),
+      base::Bind(&VideoDecoderResource::OnPluginMsgResetComplete, this));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+void VideoDecoderResource::OnReplyReceived(
+    const ResourceMessageReplyParams& params,
+    const IPC::Message& msg) {
+  IPC_BEGIN_MESSAGE_MAP(VideoDecoderResource, msg)
+  PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL(
+      PpapiPluginMsg_VideoDecoder_RequestTextures, OnPluginMsgRequestTextures)
+  PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL(PpapiPluginMsg_VideoDecoder_PictureReady,
+                                      OnPluginMsgPictureReady)
+  PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL(
+      PpapiPluginMsg_VideoDecoder_DismissPicture, OnPluginMsgDismissPicture)
+  PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL(PpapiPluginMsg_VideoDecoder_NotifyError,
+                                      OnPluginMsgNotifyError)
+  PPAPI_DISPATCH_PLUGIN_RESOURCE_CALL_UNHANDLED(
+      PluginResource::OnReplyReceived(params, msg))
+  IPC_END_MESSAGE_MAP()
+}
+
+void VideoDecoderResource::OnPluginMsgRequestTextures(
+    const ResourceMessageReplyParams& params,
+    uint32_t num_textures,
+    PP_Size size,
+    uint32_t texture_target,
+    const std::vector<gpu::Mailbox>& mailboxes) {

[Tom Sepez, 2014/05/08 20:35:07]

Are the mailboxes are supplied by a trusted process? 

[bbudge, 2014/05/14 16:40:41]

They are generated by the renderer process. I am removing this parameter for now
though, as it's only needed by a future CL.

+  std::vector<uint32_t> texture_ids;
+  for (uint32_t i = 0; i < num_textures; ++i) {
+    GLuint texture_id;
+    gles2_impl_->GenTextures(1, &texture_id);

[piman, 2014/05/08 04:26:04]

It's really better to GenTextures(num_textures, array).

[bbudge, 2014/05/14 16:40:41]

Done.

+
+    gles2_impl_->ActiveTexture(GL_TEXTURE0);

[bbudge, 2014/05/14 19:35:04]

piman: Can I move this out of the loop, to after GenTextures?

+    gles2_impl_->BindTexture(texture_target, texture_id);

[piman, 2014/05/08 04:26:04]

So, these 2 calls modify GL state.
Since this comes from a renderer message, it will really happen behind the
scenes from the plugin point-of-view, so its state will be surprisingly changed.

At the very least we should restore the state, using GetIntegerv with
GL_ACTIVE_TEXTURE and GL_TEXTURE_BINDING_*

[bbudge, 2014/05/14 16:40:41]

I've changed the resource to create its own Graphics3D resource to make
textures. It uses the plugin's Graphics3D as a share_context.

+    gles2_impl_->TexParameteri(
+        texture_target, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
+    gles2_impl_->TexParameteri(
+        texture_target, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+    gles2_impl_->TexParameterf(
+        texture_target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    gles2_impl_->TexParameterf(
+        texture_target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+    if (texture_target == GL_TEXTURE_2D) {
+      gles2_impl_->TexImage2D(texture_target,
+                              0,
+                              GL_RGBA,
+                              size.width,
+                              size.height,
+                              0,
+                              GL_RGBA,
+                              GL_UNSIGNED_BYTE,
+                              NULL);
+    }
+
+    if (!mailboxes.empty())

[piman, 2014/05/08 04:26:04]

We should check somewhere that mailbox.size() == num_textures

[bbudge, 2014/05/14 16:40:41]

Will do in future CL (mailboxes removed for now)

+      gles2_impl_->ProduceTextureCHROMIUM(GL_TEXTURE_2D, mailboxes[i].name);
+
+    textures_.insert(std::make_pair(texture_id, Texture(texture_target, size)));
+    texture_ids.push_back(texture_id);
+  }
+
+  FlushCommandBuffer();
+
+  Post(RENDERER, PpapiHostMsg_VideoDecoder_AssignTextures(size, texture_ids));
+}
+
+void VideoDecoderResource::OnPluginMsgPictureReady(
+    const ResourceMessageReplyParams& params,
+    int32_t shm_id,
+    uint32_t texture_id) {
+  // This value is inserted in OnPluginMsgDecodeComplete.
+  uint32_t decode_id = decode_ids_[shm_id];
+  received_pictures_.push(Picture(decode_id, texture_id));
+  // Get ready to accept another call to GetPicture in the callback.
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(get_picture_callback_);
+  PP_VideoPicture* picture = get_picture_;
+  get_picture_ = NULL;
+  if (TrackedCallback::IsPending(callback)) {
+    WriteNextPicture(picture);
+    callback->Run(PP_OK);
+  }
+}
+
+void VideoDecoderResource::OnPluginMsgDismissPicture(
+    const ResourceMessageReplyParams& params,
+    uint32_t texture_id) {
+  DeleteGLTexture(texture_id);
+  textures_.erase(texture_id);
+}
+
+void VideoDecoderResource::OnPluginMsgNotifyError(
+    const ResourceMessageReplyParams& params,
+    int32_t error) {
+  decoder_last_error_ = error;
+  // Cause any pending Decode or GetPicture callbacks to run immediately.
+  // Reentrancy isn't a problem, since the resource is unusable now.
+  if (TrackedCallback::IsPending(decode_callback_))
+    decode_callback_->Run(decoder_last_error_);
+  decode_callback_ = NULL;
+  if (TrackedCallback::IsPending(get_picture_callback_))
+    get_picture_callback_->Run(decoder_last_error_);
+  get_picture_callback_ = NULL;
+}
+
+void VideoDecoderResource::OnPluginMsgInitializeComplete(
+    const ResourceMessageReplyParams& params) {
+  decoder_last_error_ = params.result();
+  if (decoder_last_error_ == PP_OK)
+    initialized_ = true;
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(initialize_callback_);
+  callback->Run(decoder_last_error_);
+}
+
+void VideoDecoderResource::OnPluginMsgGetShmComplete(
+    const ResourceMessageReplyParams& params,
+    uint32_t size) {
+  get_shm_buffer_pending_ = false;
+  int32_t result = params.result();
+  if (result == PP_OK) {
+    base::SharedMemoryHandle shm_handle = base::SharedMemory::NULLHandle();
+    if (!params.TakeSharedMemoryHandleAtIndex(0, &shm_handle)) {
+      RunDecodeCallback(PP_ERROR_FAILED);
+      return;
+    }
+    uint32_t shm_id = static_cast<uint32_t>(shm_buffers_.size());
+    ShmBuffer* shm_buffer =
+        new ShmBuffer(new base::SharedMemory(shm_handle, false /* read_only */),
+                      size,
+                      shm_id);
+    shm_buffers_.push_back(shm_buffer);
+    SendDecodeMessage(shm_id);
+    RunDecodeCallback(PP_OK);
+  }
+}
+
+void VideoDecoderResource::OnPluginMsgDecodeComplete(
+    uint32_t decode_id,
+    const ResourceMessageReplyParams& params,
+    uint32_t shm_id) {

[piman, 2014/05/08 04:26:04]

It makes me a bit uncomfortable to have the shm_id round-trip through the
renderer. I don't think it's needed since you have the decode_id (so you could
keep a queue of pending decodes, where you'd stash the shm id).

A compromised renderer could cause the shm_id to be out-of-sync with the
decode_id, or be out-of-bounds. It could try to use this to gain elevated
privilege to the plugin process, which may have different access (think Flash,
that connects to renderers from other origins too). Maybe far-fetched, but I
think it's better not to make assumptions about trusting the renderer.

[bbudge, 2014/05/14 16:40:41]

I must pass the shm_id round trip to identify busy and available buffers.
I bounds check the shm_id in the host and plugin.

[bbudge, 2014/05/14 19:35:04]

Just to reiterate, shm_id is the only Id I can count on here. I'll make sure
it's validated (range checked) whenever I receive it on either side of the
proxy.

+  pending_decode_count_--;
+  available_shm_buffers_.push_back(shm_buffers_[shm_id]);
+  // Save the user id associated with the Decode now, in case it generates a
+  // call to OnPluginMsgPictureReady.
+  decode_ids_[shm_id] = decode_id;

[piman, 2014/05/08 04:26:04]

Is this ok, to use the shm_id (which is just an index in a small range) to tag
the decode calls?

2 things I'm worried about:
1- are we ensured OnPluginMsgDecodeComplete will always be called before
OnPluginMsgPictureReady?
2- since this shm buffer will be reused at the next Decode, are we ensured we'll
get *all* OnPluginMsgPictureReady for this Decode, before the
OnPluginMsgDecodeComplete for the next one?


Wouldn't it be safer to use decode_id all along the path, since they're
monotonically increasing and there's no chance of reuse? I think it'd even make
some of the logic simpler.

[bbudge, 2014/05/14 16:40:41]

The decode_id is provided by the plugin, which may pass any value if it doesn't
care about identifying pictures with decode calls.
I've changed the implementation so that the 'decode_id' is passed through
instead of the shm_id, for all the reasons you give above.

+  // If a Decode is pending, and we're not waiting for a shm buffer, attempt
+  // the Decode again.
+  if (decode_callback_ && !get_shm_buffer_pending_) {
+    int32_t result =
+        TryDecode(decode_id_, decode_size_, decode_buffer_, decode_callback_);
+    if (result == PP_OK)
+      RunDecodeCallback(PP_OK);
+  }
+}
+
+void VideoDecoderResource::OnPluginMsgFlushComplete(
+    const ResourceMessageReplyParams& params) {
+  if (get_picture_callback_) {
+    scoped_refptr<TrackedCallback> callback;
+    callback.swap(get_picture_callback_);
+    callback->Abort();
+  }
+
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(flush_callback_);
+  callback->Run(params.result());
+}
+
+void VideoDecoderResource::OnPluginMsgResetComplete(
+    const ResourceMessageReplyParams& params) {
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(reset_callback_);
+  callback->Run(params.result());
+}
+
+int32_t VideoDecoderResource::TryDecode(
+    uint32_t decode_id,
+    uint32_t size,
+    const void* buffer,
+    scoped_refptr<TrackedCallback> callback) {
+  decode_id_ = decode_id;
+  decode_size_ = size;
+  decode_buffer_ = buffer;
+  if (available_shm_buffers_.empty() ||
+      available_shm_buffers_.back()->size_ < size) {
+    decode_callback_ = callback;
+
+    if (pending_decode_count_ < kMaximumPendingDecodes) {
+      get_shm_buffer_pending_ = true;
+      uint32_t alloc_size = std::max(size, kMinimumBitstreamBufferSize);
+      Call<PpapiPluginMsg_VideoDecoder_GetShmReply>(
+          RENDERER,

[piman, 2014/05/08 04:26:04]

Why not allocate the shm by going to the browser? It would avoid round trips
(since the renderer has to go to the browser), and not involve the renderer's
main thread which can be busy.

It could also then be a sync IPC, which would remove the restriction that the
user can't touch their buffer until the callback is called. Which probably
forces a copy on most patterns (which is a shame, since we copy too), though
more likely users will forget and send flaky garbage to the video decoder.

[bbudge, 2014/05/14 19:35:04]

This was simpler, and the patch is already large. In general shm buffers are
only created as decoding is starting up, and ideally in the steady state no new
ones are created. If you like, I can add a TODO to improve this later.

[piman, 2014/05/15 04:02:53]

I disagree... the API is too subtle. See the comment in the test.

+          PpapiHostMsg_VideoDecoder_GetShm(alloc_size),
+          base::Bind(&VideoDecoderResource::OnPluginMsgGetShmComplete, this));
+    }
+
+    return PP_OK_COMPLETIONPENDING;
+  }
+
+  ShmBuffer* shm_buffer = available_shm_buffers_.back();
+  available_shm_buffers_.pop_back();
+  SendDecodeMessage(shm_buffer->shm_id_);
+  return PP_OK;
+}
+
+void VideoDecoderResource::SendDecodeMessage(uint32_t shm_id) {
+  ShmBuffer* shm_buffer = shm_buffers_[shm_id];
+  memcpy(shm_buffer->addr_, decode_buffer_, decode_size_);
+  pending_decode_count_++;
+
+  FlushCommandBuffer();
+  Call<PpapiPluginMsg_VideoDecoder_DecodeReply>(
+      RENDERER,
+      PpapiHostMsg_VideoDecoder_Decode(shm_id, decode_size_),
+      base::Bind(
+          &VideoDecoderResource::OnPluginMsgDecodeComplete, this, decode_id_));
+}
+
+void VideoDecoderResource::RunDecodeCallback(int32_t result) {
+  scoped_refptr<TrackedCallback> callback;
+  callback.swap(decode_callback_);
+  callback->Run(result);
+}
+
+void VideoDecoderResource::FlushCommandBuffer() {
+  if (gles2_impl_)
+    gles2_impl_->Flush();
+}
+
+void VideoDecoderResource::DeleteGLTexture(uint32_t id) {
+  gles2_impl_->DeleteTextures(1, &id);
+}
+
+void VideoDecoderResource::WriteNextPicture(PP_VideoPicture* pp_picture) {
+  DCHECK(!received_pictures_.empty());
+  Picture& picture = received_pictures_.front();
+  uint32_t texture_id = picture.texture_id_;
+  TextureMap::iterator it = textures_.find(texture_id);
+  DCHECK(it != textures_.end());
+  pp_picture->decode_id = picture.decode_id_;
+  pp_picture->texture_id = texture_id;
+  pp_picture->texture_target = it->second.texture_target_;
+  pp_picture->texture_size = it->second.size_;
+  received_pictures_.pop();
+}
+
+}  // namespace proxy
+}  // namespace ppapi