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 SyncPointClientImpl : public media::VideoFrame::SyncPointClient {
+ public:
+  explicit SyncPointClientImpl(gpu::gles2::GLES2Interface* gl) : gl_(gl) {}
+  virtual ~SyncPointClientImpl() {}
+  virtual uint32 InsertSyncPoint() OVERRIDE {
+    return GLC(gl_, gl_->InsertSyncPointCHROMIUM());
+  }
+  virtual void WaitSyncPoint(uint32 sync_point) OVERRIDE {
+    GLC(gl_, gl_->WaitSyncPointCHROMIUM(sync_point));
+  }
+
+ private:
+  gpu::gles2::GLES2Interface* gl_;
+};
+
+}  // 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,
+static void ReturnTexture(const scoped_refptr<media::VideoFrame>& video_frame,
+                          base::WeakPtr<ContextProvider> context_provider,

[danakj, 2014/07/10 17:09:15]

Why not WeakPtr to the VideoResourceUpdater, and pull the context_provider off
of there? See the RecycleTexture method.

Also SupportsWeakPtr is a very heavy hammer, for a DCHECK.

[dshwang, 2014/07/10 19:23:11]

I was worried the last frame glitch before video_layer_impl dies.
ContextProvider lives nearly until render process dies, but VideoResourceUpdater
dies when video_layer_impl dies.

For example,
Render process                       Browser process
----------------------------------------------------------
video_layer_impl issue drawQuad
video_layer_impl dies                Draw video drawQuad
                                     Reclaim the resource
ReturnTexture() is skipped 
due to WeakPtr<VideoResourceUpdater>

WDYT? Can we ignore the last frame glitch?

[danakj, 2014/07/10 20:00:48]

We could pass on lost_resource=true to the videoframe in that case to say that
we don't have a valid sync point to give out. So, TODO?

+                          uint32 /* sync_point */,

[danakj, 2014/07/10 17:09:15]

we don't comment out parameter names like this in chrome

                           bool lost_resource) {
-  frame->AppendReleaseSyncPoint(sync_point);
+  DCHECK(context_provider.get() || lost_resource);
+  if (lost_resource)
+    return;

[danakj, 2014/07/10 17:09:15]

can you TODO that this lost_resource should be forwarded to the decoder?

[dshwang, 2014/07/10 19:23:11]

Yes, I'll update

+  // VideoFrame::UpdateReleaseSyncPoint() creates new sync point using the same
+  // GL context which created the given |sync_point|, so discard the
+  // |sync_point|.
+  SyncPointClientImpl client(context_provider->ContextGL());
+  video_frame->UpdateReleaseSyncPoint(&client);
 }
 
 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 16 matching lines @@
     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));
+      base::Bind(&ReturnTexture, video_frame, context_provider_->AsWeakPtr()));
   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