Auto merge of #131 - servo:planar-cleanup, r=philn

Ensure video frames are stored in Rust allocated memory

To resolve the problems in issue #112 and to remove the potential use-after-free which can happen if GeckoMedia shuts down and deallocates video frames before the compositor is finished painting video frames, switch to having the video frames stored in Rust allocated Vec<u8>s which are ref counted. This means we won't deallocate the memory until the last Rust or C++ user of the data is destroyed.

examples/src/bin/test-player.rs

@@ -272,7 +272,7 @@ impl webrender::ExternalImageHandler for ImageGenerator {
             source: webrender::ExternalImageSource::RawData(
                 self.current_image
                     .as_ref()
-                    .map(|p| p.pixel_data(channel_index))
+                    .map(|img| &img.plane_for_channel(channel_index).pixels)
                     .unwrap(),
             ),
         }
@@ -320,7 +320,7 @@ impl App {
         api: &RenderApi,
         resources: &mut ResourceUpdates,
         image_key: Option<ImageKey>,
-        plane: Plane,
+        plane: &Plane,
         channel_index: u8,
     ) -> Option<ImageKey> {
         match image_key {
@@ -452,13 +452,13 @@ impl ui::Example for App {
         }
 
         let time_now = TimeStamp(time::precise_time_ns());
-        while self.frame_queue.len() > 1 && self.frame_queue[0].time_stamp > time_now {
+        while self.frame_queue.len() > 1 && self.frame_queue[0].timestamp() > time_now {
             self.frame_queue.remove(0);
         }
 
         if let Some(first_frame) = self.frame_queue.first() {
-            if self.last_frame_id != first_frame.frame_id {
-                self.last_frame_id = first_frame.frame_id;
+            if self.last_frame_id != first_frame.frame_id() {
+                self.last_frame_id = first_frame.frame_id();
                 self.current_frame_sender.as_ref().map(|sender| {
                     sender.send(first_frame.clone()).unwrap();
                 });
@@ -513,7 +513,10 @@ impl ui::Example for App {
             // intrinsic size, we'll just use the frame size.
             let (width, height) = match self.video_dimensions() {
                 Some((width, height)) => (width, height),
-                None => (frame.picture.width, frame.picture.height),
+                None => (
+                    frame.picture_region().width as i32,
+                    frame.picture_region().height as i32,
+                ),
             };
 
             // Resize so that the video is rendered as wide as the window.

gecko-media/gecko/glue/GeckoMediaDecoderOwner.cpp

@@ -5,20 +5,21 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "GeckoMediaDecoderOwner.h"
-#include "mozilla/AbstractThread.h"
-#include "VideoFrameContainer.h"
 #include "ImageContainer.h"
+#include "VideoFrameContainer.h"
+#include "mozilla/AbstractThread.h"
 
 namespace mozilla {
 
-GeckoMediaDecoderOwner::GeckoMediaDecoderOwner(PlayerCallbackObject aCallback)
+GeckoMediaDecoderOwner::GeckoMediaDecoderOwner(PlayerCallbackObject aCallback,
+                                               FrameAllocatorObject aAllocator)
   : mCallback(aCallback)
+  , mAllocator(aAllocator)
 {
 }
 
 GeckoMediaDecoderOwner::GeckoMediaDecoderOwner()
 {
-
 }
 
 void
@@ -213,9 +214,9 @@ VideoFrameContainer*
 GeckoMediaDecoderOwner::GetVideoFrameContainer()
 {
   RefPtr<layers::ImageContainer> container =
-    new layers::ImageContainer(this);
-  mVideoFrameContainer =
-    new VideoFrameContainer(this, container.forget());
+    new layers::ImageContainer(this, mAllocator);
+  mAllocator = { 0 };
+  mVideoFrameContainer = new VideoFrameContainer(this, container.forget());
   return mVideoFrameContainer;
 }
 
@@ -246,10 +247,12 @@ GeckoMediaDecoderOwner::SetDecoder(GeckoMediaDecoder* aDecoder)
 }
 
 void
-GeckoMediaDecoderOwner::UpdateCurrentImages(nsTArray<GeckoPlanarYCbCrImage> aImages)
+GeckoMediaDecoderOwner::UpdateCurrentImages(
+  nsTArray<GeckoPlanarYCbCrImage> aImages)
 {
   if (mCallback.mContext && mCallback.mUpdateCurrentImages) {
-    (*mCallback.mUpdateCurrentImages)(mCallback.mContext, aImages.Length(), aImages.Elements());
+    (*mCallback.mUpdateCurrentImages)(
+      mCallback.mContext, aImages.Length(), aImages.Elements());
   }
 }
 
@@ -267,7 +270,7 @@ GeckoMediaDecoderOwner::NotifyBuffered() const
       i++;
     }
     (*mCallback.mNotifyBuffered)(mCallback.mContext, size, ranges);
-    delete [] ranges;
+    delete[] ranges;
   }
 }
 
@@ -285,30 +288,22 @@ GeckoMediaDecoderOwner::NotifySeekable() const
       i++;
     }
     (*mCallback.mNotifySeekable)(mCallback.mContext, size, ranges);
-    delete [] ranges;
+    delete[] ranges;
   }
 }
 
 void
 GeckoMediaDecoderOwner::Shutdown()
 {
   if (mVideoFrameContainer) {
-    // The ImageContainer keeps a list of the images that it sends out to Rust,
-    // so that if we shutdown, we can deallocate and neuter the images
-    // proactively. If we don't do this, we can end up with crashes if Rust
-    // code on another thread tries to use images after we've shutdown.
-    auto imageContainer = mVideoFrameContainer->GetImageContainer();
-    if (imageContainer) {
-      imageContainer->DeallocateExportedImages();
-    }
     mVideoFrameContainer->ForgetElement();
     mVideoFrameContainer = nullptr;
   }
   mDecoder = nullptr;
   if (mCallback.mContext && mCallback.mFree) {
     (*mCallback.mFree)(mCallback.mContext);
   }
-  mCallback = {0};
+  mCallback = { 0 };
 }
 
 } // namespace mozilla