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

cc/resources/video_resource_updater.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/video_resource_updater.h"
 
 #include "base/bind.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/resources/resource_provider.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "media/base/video_frame.h"
 #include "media/filters/skcanvas_video_renderer.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 #include "ui/gfx/size_conversions.h"
 
 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() {}
 
 VideoResourceUpdater::VideoResourceUpdater(ContextProvider* context_provider,
                                            ResourceProvider* resource_provider)
     : context_provider_(context_provider),
       resource_provider_(resource_provider) {
 }
 
@@ skipping 243 matching lines @@
     external_resources.mailboxes.push_back(
         TextureMailbox(plane_resources[i].mailbox, GL_TEXTURE_2D, 0));
     external_resources.release_callbacks.push_back(
         base::Bind(&RecycleResource, AsWeakPtr(), recycle_data));
   }
 
   external_resources.type = VideoFrameExternalResources::YUV_RESOURCE;
   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(
     const scoped_refptr<media::VideoFrame>& video_frame) {
   media::VideoFrame::Format frame_format = video_frame->format();
 
   DCHECK_EQ(frame_format, media::VideoFrame::NATIVE_TEXTURE);
   if (frame_format != media::VideoFrame::NATIVE_TEXTURE)
       return VideoFrameExternalResources();
 
@@ skipping 15 matching lines @@
       break;
     default:
       NOTREACHED();
       return VideoFrameExternalResources();
   }
 
   external_resources.mailboxes.push_back(
       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;
 }
 
 // static
 void VideoResourceUpdater::RecycleResource(
     base::WeakPtr<VideoResourceUpdater> updater,
     RecycleResourceData data,
     uint32 sync_point,
     bool lost_resource) {
   if (!updater.get()) {
@@ skipping 21 matching lines @@
   }
 
   PlaneResource recycled_resource(data.resource_id,
                                   data.resource_size,
                                   data.resource_format,
                                   data.mailbox);
   updater->recycled_resources_.push_back(recycled_resource);
 }
 
 }  // namespace cc