Android media: VideoFrame should not store so many sync points.

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 1d93aebe3cce20f763da7349490d0f2a3f0d177b..d9b8d0b16d8909d9fd13d082e0b0200ac963ef8f 100644
--- a/cc/resources/video_resource_updater.cc
+++ b/cc/resources/video_resource_updater.cc
@@ -17,9 +17,35 @@
 
 namespace cc {
 
+namespace {
+
 const ResourceFormat kYUVResourceFormat = LUMINANCE_8;
 const ResourceFormat kRGBResourceFormat = RGBA_8888;
 
+class SyncPointProviderImpl : public media::VideoFrame::SyncPointProvider {
+ public:
+  explicit SyncPointProviderImpl(gpu::gles2::GLES2Interface* gl) : gl_(gl) {}
+  virtual ~SyncPointProviderImpl();
+  virtual uint32 InsertSyncPoint();
+  virtual void WaitSyncPoint(uint32 sync_point);
+
+ private:
+  gpu::gles2::GLES2Interface* gl_;
+};
+
+SyncPointProviderImpl::~SyncPointProviderImpl() {
+}
+
+uint32 SyncPointProviderImpl::InsertSyncPoint() {
+  return GLC(gl_, gl_->InsertSyncPointCHROMIUM());
+}
+
+void SyncPointProviderImpl::WaitSyncPoint(uint32 sync_point) {
+  GLC(gl_, gl_->WaitSyncPointCHROMIUM(sync_point));
+}
+
+}  // namespace
+
 VideoFrameExternalResources::VideoFrameExternalResources() : type(NONE) {}
 
 VideoFrameExternalResources::~VideoFrameExternalResources() {}
@@ -283,10 +309,14 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
   return external_resources;
 }
 
-static void ReturnTexture(const scoped_refptr<media::VideoFrame>& frame,
-                          uint32 sync_point,
-                          bool lost_resource) {
-  frame->AppendReleaseSyncPoint(sync_point);
+static void ReturnTexture(
+    const scoped_refptr<media::VideoFrame>& video_frame,
+    const scoped_refptr<ContextProvider>& context_provider,

[danakj, 2014/06/19 15:56:26]

Can we pass a WeakPtr to the VRUpdater instead so we don't have to extend the
lifetime of the ContextProvider? And if the WeakPtr is invalid, we report that
the texture is lost?

[dshwang, 2014/06/23 18:33:20]

Yes, I use WeakPtr in new patch set.

+    uint32 sync_point,
+    bool lost_resource) {
+  SyncPointProviderImpl provider(context_provider->ContextGL());
+  provider.WaitSyncPoint(sync_point);

[danakj, 2014/06/19 15:56:26]

why waiting on the sync point here? this same context inserted the sync point in
the first place

+  video_frame->UpdateReleaseSyncPoint(provider);
 }
 
 VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
@@ -322,8 +352,8 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
       TextureMailbox(mailbox_holder->mailbox,
                      mailbox_holder->texture_target,
                      mailbox_holder->sync_point));
-  external_resources.release_callbacks.push_back(
-      base::Bind(&ReturnTexture, video_frame));
+  external_resources.release_callbacks.push_back(base::Bind(
+      &ReturnTexture, video_frame, make_scoped_refptr(context_provider_)));

[danakj, 2014/06/19 15:56:26]

make_scoped_refptr() is not needed

   return external_resources;
 }