From d6025eed6571a37f15c3b2c77147ec5a3134293f Mon Sep 17 00:00:00 2001 From: Chris Pearce Date: Wed, 15 Apr 2015 12:14:49 +1200 Subject: [PATCH] Bug 1154133 - Remove sync dispatches in EMEDecryptor, and mark MediaTaskQueue::SyncDispatch as deprecated. r=edwin --- dom/media/MediaTaskQueue.cpp | 1 + dom/media/MediaTaskQueue.h | 2 + dom/media/eme/CDMProxy.cpp | 89 +++++++++++------- dom/media/eme/CDMProxy.h | 31 +++++-- dom/media/fmp4/eme/EMEDecoderModule.cpp | 115 +++++++++--------------- 5 files changed, 129 insertions(+), 109 deletions(-) diff --git a/dom/media/MediaTaskQueue.cpp b/dom/media/MediaTaskQueue.cpp index a7c1f1920594..adf3e643c69a 100644 --- a/dom/media/MediaTaskQueue.cpp +++ b/dom/media/MediaTaskQueue.cpp @@ -124,6 +124,7 @@ private: nsresult MediaTaskQueue::SyncDispatch(TemporaryRef aRunnable) { + NS_WARNING("MediaTaskQueue::SyncDispatch is dangerous and deprecated. Stop using this!"); RefPtr task(new MediaTaskQueueSyncRunnable(aRunnable)); nsresult rv = Dispatch(task); NS_ENSURE_SUCCESS(rv, rv); diff --git a/dom/media/MediaTaskQueue.h b/dom/media/MediaTaskQueue.h index 673abc257125..81f118fed3e0 100644 --- a/dom/media/MediaTaskQueue.h +++ b/dom/media/MediaTaskQueue.h @@ -84,6 +84,8 @@ public: // flushed. Normal operations should use Dispatch. nsresult ForceDispatch(TemporaryRef aRunnable); + // DEPRECATED! Do not us, if a flush happens at the same time, this function + // can hang and block forever! nsresult SyncDispatch(TemporaryRef aRunnable); // Puts the queue in a shutdown state and returns immediately. The queue will diff --git a/dom/media/eme/CDMProxy.cpp b/dom/media/eme/CDMProxy.cpp index 254630e50591..9c669e3dbf12 100644 --- a/dom/media/eme/CDMProxy.cpp +++ b/dom/media/eme/CDMProxy.cpp @@ -440,7 +440,7 @@ CDMProxy::gmp_Shutdown() // Abort any pending decrypt jobs, to awaken any clients waiting on a job. for (size_t i = 0; i < mDecryptionJobs.Length(); i++) { DecryptJob* job = mDecryptionJobs[i]; - job->mClient->Decrypted(GMPAbortedErr, nullptr); + job->PostResult(GMPAbortedErr); } mDecryptionJobs.Clear(); @@ -625,23 +625,22 @@ CDMProxy::Capabilites() { void CDMProxy::Decrypt(MediaRawData* aSample, - DecryptionClient* aClient) + DecryptionClient* aClient, + MediaTaskQueue* aTaskQueue) { - nsAutoPtr job(new DecryptJob(aSample, aClient)); + nsRefPtr job(new DecryptJob(aSample, aClient, aTaskQueue)); nsCOMPtr task( - NS_NewRunnableMethodWithArg>(this, &CDMProxy::gmp_Decrypt, job)); + NS_NewRunnableMethodWithArg>(this, &CDMProxy::gmp_Decrypt, job)); mGMPThread->Dispatch(task, NS_DISPATCH_NORMAL); } void -CDMProxy::gmp_Decrypt(nsAutoPtr aJob) +CDMProxy::gmp_Decrypt(nsRefPtr aJob) { MOZ_ASSERT(IsOnGMPThread()); - MOZ_ASSERT(aJob->mClient); - MOZ_ASSERT(aJob->mSample); if (!mCDM) { - aJob->mClient->Decrypted(GMPAbortedErr, nullptr); + aJob->PostResult(GMPAbortedErr); return; } @@ -658,34 +657,64 @@ CDMProxy::gmp_Decrypted(uint32_t aId, const nsTArray& aDecryptedData) { MOZ_ASSERT(IsOnGMPThread()); +#ifdef DEBUG + bool jobIdFound = false; +#endif for (size_t i = 0; i < mDecryptionJobs.Length(); i++) { DecryptJob* job = mDecryptionJobs[i]; if (job->mId == aId) { - if (aDecryptedData.Length() != job->mSample->mSize) { - NS_WARNING("CDM returned incorrect number of decrypted bytes"); - } - if (GMP_SUCCEEDED(aResult)) { - nsAutoPtr writer(job->mSample->CreateWriter()); - PodCopy(writer->mData, - aDecryptedData.Elements(), - std::min(aDecryptedData.Length(), job->mSample->mSize)); - job->mClient->Decrypted(GMPNoErr, job->mSample); - } else if (aResult == GMPNoKeyErr) { - NS_WARNING("CDM returned GMPNoKeyErr"); - // We still have the encrypted sample, so we can re-enqueue it to be - // decrypted again once the key is usable again. - job->mClient->Decrypted(GMPNoKeyErr, job->mSample); - } else { - nsAutoCString str("CDM returned decode failure GMPErr="); - str.AppendInt(aResult); - NS_WARNING(str.get()); - job->mClient->Decrypted(aResult, nullptr); - } +#ifdef DEBUG + jobIdFound = true; +#endif + job->PostResult(aResult, aDecryptedData); mDecryptionJobs.RemoveElementAt(i); - return; } } - NS_WARNING("GMPDecryptorChild returned incorrect job ID"); +#ifdef DEBUG + if (!jobIdFound) { + NS_WARNING("GMPDecryptorChild returned incorrect job ID"); + } +#endif +} + +void +CDMProxy::DecryptJob::PostResult(GMPErr aResult) +{ + nsTArray empty; + PostResult(aResult, empty); +} + +void +CDMProxy::DecryptJob::PostResult(GMPErr aResult, const nsTArray& aDecryptedData) +{ + if (aDecryptedData.Length() != mSample->mSize) { + NS_WARNING("CDM returned incorrect number of decrypted bytes"); + } + mResult = aResult; + if (GMP_SUCCEEDED(aResult)) { + nsAutoPtr writer(mSample->CreateWriter()); + PodCopy(writer->mData, + aDecryptedData.Elements(), + std::min(aDecryptedData.Length(), mSample->mSize)); + } else if (aResult == GMPNoKeyErr) { + NS_WARNING("CDM returned GMPNoKeyErr"); + // We still have the encrypted sample, so we can re-enqueue it to be + // decrypted again once the key is usable again. + } else { + nsAutoCString str("CDM returned decode failure GMPErr="); + str.AppendInt(aResult); + NS_WARNING(str.get()); + mSample = nullptr; + } + mTaskQueue->Dispatch(RefPtr(this).forget()); +} + +nsresult +CDMProxy::DecryptJob::Run() +{ + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); + mClient->Decrypted(mResult, mSample); + return NS_OK; } void diff --git a/dom/media/eme/CDMProxy.h b/dom/media/eme/CDMProxy.h index 476577a9a844..28c1516fc6db 100644 --- a/dom/media/eme/CDMProxy.h +++ b/dom/media/eme/CDMProxy.h @@ -141,7 +141,9 @@ public: const nsAString& aMsg); // Threadsafe. - void Decrypt(MediaRawData* aSample, DecryptionClient* aSink); + void Decrypt(MediaRawData* aSample, + DecryptionClient* aSink, + MediaTaskQueue* aTaskQueue); // Reject promise with DOMException corresponding to aExceptionCode. // Can be called from any thread. @@ -234,18 +236,35 @@ private: // GMP thread only. void gmp_RemoveSession(nsAutoPtr aData); - struct DecryptJob { - DecryptJob(MediaRawData* aSample, DecryptionClient* aClient) + class DecryptJob : public nsRunnable { + public: + explicit DecryptJob(MediaRawData* aSample, + DecryptionClient* aClient, + MediaTaskQueue* aTaskQueue) : mId(0) , mSample(aSample) , mClient(aClient) - {} + , mResult(GMPGenericErr) + , mTaskQueue(aTaskQueue) + { + MOZ_ASSERT(mClient); + MOZ_ASSERT(mSample); + } + + NS_METHOD Run() override; + void PostResult(GMPErr aResult, const nsTArray& aDecryptedData); + void PostResult(GMPErr aResult); + uint32_t mId; nsRefPtr mSample; + private: + ~DecryptJob() {} nsAutoPtr mClient; + GMPErr mResult; + nsRefPtr mTaskQueue; }; // GMP thread only. - void gmp_Decrypt(nsAutoPtr aJob); + void gmp_Decrypt(nsRefPtr aJob); class RejectPromiseTask : public nsRunnable { public: @@ -316,7 +335,7 @@ private: // Decryption jobs sent to CDM, awaiting result. // GMP thread only. - nsTArray> mDecryptionJobs; + nsTArray> mDecryptionJobs; // Number of buffers we've decrypted. Used to uniquely identify // decryption jobs sent to CDM. Note we can't just use the length of diff --git a/dom/media/fmp4/eme/EMEDecoderModule.cpp b/dom/media/fmp4/eme/EMEDecoderModule.cpp index b224478b5e01..3f5e213a6bb0 100644 --- a/dom/media/fmp4/eme/EMEDecoderModule.cpp +++ b/dom/media/fmp4/eme/EMEDecoderModule.cpp @@ -22,71 +22,38 @@ public: EMEDecryptor(MediaDataDecoder* aDecoder, MediaDataDecoderCallback* aCallback, - CDMProxy* aProxy) + CDMProxy* aProxy, + MediaTaskQueue* aDecodeTaskQueue) : mDecoder(aDecoder) , mCallback(aCallback) - , mTaskQueue(CreateFlushableMediaDecodeTaskQueue()) + , mTaskQueue(aDecodeTaskQueue) , mProxy(aProxy) , mSamplesWaitingForKey(new SamplesWaitingForKey(this, mTaskQueue, mProxy)) -#ifdef DEBUG , mIsShutdown(false) -#endif { } virtual nsresult Init() override { MOZ_ASSERT(!mIsShutdown); - nsresult rv = mTaskQueue->SyncDispatch( - NS_NewRunnableMethod(mDecoder, &MediaDataDecoder::Init)); - unused << NS_WARN_IF(NS_FAILED(rv)); - return rv; + return mDecoder->Init(); } class DeliverDecrypted : public DecryptionClient { public: - DeliverDecrypted(EMEDecryptor* aDecryptor, FlushableMediaTaskQueue* aTaskQueue) + explicit DeliverDecrypted(EMEDecryptor* aDecryptor) : mDecryptor(aDecryptor) - , mTaskQueue(aTaskQueue) - {} - virtual void Decrypted(GMPErr aResult, - MediaRawData* aSample) override { - if (aResult == GMPNoKeyErr) { - RefPtr task; - task = NS_NewRunnableMethodWithArg>( - mDecryptor, - &EMEDecryptor::Input, - nsRefPtr(aSample)); - mTaskQueue->Dispatch(task.forget()); - } else if (GMP_FAILED(aResult)) { - if (mDecryptor->mCallback) { - mDecryptor->mCallback->Error(); - } - MOZ_ASSERT(!aSample); - } else { - RefPtr task; - task = NS_NewRunnableMethodWithArg>( - mDecryptor, - &EMEDecryptor::Decrypted, - nsRefPtr(aSample)); - mTaskQueue->Dispatch(task.forget()); - } - mTaskQueue = nullptr; + { } + virtual void Decrypted(GMPErr aResult, MediaRawData* aSample) override { + mDecryptor->Decrypted(aResult, aSample); mDecryptor = nullptr; } private: nsRefPtr mDecryptor; - nsRefPtr mTaskQueue; }; virtual nsresult Input(MediaRawData* aSample) override { + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); MOZ_ASSERT(!mIsShutdown); - // We run the PDM on its own task queue. We can't run it on the decode - // task queue, because that calls into Input() in a loop and waits until - // output is delivered. We need to defer some Input() calls while we wait - // for keys to become usable, and once they do we need to dispatch an event - // to run the PDM on the same task queue, but since the decode task queue - // is waiting in MP4Reader::Decode() for output our task would never run. - // So we dispatch tasks to make all calls into the wrapped decoder. if (mSamplesWaitingForKey->WaitIfKeyNotUsable(aSample)) { return NS_OK; } @@ -95,57 +62,59 @@ public: mProxy->GetSessionIdsForKeyId(aSample->mCrypto.mKeyId, writer->mCrypto.mSessionIds); - mProxy->Decrypt(aSample, new DeliverDecrypted(this, mTaskQueue)); + mProxy->Decrypt(aSample, new DeliverDecrypted(this), mTaskQueue); return NS_OK; } - void Decrypted(MediaRawData* aSample) { - MOZ_ASSERT(!mIsShutdown); - nsresult rv = mTaskQueue->Dispatch( - NS_NewRunnableMethodWithArg>( - mDecoder, - &MediaDataDecoder::Input, - nsRefPtr(aSample))); - unused << NS_WARN_IF(NS_FAILED(rv)); + void Decrypted(GMPErr aResult, MediaRawData* aSample) { + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); + if (mIsShutdown) { + NS_WARNING("EME decrypted sample arrived after shutdown"); + return; + } + if (aResult == GMPNoKeyErr) { + // Key became unusable after we sent the sample to CDM to decrypt. + // Call Input() again, so that the sample is enqueued for decryption + // if the key becomes usable again. + Input(aSample); + } else if (GMP_FAILED(aResult)) { + if (mCallback) { + mCallback->Error(); + } + MOZ_ASSERT(!aSample); + } else { + MOZ_ASSERT(!mIsShutdown); + nsresult rv = mDecoder->Input(aSample); + unused << NS_WARN_IF(NS_FAILED(rv)); + } } virtual nsresult Flush() override { + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); MOZ_ASSERT(!mIsShutdown); - nsresult rv = mTaskQueue->SyncDispatch( - NS_NewRunnableMethod( - mDecoder, - &MediaDataDecoder::Flush)); + nsresult rv = mDecoder->Flush(); unused << NS_WARN_IF(NS_FAILED(rv)); mSamplesWaitingForKey->Flush(); return rv; } virtual nsresult Drain() override { + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); MOZ_ASSERT(!mIsShutdown); - nsresult rv = mTaskQueue->Dispatch( - NS_NewRunnableMethod( - mDecoder, - &MediaDataDecoder::Drain)); + nsresult rv = mDecoder->Drain(); unused << NS_WARN_IF(NS_FAILED(rv)); return rv; } virtual nsresult Shutdown() override { + MOZ_ASSERT(mTaskQueue->IsCurrentThreadIn()); MOZ_ASSERT(!mIsShutdown); -#ifdef DEBUG mIsShutdown = true; -#endif - nsresult rv = mTaskQueue->SyncDispatch( - NS_NewRunnableMethod( - mDecoder, - &MediaDataDecoder::Shutdown)); + nsresult rv = mDecoder->Shutdown(); unused << NS_WARN_IF(NS_FAILED(rv)); mSamplesWaitingForKey->BreakCycles(); mSamplesWaitingForKey = nullptr; mDecoder = nullptr; - mTaskQueue->BeginShutdown(); - mTaskQueue->AwaitShutdownAndIdle(); - mTaskQueue = nullptr; mProxy = nullptr; mCallback = nullptr; return rv; @@ -155,12 +124,10 @@ private: nsRefPtr mDecoder; MediaDataDecoderCallback* mCallback; - nsRefPtr mTaskQueue; + nsRefPtr mTaskQueue; nsRefPtr mProxy; nsRefPtr mSamplesWaitingForKey; -#ifdef DEBUG bool mIsShutdown; -#endif }; class EMEMediaDataDecoderProxy : public MediaDataDecoderProxy { @@ -273,7 +240,8 @@ EMEDecoderModule::CreateVideoDecoder(const VideoInfo& aConfig, nsRefPtr emeDecoder(new EMEDecryptor(decoder, aCallback, - mProxy)); + mProxy, + MediaTaskQueue::GetCurrentQueue())); return emeDecoder.forget(); } @@ -303,7 +271,8 @@ EMEDecoderModule::CreateAudioDecoder(const AudioInfo& aConfig, nsRefPtr emeDecoder(new EMEDecryptor(decoder, aCallback, - mProxy)); + mProxy, + MediaTaskQueue::GetCurrentQueue())); return emeDecoder.forget(); }