Add CDM allocator interface.

webkit/media/crypto/ppapi/cdm_wrapper.cc

 // 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.
 
-#include <cstring>  // For memcpy.
+#include <cstring>
 #include <vector>
 
-#include "base/compiler_specific.h"  // For OVERRIDE.
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/hash_tables.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_stdint.h"
 #include "ppapi/c/private/pp_content_decryptor.h"
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/core.h"
 #include "ppapi/cpp/instance.h"
 #include "ppapi/cpp/logging.h"
 #include "ppapi/cpp/module.h"
 #include "ppapi/cpp/pass_ref.h"
 #include "ppapi/cpp/resource.h"
@@ skipping 29 matching lines @@
   if (IsMainThread())
     cb.Run(PP_OK);
   else
     pp::Module::Get()->core()->CallOnMainThread(0, cb, PP_OK);
 }
 
 }  // namespace
 
 namespace webkit_media {
 
+class PpbCdmBuffer : public cdm::Buffer {
+ public:
+  // Constructs an empty buffer.
+  PpbCdmBuffer() : buffer_(NULL), size_(0), id_(0) {}
+
+  // Constructs a buffer object containing all information necessary for a

[ddorwin, 2012/09/12 23:53:01]

This wording is a bit odd, especially such it doesn't allocate any buffer.
Does it make sense for the atual PPAPI calls to occur in here? I guess that
depends on whether we copy these buffers around.

[Tom Finegan, 2012/09/13 10:33:40]

Removed. There's only one constructor now, and hopefully the class comment
suffices.

+  // writable buffer.
+  PpbCdmBuffer(uint8_t* buffer, int32_t size, PP_Resource id)
+      : buffer_(buffer), size_(size), id_(id) {}
+
+  virtual ~PpbCdmBuffer() {}
+
+  uint8_t* buffer() const OVERRIDE { return buffer_; }
+  int32_t size() const OVERRIDE { return size_; }
+  PP_Resource id() const { return id_; }

[ddorwin, 2012/09/12 23:53:01]

nit: Space to separate overrides.

[Tom Finegan, 2012/09/13 10:33:40]

Done.

+
+ private:
+  uint8_t* const buffer_;
+  const int32_t size_;
+  const PP_Resource id_;
+
+  DISALLOW_COPY_AND_ASSIGN(PpbCdmBuffer);
+};
+
+class CdmAllocatorImpl : public cdm::Allocator {
+ public:
+  explicit CdmAllocatorImpl(pp::Instance* instance);
+  virtual ~CdmAllocatorImpl();
+
+  // CdmAllocator methods.
+  // Creates a pp::Buffer_Dev, stores it in |buffer_map_|, and returns the
+  // buffer identifier, pointer, and size wrapped in a cdm::Buffer*. Returns
+  // NULL on failure.

[ddorwin, 2012/09/12 23:53:01]

Caller takes ownership?

[Tom Finegan, 2012/09/13 10:33:40]

Done.

+  virtual cdm::Buffer* Allocate(int32_t size) OVERRIDE;
+
+  // Relinquishes buffer ownership by removing it from |buffer_map_|. The buffer
+  // memory will become invalid after this call returns when it results in a
+  // zero reference count on the pp::Buffer_Dev resource.
+  virtual void ReleaseBuffer(const cdm::Buffer& buffer) OVERRIDE;
+
+ private:
+  typedef base::hash_map<PP_Resource, pp::Buffer_Dev> BufferMap;
+
+  pp::Instance* instance_;
+  BufferMap buffer_map_;

[ddorwin, 2012/09/12 23:53:01]

What do we need this map for? We only insert and remove from it but never look
it up. If PpbCdmBuffer held the Buffer_Dev as a member, then I don't think we'd
need it or the ReleaseBuffer() method.

[Tom Finegan, 2012/09/13 10:33:40]

Removed.

+};
+
 // A wrapper class for abstracting away PPAPI interaction and threading for a
 // Content Decryption Module (CDM).
 class CdmWrapper : public pp::Instance,
                    public pp::ContentDecryptor_Private {
  public:
   CdmWrapper(PP_Instance instance, pp::Module* module);
   virtual ~CdmWrapper();
-
   virtual bool Init(uint32_t argc, const char* argn[], const char* argv[]) {
     return true;
   }
 
   // PPP_ContentDecryptor_Private methods
   // Note: As per comments in PPP_ContentDecryptor_Private, these calls should
   // return false if the call was not forwarded to the CDM and should return
   // true otherwise. Once the call reaches the CDM, the call result/status
   // should be reported through the PPB_ContentDecryptor_Private interface.
   virtual bool GenerateKeyRequest(const std::string& key_system,
@@ skipping 20 matching lines @@
   // <code>callback_factory_</code> to ensure that calls into
   // <code>PPP_ContentDecryptor_Private</code> are asynchronous.
   void KeyAdded(int32_t result, const std::string& session_id);
   void KeyMessage(int32_t result, cdm::KeyMessage& key_message);
   void KeyError(int32_t result, const std::string& session_id);
   void DeliverBlock(int32_t result,
                     const cdm::Status& status,
                     cdm::OutputBuffer& output_buffer,
                     const PP_DecryptTrackingInfo& tracking_info);
 
+  CdmAllocatorImpl allocator_;
   pp::CompletionCallbackFactory<CdmWrapper> callback_factory_;
   cdm::ContentDecryptionModule* cdm_;
   std::string key_system_;
 };
 
+CdmAllocatorImpl::CdmAllocatorImpl(pp::Instance* instance)
+    : instance_(instance) {
+}
+
+CdmAllocatorImpl::~CdmAllocatorImpl() {
+}
+
+cdm::Buffer* CdmAllocatorImpl::Allocate(int32_t size) {
+  PP_DCHECK(size > 0);
+
+  pp::Buffer_Dev buffer(instance_, size);
+  if (buffer.is_null())
+    return NULL;
+
+  if (!buffer_map_.insert(std::make_pair(buffer.pp_resource(), buffer)).second)
+    return NULL;
+
+  return new PpbCdmBuffer(reinterpret_cast<uint8_t*>(buffer.data()),
+                          buffer.size(),
+                          buffer.pp_resource());
+}
+
+void CdmAllocatorImpl::ReleaseBuffer(const cdm::Buffer& buffer) {
+  const PpbCdmBuffer* cdm_buffer = static_cast<const PpbCdmBuffer*>(&buffer);
+  buffer_map_.erase(cdm_buffer->id());
+}
+
 CdmWrapper::CdmWrapper(PP_Instance instance, pp::Module* module)
     : pp::Instance(instance),
       pp::ContentDecryptor_Private(this),
+      allocator_(this),
       cdm_(NULL) {
   callback_factory_.Initialize(this);
 }
 
 CdmWrapper::~CdmWrapper() {
   if (cdm_)
     DestroyCdmInstance(cdm_);
 }
 
 bool CdmWrapper::GenerateKeyRequest(const std::string& key_system,
                                     pp::VarArrayBuffer init_data) {
   PP_DCHECK(!key_system.empty());
 
   if (!cdm_) {
-    cdm_ = CreateCdmInstance();
+    cdm_ = CreateCdmInstance(&allocator_);
     if (!cdm_)
       return false;
   }
 
   cdm::KeyMessage key_request;
   cdm::Status status = cdm_->GenerateKeyRequest(
       reinterpret_cast<const uint8_t*>(init_data.Map()),
       init_data.ByteLength(),
       &key_request);
 
@@ skipping 81 matching lines @@
   for (uint32_t i = 0; i < encrypted_block_info.num_subsamples; ++i) {
     subsamples.push_back(cdm::SubsampleEntry(
         encrypted_block_info.subsamples[i].clear_bytes,
         encrypted_block_info.subsamples[i].cipher_bytes));
   }
   input_buffer.subsamples = &subsamples[0];
   input_buffer.timestamp = encrypted_block_info.tracking_info.timestamp;
 
   cdm::OutputBuffer output_buffer;
   cdm::Status status = cdm_->Decrypt(input_buffer, &output_buffer);
+  PP_DCHECK(status == cdm::kSuccess);
 
   CallOnMain(callback_factory_.NewCallback(
       &CdmWrapper::DeliverBlock,
       status,
       output_buffer,

[ddorwin, 2012/09/12 23:53:01]

The .buffer member (PpbCdmBuffer) will be leaked if this callback is not called.
If we made OutputBuffer own it, we wouldn't need to worry about it.

We also have the ReleaseBuffer issue, but at least that memory would be released
when the instance was destroyed. It would be a plus if we could manage ownership
of that.

[Tom Finegan, 2012/09/13 10:33:40]

I've moved things around a little so that OutputBuffer can own the Buffer*.
I think I still need ReleaseBuffer... can't delete the cdm::Buffer* without
seeing its implementation.

[ddorwin, 2012/09/13 19:47:00]

Correct, you need it. It could be deleted via the abstract class, but then the
memory free would be on the wrong side of the so/dll boundary.

       encrypted_block_info.tracking_info));
   return true;
 }
 
 bool CdmWrapper::DecryptAndDecode(
     pp::Buffer_Dev encrypted_buffer,
     const PP_EncryptedBlockInfo& encrypted_block_info) {
   return false;
 }
 
@@ skipping 40 matching lines @@
   pp::ContentDecryptor_Private::KeyError(key_system_,
                                          session_id,
                                          kUnknownError,
                                          0);
 }
 
 void CdmWrapper::DeliverBlock(int32_t result,
                               const cdm::Status& status,
                               cdm::OutputBuffer& output_buffer,
                               const PP_DecryptTrackingInfo& tracking_info) {
-  pp::Buffer_Dev decrypted_buffer(MakeBufferResource(output_buffer.data,
-                                                     output_buffer.data_size));
   PP_DecryptedBlockInfo decrypted_block_info;
   decrypted_block_info.tracking_info.request_id = tracking_info.request_id;
   decrypted_block_info.tracking_info.timestamp = output_buffer.timestamp;
 
   switch (status) {
     case cdm::kSuccess:
       decrypted_block_info.result = PP_DECRYPTRESULT_SUCCESS;
       break;
     case cdm::kNoKey:
       decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_NOKEY;
       break;
     default:
       decrypted_block_info.result = PP_DECRYPTRESULT_DECRYPT_ERROR;
   }
 
-  pp::ContentDecryptor_Private::DeliverBlock(decrypted_buffer,
-                                             decrypted_block_info);
+  const PpbCdmBuffer* const cdm_buffer =
+      static_cast<PpbCdmBuffer*>(output_buffer.buffer);
+  PP_DCHECK(cdm_buffer);
 
-  // TODO(xhwang): Fix this. This is not always safe as the memory is allocated
-  // in another shared object.
-  delete [] output_buffer.data;
-  output_buffer.data = NULL;
+  pp::ContentDecryptor_Private::DeliverBlock(
+      pp::Buffer_Dev(cdm_buffer->id()),
+      decrypted_block_info);
+  allocator_.ReleaseBuffer(*cdm_buffer);
+  delete cdm_buffer;
 }
 
 // This object is the global object representing this plugin library as long
 // as it is loaded.
-class MyModule : public pp::Module {
+class CdmWrapperModule : public pp::Module {
  public:
-  MyModule() : pp::Module() {}
-  virtual ~MyModule() {}
+  CdmWrapperModule() : pp::Module() {}
+  virtual ~CdmWrapperModule() {}
 
   virtual pp::Instance* CreateInstance(PP_Instance instance) {
     return new CdmWrapper(instance, this);
   }
 };
 
 }  // namespace webkit_media
 
 namespace pp {
 
 // Factory function for your specialization of the Module object.
 Module* CreateModule() {
-  return new webkit_media::MyModule();
+  return new webkit_media::CdmWrapperModule();
 }
 
 }  // namespace pp