Prime the landing pad for the new video rendering pipeline.

cc/layers/video_frame_provider.h

Index: cc/layers/video_frame_provider.h
diff --git a/cc/layers/video_frame_provider.h b/cc/layers/video_frame_provider.h
index 45e6c4110c5364ec5deb36e2e1a7504fcbbf007d..9b4d89bc4f52704f8ca32def2afa3328b4b0e390 100644
--- a/cc/layers/video_frame_provider.h
+++ b/cc/layers/video_frame_provider.h
@@ -6,6 +6,7 @@
 #define CC_LAYERS_VIDEO_FRAME_PROVIDER_H_
 
 #include "base/memory/ref_counted.h"
+#include "base/time/time.h"
 #include "cc/base/cc_export.h"
 
 namespace media {
@@ -14,27 +15,35 @@ class VideoFrame;
 
 namespace cc {
 
-// Threading notes: This class may be used in a multi threaded manner.
-// Specifically, the implementation may call GetCurrentFrame() or
-// PutCurrentFrame() from the compositor thread. If so, the caller is
-// responsible for making sure Client::DidReceiveFrame() and
-// Client::DidUpdateMatrix() are only called from this same thread.
+// VideoFrameProvider and VideoFrameProvider::Client define the relationship by
+// which video frames are exchanged between a provider and client.
+//
+// Threading notes: This class may be used in a multithreaded manner. However,
+// if the Client implementation calls GetCurrentFrame()/PutCurrentFrame() from
+// one thread, the provider must ensure that all client methods are called from
+// that thread (typically the compositor thread).
 class CC_EXPORT VideoFrameProvider {
  public:
-  virtual ~VideoFrameProvider() {}
-
   class CC_EXPORT Client {
    public:
-    // Provider will call this method to tell the client to stop using it.
+    // The provider will call this method to tell the client to stop using it.
     // StopUsingProvider() may be called from any thread. The client should

[xhwang, 2015/04/13 20:50:29]

This is inconsistent with the previous comment that:

"if the Client implementation calls GetCurrentFrame()/PutCurrentFrame() from one
thread, the provider must ensure that all client methods are called from that
thread (typically the compositor thread)".

[DaleCurtis, 2015/04/13 23:15:06]

Looks like the current comment is correct, StopUsingProvider() is called from
multiple threads in the MediaStream implementation. I've updated the class level
comment to exclude StopUsingProvider().

     // block until it has PutCurrentFrame() any outstanding frames.
     virtual void StopUsingProvider() = 0;
 
+    // Notifies the client that it should start or stop making regular Render()
+    // calls to the provider. No further calls to Render() will be made after
+    // StopRendering() returns. Clients should handle redundant calls.

[xhwang, 2015/04/13 20:50:28]

Maybe add an example to explain why these are needed, e.g. during Pause(),
nothing needs to be rendered...

[DaleCurtis, 2015/04/13 23:15:05]

Done.

+    virtual void StartRendering() = 0;
+    virtual void StopRendering() = 0;
+
     // Notifies the provider's client that a call to GetCurrentFrame() will

[xhwang, 2015/04/13 20:50:29]

nit: s/provider's//?

[DaleCurtis, 2015/04/13 23:15:05]

Done.

     // return new data.
+    // TODO(dalecurtis): Nuke this once VideoFrameProviderClientImpl is using a
+    // BeginFrameObserver based approach. http://crbug.com/336733

[xhwang, 2015/04/13 20:50:29]

What's the relationship b/w this and the return value of UpdateCurrentFrame()
below?

[DaleCurtis, 2015/04/13 23:15:06]

Nothing, it's only for the old path. UpdateCurrentFrame() will not be called for
the old path; instead the VideoFrameProvider will call DidReceiveFrame() and the
client will call GetCurrentFrame() (as it does today).

     virtual void DidReceiveFrame() = 0;
 
-    // Notifies the provider's client of a new UV transform matrix to be used.
+    // Notifies the client of a new UV transform matrix to be used.
     virtual void DidUpdateMatrix(const float* matrix) = 0;
 
    protected:
@@ -45,18 +54,23 @@ class CC_EXPORT VideoFrameProvider {
   // that the provider is not destroyed before this call returns.
   virtual void SetVideoFrameProviderClient(Client* client) = 0;
 
-  // This function places a lock on the current frame and returns a pointer to
-  // it. Calls to this method should always be followed with a call to
-  // PutCurrentFrame().
-  // Only the current provider client should call this function.
+  // Called by the client on a regular interval. Returns true if a new frame
+  // will be available via GetCurrentFrame() which should be displayed within
+  // the presentation interval [|deadline_min|, |deadline_max|].

[xhwang, 2015/04/13 20:50:28]

nit to double check: Are you sure it's [] instead of (] or [)?

[DaleCurtis, 2015/04/13 23:15:06]

Double checked with Sunny, it's [].

[xhwang, 2015/04/14 00:29:52]

Okay, but a bit surprised... I assume cc code will make sure there's no
overlaps...

+  virtual bool UpdateCurrentFrame(base::TimeTicks deadline_min,
+                                  base::TimeTicks deadline_max) = 0;
+
+  // Returns the last frame that was acquired during UpdateCurrentFrame(). A

[xhwang, 2015/04/13 20:50:29]

nit: "acquired" is not mentioned in the documentation for UpdateCurrentFrame().

[DaleCurtis, 2015/04/13 23:15:05]

I've reworded the text to refer to calling UpdateCurrentFrame().

+  // call to this method does not ensure that the frame will be rendered. A
+  // subsequent call to PutCurrentFrame() will be made if the frame is rendered.
   virtual scoped_refptr<media::VideoFrame> GetCurrentFrame() = 0;
 
-  // This function releases the lock on the video frame. It should always be
-  // called after GetCurrentFrame(). Frames passed into this method
-  // should no longer be referenced after the call is made. Only the current
-  // provider client should call this function.
-  virtual void PutCurrentFrame(
-      const scoped_refptr<media::VideoFrame>& frame) = 0;
+  // Indicates that the frame returned via GetCurrentFrame() was rendered. Must
+  // only occur after a previous call to GetCurrentFrame().
+  virtual void PutCurrentFrame() = 0;

[xhwang, 2015/04/13 20:50:29]

nit: This naming is really confusing. It probably should be something like
OnFrameRendered(). This can be addressed later though.

[DaleCurtis, 2015/04/13 23:15:06]

Unfortunately it doesn't necessary mean rendered either since the compositor may
drop the buffer on the way to the gpu process. I agree it's a bit confusing
though; for this patch lets just keep the current name.

[xhwang, 2015/04/14 00:29:52]

Acknowledged.

+
+ protected:
+  virtual ~VideoFrameProvider() {}
 };
 
 }  // namespace cc