Replace VideoRendererBase Get/PutCurrentFrame() with a VideoFrame-containing callback.

media/filters/video_renderer_base.h

Index: media/filters/video_renderer_base.h
diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h
index cb44d1b9a459640b4f620919b508be9cca199579..ed0177bbbb019ea63b7b9e9e414d631607023728 100644
--- a/media/filters/video_renderer_base.h
+++ b/media/filters/video_renderer_base.h
@@ -35,13 +35,14 @@ class MEDIA_EXPORT VideoRendererBase
     : public VideoRenderer,
       public base::PlatformThread::Delegate {
  public:
+  typedef base::Callback<void(const scoped_refptr<VideoFrame>&)> PaintCB;
   typedef base::Callback<void(bool)> SetOpaqueCB;
 
   // Maximum duration of the last frame.
   static base::TimeDelta kMaxLastFrameDuration();
 
   // |paint_cb| is executed on the video frame timing thread whenever a new
-  // frame is available for painting via GetCurrentFrame().
+  // frame is available for painting.
   //
   // |set_opaque_cb| is executed when the renderer is initialized to inform
   // the player whether the decoder's output will be opaque or not.
@@ -56,7 +57,7 @@ class MEDIA_EXPORT VideoRendererBase
   // Get/PutCurrentFrame() http://crbug.com/108435
   VideoRendererBase(const scoped_refptr<base::MessageLoopProxy>& message_loop,
                     const SetDecryptorReadyCB& set_decryptor_ready_cb,
-                    const base::Closure& paint_cb,
+                    const PaintCB& paint_cb,
                     const SetOpaqueCB& set_opaque_cb,
                     bool drop_frames);
 
@@ -82,15 +83,6 @@ class MEDIA_EXPORT VideoRendererBase
   // PlatformThread::Delegate implementation.
   virtual void ThreadMain() OVERRIDE;
 
-  // Clients of this class (painter/compositor) should use GetCurrentFrame()
-  // obtain ownership of VideoFrame, it should always relinquish the ownership
-  // by use PutCurrentFrame(). Current frame is not guaranteed to be non-NULL.
-  // It expects clients to use color-fill the background if current frame
-  // is NULL. This could happen before pipeline is pre-rolled or during
-  // pause/flush/preroll.
-  void GetCurrentFrame(scoped_refptr<VideoFrame>* frame_out);
-  void PutCurrentFrame(scoped_refptr<VideoFrame> frame);
-
  protected:
   virtual ~VideoRendererBase();
 
@@ -124,7 +116,7 @@ class MEDIA_EXPORT VideoRendererBase
   // Attempts to complete flushing and transition into the flushed state.
   void AttemptFlush_Locked();
 
-  // Calculates the duration to sleep for based on |current_frame_|'s timestamp,
+  // Calculates the duration to sleep for based on |last_timestamp_|,
   // the next frame timestamp (may be NULL), and the provided playback rate.
   //
   // We don't use |playback_rate_| to avoid locking.
@@ -138,9 +130,17 @@ class MEDIA_EXPORT VideoRendererBase
   // Return the number of frames currently held by this class.
   int NumFrames_Locked() const;
 
-  // Updates |current_frame_| to the next frame on |ready_frames_| and calls
-  // |size_changed_cb_| if the natural size changes.
-  void SetCurrentFrameToNextReadyFrame();
+  // Runs |paint_cb_| with the next frame from |ready_frames_|, updating
+  // |last_natural_size_| and running |size_changed_cb_| if the natural size
+  // changes.
+  //
+  // A read is scheduled to replace the frame.
+  void PaintNextReadyFrame_Locked();
+
+  // Drops the next frame from |ready_frames_| and runs |statistics_cb_|.
+  //
+  // A read is scheduled to replace the frame.
+  void DropNextReadyFrame_Locked();
 
   void ResetDecoder();
   void StopDecoder(const base::Closure& callback);
@@ -168,24 +168,10 @@ class MEDIA_EXPORT VideoRendererBase
   scoped_refptr<VideoDecoder> decoder_;
   scoped_refptr<DecryptingDemuxerStream> decrypting_demuxer_stream_;
 
-  // Queue of incoming frames as well as the current frame since the last time
-  // OnFrameAvailable() was called.
+  // Queue of incoming frames yet to be painted.
   typedef std::deque<scoped_refptr<VideoFrame> > VideoFrameQueue;
   VideoFrameQueue ready_frames_;
 
-  // The current frame available to subclasses for rendering via
-  // GetCurrentFrame().  |current_frame_| can only be altered when
-  // |pending_paint_| is false.
-  scoped_refptr<VideoFrame> current_frame_;
-
-  // The previous |current_frame_| and is returned via GetCurrentFrame() in the
-  // situation where all frames were deallocated (i.e., during a flush).
-  //
-  // TODO(scherkus): remove this after getting rid of Get/PutCurrentFrame() in
-  // favour of passing ownership of the current frame to the renderer via
-  // callback.
-  scoped_refptr<VideoFrame> last_available_frame_;
-
   // Used to signal |thread_| as frames are added to |frames_|.  Rule of thumb:
   // always check |state_| to see if it was set to STOPPED after waking up!
   base::ConditionVariable frame_available_;
@@ -232,18 +218,9 @@ class MEDIA_EXPORT VideoRendererBase
   // Video thread handle.
   base::PlatformThreadHandle thread_;
 
-  // Keep track of various pending operations:
-  //   - |pending_read_| is true when there's an active video decoding request.
-  //   - |pending_paint_| is true when |current_frame_| is currently being
-  //     accessed by the subclass.
-  //   - |pending_paint_with_last_available_| is true when
-  //     |last_available_frame_| is currently being accessed by the subclass.
-  //
-  // Flushing cannot complete until both |pending_read_| and |pending_paint_|
-  // are false.
+  // Keep track of outstanding reads on the video decoder. Flushing can only
+  // complete once reads have completed.
   bool pending_read_;
-  bool pending_paint_;
-  bool pending_paint_with_last_available_;
 
   bool drop_frames_;
 
@@ -270,15 +247,24 @@ class MEDIA_EXPORT VideoRendererBase
   scoped_refptr<VideoFrame> prerolling_delayed_frame_;
 
   // Embedder callback for notifying a new frame is available for painting.
-  base::Closure paint_cb_;
+  PaintCB paint_cb_;
 
   // Callback to execute to inform the player if the video decoder's output is
   // opaque.
   SetOpaqueCB set_opaque_cb_;
 
   // The last natural size |size_changed_cb_| was called with.
+  //
+  // TODO(scherkus): WebMediaPlayerImpl should track this instead of plumbing
+  // this through Pipeline. The one tricky bit might be guaranteeing we deliver
+  // the size information before we reach HAVE_METADATA.
   gfx::Size last_natural_size_;
 
+  // The timestamp of the last frame removed from the |ready_frames_| queue,
+  // either for calling |paint_cb_| or for dropping. Set to kNoTimestamp()
+  // during flushing.
+  base::TimeDelta last_timestamp_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoRendererBase);
 };