Implementation for Pepper C++ Video Decoder API (wrapper on top of C API).

ppapi/cpp/dev/video_decoder_dev.cc

Index: ppapi/cpp/dev/video_decoder_dev.cc
diff --git a/ppapi/cpp/dev/video_decoder_dev.cc b/ppapi/cpp/dev/video_decoder_dev.cc
index 504530a29616642ddb08da314695e13f02a0d2cb..54916485acf16916e90758389e27bbb1e4e864fd 100644
--- a/ppapi/cpp/dev/video_decoder_dev.cc
+++ b/ppapi/cpp/dev/video_decoder_dev.cc
@@ -4,6 +4,8 @@
 
 #include "ppapi/cpp/dev/video_decoder_dev.h"
 
+#include <algorithm>
+
 #include "ppapi/c/dev/ppb_video_decoder_dev.h"
 #include "ppapi/c/dev/ppp_video_decoder_dev.h"
 #include "ppapi/c/pp_errors.h"
@@ -24,64 +26,118 @@ template <> const char* interface_name<PPB_VideoDecoder_Dev>() {
 
 }  // namespace
 
-VideoDecoder::VideoDecoder(const Instance* /* instance */,
-                           const std::vector<uint32_t>& /* config */,
-                           CompletionCallback /* callback */,
+VideoDecoder::VideoDecoder(const Instance* instance,
+                           const std::vector<uint32_t>& config,
+                           CompletionCallback callback,
                            Client* client)
     : client_(client) {
   if (!has_interface<PPB_VideoDecoder_Dev>())
     return;
-  // TODO(vmr): Implement.
+  PP_VideoConfigElement* c_config = new PP_VideoConfigElement[config.size()];
+  std::copy(config.begin(), config.end(), c_config);

[brettw, 2011/05/31 12:46:44]

I don't really understand what you're doing here. The config is in a vector but
you're still requiring the terminator at the end, and it will probably crash if
you forget that.

It frankly seems safer from a caller's perspective to just take the pointer to
the config element array, since then it's more clear that "you need to do
something to tell it about the end of the array".

Also, the copying here seems totally unnecessary.

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

+  PassRefFromConstructor(get_interface<PPB_VideoDecoder_Dev>()->Create(
+      instance->pp_instance(), c_config, callback.pp_completion_callback()));
+  delete [] c_config;

[brettw, 2011/05/31 12:46:44]

Normally I think we don't put a space in delete[].

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

 }
 
 VideoDecoder::~VideoDecoder() {}
 
 vector<uint32_t> VideoDecoder::GetConfigs(
-    Instance* /* instance */,
-    const vector<uint32_t>& /* prototype_config */) {
-  // TODO(vmr): Implement.
+    Instance* instance,
+    const vector<uint32_t>& prototype_config) {

[brettw, 2011/05/31 12:46:44]

You're not very consistent about std:: vs. not. I'd just use std:: everywhere
and delete the using at the top of this file.

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

   vector<uint32_t> matching_configs;
   if (!has_interface<PPB_VideoDecoder_Dev>())
     return matching_configs;
+  // Convert prototype config into C-array and ask how many matching configs
+  // there are.
+  PP_VideoConfigElement* c_proto = NULL;
+  if (prototype_config.size() > 0) {
+    c_proto = new PP_VideoConfigElement[prototype_config.size()];
+    if (c_proto == NULL)
+      return matching_configs;
+    std::copy(prototype_config.begin(), prototype_config.end(), c_proto);

[brettw, 2011/05/31 12:46:44]

Is this copy necessary? Why not just pass &prototype_config[0] if there's an
element (being careful to pass NULL if there's no element). I also don't see why
these vectors are all uint32_t instead of the PP_VideoConfigElement typedef.

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

I changed this as well. From now on, we are absolutely trust that vector
implementation is on all targets implemented as C-array, right? 
Done.

+  }
+  uint32_t num_of_matching_configs = 0;
+  get_interface<PPB_VideoDecoder_Dev>()->GetConfigs(
+      instance->pp_instance(), c_proto, NULL, 0, &num_of_matching_configs);
+  if (num_of_matching_configs == 0) {
+    delete[] c_proto;
+    return matching_configs;
+  }
+  // Then ask the actually matching configs and copy them to the return vector.
+  PP_VideoConfigElement* c_matching_configs =
+      new PP_VideoConfigElement[num_of_matching_configs];

[brettw, 2011/05/31 12:46:44]

You can also copy straight into the vector and avoid this.

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

+  get_interface<PPB_VideoDecoder_Dev>()->GetConfigs(
+      instance->pp_instance(), c_proto, c_matching_configs,
+      num_of_matching_configs, &num_of_matching_configs);
+  std::copy(c_matching_configs,
+            c_matching_configs + num_of_matching_configs,
+            matching_configs.end());
+  delete[] c_proto;
+  delete[] c_matching_configs;
   return matching_configs;
 }
 
-void VideoDecoder::AssignGLESBuffers(uint32_t /* no_of_buffers */,
-                                     const PP_GLESBuffer_Dev& /* buffers */) {
-  // TODO(vmr): Implement.
+void VideoDecoder::AssignGLESBuffers(
+    const std::vector<PP_GLESBuffer_Dev> buffers) {
+  if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
+    return;
+  PP_GLESBuffer_Dev* gles_buffers = new PP_GLESBuffer_Dev[buffers.size()];
+  if (!gles_buffers) {

[brettw, 2011/05/31 12:46:44]

We don't normally check for OOM (and neither do you above) unless it's something
in particular that may fail. I'd just remove these checks (next function as
well).

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

+    assert(!"Out of memory.");
+    return;
+  }
+  std::copy(buffers.begin(), buffers.end(), gles_buffers);

[brettw, 2011/05/31 12:46:44]

I think you can avoid this copy.

[Ville-Mikko Rautio, 2011/05/31 13:33:14]

Done.

+  get_interface<PPB_VideoDecoder_Dev>()->AssignGLESBuffers(
+      pp_resource(), buffers.size(), gles_buffers);
+  delete[] gles_buffers;
 }
 
 void VideoDecoder::AssignSysmemBuffers(
-    uint32_t /* no_of_buffers */,
-    const PP_SysmemBuffer_Dev& /* buffers */) {
-  // TODO(vmr): Implement.
+    const std::vector<PP_SysmemBuffer_Dev> buffers) {
+  if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
+    return;
+  PP_SysmemBuffer_Dev* sysmem_buffers = new PP_SysmemBuffer_Dev[buffers.size()];
+  if (!sysmem_buffers) {
+    assert(!"Out of memory.");
+    return;
+  }
+  std::copy(buffers.begin(), buffers.end(), sysmem_buffers);
+  get_interface<PPB_VideoDecoder_Dev>()->AssignSysmemBuffers(
+      pp_resource(), buffers.size(), sysmem_buffers);
+  delete[] sysmem_buffers;
 }
 
 bool VideoDecoder::Decode(
-    const PP_VideoBitstreamBuffer_Dev& /* bitstream_buffer */,
-    CompletionCallback /* callback */) {
-  // TODO(vmr): Implement.
+    const PP_VideoBitstreamBuffer_Dev& bitstream_buffer,
+    CompletionCallback callback) {
   if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
     return false;
-  return false;
+  return PPBoolToBool(get_interface<PPB_VideoDecoder_Dev>()->Decode(
+      pp_resource(),
+      const_cast<PP_VideoBitstreamBuffer_Dev*>(&bitstream_buffer),
+      callback.pp_completion_callback()));
 }
 
-void VideoDecoder::ReusePictureBuffer(int32_t /* picture_buffer_id */) {
-  // TODO(vmr): Implement.
+void VideoDecoder::ReusePictureBuffer(int32_t picture_buffer_id) {
+  if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
+    return;
+  get_interface<PPB_VideoDecoder_Dev>()->ReusePictureBuffer(
+      pp_resource(), picture_buffer_id);
 }
 
-bool VideoDecoder::Flush(CompletionCallback /* callback */) {
-  // TODO(vmr): Implement.
-  if (!has_interface<PPB_VideoDecoder_Dev>())
+bool VideoDecoder::Flush(CompletionCallback callback) {
+  if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
     return false;
-  return true;
+  return PPBoolToBool(get_interface<PPB_VideoDecoder_Dev>()->Flush(
+      pp_resource(), callback.pp_completion_callback()));
 }
 
-bool VideoDecoder::Abort(CompletionCallback /* callback */) {
-  // TODO(vmr): Implement.
-  if (!has_interface<PPB_VideoDecoder_Dev>())
+bool VideoDecoder::Abort(CompletionCallback callback) {
+  if (!has_interface<PPB_VideoDecoder_Dev>() || !pp_resource())
     return false;
-  return true;
+  return PPBoolToBool(get_interface<PPB_VideoDecoder_Dev>()->Abort(
+      pp_resource(), callback.pp_completion_callback()));
 }
 
 }  // namespace pp