Bug 1266336 - Clarify expected usage of CDM wrapper - r?cpearce draft
authorGerald Squelart <gsquelart@mozilla.com>
Fri, 06 May 2016 12:10:59 +1000
changeset 364171 c8558ed684abf2c9fb4c534d950df59d16f56eea
parent 364170 c089d5ef5c2342f03c557c6be87871e3da14169b
child 520200 dd089d54b7c6b2cfeb4595e45e342e799646980a
push id17375
push usergsquelart@mozilla.com
push dateFri, 06 May 2016 02:13:52 +0000
reviewerscpearce
bugs1266336
milestone49.0a1
Bug 1266336 - Clarify expected usage of CDM wrapper - r?cpearce 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