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

@@ -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<RemoteAudioDataIPDL> 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<AudioDataValue>()) {
-      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<AudioDataValue>(), 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

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<ShmemBuffer, 4> mUsedShmems;
 };
 
 }  // namespace mozilla

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<uint8_t>()) {
+    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() {

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<RemoteDecoderManagerParent> mParent;
-  RefPtr<RemoteDecoderParent> mIPDLSelfRef;
-  RefPtr<TaskQueue> mManagerTaskQueue;
-  RefPtr<TaskQueue> mDecodeTaskQueue;
+  const RefPtr<RemoteDecoderManagerParent> mParent;
+  const RefPtr<TaskQueue> mDecodeTaskQueue;
   RefPtr<MediaDataDecoder> mDecoder;
+
+ private:
+  void ReleaseBuffer(ShmemBuffer&& aBuffer);
+  void ReleaseUsedShmems();
+  RefPtr<RemoteDecoderParent> mIPDLSelfRef;
+  const RefPtr<TaskQueue> mManagerTaskQueue;
+  ShmemPool mDecodedFramePool;
+  AutoTArray<ShmemBuffer, 4> mUsedShmems;
 };
 
 }  // namespace mozilla

dom/media/ipc/RemoteVideoDecoder.cpp

@@ -128,7 +128,7 @@ RefPtr<mozilla::layers::Image> RemoteVideoDecoderChild::DeserializeImage(
       delete[] reinterpret_cast<uint8_t*>(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<PlanarYCbCrImage*>(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<uint8_t>()) {
-        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;