Bug 1186406 - Copy input to ClearKey's decoder, so we can return its containing shmem earlier. r=gerald
authorChris Pearce <cpearce@mozilla.com>
Tue, 01 Dec 2015 18:13:58 +1300
changeset 309002 4ec205ad08157abb71b492a10e423de4e971770e
parent 309001 39cbbf8f1c0ae776822ebf3800b69c4c9e1c7fa2
child 309003 f647c855f10b4c8e9e4d5b43c5bb1a4a05d38e50
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1186406
milestone45.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1186406 - Copy input to ClearKey's decoder, so we can return its containing shmem earlier. r=gerald We're failing in the "Very rough kill-switch" case in GMPVideoDecoderParent::Decode() we find that too many shmems are in use when we come to send a "Decode" message to the GMP, and that causes an error which percolates up to cause the test failure. This patch changes gmp-clearkey to copy the input encrypted and compressed sample and immediately return the shmem to the parent process. We are copying the data anyway when we decrypt, so we can rejigg things so that we don't actually end up doing a second copy.
media/gmp-clearkey/0.1/AudioDecoder.cpp
media/gmp-clearkey/0.1/ClearKeyDecryptionManager.cpp
media/gmp-clearkey/0.1/ClearKeyDecryptionManager.h
media/gmp-clearkey/0.1/ClearKeySessionManager.cpp
media/gmp-clearkey/0.1/ClearKeyUtils.h
media/gmp-clearkey/0.1/VideoDecoder.cpp
media/gmp-clearkey/0.1/VideoDecoder.h
--- a/media/gmp-clearkey/0.1/AudioDecoder.cpp
+++ b/media/gmp-clearkey/0.1/AudioDecoder.cpp
@@ -114,17 +114,17 @@ AudioDecoder::DecodeTask(GMPAudioSamples
   }
 
   const GMPEncryptedBufferMetadata* crypto = aInput->GetDecryptionData();
   std::vector<uint8_t> buffer(inBuffer, inBuffer + aInput->Size());
   if (crypto) {
     // Plugin host should have set up its decryptor/key sessions
     // before trying to decode!
     GMPErr rv =
-      ClearKeyDecryptionManager::Get()->Decrypt(&buffer[0], buffer.size(), crypto);
+      ClearKeyDecryptionManager::Get()->Decrypt(buffer, CryptoMetaData(crypto));
 
     if (GMP_FAILED(rv)) {
       CK_LOGE("Failed to decrypt with key id %08x...", *(uint32_t*)crypto->KeyId());
       MaybeRunOnMainThread(WrapTask(mCallback, &GMPAudioDecoderCallback::Error, rv));
       return;
     }
   }
 
--- a/media/gmp-clearkey/0.1/ClearKeyDecryptionManager.cpp
+++ b/media/gmp-clearkey/0.1/ClearKeyDecryptionManager.cpp
@@ -25,17 +25,17 @@ class ClearKeyDecryptor : public RefCoun
 {
 public:
   ClearKeyDecryptor();
 
   void InitKey(const Key& aKey);
   bool HasKey() const { return !!mKey.size(); }
 
   GMPErr Decrypt(uint8_t* aBuffer, uint32_t aBufferSize,
-                 const GMPEncryptedBufferMetadata* aMetadata);
+                 const CryptoMetaData& aMetadata);
 
   const Key& DecryptionKey() const { return mKey; }
 
 private:
   ~ClearKeyDecryptor();
 
   Key mKey;
 };
@@ -126,27 +126,32 @@ ClearKeyDecryptionManager::ReleaseKeyId(
 
   ClearKeyDecryptor* decryptor = mDecryptors[aKeyId];
   if (!decryptor->Release()) {
     mDecryptors.erase(aKeyId);
   }
 }
 
 GMPErr
+ClearKeyDecryptionManager::Decrypt(std::vector<uint8_t>& aBuffer,
+                                   const CryptoMetaData& aMetadata)
+{
+  return Decrypt(&aBuffer[0], aBuffer.size(), aMetadata);
+}
+
+GMPErr
 ClearKeyDecryptionManager::Decrypt(uint8_t* aBuffer, uint32_t aBufferSize,
-                                   const GMPEncryptedBufferMetadata* aMetadata)
+                                   const CryptoMetaData& aMetadata)
 {
   CK_LOGD("ClearKeyDecryptionManager::Decrypt");
-  KeyId keyId(aMetadata->KeyId(), aMetadata->KeyId() + aMetadata->KeyIdSize());
-
-  if (!HasKeyForKeyId(keyId)) {
+  if (!HasKeyForKeyId(aMetadata.mKeyId)) {
     return GMPNoKeyErr;
   }
 
-  return mDecryptors[keyId]->Decrypt(aBuffer, aBufferSize, aMetadata);
+  return mDecryptors[aMetadata.mKeyId]->Decrypt(aBuffer, aBufferSize, aMetadata);
 }
 
 ClearKeyDecryptor::ClearKeyDecryptor()
 {
   CK_LOGD("ClearKeyDecryptor ctor");
 }
 
 ClearKeyDecryptor::~ClearKeyDecryptor()
@@ -157,57 +162,57 @@ ClearKeyDecryptor::~ClearKeyDecryptor()
 void
 ClearKeyDecryptor::InitKey(const Key& aKey)
 {
   mKey = aKey;
 }
 
 GMPErr
 ClearKeyDecryptor::Decrypt(uint8_t* aBuffer, uint32_t aBufferSize,
-                           const GMPEncryptedBufferMetadata* aMetadata)
+                           const CryptoMetaData& aMetadata)
 {
   CK_LOGD("ClearKeyDecryptor::Decrypt");
   // If the sample is split up into multiple encrypted subsamples, we need to
   // stitch them into one continuous buffer for decryption.
   std::vector<uint8_t> tmp(aBufferSize);
 
-  if (aMetadata->NumSubsamples()) {
+  if (aMetadata.NumSubsamples()) {
     // Take all encrypted parts of subsamples and stitch them into one
     // continuous encrypted buffer.
-    unsigned char* data = aBuffer;
-    unsigned char* iter = &tmp[0];
-    for (size_t i = 0; i < aMetadata->NumSubsamples(); i++) {
-      data += aMetadata->ClearBytes()[i];
-      uint32_t cipherBytes = aMetadata->CipherBytes()[i];
+    uint8_t* data = aBuffer;
+    uint8_t* iter = &tmp[0];
+    for (size_t i = 0; i < aMetadata.NumSubsamples(); i++) {
+      data += aMetadata.mClearBytes[i];
+      uint32_t cipherBytes = aMetadata.mCipherBytes[i];
 
       memcpy(iter, data, cipherBytes);
 
       data += cipherBytes;
       iter += cipherBytes;
     }
 
     tmp.resize((size_t)(iter - &tmp[0]));
   } else {
     memcpy(&tmp[0], aBuffer, aBufferSize);
   }
 
-  assert(aMetadata->IVSize() == 8 || aMetadata->IVSize() == 16);
-  std::vector<uint8_t> iv(aMetadata->IV(), aMetadata->IV() + aMetadata->IVSize());
-  iv.insert(iv.end(), CLEARKEY_KEY_LEN - aMetadata->IVSize(), 0);
+  assert(aMetadata.mIV.size() == 8 || aMetadata.mIV.size() == 16);
+  std::vector<uint8_t> iv(aMetadata.mIV);
+  iv.insert(iv.end(), CLEARKEY_KEY_LEN - aMetadata.mIV.size(), 0);
 
   ClearKeyUtils::DecryptAES(mKey, tmp, iv);
 
-  if (aMetadata->NumSubsamples()) {
+  if (aMetadata.NumSubsamples()) {
     // Take the decrypted buffer, split up into subsamples, and insert those
     // subsamples back into their original position in the original buffer.
-    unsigned char* data = aBuffer;
-    unsigned char* iter = &tmp[0];
-    for (size_t i = 0; i < aMetadata->NumSubsamples(); i++) {
-      data += aMetadata->ClearBytes()[i];
-      uint32_t cipherBytes = aMetadata->CipherBytes()[i];
+    uint8_t* data = aBuffer;
+    uint8_t* iter = &tmp[0];
+    for (size_t i = 0; i < aMetadata.NumSubsamples(); i++) {
+      data += aMetadata.mClearBytes[i];
+      uint32_t cipherBytes = aMetadata.mCipherBytes[i];
 
       memcpy(data, iter, cipherBytes);
 
       data += cipherBytes;
       iter += cipherBytes;
     }
   } else {
     memcpy(aBuffer, &tmp[0], aBufferSize);
--- a/media/gmp-clearkey/0.1/ClearKeyDecryptionManager.h
+++ b/media/gmp-clearkey/0.1/ClearKeyDecryptionManager.h
@@ -19,16 +19,55 @@
 
 #include <map>
 
 #include "ClearKeyUtils.h"
 #include "RefCounted.h"
 
 class ClearKeyDecryptor;
 
+class CryptoMetaData {
+public:
+  CryptoMetaData() {}
+
+  explicit CryptoMetaData(const GMPEncryptedBufferMetadata* aCrypto)
+  {
+    Init(aCrypto);
+  }
+
+  void Init(const GMPEncryptedBufferMetadata* aCrypto)
+  {
+    if (!aCrypto) {
+      assert(!IsValid());
+      return;
+    }
+    Assign(mKeyId, aCrypto->KeyId(), aCrypto->KeyIdSize());
+    Assign(mIV, aCrypto->IV(), aCrypto->IVSize());
+    Assign(mClearBytes, aCrypto->ClearBytes(), aCrypto->NumSubsamples());
+    Assign(mCipherBytes, aCrypto->CipherBytes(), aCrypto->NumSubsamples());
+  }
+
+  bool IsValid() const {
+    return !mKeyId.empty() &&
+           !mIV.empty() &&
+           !mCipherBytes.empty() &&
+           !mClearBytes.empty();
+  }
+
+  size_t NumSubsamples() const {
+    assert(mClearBytes.size() == mCipherBytes.size());
+    return mClearBytes.size();
+  }
+
+  std::vector<uint8_t> mKeyId;
+  std::vector<uint8_t> mIV;
+  std::vector<uint16_t> mClearBytes;
+  std::vector<uint32_t> mCipherBytes;
+};
+
 class ClearKeyDecryptionManager : public RefCounted
 {
 private:
   ClearKeyDecryptionManager();
   ~ClearKeyDecryptionManager();
 
   static ClearKeyDecryptionManager* sInstance;
 
@@ -40,18 +79,21 @@ public:
 
   const Key& GetDecryptionKey(const KeyId& aKeyId);
 
   // Create a decryptor for the given KeyId if one does not already exist.
   void InitKey(KeyId aKeyId, Key aKey);
   void ExpectKeyId(KeyId aKeyId);
   void ReleaseKeyId(KeyId aKeyId);
 
+  // Decrypts buffer *in place*.
   GMPErr Decrypt(uint8_t* aBuffer, uint32_t aBufferSize,
-                 const GMPEncryptedBufferMetadata* aMetadata);
+                 const CryptoMetaData& aMetadata);
+  GMPErr Decrypt(std::vector<uint8_t>& aBuffer,
+                 const CryptoMetaData& aMetadata);
 
   void Shutdown();
 
 private:
   bool IsExpectingKeyForKeyId(const KeyId& aKeyId) const;
 
   std::map<KeyId, ClearKeyDecryptor*> mDecryptors;
 };
--- a/media/gmp-clearkey/0.1/ClearKeySessionManager.cpp
+++ b/media/gmp-clearkey/0.1/ClearKeySessionManager.cpp
@@ -395,17 +395,17 @@ ClearKeySessionManager::Decrypt(GMPBuffe
 
 void
 ClearKeySessionManager::DoDecrypt(GMPBuffer* aBuffer,
                                   GMPEncryptedBufferMetadata* aMetadata)
 {
   CK_LOGD("ClearKeySessionManager::DoDecrypt");
 
   GMPErr rv = mDecryptionManager->Decrypt(aBuffer->Data(), aBuffer->Size(),
-                                              aMetadata);
+                                          CryptoMetaData(aMetadata));
   CK_LOGD("DeDecrypt finished with code %x\n", rv);
   mCallback->Decrypted(aBuffer, rv);
 }
 
 void
 ClearKeySessionManager::Shutdown()
 {
   CK_LOGD("ClearKeySessionManager::Shutdown %p", this);
--- a/media/gmp-clearkey/0.1/ClearKeyUtils.h
+++ b/media/gmp-clearkey/0.1/ClearKeyUtils.h
@@ -98,9 +98,16 @@ public:
     }
   }
 private:
   GMPMutex* mMutex;
 };
 
 GMPMutex* GMPCreateMutex();
 
+template<typename T>
+inline void
+Assign(std::vector<T>& aVec, const T* aData, size_t aLength)
+{
+  aVec.assign(aData, aData + aLength);
+}
+
 #endif // __ClearKeyUtils_h__
--- a/media/gmp-clearkey/0.1/VideoDecoder.cpp
+++ b/media/gmp-clearkey/0.1/VideoDecoder.cpp
@@ -110,91 +110,86 @@ VideoDecoder::Decode(GMPVideoEncodedFram
   {
     AutoLock lock(mMutex);
     mNumInputTasks++;
   }
 
   // Note: we don't need the codec specific info on a per-frame basis.
   // It's mostly useful for WebRTC use cases.
 
+  // Make a copy of the data, so we can release aInputFrame ASAP,
+  // to avoid too many shmem handles being held by the GMP process.
+  // If the GMP process holds on to too many shmem handles, the Gecko
+  // side can fail to allocate a shmem to send more input. This is
+  // particularly a problem in Gecko mochitests, which can open multiple
+  // actors at once which share the same pool of shmems.
+  DecodeData* data = new DecodeData();
+  Assign(data->mBuffer, aInputFrame->Buffer(), aInputFrame->Size());
+  data->mTimestamp = aInputFrame->TimeStamp();
+  data->mDuration = aInputFrame->Duration();
+  data->mIsKeyframe = (aInputFrame->FrameType() == kGMPKeyFrame);
+  const GMPEncryptedBufferMetadata* crypto = aInputFrame->GetDecryptionData();
+  if (crypto) {
+    data->mCrypto.Init(crypto);
+  }
+  aInputFrame->Destroy();
   mWorkerThread->Post(WrapTaskRefCounted(this,
                                          &VideoDecoder::DecodeTask,
-                                         aInputFrame));
+                                         data));
 }
 
-class AutoReleaseVideoFrame {
-public:
-  AutoReleaseVideoFrame(GMPVideoEncodedFrame* aFrame)
-    : mFrame(aFrame)
-  {
-  }
-  ~AutoReleaseVideoFrame()
-  {
-    GetPlatform()->runonmainthread(WrapTask(mFrame, &GMPVideoEncodedFrame::Destroy));
-  }
-private:
-  GMPVideoEncodedFrame* mFrame;
-};
-
 void
-VideoDecoder::DecodeTask(GMPVideoEncodedFrame* aInput)
+VideoDecoder::DecodeTask(DecodeData* aData)
 {
   CK_LOGD("VideoDecoder::DecodeTask");
-  AutoReleaseVideoFrame ensureFrameReleased(aInput);
+  AutoPtr<DecodeData> d(aData);
   HRESULT hr;
 
   {
     AutoLock lock(mMutex);
     mNumInputTasks--;
     assert(mNumInputTasks >= 0);
   }
 
   if (mIsFlushing) {
     CK_LOGD("VideoDecoder::DecodeTask rejecting frame: flushing.");
     return;
   }
 
-  if (!aInput || !mHostAPI || !mDecoder) {
+  if (!aData || !mHostAPI || !mDecoder) {
     CK_LOGE("Decode job not set up correctly!");
     return;
   }
 
-  const uint8_t* inBuffer = aInput->Buffer();
-  if (!inBuffer) {
-    CK_LOGE("No buffer for encoded frame!\n");
-    return;
-  }
-
-  const GMPEncryptedBufferMetadata* crypto = aInput->GetDecryptionData();
-  std::vector<uint8_t> buffer(inBuffer, inBuffer + aInput->Size());
-  if (crypto) {
+  std::vector<uint8_t>& buffer = aData->mBuffer;
+  if (aData->mCrypto.IsValid()) {
     // Plugin host should have set up its decryptor/key sessions
     // before trying to decode!
     GMPErr rv =
-      ClearKeyDecryptionManager::Get()->Decrypt(&buffer[0], buffer.size(), crypto);
+      ClearKeyDecryptionManager::Get()->Decrypt(buffer, aData->mCrypto);
 
     if (GMP_FAILED(rv)) {
       MaybeRunOnMainThread(WrapTask(mCallback, &GMPVideoDecoderCallback::Error, rv));
       return;
     }
   }
 
   AnnexB::ConvertFrameInPlace(buffer);
 
-  if (aInput->FrameType() == kGMPKeyFrame) {
+  if (aData->mIsKeyframe) {
     // We must send the SPS and PPS to Windows Media Foundation's decoder.
     // Note: We do this *after* decryption, otherwise the subsample info
     // would be incorrect.
     buffer.insert(buffer.begin(), mAnnexB.begin(), mAnnexB.end());
   }
 
   hr = mDecoder->Input(buffer.data(),
                        buffer.size(),
-                       aInput->TimeStamp(),
-                       aInput->Duration());
+                       aData->mTimestamp,
+                       aData->mDuration);
 
   CK_LOGD("VideoDecoder::DecodeTask() Input ret hr=0x%x\n", hr);
   if (FAILED(hr)) {
     CK_LOGE("VideoDecoder::DecodeTask() decode failed ret=0x%x%s\n",
         hr,
         ((hr == MF_E_NOTACCEPTING) ? " (MF_E_NOTACCEPTING)" : ""));
     return;
   }
--- a/media/gmp-clearkey/0.1/VideoDecoder.h
+++ b/media/gmp-clearkey/0.1/VideoDecoder.h
@@ -55,17 +55,30 @@ public:
 private:
 
   virtual ~VideoDecoder();
 
   void EnsureWorker();
 
   void DrainTask();
 
-  void DecodeTask(GMPVideoEncodedFrame* aInputFrame);
+  struct DecodeData {
+    DecodeData()
+      : mTimestamp(0)
+      , mDuration(0)
+      , mIsKeyframe(false)
+    {}
+    std::vector<uint8_t> mBuffer;
+    uint64_t mTimestamp;
+    uint64_t mDuration;
+    bool mIsKeyframe;
+    CryptoMetaData mCrypto;
+  };
+
+  void DecodeTask(DecodeData* aData);
 
   void ResetCompleteTask();
 
   void ReturnOutput(IMFSample* aSample,
                     int32_t aWidth,
                     int32_t aHeight,
                     int32_t aStride);