Add CDM allocator interface.

webkit/media/crypto/ppapi/content_decryption_module.h

 // 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.
 
 #ifndef WEBKIT_MEDIA_CRYPTO_PPAPI_CONTENT_DECRYPTION_MODULE_H_
 #define WEBKIT_MEDIA_CRYPTO_PPAPI_CONTENT_DECRYPTION_MODULE_H_
 
 #if defined(_MSC_VER)
 typedef unsigned char uint8_t;
 typedef unsigned int uint32_t;
 typedef int int32_t;
 typedef __int64 int64_t;
 #else
 #include <stdint.h>
 #endif
 
 #include "webkit/media/crypto/ppapi/cdm_export.h"
 
 namespace cdm {
+class CdmAllocator;

[ddorwin, 2012/09/07 09:48:48]

"Cdm" is redundant in the "cdm" interface.

[Tom Finegan, 2012/09/08 01:02:47]

Done. Also renamed CdmBuffer to Buffer. For the same reason.

 class ContentDecryptionModule;
 }
 
 extern "C" {
-CDM_EXPORT cdm::ContentDecryptionModule* CreateCdmInstance();
+CDM_EXPORT cdm::ContentDecryptionModule* CreateCdmInstance(
+    cdm::CdmAllocator* allocator);
 CDM_EXPORT void DestroyCdmInstance(cdm::ContentDecryptionModule* instance);
 CDM_EXPORT const char* GetCdmVersion();
 }
 
 namespace cdm {
 
 enum Status {
   kSuccess = 0,
   kNeedMoreData,  // Decoder needs more data to produce a decoded frame/sample.
   kNoKey,  // The required decryption key is not available.
   kError
 };
 
 // Represents a key message sent by the CDM. It does not own any pointers in
 // this struct.
-// TODO(xhwang): Use int32_t instead of uint32_t for sizes here and below and
-// update checks to include <0.
 struct KeyMessage {
   KeyMessage()
       : session_id(NULL),
         session_id_size(0),
         message(NULL),
         message_size(0),
         default_url(NULL),
         default_url_size(0) {}
 
   char* session_id;
-  uint32_t session_id_size;
+  int32_t session_id_size;
+
+  // TODO(tomfinegan): Replace |message| and |message_size| with a CdmBuffer.
   uint8_t* message;
-  uint32_t message_size;
+  int32_t message_size;
+
   char* default_url;
-  uint32_t default_url_size;
+  int32_t default_url_size;
 };
 
 // An input buffer can be split into several continuous subsamples.
 // A SubsampleEntry specifies the number of clear and cipher bytes in each
 // subsample. For example, the following buffer has three subsamples:
 //
 // |<----- subsample1 ----->|<----- subsample2 ----->|<----- subsample3 ----->|
 // |   clear1   |  cipher1  |  clear2  |   cipher2   | clear3 |    cipher3    |
 //
 // For decryption, all of the cipher bytes in a buffer should be concatenated
@@ skipping 30 matching lines @@
         key_id_size(0),
         iv(NULL),
         iv_size(0),
         checksum(NULL),
         checksum_size(0),
         subsamples(NULL),
         num_subsamples(0),
         timestamp(0) {}
 
   const uint8_t* data;  // Pointer to the beginning of the input data.
-  uint32_t data_size;  // Size (in bytes) of |data|.
+  int32_t data_size;  // Size (in bytes) of |data|.
 
-  uint32_t data_offset;  // Number of bytes to be discarded before decryption.
+  int32_t data_offset;  // Number of bytes to be discarded before decryption.
 
   const uint8_t* key_id;  // Key ID to identify the decryption key.
-  uint32_t key_id_size;  // Size (in bytes) of |key_id|.
+  int32_t key_id_size;  // Size (in bytes) of |key_id|.
 
   const uint8_t* iv;  // Initialization vector.
-  uint32_t iv_size;  // Size (in bytes) of |iv|.
+  int32_t iv_size;  // Size (in bytes) of |iv|.
 
   const uint8_t* checksum;
-  uint32_t checksum_size;  // Size (in bytes) of the |checksum|.
+  int32_t checksum_size;  // Size (in bytes) of the |checksum|.
 
   const struct SubsampleEntry* subsamples;
-  uint32_t num_subsamples;  // Number of subsamples in |subsamples|.
+  int32_t num_subsamples;  // Number of subsamples in |subsamples|.
 
   int64_t timestamp;  // Presentation timestamp in microseconds.
 };
 
 // Represents an output decrypted buffer. It does not own |data|.
 struct OutputBuffer {
   OutputBuffer()
       : data(NULL),
         data_size(0),
-        timestamp(0) {}
+        timestamp(0),
+        buffer_id(0) {}
 
-  const uint8_t* data;  // Pointer to the beginning of the output data.
-  uint32_t data_size;  // Size (in bytes) of |data|.
+  uint8_t* data;  // Pointer to the beginning of the output data.
+  int32_t data_size;  // Size (in bytes) of |data|.
 
   int64_t timestamp;  // Presentation timestamp in microseconds.
+  int32_t buffer_id;  // Buffer identifier. Issued by CdmAllocator, and used by

[ddorwin, 2012/09/07 09:48:48]

Why is this not just a CdmBuffer?

[Tom Finegan, 2012/09/08 01:02:47]

Removed data, data_size, and buffer_id. Replaced with cdm::Buffer*.

+                      // the CDM wrapper.
 };
 
 // Surface formats based on FOURCC labels, see:
 // http://www.fourcc.org/yuv.php
 enum VideoFormat {
   kUnknownVideoFormat = 0,  // Unknown format value.  Used for error reporting.
   kEmptyVideoFrame,  // An empty frame.
   kYv12,  // 12bpp YVU planar 1x1 Y, 2x2 VU samples.
   kI420  // 12bpp YVU planar 1x1 Y, 2x2 UV samples.
 };
@@ skipping 153 matching lines @@
   virtual void ResetVideoDecoder() = 0;
 
   // Stops the CDM video decoder and sets it to an uninitialized state. The
   // caller can call InitializeVideoDecoder() again after this call to
   // re-initialize the video decoder. This can be used to reconfigure the
   // video decoder if the config changes.
   virtual void StopVideoDecoder() = 0;
 
   virtual ~ContentDecryptionModule() {}
 };
 

[ddorwin, 2012/09/07 09:48:48]

The comment should probably document the behavior. Will the buffer be freed when
this object is destroyed?
You might have similar questions on construction unless you prevent anything but
an empty buffer from being created in the base class.

[Tom Finegan, 2012/09/08 01:02:47]

Added note saying that the buffer is not freed.
Base class is now abstract, so moot, no?

+// Utility class that encapsulates all data required to interact with buffers

[ddorwin, 2012/09/07 09:48:48]

Remove "Utility class that ".

[Tom Finegan, 2012/09/08 01:02:47]

Done.

+// allocated by |CdmAllocator|s.

[ddorwin, 2012/09/07 09:48:48]

I don't think | is necessary around class names.

[Tom Finegan, 2012/09/08 01:02:47]

Done.

+class CdmBuffer {

[ddorwin, 2012/09/07 09:48:48]

This should be an abstract class that only exposes buffer()/data().
id and size are implementation details of the Impl. The former is specifically
related to the buffer.

[Tom Finegan, 2012/09/08 01:02:47]

I left the size method in the class since doing so allowed removal of the data
and data_size members from OutputBuffer.

+ public:
+  // Constructs an empty buffer.

[ddorwin, 2012/09/07 09:48:48]

Constructing a buffer sounds like allocation. But really it just represents it.
(Unless you change the Impl to hold the actual buffer.

[Tom Finegan, 2012/09/08 01:02:47]

Removed.

+  CdmBuffer() : buffer_(0), id_(0), size_(0) {}
+
+  // Constructs a buffer object containing all information necessary for a
+  // user writable buffer.

[ddorwin, 2012/09/07 09:48:48]

"user"?

[Tom Finegan, 2012/09/08 01:02:47]

Meant writable by user of the class, but I dropped the user.

+  CdmBuffer(uint8_t* buffer, int32_t id, int32_t size)
+      : buffer_(buffer), id_(id), size_(size) {}
+
+  // Constructs a buffer containing only a valid ID. Provided for the

[ddorwin, 2012/09/07 09:48:48]

This seems odd. Maybe it's not necessary if we store the CdmBuffer in
OutputBuffer.

[Tom Finegan, 2012/09/08 01:02:47]

You were right: storing the cdm::Buffer in OutputBuffer made this constructor
unnecessary.

+  // sake of convenience. For example, allows caller of
+  // CdmAllocator::ReleaseBuffer to pass CdmBuffer(value) when concerned only
+  // with releasing the buffer wrapped by this object.
+  explicit CdmBuffer(int32_t id) : buffer_(0), id_(id), size_(0) {}
+  ~CdmBuffer() {}

[ddorwin, 2012/09/07 09:48:48]

Be sure this is virtual after making this a base class.

[Tom Finegan, 2012/09/08 01:02:47]

Done.

+
+  uint8_t* buffer() const { return buffer_; }
+  int32_t id() const { return id_; }
+  int32_t size() const { return size_; }
+
+ private:
+  uint8_t* buffer_;

[ddorwin, 2012/09/07 09:48:48]

*const

[Tom Finegan, 2012/09/08 01:02:47]

cdm::Buffer no longer has data members.

+  const int32_t id_;
+  const int32_t size_;
+};
+
+// Interface class intended to hide cross object memory allocation details from

[ddorwin, 2012/09/07 09:48:48]

s/intended to/that/

[Tom Finegan, 2012/09/08 01:02:47]

Done.

+// CDMs. The CDM wrapper takes ownership of the allocated data when it is

[ddorwin, 2012/09/07 09:48:48]

This sentence sounds like it should be on those methods. What do we really need
to say here. Is my comment below sufficient?

[Tom Finegan, 2012/09/08 01:02:47]

Used your comment, but added a little bit...

+// returned to the wrapper by the CDM (i.e. via OutputBuffer in Decrypt()).

[ddorwin, 2012/09/07 09:48:48]

Allocated buffers should only be freed by the CDM wrapper.

[Tom Finegan, 2012/09/08 01:02:47]

Done.

+class CdmAllocator {
+ public:
+  CdmAllocator() {}
+  virtual ~CdmAllocator() {}
+
+  // Returns a CdmBuffer containing non-zero members upon success. Returns an
+  // empty CdmBuffer with all members equal to zero on failure.
+  virtual CdmBuffer Allocate(int32_t size) = 0;
+
+  // Used by the CDM to release a buffer that will not be delivered to the CDM
+  // wrapper. Returns true when |buffer| contains a valid buffer identifier,
+  // false otherwise.
+  virtual bool ReleaseBuffer(const CdmBuffer& buffer) = 0;
+};
+
 }  // namespace cdm
 
 #endif  // WEBKIT_MEDIA_CRYPTO_PPAPI_CONTENT_DECRYPTION_MODULE_H_