Update VideoDecode PPAPI structs to be consistent with media structures, part 1

webkit/plugins/ppapi/ppb_video_decoder_impl.cc

Index: webkit/plugins/ppapi/ppb_video_decoder_impl.cc
diff --git a/webkit/plugins/ppapi/ppb_video_decoder_impl.cc b/webkit/plugins/ppapi/ppb_video_decoder_impl.cc
index ec2d3f10e07813bb12b4fc99a09f190227240200..bef7717c4e22c4b0f591523d3e91fdb6dee3a748 100644
--- a/webkit/plugins/ppapi/ppb_video_decoder_impl.cc
+++ b/webkit/plugins/ppapi/ppb_video_decoder_impl.cc
@@ -7,13 +7,17 @@
 #include <string>
 
 #include "base/logging.h"
+#include "media/video/picture.h"
 #include "ppapi/c/dev/pp_video_dev.h"
 #include "ppapi/c/dev/ppb_video_decoder_dev.h"
+#include "ppapi/c/dev/ppp_video_decoder_dev.h"
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_errors.h"
 #include "webkit/plugins/ppapi/common.h"
+#include "webkit/plugins/ppapi/plugin_module.h"
 #include "webkit/plugins/ppapi/var.h"

[scherkus (not reviewing), 2011/04/26 22:36:34]

nit: include order

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Done.

 #include "webkit/plugins/ppapi/ppapi_plugin_instance.h"
+#include "webkit/plugins/ppapi/ppb_buffer_impl.h"
 #include "webkit/plugins/ppapi/ppb_file_ref_impl.h"
 #include "webkit/plugins/ppapi/resource_tracker.h"
 
@@ -72,7 +76,7 @@ PP_Bool Decode(PP_Resource decoder_id,
 
 void AssignPictureBuffer(PP_Resource video_decoder,
                          uint32_t no_of_buffers,
-                         union PP_PictureData_Dev* picture_buffer) {
+                         struct PP_PictureBuffer_Dev* picture_buffer) {
   scoped_refptr<PPB_VideoDecoder_Impl> decoder(
       Resource::GetAs<PPB_VideoDecoder_Impl>(video_decoder));
   if (!decoder)
@@ -82,13 +86,13 @@ void AssignPictureBuffer(PP_Resource video_decoder,
 }
 
 void ReusePictureBuffer(PP_Resource video_decoder,
-                        union PP_PictureData_Dev* picture_buffer) {
+                        int32_t picture_buffer_id) {
   scoped_refptr<PPB_VideoDecoder_Impl> decoder(
       Resource::GetAs<PPB_VideoDecoder_Impl>(video_decoder));
   if (!decoder)
     return;
 
-  decoder->ReusePictureBuffer(picture_buffer);
+  decoder->ReusePictureBuffer(picture_buffer_id);
 }
 
 PP_Bool Flush(PP_Resource video_decoder,
@@ -166,7 +170,7 @@ bool PPB_VideoDecoder_Impl::Init(PP_VideoDecoderConfig_Dev* decoder_config) {
     return false;
 
   platform_video_decoder_.reset(
-      instance()->delegate()->CreateVideoDecoder(decoder_config));
+      instance()->delegate()->CreateVideoDecoder(decoder_config, this));
 
   return platform_video_decoder_.get()? true : false;
 }
@@ -177,9 +181,13 @@ bool PPB_VideoDecoder_Impl::Decode(
   if (!platform_video_decoder_.get())
     return false;
 
-  media::BitstreamBuffer* decode_buffer = NULL;
-  // TODO(vmr): Convert bitstream_buffer to BitstreamBuffer object.
-  NOTIMPLEMENTED();
+  scoped_refptr<PPB_Buffer_Impl> pepper_buffer =
+      Resource::GetAs<PPB_Buffer_Impl>(bitstream_buffer->bitstream);
+
+  media::BitstreamBuffer* decode_buffer =
+      new media::BitstreamBuffer(pepper_buffer->mapped_buffer(),
+                                 bitstream_buffer->bitstream_size,
+                                 bitstream_buffer->user_handle);
 
   // Store the callback to inform when bitstream buffer has been processed.
   // TODO(vmr): handle simultaneous decodes + callbacks.
@@ -193,27 +201,24 @@ bool PPB_VideoDecoder_Impl::Decode(
 
 void PPB_VideoDecoder_Impl::AssignPictureBuffer(
     uint32_t no_of_picture_buffers,
-    PP_PictureData_Dev* picture_buffers) {
+    PP_PictureBuffer_Dev* picture_buffers) {
   if (!platform_video_decoder_.get())
     return;
 
-  // TODO(vmr): Map PP_PictureData_Dev into PictureBuffer object.
-  NOTIMPLEMENTED();
-
-  media::VideoDecodeAccelerator::PictureBuffer* buffer = NULL;
-  platform_video_decoder_->ReusePictureBuffer(buffer);
+  std::vector<media::VideoDecodeAccelerator::PictureBuffer*> wrapped_buffers;
+  for (uint i = 0; i < no_of_picture_buffers; i++) {
+    struct PP_PictureBuffer_Dev in_buf = picture_buffers[i];
+    media::VideoDecodeAccelerator::PictureBuffer* buffer =
+        CreateMediaPictureBuffer(in_buf);
+    wrapped_buffers.push_back(buffer);
+    platform_video_decoder_->AssignPictureBuffer(wrapped_buffers);
+  }
 }
 
-void PPB_VideoDecoder_Impl::ReusePictureBuffer(
-    PP_PictureData_Dev* picture_buffer) {
+void PPB_VideoDecoder_Impl::ReusePictureBuffer(int32_t picture_buffer_id) {
   if (!platform_video_decoder_.get())
     return;
-
-  // TODO(vmr): Map PP_PictureData_Dev into PictureBuffer object.
-  NOTIMPLEMENTED();
-
-  media::VideoDecodeAccelerator::PictureBuffer* buffer = NULL;
-  platform_video_decoder_->ReusePictureBuffer(buffer);
+  platform_video_decoder_->ReusePictureBuffer(picture_buffer_id);
 }
 
 bool PPB_VideoDecoder_Impl::Flush(PP_CompletionCallback callback) {
@@ -245,39 +250,36 @@ bool PPB_VideoDecoder_Impl::Abort(PP_CompletionCallback callback) {
 void PPB_VideoDecoder_Impl::ProvidePictureBuffers(
     uint32_t requested_num_of_buffers,
     const std::vector<uint32_t>& buffer_properties) {
-  // TODO(vmr): Implement.
+  // TODO(vrk): The structures to represent buffer properties in media::
+  // versus ppapi:: do not match. These need to be consistent before
+  // this can be implemented.
   NOTIMPLEMENTED();
 }
 
 void PPB_VideoDecoder_Impl::PictureReady(
-    media::VideoDecodeAccelerator::Picture* picture) {
-  // TODO(vmr): Implement.
-  NOTIMPLEMENTED();
-
-  // Convert the picture.
-  // Get plugin's PPP interface function pointers.
-  // PPP_VideoDecoder* ppp_videodecoder;
-  // Call ProvidePictureBuffers function pointer and return.
-  // ppp_videodecoder->PictureReady(resource_decoder, picture,
-  //                                pic_buffer_used_again);
+    media::VideoDecodeAccelerator::Picture& picture) {
+  const PPP_VideoDecoder_Dev* ppp_videodecoder =

[Ami GONE FROM CHROMIUM, 2011/04/26 23:01:57]

worth doing this once in the ctor to avoid this 3-line stanza that repeats
multiple times below?

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Done.

+      static_cast<const PPP_VideoDecoder_Dev*>(instance()->module()->
+          GetPluginInterface(PPP_VIDEODECODER_DEV_INTERFACE));
+  ScopedResourceId resource(this);
+  struct PP_Picture_Dev* out_pic = CreatePictureDev(picture);

[Ami GONE FROM CHROMIUM, 2011/04/26 23:01:57]

I think this leaks all out_pics?
To make the current API work, you need to keep your own map from
picture_buffer_id to the out_pic that currently points to it, so in
{Dismiss,Reuse}PictureBuffer(id) you can free/reuse the out_pic.

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Yeah, that sounds right. Hmm... not quite sure how we should fix this yet (API
change or using a map to manage the memory ourselves).

+  ppp_videodecoder->PictureReady(resource.id, out_pic);
 }
 
-void PPB_VideoDecoder_Impl::DismissPictureBuffer(
-      media::VideoDecodeAccelerator::PictureBuffer* picture_buffer) {
-  // TODO(vmr): Implement.
-  NOTIMPLEMENTED();
+void PPB_VideoDecoder_Impl::DismissPictureBuffer(int32 picture_buffer_id) {
+  const PPP_VideoDecoder_Dev* ppp_videodecoder =
+      static_cast<const PPP_VideoDecoder_Dev*>(instance()->module()->
+          GetPluginInterface(PPP_VIDEODECODER_DEV_INTERFACE));
+  ScopedResourceId resource(this);
+  ppp_videodecoder->DismissPictureBuffer(resource.id, picture_buffer_id);
 }
 
 void PPB_VideoDecoder_Impl::NotifyEndOfStream() {
-  // TODO(vmr): Implement.
-  NOTIMPLEMENTED();
-
-  // Get decoder's resource handle.
-  // PP_Resource resource_decoder;
-  // Get plugin's PPP interface function pointers.
-  // PPP_VideoDecoder* ppp_videodecoder;
-  // Call EndOfStream function pointer and return.
-  // ppp_videodecoder->EndOfStream(resource_decoder);
+  const PPP_VideoDecoder_Dev* ppp_videodecoder =
+      static_cast<const PPP_VideoDecoder_Dev*>(instance()->module()->
+          GetPluginInterface(PPP_VIDEODECODER_DEV_INTERFACE));
+  ScopedResourceId resource(this);
+  ppp_videodecoder->EndOfStream(resource.id);
 }
 
 void PPB_VideoDecoder_Impl::NotifyError(
@@ -293,6 +295,62 @@ void PPB_VideoDecoder_Impl::NotifyError(
   // ppp_videodecoder->NotifyError(error, error_data, data_size);
 }
 
+media::VideoDecodeAccelerator::PictureBuffer*
+PPB_VideoDecoder_Impl::CreateMediaPictureBuffer(
+    struct PP_PictureBuffer_Dev& input) {
+
+  gfx::Size size(input.width, input.height);
+
+  std::vector<media::PictureBuffer::DataPlaneHandle> data_plane_handles;
+  media::PictureBuffer::DataPlaneHandle data_plane;
+  media::PictureBuffer::MemoryType memory_type;
+
+  if (input.storage_type ==  PP_PICTUREBUFFERTYPE_SYSTEM) {

[scherkus (not reviewing), 2011/04/26 22:36:34]

nit: two spaces should be one

[Ami GONE FROM CHROMIUM, 2011/04/26 23:01:57]

extra space in there, but maybe use a switch instead (since that gives you a
free compiler-provided compile-time assertion you're not missing cases).

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Done.

+    memory_type = media::PictureBuffer::PICTUREBUFFER_MEMORYTYPE_SYSTEM;
+    scoped_refptr<PPB_Buffer_Impl> pepper_buffer =
+        Resource::GetAs<PPB_Buffer_Impl>(input.storage.sysmem);
+    assert(pepper_buffer->is_mapped());
+    data_plane.sysmem = static_cast<void*>(pepper_buffer->mapped_buffer());
+  } else if (input.storage_type ==  PP_PICTUREBUFFERTYPE_SYSTEM) {
+    memory_type = media::PictureBuffer::PICTUREBUFFER_MEMORYTYPE_GL_TEXTURE;
+    data_plane.gles2_texture.texture_id =
+        input.storage.gles2_texture.texture_id;
+    data_plane.gles2_texture.context_id = input.storage.gles2_texture.context;
+  } else {
+    // Storage type unrecognized.
+    NOTREACHED();

[scherkus (not reviewing), 2011/04/26 22:36:34]

warning: on release builds this will be optimized out (it's a DCHECK(false))

in which case data_plane might be uninitialized and get pushed onto
data_plan_handles etc etc

perhaps return NULL?

can we signal an error?

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

For now, returning NULL and not adding it to the picture buffers, though I think
the current API says the I should call DismissPictureBuffer on the malformed
buffer. Not sure if that's the "proper" behavior. I'll fix this after I fix the
API+memory issues!

+  }
+  data_plane_handles.push_back(data_plane);
+
+  media::PictureBuffer* buffer =
+      new media::PictureBuffer(input.id, size, memory_type, data_plane_handles);
+
+  return buffer;
+}
+
+media::VideoDecodeAccelerator::Picture*
+PPB_VideoDecoder_Impl::CreateMediaPicture(struct PP_Picture_Dev& input) {
+  gfx::Size visible_size(input.visible_width, input.visible_height);
+  gfx::Size decoded_size(input.decoded_width, input.decoded_height);
+  media::Picture* picture =
+      new media::Picture(input.picture_buffer_id, decoded_size,
+                         visible_size, input.bitstream_user_handle);
+  return picture;
+}
+
+struct PP_Picture_Dev* PPB_VideoDecoder_Impl::CreatePictureDev(
+    media::VideoDecodeAccelerator::Picture& input) {
+  struct PP_Picture_Dev* picture = (struct PP_Picture_Dev*)
+      malloc(sizeof(struct PP_Picture_Dev));

[scherkus (not reviewing), 2011/04/26 22:36:34]

not a pepper master but I'm not seeing any other ppb *.cc files use malloc+cast
but instead use new

I guess since this is C++ new should be OK as long as it's still C++ that still
de-allocates the pictures

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Yeah I wasn't sure on this case. Again, since I'm not sure if I want to be
allocating memory here in the first place, I'll fix this once I figure out the
API+mem stuff should work.

+  picture->picture_buffer_id = input.GetId();
+  picture->bitstream_user_handle = input.GetUserHandle();
+  picture->visible_width = input.GetVisibleSize().width();
+  picture->visible_height = input.GetVisibleSize().height();
+  picture->decoded_width = input.GetDecodedSize().width();
+  picture->decoded_height = input.GetDecodedSize().height();
+  return picture;
+}
+
 void PPB_VideoDecoder_Impl::OnAbortComplete() {
   if (abort_callback_.func == NULL)
     return;
@@ -307,6 +365,15 @@ void PPB_VideoDecoder_Impl::OnBitstreamBufferProcessed() {
   if (bitstream_buffer_callback_.func == NULL)
     return;
 
+  const PPP_VideoDecoder_Dev* ppp_videodecoder =
+      static_cast<const PPP_VideoDecoder_Dev*>(instance()->module()->
+          GetPluginInterface(PPP_VIDEODECODER_DEV_INTERFACE));
+
+  ScopedResourceId resource(this);
+  // TODO: Does this really need to be a ptr?

[scherkus (not reviewing), 2011/04/26 22:36:34]

todo for who?

[vrk (LEFT CHROMIUM), 2011/04/27 00:40:33]

Whoops, this whole block of added code doesn't make sense. Meant to delete this
before uploading patch. D'oh. Deleted!

+  PP_Picture_Dev decoded_picture;
+  ppp_videodecoder->PictureReady(resource.id, &decoded_picture);
+
   // Call the callback that was stored to be called when bitstream was sent for
   // decoding.
   PP_CompletionCallback callback = PP_BlockUntilComplete();