Bug 1266336 - Clarify expected usage of CDM wrapper - r=cpearce,a=ritu
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 06 May 2016 12:10:59 +1000
changeset 326192 5f0df9e62467f647b93c1df8dbec87227c0d1cb6
parent 326191 5495417dc2fc341cd9efa55bb81c6a8ebed58c52
child 326193 073989f714c8ade2740d57fa6b64b9ea388e88a0
push id1128
push userjlund@mozilla.com
push dateWed, 01 Jun 2016 01:31:59 +0000
treeherdermozilla-release@fe0d30de989d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscpearce, ritu
bugs1266336
milestone47.0
Bug 1266336 - Clarify expected usage of CDM wrapper - r=cpearce,a=ritu Assert that the CDM wrapper is given a non-null CDM pointer. (so GetCDM() doesn't need to be null-checked.) Renamed WidevineVideoDecoder mCDM to mCDMWrapper, to avoid (my) confusion. Assert that WidevineVideoDecoder is given a non-null CDM-wrapper pointer. Assert that WidevineVideoDecoder only accesses the CDM before DecodingComplete. Small optimization: Move aCDM into mCDM (to save an AddRef/Release pair). MozReview-Commit-ID: yKupY067ly
dom/media/gmp/widevine-adapter/WidevineUtils.h
dom/media/gmp/widevine-adapter/WidevineVideoDecoder.cpp
dom/media/gmp/widevine-adapter/WidevineVideoDecoder.h
--- a/dom/media/gmp/widevine-adapter/WidevineUtils.h
+++ b/dom/media/gmp/widevine-adapter/WidevineUtils.h
@@ -43,17 +43,19 @@ GMPErr
 ToGMPErr(cdm::Status aStatus);
 
 class CDMWrapper {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CDMWrapper)
 
   CDMWrapper(cdm::ContentDecryptionModule_8* aCDM)
     : mCDM(aCDM)
-  {}
+  {
+    MOZ_ASSERT(mCDM);
+  }
   cdm::ContentDecryptionModule_8* GetCDM() const { return mCDM; }
 private:
   cdm::ContentDecryptionModule_8* mCDM;
   ~CDMWrapper() {
     Log("CDMWrapper destroying CDM=%p", mCDM);
     mCDM->Destroy();
     mCDM = nullptr;
   }
--- a/dom/media/gmp/widevine-adapter/WidevineVideoDecoder.cpp
+++ b/dom/media/gmp/widevine-adapter/WidevineVideoDecoder.cpp
@@ -9,24 +9,27 @@
 #include "WidevineUtils.h"
 #include "WidevineVideoFrame.h"
 
 using namespace cdm;
 
 namespace mozilla {
 
 WidevineVideoDecoder::WidevineVideoDecoder(GMPVideoHost* aVideoHost,
-                                           RefPtr<CDMWrapper> aCDM)
+                                           RefPtr<CDMWrapper> aCDMWrapper)
   : mVideoHost(aVideoHost)
-  , mCDM(aCDM)
+  , mCDMWrapper(Move(aCDMWrapper))
   , mExtraData(new MediaByteBuffer())
   , mSentInput(false)
 {
+  // Expect to start with a CDM wrapper, will release it in DecodingComplete().
+  MOZ_ASSERT(mCDMWrapper);
   Log("WidevineVideoDecoder created this=%p", this);
 
+  // Corresponding Release is in DecodingComplete().
   AddRef();
 }
 
 WidevineVideoDecoder::~WidevineVideoDecoder()
 {
   Log("WidevineVideoDecoder destroyed this=%p", this);
 }
 
@@ -228,16 +231,17 @@ WidevineVideoDecoder::Drain()
   CDM()->ResetDecoder(kStreamTypeVideo);
   mCallback->DrainComplete();
 }
 
 void
 WidevineVideoDecoder::DecodingComplete()
 {
   Log("WidevineVideoDecoder::DecodingComplete()");
-  if (mCDM) {
+  if (mCDMWrapper) {
     CDM()->DeinitializeDecoder(kStreamTypeVideo);
-    mCDM = nullptr;
+    mCDMWrapper = nullptr;
   }
+  // Release that corresponds to AddRef() in constructor.
   Release();
 }
 
 } // namespace mozilla
--- a/dom/media/gmp/widevine-adapter/WidevineVideoDecoder.h
+++ b/dom/media/gmp/widevine-adapter/WidevineVideoDecoder.h
@@ -20,17 +20,17 @@
 namespace mozilla {
 
 class WidevineVideoDecoder : public GMPVideoDecoder {
 public:
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(WidevineVideoDecoder)
 
   WidevineVideoDecoder(GMPVideoHost* aVideoHost,
-                       RefPtr<CDMWrapper> aCDM);
+                       RefPtr<CDMWrapper> aCDMWrapper);
   void InitDecode(const GMPVideoCodec& aCodecSettings,
                   const uint8_t* aCodecSpecific,
                   uint32_t aCodecSpecificLength,
                   GMPVideoDecoderCallback* aCallback,
                   int32_t aCoreCount) override;
   void Decode(GMPVideoEncodedFrame* aInputFrame,
               bool aMissingFrames,
               const uint8_t* aCodecSpecificInfo,
@@ -39,22 +39,27 @@ public:
   void Reset() override;
   void Drain() override;
   void DecodingComplete() override;
 
 private:
 
   ~WidevineVideoDecoder();
 
-  cdm::ContentDecryptionModule_8* CDM() { return mCDM->GetCDM(); }
+  cdm::ContentDecryptionModule_8* CDM() const {
+    // CDM should only be accessed before 'DecodingComplete'.
+    MOZ_ASSERT(mCDMWrapper);
+    // CDMWrapper ensure the CDM is non-null, no need to check again.
+    return mCDMWrapper->GetCDM();
+  }
 
   bool ReturnOutput(WidevineVideoFrame& aFrame);
 
   GMPVideoHost* mVideoHost;
-  RefPtr<CDMWrapper> mCDM;
+  RefPtr<CDMWrapper> mCDMWrapper;
   RefPtr<MediaByteBuffer> mExtraData;
   RefPtr<MediaByteBuffer> mAnnexB;
   GMPVideoDecoderCallback* mCallback;
   std::map<uint64_t, uint64_t> mFrameDurations;
   bool mSentInput;
 };
 
 } // namespace mozilla