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

ppapi/cpp/dev/video_decoder_dev.h

 // 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.
 
 #ifndef PPAPI_CPP_DEV_VIDEO_DECODER_DEV_H_
 #define PPAPI_CPP_DEV_VIDEO_DECODER_DEV_H_
 
 #include <vector>
 
 #include "ppapi/c/dev/pp_video_dev.h"
@@ skipping 39 matching lines @@
 
   // Constructor for the video decoder. Calls the Create on the
   // PPB_VideoDecoder_Dev interface.
   //
   // Parameters:
   //  |instance| is the pointer to the plug-in instance.
   //  |config| is the configuration on which the decoder should be initialized.
   //  |callback| will be called when decoder is initialized.
   //  |client| is the pointer to the client object. Ownership of the object is
   //  not transferred and it must outlive the lifetime of this class.
-  VideoDecoder(const Instance* instance, const std::vector<uint32_t>& config,
+  VideoDecoder(const Instance* instance,
+               const std::vector<PP_VideoConfigElement>& config,

[brettw, 2011/06/01 07:57:03]

I still have concerns about using a vector here, are you sure this is an
improvement over a C string?

As I mentioned in the previous review, the user must add the terminator in the
list even though the vector has a specific size and you're not used to having to
have some "ending marker" on a vector. This seems surprising and will crash.

How do we expect people to typically call it? For some things like this you have
a hardcoded list, and the vector makes this less convenient (since you'd just
make an array in C declaring the things you want, and pass it in). If we expect
this is the common usage, it seems like a regular C pointer like the C interface
is better.

If we expect callers to have to dynamically build it, the vector seems better,
but it seems like we should have some checks that it has the terminator at the
end. In this case, I'd make the constructor assert that it has the terminator,
and just return early (leaving the object uninitialized) if it's not there.

[Ville-Mikko Rautio, 2011/06/01 09:45:26]

Agreed. C-array style is better from user perspective. Changed it back.

                CompletionCallback callback, Client* client);
   ~VideoDecoder();
 
   // GetConfigs returns supported configurations that are subsets of given
   // |prototype_config|.
-  static std::vector<uint32_t> GetConfigs(
+  static std::vector<PP_VideoConfigElement> GetConfigs(
       Instance* instance,
-      const std::vector<uint32_t>& prototype_config);
+      const std::vector<PP_VideoConfigElement>& prototype_config);
 
   // Provides the decoder with picture buffers for video decoding.
   // AssignGLESBuffers provides texture-backed buffers, whereas
   // AssignSysmemBuffers provides system memory-backed buffers.
-  void AssignGLESBuffers(uint32_t no_of_buffers,
-                         const PP_GLESBuffer_Dev& buffers);
-  void AssignSysmemBuffers(uint32_t no_of_buffers,
-                           const PP_SysmemBuffer_Dev& buffers);
+  void AssignGLESBuffers(const std::vector<PP_GLESBuffer_Dev>& buffers);
+  void AssignSysmemBuffers(const std::vector<PP_SysmemBuffer_Dev>& buffers);
 
   // Decodes given bitstream buffer. Once decoder is done with processing
   // |bitstream_buffer| is will call |callback| with provided user data.
   bool Decode(const PP_VideoBitstreamBuffer_Dev& bitstream_buffer,
               CompletionCallback callback);
 
   // Tells the decoder to reuse given picture buffer.
   void ReusePictureBuffer(int32_t picture_buffer_id);
 
   // Flushes the decoder. |callback| will be called as soon as Flush has been
@@ skipping 10 matching lines @@
   Client* client_;
 
   // Suppress compiler-generated copy constructors.
   VideoDecoder(const VideoDecoder&);
   void operator=(const VideoDecoder&);
 };
 
 }  // namespace pp
 
 #endif  // PPAPI_CPP_DEV_VIDEO_DECODER_DEV_H_