From b9de3db7d3641d4e97e7d2d05ac54b2e7ce4eb4c Mon Sep 17 00:00:00 2001 From: Jean-Yves Avenard Date: Tue, 1 Oct 2019 11:28:06 +0000 Subject: [PATCH] Bug 1584721 - P4. Recycle all ShmemBuffer including for video. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D47556 --HG-- extra : moz-landing-system : lando --- dom/media/ipc/RemoteAudioDecoder.cpp | 33 +++--------------------- dom/media/ipc/RemoteAudioDecoder.h | 5 ---- dom/media/ipc/RemoteDecoderParent.cpp | 36 +++++++++++++++++++++++++-- dom/media/ipc/RemoteDecoderParent.h | 16 ++++++++---- dom/media/ipc/RemoteVideoDecoder.cpp | 14 +++-------- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/dom/media/ipc/RemoteAudioDecoder.cpp b/dom/media/ipc/RemoteAudioDecoder.cpp index 8ddd4244c573..e3647bc4bebd 100644 --- a/dom/media/ipc/RemoteAudioDecoder.cpp +++ b/dom/media/ipc/RemoteAudioDecoder.cpp @@ -80,8 +80,7 @@ RemoteAudioDecoderParent::RemoteAudioDecoderParent( TaskQueue* aManagerTaskQueue, TaskQueue* aDecodeTaskQueue, bool* aSuccess, nsCString* aErrorDescription) : RemoteDecoderParent(aParent, aManagerTaskQueue, aDecodeTaskQueue), - mAudioInfo(aAudioInfo), - mDecodedFramePool(4) { + mAudioInfo(aAudioInfo) { CreateDecoderParams params(mAudioInfo); params.mTaskQueue = mDecodeTaskQueue; params.mOptions = aOptions; @@ -109,11 +108,6 @@ MediaResult RemoteAudioDecoderParent::ProcessDecodedData( DecodedOutputIPDL& aDecodedData) { MOZ_ASSERT(OnManagerThread()); - // If we are here, we know all previously returned RemoteAudioDataIPDL got - // used by the child. We can mark all previously sent ShmemBuffer as available - // again. - ReleaseUsedShmems(); - nsTArray array; for (const auto& data : aData) { @@ -125,26 +119,17 @@ MediaResult RemoteAudioDecoderParent::ProcessDecodedData( "Decoded audio must output an AlignedAudioBuffer " "to be used with RemoteAudioDecoderParent"); - ShmemBuffer buffer = mDecodedFramePool.Get( - this, audio->Data().Length() * sizeof(AudioDataValue), - ShmemPool::AllocationPolicy::Unsafe); + ShmemBuffer buffer = + AllocateBuffer(audio->Data().Length() * sizeof(AudioDataValue)); if (!buffer.Valid()) { return MediaResult(NS_ERROR_OUT_OF_MEMORY, "ShmemBuffer::Get failed in " "RemoteAudioDecoderParent::ProcessDecodedData"); } - if (audio->Data().Length() > buffer.Get().Size()) { - mDecodedFramePool.Put(std::move(buffer)); - return MediaResult(NS_ERROR_OUT_OF_MEMORY, - "ShmemBuffer::Get returned less than requested in " - "RemoteAudioDecoderParent::ProcessDecodedData"); - } PodCopy(buffer.Get().get(), audio->Data().Elements(), audio->Data().Length()); - mUsedShmems.AppendElement(buffer.Get()); - RemoteAudioDataIPDL output( MediaDataIPDL(data->mOffset, data->mTime, data->mTimecode, data->mDuration, data->mKeyframe), @@ -158,16 +143,4 @@ MediaResult RemoteAudioDecoderParent::ProcessDecodedData( return NS_OK; } -void RemoteAudioDecoderParent::CleanupOnActorDestroy() { - ReleaseUsedShmems(); - mDecodedFramePool.Cleanup(this); -} - -void RemoteAudioDecoderParent::ReleaseUsedShmems() { - for (ShmemBuffer& mem : mUsedShmems) { - mDecodedFramePool.Put(ShmemBuffer(mem.Get())); - } - mUsedShmems.Clear(); -} - } // namespace mozilla diff --git a/dom/media/ipc/RemoteAudioDecoder.h b/dom/media/ipc/RemoteAudioDecoder.h index a6545b079333..ebf7d87374e9 100644 --- a/dom/media/ipc/RemoteAudioDecoder.h +++ b/dom/media/ipc/RemoteAudioDecoder.h @@ -35,18 +35,13 @@ class RemoteAudioDecoderParent final : public RemoteDecoderParent { protected: MediaResult ProcessDecodedData(const MediaDataDecoder::DecodedData& aData, DecodedOutputIPDL& aDecodedData) override; - void CleanupOnActorDestroy() override; - private: - void ReleaseUsedShmems(); // Can only be accessed from the manager thread // Note: we can't keep a reference to the original AudioInfo here // because unlike in typical MediaDataDecoder situations, we're being // passed a deserialized AudioInfo from RecvPRemoteDecoderConstructor // which is destroyed when RecvPRemoteDecoderConstructor returns. const AudioInfo mAudioInfo; - ShmemPool mDecodedFramePool; - AutoTArray mUsedShmems; }; } // namespace mozilla diff --git a/dom/media/ipc/RemoteDecoderParent.cpp b/dom/media/ipc/RemoteDecoderParent.cpp index 8ea11cfdd9ce..1405245bea16 100644 --- a/dom/media/ipc/RemoteDecoderParent.cpp +++ b/dom/media/ipc/RemoteDecoderParent.cpp @@ -17,8 +17,9 @@ RemoteDecoderParent::RemoteDecoderParent(RemoteDecoderManagerParent* aParent, TaskQueue* aManagerTaskQueue, TaskQueue* aDecodeTaskQueue) : mParent(aParent), + mDecodeTaskQueue(aDecodeTaskQueue), mManagerTaskQueue(aManagerTaskQueue), - mDecodeTaskQueue(aDecodeTaskQueue) { + mDecodedFramePool(1, ShmemPool::PoolType::DynamicPool) { MOZ_COUNT_CTOR(RemoteDecoderParent); MOZ_ASSERT(OnManagerThread()); // We hold a reference to ourselves to keep us alive until IPDL @@ -97,6 +98,11 @@ mozilla::ipc::IPCResult RemoteDecoderParent::RecvDecode( mManagerTaskQueue, __func__, [self, this, resolver = std::move(aResolver)]( MediaDataDecoder::DecodePromise::ResolveOrRejectValue&& aValue) { + // If we are here, we know all previously returned DecodedOutputIPDL got + // used by the child. We can mark all previously sent ShmemBuffer as + // available again. + ReleaseUsedShmems(); + if (!self->CanRecv()) { // Avoid unnecessarily creating shmem objects later. return; @@ -191,7 +197,33 @@ void RemoteDecoderParent::ActorDestroy(ActorDestroyReason aWhy) { mDecoder->Shutdown(); mDecoder = nullptr; } - CleanupOnActorDestroy(); + ReleaseUsedShmems(); + mDecodedFramePool.Cleanup(this); +} + +ShmemBuffer RemoteDecoderParent::AllocateBuffer(size_t aSize) { + ShmemBuffer buffer = + mDecodedFramePool.Get(this, aSize, ShmemPool::AllocationPolicy::Unsafe); + if (!buffer.Valid()) { + return buffer; + } + if (aSize > buffer.Get().Size()) { + ReleaseBuffer(std::move(buffer)); + return ShmemBuffer(); + } + mUsedShmems.AppendElement(buffer.Get()); + return buffer; +} + +void RemoteDecoderParent::ReleaseBuffer(ShmemBuffer&& aBuffer) { + mDecodedFramePool.Put(std::move(aBuffer)); +} + +void RemoteDecoderParent::ReleaseUsedShmems() { + for (ShmemBuffer& mem : mUsedShmems) { + ReleaseBuffer(ShmemBuffer(mem.Get())); + } + mUsedShmems.Clear(); } bool RemoteDecoderParent::OnManagerThread() { diff --git a/dom/media/ipc/RemoteDecoderParent.h b/dom/media/ipc/RemoteDecoderParent.h index dd021b043c2d..44104778ae2c 100644 --- a/dom/media/ipc/RemoteDecoderParent.h +++ b/dom/media/ipc/RemoteDecoderParent.h @@ -48,13 +48,19 @@ class RemoteDecoderParent : public PRemoteDecoderParent { virtual MediaResult ProcessDecodedData( const MediaDataDecoder::DecodedData& aDatam, DecodedOutputIPDL& aDecodedData) = 0; - virtual void CleanupOnActorDestroy() {} + ShmemBuffer AllocateBuffer(size_t aLength); - RefPtr mParent; - RefPtr mIPDLSelfRef; - RefPtr mManagerTaskQueue; - RefPtr mDecodeTaskQueue; + const RefPtr mParent; + const RefPtr mDecodeTaskQueue; RefPtr mDecoder; + + private: + void ReleaseBuffer(ShmemBuffer&& aBuffer); + void ReleaseUsedShmems(); + RefPtr mIPDLSelfRef; + const RefPtr mManagerTaskQueue; + ShmemPool mDecodedFramePool; + AutoTArray mUsedShmems; }; } // namespace mozilla diff --git a/dom/media/ipc/RemoteVideoDecoder.cpp b/dom/media/ipc/RemoteVideoDecoder.cpp index fdaf56c9b2c7..a8523cbfae04 100644 --- a/dom/media/ipc/RemoteVideoDecoder.cpp +++ b/dom/media/ipc/RemoteVideoDecoder.cpp @@ -128,7 +128,7 @@ RefPtr RemoteVideoDecoderChild::DeserializeImage( delete[] reinterpret_cast(memOrShmem.get_uintptr_t()); break; case MemoryOrShmem::TShmem: - DeallocShmem(memOrShmem.get_Shmem()); + // Memory buffer will be recycled by the parent automatically. break; default: MOZ_ASSERT(false, "Unknown MemoryOrShmem type"); @@ -355,20 +355,14 @@ MediaResult RemoteVideoDecoderParent::ProcessDecodedData( static_cast(video->mImage.get()); SurfaceDescriptorBuffer sdBuffer; - Shmem buffer; - if (!AllocShmem(image->GetDataSize(), Shmem::SharedMemory::TYPE_BASIC, - &buffer)) { + ShmemBuffer buffer = AllocateBuffer(image->GetDataSize()); + if (!buffer.Valid()) { return MediaResult(NS_ERROR_OUT_OF_MEMORY, "AllocShmem failed in " "RemoteVideoDecoderParent::ProcessDecodedData"); } - if (image->GetDataSize() > buffer.Size()) { - return MediaResult(NS_ERROR_OUT_OF_MEMORY, - "AllocShmem returned less than requested in " - "RemoteVideoDecoderParent::ProcessDecodedData"); - } - sdBuffer.data() = std::move(buffer); + sdBuffer.data() = std::move(buffer.Get()); image->BuildSurfaceDescriptorBuffer(sdBuffer); sd = sdBuffer;