Implement PPAPI VideoDecode out-of-process support

webkit/plugins/ppapi/ppb_video_decoder_impl.cc

 // Copyright (c) 2011 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/plugins/ppapi/ppb_video_decoder_impl.h"
 
 #include <string>
 
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ skipping 16 matching lines @@
 using ppapi::thunk::EnterResourceNoLock;
 using ppapi::thunk::PPB_Buffer_API;
 using ppapi::thunk::PPB_Context3D_API;
 using ppapi::thunk::PPB_VideoDecoder_API;
 
 namespace webkit {
 namespace ppapi {
 
 PPB_VideoDecoder_Impl::PPB_VideoDecoder_Impl(PluginInstance* instance)
     : Resource(instance),
-      callback_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
-      context3d_id_(0),
-      flush_callback_(PP_BlockUntilComplete()),
-      reset_callback_(PP_BlockUntilComplete()),
-      gles2_impl_(NULL) {
+      VideoDecoderImpl(true) {
   ppp_videodecoder_ =
       static_cast<const PPP_VideoDecoder_Dev*>(instance->module()->
           GetPluginInterface(PPP_VIDEODECODER_DEV_INTERFACE));
 }
 
 PPB_VideoDecoder_Impl::~PPB_VideoDecoder_Impl() {
 }
 
 PPB_VideoDecoder_API* PPB_VideoDecoder_Impl::AsPPB_VideoDecoder_API() {
   return this;
 }
 
 // static
 PP_Resource PPB_VideoDecoder_Impl::Create(PluginInstance* instance,
                                           PP_Resource context3d_id,
                                           const PP_VideoConfigElement* config) {
   scoped_refptr<PPB_VideoDecoder_Impl> decoder(
       new PPB_VideoDecoder_Impl(instance));
   if (decoder->Init(context3d_id, config))
     return decoder->GetReference();
   return 0;
 }
 
 bool PPB_VideoDecoder_Impl::Init(PP_Resource context3d_id,
                                  const PP_VideoConfigElement* config) {
-  if (!instance() || !context3d_id || !config)
+  if(!instance() || !::ppapi::VideoDecoderImpl::Init(context3d_id, config))

[Ami GONE FROM CHROMIUM, 2011/08/02 00:49:08]

How can instance() be NULL, do you know?

[vrk (LEFT CHROMIUM), 2011/08/03 22:05:22]

It appears it can't be -- when ResourceCreationImpl calls
PPB_VideoDecoder_Impl::Create, it passes as an instance its own instance_ field,
ignoring what is passed into it? And that instance_ field has been vetted via
another method in ResourceCreationImpl. Tentatively removing the check...

brettw: does this sound right?

     return false;
 
-  EnterResourceNoLock<PPB_Context3D_API> enter(context3d_id, true);
-  if (enter.failed())
+  EnterResourceNoLock<PPB_Context3D_API> enter_context(context3d_id, true);
+  DCHECK(enter_context.succeeded());

[Ami GONE FROM CHROMIUM, 2011/08/02 00:49:08]

I'm not entirely sure about this; it's only correct b/c the super-class Init()
would have returned false if its copy of this failed, but that seems brittle
without at least a comment documenting the dependency.

[vrk (LEFT CHROMIUM), 2011/08/03 19:04:30]

Agreed. To also address piman's concern w/ the double map lookup, I changed Init
to take a PPB_Context3D_API* in addition to PP_Resource (can't drop the
PP_Resource so that we keep resource tracking). Let me know what you think!

+
+  std::vector<int32> copied;
+  if (!CopyConfigsToVector(config, &copied))
     return false;
+
   PPB_Context3D_Impl* context3d =
-      static_cast<PPB_Context3D_Impl*>(enter.object());
-
-  context3d_id_ = context3d_id;
-  ResourceTracker::Get()->AddRefResource(context3d_id_);
+      static_cast<PPB_Context3D_Impl*>(enter_context.object());
 
   int command_buffer_route_id =
       context3d->platform_context()->GetCommandBufferRouteId();
   if (command_buffer_route_id == 0)
     return false;
   platform_video_decoder_ = instance()->delegate()->CreateVideoDecoder(
       this, command_buffer_route_id);
-
-  gles2_impl_ = context3d->gles2_impl();
-
   if (!platform_video_decoder_)
     return false;
 
-  std::vector<uint32> copied;
-  // TODO(fischman,vrk): this is completely broken in that it fails to account
-  // for the semantic distinction between keys and values; it is certainly
-  // possible for a value to show up as 0, and that shouldn't terminate the
-  // config vector.  Only a *key* of 0 should do so.
-  // TODO(vrk): This is assuming PP_VideoAttributeDictionary and
-  // VideoAttributeKey have identical enum values. There is no compiler
-  // assert to guarantee this. We either need to add such asserts or
-  // merge PP_VideoAttributeDictionary and VideoAttributeKey.
-  for (const PP_VideoConfigElement* current = config;
-       *current != PP_VIDEOATTR_DICTIONARY_TERMINATOR; ++current) {
-    copied.push_back(static_cast<uint32>(*current));
-  }
-
   FlushCommandBuffer();
   return platform_video_decoder_->Initialize(copied);
 }
 
 int32_t PPB_VideoDecoder_Impl::Decode(
     const PP_VideoBitstreamBuffer_Dev* bitstream_buffer,
     PP_CompletionCallback callback) {
   if (!platform_video_decoder_)
     return PP_ERROR_BADRESOURCE;
 
   EnterResourceNoLock<PPB_Buffer_API> enter(bitstream_buffer->data, true);
   if (enter.failed())
     return PP_ERROR_FAILED;
 
   PPB_Buffer_Impl* buffer = static_cast<PPB_Buffer_Impl*>(enter.object());
   media::BitstreamBuffer decode_buffer(bitstream_buffer->id,
                                        buffer->shared_memory()->handle(),
                                        static_cast<size_t>(buffer->size()));
-  CHECK(bitstream_buffer_callbacks_.insert(std::make_pair(
-      bitstream_buffer->id, callback)).second);
+  AddBitstreamBufferCallback(bitstream_buffer->id, callback);
 
   FlushCommandBuffer();
   platform_video_decoder_->Decode(decode_buffer);
   return PP_OK_COMPLETIONPENDING;
 }
 
 void PPB_VideoDecoder_Impl::AssignPictureBuffers(
     uint32_t no_of_buffers,
     const PP_PictureBuffer_Dev* buffers) {
   if (!platform_video_decoder_)
@@ skipping 19 matching lines @@
 
   FlushCommandBuffer();
   platform_video_decoder_->ReusePictureBuffer(picture_buffer_id);
 }
 
 int32_t PPB_VideoDecoder_Impl::Flush(PP_CompletionCallback callback) {
   if (!platform_video_decoder_)
     return PP_ERROR_BADRESOURCE;
 
   // Store the callback to be called when Flush() is done.
   // TODO(fischman,vrk): consider implications of already-outstanding callback.

[Ami GONE FROM CHROMIUM, 2011/08/02 00:49:08]

This TODO (and its sibling below) don't belong here anymore.  If anywhere, they
should probably go in the shared impl's header above the declaration of the
Set*Callback() methods.

[vrk (LEFT CHROMIUM), 2011/08/03 19:04:30]

Done.

-  flush_callback_ = callback;
+  SetFlushCallback(callback);
 
   FlushCommandBuffer();
   platform_video_decoder_->Flush();
   return PP_OK_COMPLETIONPENDING;
 }
 
 int32_t PPB_VideoDecoder_Impl::Reset(PP_CompletionCallback callback) {
   if (!platform_video_decoder_)
     return PP_ERROR_BADRESOURCE;
 
   // Store the callback to be called when Reset() is done.
   // TODO(fischman,vrk): consider implications of already-outstanding callback.
-  reset_callback_ = callback;
+  SetResetCallback(callback);
 
   FlushCommandBuffer();
   platform_video_decoder_->Reset();
   return PP_OK_COMPLETIONPENDING;
 }
 
 void PPB_VideoDecoder_Impl::Destroy() {
   if (!platform_video_decoder_)
     return;
 
   FlushCommandBuffer();
   platform_video_decoder_->Destroy();
-  gles2_impl_ = NULL;
-  if (context3d_id_)
-    ResourceTracker::Get()->UnrefResource(context3d_id_);
   platform_video_decoder_ = NULL;
   ppp_videodecoder_ = NULL;
+
+  ::ppapi::VideoDecoderImpl::Destroy();

[Ami GONE FROM CHROMIUM, 2011/08/02 00:49:08]

Is it obvious why this belongs after the 2 =NULL's above, as opposed to before
them?

[vrk (LEFT CHROMIUM), 2011/08/03 19:04:30]

Talked in person, done.

 }
 
 void PPB_VideoDecoder_Impl::ProvidePictureBuffers(
     uint32 requested_num_of_buffers, const gfx::Size& dimensions) {
   if (!ppp_videodecoder_)
     return;
 
   PP_Size out_dim = PP_MakeSize(dimensions.width(), dimensions.height());
   ScopedResourceId resource(this);
   ppp_videodecoder_->ProvidePictureBuffers(
@@ skipping 38 matching lines @@
   ScopedResourceId resource(this);
   // TODO(vrk): This is assuming VideoDecodeAccelerator::Error and
   // PP_VideoDecodeError_Dev have identical enum values. There is no compiler
   // assert to guarantee this. We either need to add such asserts or
   // merge these two enums.
   ppp_videodecoder_->NotifyError(instance()->pp_instance(), resource.id,
                                  static_cast<PP_VideoDecodeError_Dev>(error));
 }
 
 void PPB_VideoDecoder_Impl::NotifyResetDone() {
-  if (reset_callback_.func == NULL)
-    return;
-
-  // Call the callback that was stored to be called when Reset is done.
-  PP_RunAndClearCompletionCallback(&reset_callback_, PP_OK);
+  RunResetCallback();
 }
 
 void PPB_VideoDecoder_Impl::NotifyEndOfBitstreamBuffer(
     int32 bitstream_buffer_id) {
-  CallbackById::iterator it =
-      bitstream_buffer_callbacks_.find(bitstream_buffer_id);
-  DCHECK(it != bitstream_buffer_callbacks_.end());
-  PP_CompletionCallback cc = it->second;
-  bitstream_buffer_callbacks_.erase(it);
-  PP_RunCompletionCallback(&cc, PP_OK);
+  RunBitstreamBufferCallback(bitstream_buffer_id);
 }
 
 void PPB_VideoDecoder_Impl::NotifyFlushDone() {
-  if (flush_callback_.func == NULL)
-    return;
-
-  // Call the callback that was stored to be called when Flush is done.
-  PP_RunAndClearCompletionCallback(&flush_callback_, PP_OK);
+  RunFlushCallback();
 }
 
 void PPB_VideoDecoder_Impl::NotifyInitializeDone() {
   NOTREACHED() << "PlatformVideoDecoder::Initialize() is synchronous!";
 }
 
-void PPB_VideoDecoder_Impl::FlushCommandBuffer() {
-  // For the out-of-process case, |gles2_impl_| will be NULL in the renderer
-  // process. The VideoDecoder_Proxy is charged with the responsibility of
-  // doing this Flush() in the analogous places in the plugin process.
-  if (gles2_impl_)
-    gles2_impl_->Flush();
-}
-
 }  // namespace ppapi
 }  // namespace webkit