Bug 792252 - Clean up ImageContainerChild (before implementing SharedPlanarYCbCrImage to remove the extra video frame copy). r=kanru
authorNicolas Silva <nical.bugzilla@gmail.com>
Mon, 29 Oct 2012 16:08:06 +0100
changeset 111831 42b38e4e9068ac3d0af266f83ccdf0be97e52fd3
parent 111830 63ce297be1f21ec26001fdcd20b9318fb29b6391
child 111832 acc131324855c36a695d5fa94f4adca66544eed9
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewerskanru
bugs792252
milestone19.0a1
Bug 792252 - Clean up ImageContainerChild (before implementing SharedPlanarYCbCrImage to remove the extra video frame copy). r=kanru
gfx/layers/ipc/ImageContainerChild.cpp
gfx/layers/ipc/ImageContainerChild.h
gfx/layers/ipc/ShmemYCbCrImage.cpp
gfx/layers/ipc/ShmemYCbCrImage.h
--- a/gfx/layers/ipc/ImageContainerChild.cpp
+++ b/gfx/layers/ipc/ImageContainerChild.cpp
@@ -9,16 +9,18 @@
 #include "ShadowLayers.h"
 #include "mozilla/layers/PLayers.h"
 #include "mozilla/layers/SharedImageUtils.h"
 #include "ImageContainer.h"
 #include "GonkIOSurfaceImage.h"
 #include "GrallocImages.h"
 #include "mozilla/layers/ShmemYCbCrImage.h"
 
+using namespace mozilla::ipc;
+
 namespace mozilla {
 namespace layers {
 
 /*
  * - POOL_MAX_SHARED_IMAGES is the maximum number number of shared images to
  * store in the ImageContainerChild's pool.
  *
  * - MAX_ACTIVE_SHARED_IMAGES is the maximum number of active shared images.
@@ -117,54 +119,62 @@ void ImageContainerChild::DestroySharedI
 {
   NS_ABORT_IF_FALSE(InImageBridgeChildThread(),
                     "Should be in ImageBridgeChild thread.");
 
   --mActiveImageCount;
   DeallocSharedImageData(this, aImage);
 }
 
+SharedImage* ImageContainerChild::AsSharedImage(Image* aImage)
+{
+#ifdef MOZ_WIDGET_GONK
+  if (aImage->GetFormat() == GONK_IO_SURFACE) {
+    GonkIOSurfaceImage* gonkImage = static_cast<GonkIOSurfaceImage*>(aImage);
+    SharedImage* result = new SharedImage(gonkImage->GetSurfaceDescriptor());
+    return result;
+  } else if (aImage->GetFormat() == GRALLOC_PLANAR_YCBCR) {
+    GrallocPlanarYCbCrImage* GrallocImage = static_cast<GrallocPlanarYCbCrImage*>(aImage);
+    SharedImage* result = new SharedImage(GrallocImage->GetSurfaceDescriptor());
+    return result;
+  }
+#endif
+  return nullptr; // XXX: bug 773440
+}
+
 bool ImageContainerChild::CopyDataIntoSharedImage(Image* src, SharedImage* dest)
 {
   if ((src->GetFormat() == PLANAR_YCBCR) && 
       (dest->type() == SharedImage::TYCbCrImage)) {
     PlanarYCbCrImage *planarYCbCrImage = static_cast<PlanarYCbCrImage*>(src);
-    const PlanarYCbCrImage::Data *data =planarYCbCrImage->GetData();
+    const PlanarYCbCrImage::Data *data = planarYCbCrImage->GetData();
     NS_ASSERTION(data, "Must be able to retrieve yuv data from image!");
     YCbCrImage& yuv = dest->get_YCbCrImage();
 
     ShmemYCbCrImage shmemImage(yuv.data(), yuv.offset());
 
-    for (int i = 0; i < data->mYSize.height; i++) {
-      memcpy(shmemImage.GetYData() + i * shmemImage.GetYStride(),
-             data->mYChannel + i * data->mYStride,
-             data->mYSize.width);
+    if (!shmemImage.CopyData(data->mYChannel, data->mCbChannel, data->mCrChannel,
+                             data->mYSize, data->mYStride,
+                             data->mCbCrSize, data->mCbCrStride)) {
+      NS_WARNING("Failed to copy image data!");
+      return false;
     }
-    for (int i = 0; i < data->mCbCrSize.height; i++) {
-      memcpy(shmemImage.GetCbData() + i * shmemImage.GetCbCrStride(),
-             data->mCbChannel + i * data->mCbCrStride,
-             data->mCbCrSize.width);
-      memcpy(shmemImage.GetCrData() + i * shmemImage.GetCbCrStride(),
-             data->mCrChannel + i * data->mCbCrStride,
-             data->mCbCrSize.width);
-    }
-
     return true;
   }
   return false; // TODO: support more image formats
 }
 
-SharedImage* ImageContainerChild::CreateSharedImageFromData(Image* image)
+SharedImage* ImageContainerChild::AllocateSharedImageFor(Image* image)
 {
   NS_ABORT_IF_FALSE(InImageBridgeChildThread(),
                   "Should be in ImageBridgeChild thread.");
-
   if (!image) {
     return nullptr;
   }
+
   if (image->GetFormat() == PLANAR_YCBCR ) {
     PlanarYCbCrImage *planarYCbCrImage = static_cast<PlanarYCbCrImage*>(image);
     const PlanarYCbCrImage::Data *data = planarYCbCrImage->GetData();
     NS_ASSERTION(data, "Must be able to retrieve yuv data from image!");
     if (!data) {
       return nullptr;
     }
 
@@ -176,48 +186,24 @@ SharedImage* ImageContainerChild::Create
       return nullptr;
     }
 
     ShmemYCbCrImage::InitializeBufferInfo(shmem.get<uint8_t>(),
                                           data->mYSize,
                                           data->mCbCrSize);
     ShmemYCbCrImage shmemImage(shmem);
 
-    if (!shmemImage.IsValid() || shmem.Size<uint8_t>() < size) {
+    if (!shmemImage.IsValid()) {
+      NS_WARNING("Failed to properly allocate image data!");
       DeallocShmem(shmem);
       return nullptr;
     }
 
-    for (int i = 0; i < data->mYSize.height; i++) {
-      memcpy(shmemImage.GetYData() + i * shmemImage.GetYStride(),
-             data->mYChannel + i * data->mYStride,
-             data->mYSize.width);
-    }
-    for (int i = 0; i < data->mCbCrSize.height; i++) {
-      memcpy(shmemImage.GetCbData() + i * shmemImage.GetCbCrStride(),
-             data->mCbChannel + i * data->mCbCrStride,
-             data->mCbCrSize.width);
-      memcpy(shmemImage.GetCrData() + i * shmemImage.GetCbCrStride(),
-             data->mCrChannel + i * data->mCbCrStride,
-             data->mCbCrSize.width);
-    }
-
     ++mActiveImageCount;
-    SharedImage* result = new SharedImage(YCbCrImage(shmem, 0, data->GetPictureRect()));
-    return result;
-#ifdef MOZ_WIDGET_GONK
-  } else if (image->GetFormat() == GONK_IO_SURFACE) {
-    GonkIOSurfaceImage* gonkImage = static_cast<GonkIOSurfaceImage*>(image);
-    SharedImage* result = new SharedImage(gonkImage->GetSurfaceDescriptor());
-    return result;
-  } else if (image->GetFormat() == GRALLOC_PLANAR_YCBCR) {
-    GrallocPlanarYCbCrImage* GrallocImage = static_cast<GrallocPlanarYCbCrImage*>(image);
-    SharedImage* result = new SharedImage(GrallocImage->GetSurfaceDescriptor());
-    return result;
-#endif
+    return new SharedImage(YCbCrImage(shmem, 0, data->GetPictureRect()));
   } else {
     NS_RUNTIMEABORT("TODO: Only YCbCrImage is supported here right now.");
   }
   return nullptr;
 }
 
 bool ImageContainerChild::AddSharedImageToPool(SharedImage* img)
 {
@@ -289,82 +275,81 @@ void ImageContainerChild::ClearSharedIma
   NS_ABORT_IF_FALSE(InImageBridgeChildThread(),
                     "Should be in ImageBridgeChild thread.");
   for(unsigned int i = 0; i < mSharedImagePool.Length(); ++i) {
     DeallocSharedImageData(this, *mSharedImagePool[i]);
   }
   mSharedImagePool.Clear();
 }
 
+void ImageContainerChild::SendImageNow(Image* aImage)
+{
+  NS_ABORT_IF_FALSE(InImageBridgeChildThread(),
+                    "Should be in ImageBridgeChild thread.");
+
+  if (mStop) {
+    return;
+  }
+
+  bool needsCopy = false;
+  // If the image can be converted to a shared image, no need to do a copy.
+  SharedImage* img = AsSharedImage(aImage);
+  if (!img) {
+    needsCopy = true;
+    // Try to get a compatible shared image from the pool
+    img = GetSharedImageFor(aImage);
+    if (!img && mActiveImageCount < (int)MAX_ACTIVE_SHARED_IMAGES) {
+      // If no shared image available, allocate a new one
+      img = AllocateSharedImageFor(aImage);
+    }
+  }
+
+  if (img && (!needsCopy || CopyDataIntoSharedImage(aImage, img))) {
+    // Keep a reference to the image we sent to compositor to maintain a
+    // correct reference count.
+    mImageQueue.AppendElement(aImage);
+    SendPublishImage(*img);
+  } else {
+    NS_WARNING("Failed to send an image to the compositor");
+  }
+  delete img;
+  return;
+}
+
 class ImageBridgeCopyAndSendTask : public Task
 {
 public:
   ImageBridgeCopyAndSendTask(ImageContainerChild * child, 
                              ImageContainer * aContainer, 
                              Image * aImage)
   : mChild(child), mImageContainer(aContainer), mImage(aImage) {}
 
   void Run()
   { 
-    SharedImage* img = mChild->ImageToSharedImage(mImage.get());
-    if (img) {
-      mChild->SendPublishImage(*img);
-    }
+    mChild->SendImageNow(mImage);
   }
 
-  ImageContainerChild *mChild;
+  ImageContainerChild* mChild;
   nsRefPtr<ImageContainer> mImageContainer;
   nsRefPtr<Image> mImage;
 };
 
-SharedImage* ImageContainerChild::ImageToSharedImage(Image* aImage)
-{
-  if (mStop) {
-    return nullptr;
-  }
-  if (mActiveImageCount > (int)MAX_ACTIVE_SHARED_IMAGES) {
-    // Too many active shared images, perhaps the compositor is hanging.
-    // Skipping current image
-    return nullptr;
-  }
-
-  NS_ABORT_IF_FALSE(InImageBridgeChildThread(),
-                    "Should be in ImageBridgeChild thread.");
-  SharedImage *img = GetSharedImageFor(aImage);
-  if (img) {
-    CopyDataIntoSharedImage(aImage, img);  
-  } else {
-    img = CreateSharedImageFromData(aImage);
-  }
-  // Keep a reference to the image we sent to compositor to maintain a
-  // correct reference count.
-  mImageQueue.AppendElement(aImage);
-  return img;
-}
-
 void ImageContainerChild::SendImageAsync(ImageContainer* aContainer,
                                          Image* aImage)
 {
   if(!aContainer || !aImage) {
       return;
   }
 
   if (mStop) {
     return;
   }
 
   if (InImageBridgeChildThread()) {
-    SharedImage *img = ImageToSharedImage(aImage);
-    if (img) {
-      SendPublishImage(*img);
-    } else {
-      NS_WARNING("Failed to create a shared image!");
-    }
-    delete img;
-    return;
+    SendImageNow(aImage);
   }
 
   // Sending images and (potentially) allocating shmems must be done 
   // on the ImageBridgeChild thread.
   Task *t = new ImageBridgeCopyAndSendTask(this, aContainer, aImage);   
   GetMessageLoop()->PostTask(FROM_HERE, t);
 }
 
--- a/gfx/layers/ipc/ImageContainerChild.h
+++ b/gfx/layers/ipc/ImageContainerChild.h
@@ -37,17 +37,17 @@ class ImageContainerChild : public PImag
 
 public:
   ImageContainerChild();
   virtual ~ImageContainerChild();
 
   /**
    * Sends an image to the compositor without using the main thread.
    *
-   * This method should be called by ImageContainer only, and can be called 
+   * This method should be called by ImageContainer only, and can be called
    * from any thread.
    */
   void SendImageAsync(ImageContainer* aContainer, Image* aImage);
 
   /**
    * Each ImageContainerChild is associated to an ID. This ID is used in the 
    * compositor side to fetch the images without having to keep direct references
    * between the ShadowImageLayer and the InmageBridgeParent.
@@ -153,46 +153,61 @@ protected:
    * deallocationg/reallocating too often.
    * This method is typically called when the a shared image is received from
    * the compositor.
    * 
    */
   bool AddSharedImageToPool(SharedImage* img);
 
   /**
-   * Removes a shared image from the pool and returns it.
-   * Returns nullptr if the pool is empty.
-   */
-  SharedImage* GetSharedImageFor(Image* aImage);
-  /**
-   * Seallocates all the shared images from the pool and clears the pool.
-   */
-
-  void ClearSharedImagePool();
-  /**
-   * Returns a shared image containing the same data as the image passed in 
-   * parameter.
+   * Must be called on the ImageBridgeChild's thread.
+   *
+   * creates and sends a shared image containing the same data as the image passed
+   * in parameter.
    * - If the image's data is already in shared memory, this should just acquire
-   * the data rather tahn copying it (TODO)
+   * the data rather tahn copying it
    * - If There is an available shared image of the same size in the pool, reuse
    * it rather than allocating shared memory.
    * - worst case, allocate shared memory and copy the image's data into it. 
    */
-  SharedImage* ImageToSharedImage(Image* toCopy);
+  void SendImageNow(Image* aImage);
+
+
+  /**
+   * Returns a SharedImage if the image passed in parameter can be shared
+   * directly without a copy, returns nullptr otherwise.
+   */
+  SharedImage* AsSharedImage(Image* aImage);
+
+  /**
+   * Removes a shared image from the pool and returns it.
+   * Returns nullptr if the pool is empty.
+   * The returned image does not contain the image data, a copy still needs to
+   * be done afterward (see CopyDataIntoSharedImage).
+   */
+  SharedImage* GetSharedImageFor(Image* aImage);
+
+  /**
+   * Allocates a SharedImage.
+   * Returns nullptr in case of failure.
+   * The returned image does not contain the image data, a copy still needs to
+   * be done afterward (see CopyDataIntoSharedImage).
+   */
+  SharedImage* AllocateSharedImageFor(Image* aImage);
 
   /**
    * Called by ImageToSharedImage.
    */
   bool CopyDataIntoSharedImage(Image* src, SharedImage* dest);
 
+
   /**
-   * Allocates a SharedImage and copy aImage's data into it.
-   * Called by ImageToSharedImage.
+   * Deallocates all the shared images from the pool and clears the pool.
    */
-  SharedImage * CreateSharedImageFromData(Image* aImage);
+  void ClearSharedImagePool();
 
 private:
   uint64_t mImageContainerID;
   nsTArray<SharedImage*> mSharedImagePool;
 
   /**
    * Save a reference to the outgoing images and remove the reference
    * once the image is returned from the compositor.
--- a/gfx/layers/ipc/ShmemYCbCrImage.cpp
+++ b/gfx/layers/ipc/ShmemYCbCrImage.cpp
@@ -145,10 +145,34 @@ bool ShmemYCbCrImage::IsValid()
   if (mShmem.Size<uint8_t>() < bufferInfoSize ||
       GetYCbCrBufferInfo(mShmem, mOffset)->mYOffset != bufferInfoSize ||
       mShmem.Size<uint8_t>() < mOffset + ComputeMinBufferSize(GetYSize(),GetCbCrSize())) {
     return false;
   }
   return true;
 }
 
+bool ShmemYCbCrImage::CopyData(uint8_t* aYData, uint8_t* aCbData, uint8_t* aCrData,
+                               gfxIntSize aYSize, uint32_t aYStride,
+                               gfxIntSize aCbCrSize, uint32_t aCbCrStride)
+{
+  if (!IsValid() || GetYSize() != aYSize || GetCbCrSize() != aCbCrSize) {
+    return false;
+  }
+  for (int i = 0; i < aYSize.height; i++) {
+    memcpy(GetYData() + i * GetYStride(),
+           aYData + i * aYStride,
+           aYSize.width);
+  }
+  for (int i = 0; i < aCbCrSize.height; i++) {
+    memcpy(GetCbData() + i * GetCbCrStride(),
+           aCbData + i * aCbCrStride,
+           aCbCrSize.width);
+    memcpy(GetCrData() + i * GetCbCrStride(),
+           aCrData + i * aCbCrStride,
+           aCbCrSize.width);
+  }
+  return true;
+}
+
+
 } // namespace
 } // namespace
--- a/gfx/layers/ipc/ShmemYCbCrImage.h
+++ b/gfx/layers/ipc/ShmemYCbCrImage.h
@@ -88,16 +88,23 @@ public:
    */
   gfxIntSize GetYSize();
 
   /**
    * Returns the dimensions of the Cb and Cr Channel.
    */
   gfxIntSize GetCbCrSize();
 
+  /**
+   * Copies the data passed in parameter into the shmem.
+   */
+  bool CopyData(uint8_t* aYData, uint8_t* aCbData, uint8_t* aCrData,
+                gfxIntSize aYSize, uint32_t aYStride,
+                gfxIntSize aCbCrSize, uint32_t aCbCrStride);
+
 private:
   bool Open(Shmem& aShmem, size_t aOffset = 0);
 
   ipc::Shmem mShmem;
   size_t mOffset;
 };
 
 } // namespace