[media] Enable VDAs to request read lock fences, use in VTVDA.

VTVDA binds IOSurfaces to textures, and therefore it is possible for
a GL operation on the IOSurface to still be queued (at the platform
level) after all references to the IOSurface have been dropped.

This CL enables VDAs to request the READ_LOCK_FENCES_ENABLED flag on
their VideoFrames, which ensures that platform operations have
completed before references are dropped.

Bug: 934578
Change-Id: Ia369c66d06100f58f991494b354d00dc628b38c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1487698Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637946}

media/filters/gpu_video_decoder.cc

@@ -687,6 +687,10 @@ void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
   frame->set_color_space(picture.color_space());
   if (picture.allow_overlay())
     frame->metadata()->SetBoolean(VideoFrameMetadata::ALLOW_OVERLAY, true);
+  if (picture.read_lock_fences_enabled()) {
+    frame->metadata()->SetBoolean(VideoFrameMetadata::READ_LOCK_FENCES_ENABLED,
+                                  true);
+  }
   if (picture.texture_owner())
     frame->metadata()->SetBoolean(VideoFrameMetadata::TEXTURE_OWNER, true);
   if (picture.wants_promotion_hint()) {

media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc

@@ -262,6 +262,7 @@ void GpuVideoDecodeAcceleratorHost::OnPictureReady(
   Picture picture(params.picture_buffer_id, params.bitstream_buffer_id,
                   params.visible_rect, params.color_space,
                   params.allow_overlay);
+  picture.set_read_lock_fences_enabled(params.read_lock_fences_enabled);
   picture.set_size_changed(params.size_changed);
   picture.set_texture_owner(params.surface_texture);
   picture.set_wants_promotion_hint(params.wants_promotion_hint);

media/gpu/ipc/common/media_messages.h

@@ -27,6 +27,7 @@ IPC_STRUCT_BEGIN(AcceleratedVideoDecoderHostMsg_PictureReady_Params)
   IPC_STRUCT_MEMBER(gfx::Rect, visible_rect)
   IPC_STRUCT_MEMBER(gfx::ColorSpace, color_space)
   IPC_STRUCT_MEMBER(bool, allow_overlay)
+  IPC_STRUCT_MEMBER(bool, read_lock_fences_enabled)
   IPC_STRUCT_MEMBER(bool, size_changed)
   IPC_STRUCT_MEMBER(bool, surface_texture)
   IPC_STRUCT_MEMBER(bool, wants_promotion_hint)

media/gpu/ipc/service/gpu_video_decode_accelerator.cc

@@ -288,6 +288,7 @@ void GpuVideoDecodeAccelerator::PictureReady(const Picture& picture) {
   params.visible_rect = picture.visible_rect();
   params.color_space = picture.color_space();
   params.allow_overlay = picture.allow_overlay();
+  params.read_lock_fences_enabled = picture.read_lock_fences_enabled();
   params.size_changed = picture.size_changed();
   params.surface_texture = picture.texture_owner();
   params.wants_promotion_hint = picture.wants_promotion_hint();

media/gpu/ipc/service/picture_buffer_manager.cc

@@ -220,6 +220,10 @@ class PictureBufferManagerImpl : public PictureBufferManager {
 
     if (picture.allow_overlay())
       frame->metadata()->SetBoolean(VideoFrameMetadata::ALLOW_OVERLAY, true);
+    if (picture.read_lock_fences_enabled()) {
+      frame->metadata()->SetBoolean(
+          VideoFrameMetadata::READ_LOCK_FENCES_ENABLED, true);
+    }
 
     // TODO(sandersd): Provide an API for VDAs to control this.
     frame->metadata()->SetBoolean(VideoFrameMetadata::POWER_EFFICIENT, true);

media/gpu/vt_video_decode_accelerator_mac.cc

@@ -1344,9 +1344,21 @@ bool VTVideoDecodeAccelerator::SendFrame(const Frame& frame) {
 
   DVLOG(3) << "PictureReady(picture_id=" << picture_id << ", "
            << "bitstream_id=" << frame.bitstream_id << ")";
-  client_->PictureReady(Picture(picture_id, frame.bitstream_id,
-                                gfx::Rect(frame.image_size), color_space,
-                                true));
+  Picture picture(picture_id, frame.bitstream_id, gfx::Rect(frame.image_size),
+                  color_space, true);
+  // The GLImageIOSurface keeps the IOSurface alive as long as it exists, but
+  // bound textures do not, and they can outlive the GLImageIOSurface if they
+  // are deleted in the command buffer before they are used by the platform GL
+  // implementation. (https://crbug.com/930479#c69)
+  //
+  // A fence is required whenever a GLImage is bound, but we can't know in
+  // advance whether that will happen.
+  //
+  // TODO(sandersd): Can GLImageIOSurface be responsible for fences, so that
+  // we don't need to use them when the image is never bound? Bindings are
+  // typically only created when WebGL is in use.
+  picture.set_read_lock_fences_enabled(true);
+  client_->PictureReady(std::move(picture));
   return true;
 }
 

media/video/picture.cc

@@ -74,6 +74,7 @@ Picture::Picture(int32_t picture_buffer_id,
       visible_rect_(visible_rect),
       color_space_(color_space),
       allow_overlay_(allow_overlay),
+      read_lock_fences_enabled_(false),
       size_changed_(false),
       texture_owner_(false),
       wants_promotion_hint_(false) {}

media/video/picture.h

@@ -94,10 +94,6 @@ class MEDIA_EXPORT Picture {
   // Returns the id of the bitstream buffer from which this frame was decoded.
   int32_t bitstream_buffer_id() const { return bitstream_buffer_id_; }
 
-  void set_bitstream_buffer_id(int32_t bitstream_buffer_id) {
-    bitstream_buffer_id_ = bitstream_buffer_id;
-  }
-
   // Returns the color space of the picture.
   const gfx::ColorSpace& color_space() const { return color_space_; }
 
@@ -108,6 +104,12 @@ class MEDIA_EXPORT Picture {
 
   bool allow_overlay() const { return allow_overlay_; }
 
+  bool read_lock_fences_enabled() const { return read_lock_fences_enabled_; }
+
+  void set_read_lock_fences_enabled(bool read_lock_fences_enabled) {
+    read_lock_fences_enabled_ = read_lock_fences_enabled;
+  }
+
   // Returns true when the VDA has adjusted the resolution of this Picture
   // without requesting new PictureBuffers. GpuVideoDecoder should read this
   // as a signal to update the size of the corresponding PicutreBuffer using
@@ -132,6 +134,7 @@ class MEDIA_EXPORT Picture {
   gfx::Rect visible_rect_;
   gfx::ColorSpace color_space_;
   bool allow_overlay_;
+  bool read_lock_fences_enabled_;
   bool size_changed_;
   bool texture_owner_;
   bool wants_promotion_hint_;