HW Video: Make media::VideoFrame handle the sync point of the compositor as well as webgl

cc/resources/video_resource_updater.cc

Index: cc/resources/video_resource_updater.cc
diff --git a/cc/resources/video_resource_updater.cc b/cc/resources/video_resource_updater.cc
index b170be6784b2a443978d6d2fcc94782fb7a4fe2d..50afdd2ef18446153460b20d9fd1afa39cb04379 100644
--- a/cc/resources/video_resource_updater.cc
+++ b/cc/resources/video_resource_updater.cc
@@ -310,10 +310,20 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
   return external_resources;
 }
 
-static void ReturnTexture(const scoped_refptr<media::VideoFrame>& frame,
-                          uint32 sync_point,
-                          bool lost_resource) {
-  frame->mailbox_holder()->sync_point = sync_point;
+// static
+void VideoResourceUpdater::ReturnTexture(
+    base::WeakPtr<VideoResourceUpdater> updater,
+    const scoped_refptr<media::VideoFrame>& frame,
+    uint32 sync_point,
+    bool lost_resource) {
+  uint32 previous_sync_point = frame->SwapReleaseSyncPoint(sync_point);
+  // TODO(dshwang): |updater| can be deleted while ResourceProvider keeps
+  // this mailbox. If below code causes critical issue, ResourceProvider

[danakj, 2014/02/28 21:47:03]

This is kinda ugly.. and since cc is giving the resource back it doesn't have a
good reason to wait on the sync point other than making sure its sync point gets
heard. I do like the idea of holding multiple sync points on the video frame
more than inserting buggy weakptr code here.

[dshwang, 2014/02/28 21:54:17]

Yeah, I agree. your idea is better. I'll change VideoFrame can keep multiple
sync points. It makes client code clean also.

+  // can provide ContextProvider* like |sync_point|.
+  if (updater && previous_sync_point) {
+    gpu::gles2::GLES2Interface* gl = updater->context_provider_->ContextGL();
+    GLC(gl, gl->WaitSyncPointCHROMIUM(previous_sync_point));

[danakj, 2014/02/28 21:47:03]

This is inverted, you'd need to wait on the existing sync point before inserting
a new one.

stuff1
insert1

stuff2
insert2  <- waiting on this should guarantee stuff1 and stuff2 complete

<- someone else reads the sync point and does wait2, it expects stuff1 and
stuff2 to be done but it's not ->

wait1 <- now stuff1 is done.

[dshwang, 2014/02/28 21:54:17]

Thank you for explanation. After VideoFrame keep multiple release sync points,
this code will be removed.
I'll take care of release sync point ordering as your explanation :)

Time to sleep in Europe. have a nice weekend.

+  }
 }
 
 VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
@@ -350,7 +360,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
                      mailbox_holder->texture_target,
                      mailbox_holder->sync_point));
   external_resources.release_callbacks.push_back(
-      base::Bind(&ReturnTexture, video_frame));
+      base::Bind(&ReturnTexture, AsWeakPtr(), video_frame));
   return external_resources;
 }