Replace subclass interface from VideoRendererBase with a single paint callback.

As a result, many of the existing VideoRendererBase subclasses have been simplified or even removed entirely.

BUG=28208

Review URL: http://codereview.chromium.org/8999029

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@115583 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 205 insertions, 423 deletions

diff --git a/media/filters/null_video_renderer.cc b/media/filters/null_video_renderer.cc
deleted file mode 100644
index 2c81ded..0000000
--- a/media/filters/null_video_renderer.cc
+++ /dev/null
@@ -1,24 +0,0 @@
-// Copyright (c) 2011 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 "media/filters/null_video_renderer.h"
-
-namespace media {
-
-NullVideoRenderer::NullVideoRenderer() {}
-NullVideoRenderer::~NullVideoRenderer() {}
-
-bool NullVideoRenderer::OnInitialize(media::VideoDecoder* decoder) {
-  return true;
-}
-
-void NullVideoRenderer::OnStop(const base::Closure& callback) {
-  callback.Run();
-}
-
-void NullVideoRenderer::OnFrameAvailable() {
-  // Do nothing.
-}
-
-}  // namespace media
diff --git a/media/filters/null_video_renderer.h b/media/filters/null_video_renderer.h
deleted file mode 100644
index 98d8deb..0000000
--- a/media/filters/null_video_renderer.h
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (c) 2011 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.
-
-#ifndef MEDIA_FILTERS_NULL_VIDEO_RENDERER_H_
-#define MEDIA_FILTERS_NULL_VIDEO_RENDERER_H_
-
-#include "media/filters/video_renderer_base.h"
-
-namespace media {
-
-class NullVideoRenderer : public VideoRendererBase {
- public:
-  NullVideoRenderer();
-
-  // VideoRendererBase implementation.
-  virtual bool OnInitialize(VideoDecoder* decoder) OVERRIDE;
-  virtual void OnStop(const base::Closure& callback) OVERRIDE;
-  virtual void OnFrameAvailable() OVERRIDE;
-
- private:
-  // Only allow to be deleted by reference counting.
-  friend class scoped_refptr<NullVideoRenderer>;
-  virtual ~NullVideoRenderer();
-
-  DISALLOW_COPY_AND_ASSIGN(NullVideoRenderer);
-};
-
-}  // namespace media
-
-#endif  // MEDIA_FILTERS_NULL_VIDEO_RENDERER_H_
diff --git a/media/filters/video_renderer_base.cc b/media/filters/video_renderer_base.cc
index 807594b..697a91b 100644
--- a/media/filters/video_renderer_base.cc
+++ b/media/filters/video_renderer_base.cc
@@ -13,7 +13,7 @@
 
 namespace media {
 
-VideoRendererBase::VideoRendererBase()
+VideoRendererBase::VideoRendererBase(const base::Closure& paint_cb)
     : frame_available_(&lock_),
       state_(kUninitialized),
       thread_(base::kNullThreadHandle),
@@ -22,7 +22,9 @@ VideoRendererBase::VideoRendererBase()
       pending_paint_with_last_available_(false),
       playback_rate_(0),
       read_cb_(base::Bind(&VideoRendererBase::FrameReady,
-                          base::Unretained(this))) {
+                          base::Unretained(this))),
+      paint_cb_(paint_cb) {
+  DCHECK(!paint_cb_.is_null());
 }
 
 VideoRendererBase::~VideoRendererBase() {
@@ -74,8 +76,7 @@ void VideoRendererBase::Stop(const base::Closure& callback) {
   if (thread_to_join != base::kNullThreadHandle)
     base::PlatformThread::Join(thread_to_join);
 
-  // Signal the subclass we're stopping.
-  OnStop(callback);
+  callback.Run();
 }
 
 void VideoRendererBase::SetPlaybackRate(float playback_rate) {
@@ -110,16 +111,6 @@ void VideoRendererBase::Initialize(VideoDecoder* decoder,
   // Notify the pipeline of the video dimensions.
   host()->SetNaturalVideoSize(decoder_->natural_size());
 
-  // Initialize the subclass.
-  // TODO(scherkus): do we trust subclasses not to do something silly while
-  // we're holding the lock?
-  if (!OnInitialize(decoder)) {
-    state_ = kError;
-    host()->SetError(PIPELINE_ERROR_INITIALIZATION_FAILED);
-    callback.Run();
-    return;
-  }
-
   // We're all good!  Consider ourselves flushed. (ThreadMain() should never
   // see us in the kUninitialized state).
   // Since we had an initial Seek, we consider ourself flushed, because we
@@ -278,7 +269,7 @@ void VideoRendererBase::ThreadMain() {
     AttemptRead_Locked();
 
     base::AutoUnlock auto_unlock(lock_);
-    OnFrameAvailable();
+    paint_cb_.Run();
   }
 }
 
@@ -408,7 +399,7 @@ void VideoRendererBase::FrameReady(scoped_refptr<VideoFrame> frame) {
     ResetAndRunCB(&seek_cb_, PIPELINE_OK);
 
     base::AutoUnlock ul(lock_);
-    OnFrameAvailable();
+    paint_cb_.Run();
   }
 }
 
diff --git a/media/filters/video_renderer_base.h b/media/filters/video_renderer_base.h
index 7891125..e5dcc5a 100644
--- a/media/filters/video_renderer_base.h
+++ b/media/filters/video_renderer_base.h
@@ -2,16 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// VideoRendererBase creates its own thread for the sole purpose of timing frame
-// presentation.  It handles reading from the decoder and stores the results in
-// a queue of decoded frames, calling OnFrameAvailable() on subclasses to notify
-// when a frame is ready to display.
-//
-// The media filter methods Initialize(), Stop(), SetPlaybackRate() and Seek()
-// should be serialized, which they commonly are the pipeline thread.
-// As long as VideoRendererBase is initialized, GetCurrentFrame() is safe to
-// call from any thread, at any time, including inside of OnFrameAvailable().
-
 #ifndef MEDIA_FILTERS_VIDEO_RENDERER_BASE_H_
 #define MEDIA_FILTERS_VIDEO_RENDERER_BASE_H_
 
@@ -27,13 +17,24 @@
 
 namespace media {
 
-// TODO(scherkus): to avoid subclasses, consider using a peer/delegate interface
-// and pass in a reference to the constructor.
+// VideoRendererBase creates its own thread for the sole purpose of timing frame
+// presentation.  It handles reading from the decoder and stores the results in
+// a queue of decoded frames and executing a callback when a frame is ready for
+// rendering.
 class MEDIA_EXPORT VideoRendererBase
     : public VideoRenderer,
       public base::PlatformThread::Delegate {
  public:
-  VideoRendererBase();
+  // |paint_cb| is executed on the video frame timing thread whenever a new
+  // frame is available for painting via GetCurrentFrame().
+  //
+  // Implementors should avoid doing any sort of heavy work in this method and
+  // instead post a task to a common/worker thread to handle rendering.  Slowing
+  // down the video thread may result in losing synchronization with audio.
+  //
+  // TODO(scherkus): pass the VideoFrame* to this callback and remove
+  // Get/PutCurrentFrame() http://crbug.com/108435
+  explicit VideoRendererBase(const base::Closure& paint_cb);
   virtual ~VideoRendererBase();
 
   // Filter implementation.
@@ -62,32 +63,6 @@ class MEDIA_EXPORT VideoRendererBase
   void GetCurrentFrame(scoped_refptr<VideoFrame>* frame_out);
   void PutCurrentFrame(scoped_refptr<VideoFrame> frame);
 
- protected:
-  // Subclass interface.  Called before any other initialization in the base
-  // class takes place.
-  //
-  // Implementors typically use the media format of |decoder| to create their
-  // output surfaces.
-  virtual bool OnInitialize(VideoDecoder* decoder) = 0;
-
-  // Subclass interface.  Called after all other stopping actions take place.
-  //
-  // Implementors should perform any necessary cleanup before calling the
-  // callback.
-  virtual void OnStop(const base::Closure& callback) = 0;
-
-  // Subclass interface.  Called when a new frame is ready for display, which
-  // can be accessed via GetCurrentFrame().
-  //
-  // Implementors should avoid doing any sort of heavy work in this method and
-  // instead post a task to a common/worker thread to handle rendering.  Slowing
-  // down the video thread may result in losing synchronization with audio.
-  //
-  // IMPORTANT: This method is called on the video renderer thread, which is
-  // different from the thread OnInitialize(), OnStop(), and the rest of the
-  // class executes on.
-  virtual void OnFrameAvailable() = 0;
-
  private:
   // Callback from the video decoder delivering decoded video frames.
   void FrameReady(scoped_refptr<VideoFrame> frame);
@@ -205,6 +180,9 @@ class MEDIA_EXPORT VideoRendererBase
 
   VideoDecoder::ReadCB read_cb_;
 
+  // Embedder callback for notifying a new frame is available for painting.
+  base::Closure paint_cb_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoRendererBase);
 };
 
diff --git a/media/filters/video_renderer_base_unittest.cc b/media/filters/video_renderer_base_unittest.cc
index 552453c..22e70a9 100644
--- a/media/filters/video_renderer_base_unittest.cc
+++ b/media/filters/video_renderer_base_unittest.cc
@@ -40,34 +40,17 @@ ACTION(OnStop) {
   arg0.Run();
 }
 
-// Mocked subclass of VideoRendererBase for testing purposes.
-class MockVideoRendererBase : public VideoRendererBase {
- public:
-  MockVideoRendererBase() {}
-  virtual ~MockVideoRendererBase() {}
-
-  // VideoRendererBase implementation.
-  MOCK_METHOD1(OnInitialize, bool(VideoDecoder* decoder));
-  MOCK_METHOD1(OnStop, void(const base::Closure& callback));
-  MOCK_METHOD0(OnFrameAvailable, void());
-
-  // Used for verifying check points during tests.
-  MOCK_METHOD1(CheckPoint, void(int id));
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(MockVideoRendererBase);
-};
-
 class VideoRendererBaseTest : public ::testing::Test {
  public:
   VideoRendererBaseTest()
-      : renderer_(new MockVideoRendererBase()),
-        decoder_(new MockVideoDecoder()),
+      : decoder_(new MockVideoDecoder()),
         cv_(&lock_),
         event_(false, false),
         timeout_(base::TimeDelta::FromMilliseconds(
             TestTimeouts::action_timeout_ms())),
         seeking_(false) {
+    renderer_ = new VideoRendererBase(base::Bind(
+        &VideoRendererBaseTest::PaintCBWasCalled, base::Unretained(this)));
     renderer_->set_host(&host_);
 
     EXPECT_CALL(*decoder_, natural_size())
@@ -85,6 +68,8 @@ class VideoRendererBaseTest : public ::testing::Test {
     }
   }
 
+  MOCK_METHOD0(PaintCBWasCalled, void());
+
   void Initialize() {
     // TODO(scherkus): really, really, really need to inject a thread into
     // VideoRendererBase... it makes mocking much harder.
@@ -105,10 +90,6 @@ class VideoRendererBaseTest : public ::testing::Test {
     // We expect the video size to be set.
     EXPECT_CALL(host_, SetNaturalVideoSize(kNaturalSize));
 
-    // Then our subclass will be asked to initialize.
-    EXPECT_CALL(*renderer_, OnInitialize(_))
-        .WillOnce(Return(true));
-
     // Set playback rate before anything else happens.
     renderer_->SetPlaybackRate(1.0f);
 
@@ -160,12 +141,6 @@ class VideoRendererBaseTest : public ::testing::Test {
 
   void Stop() {
     SCOPED_TRACE("Stop()");
-
-    // Expect a call into the subclass.
-    EXPECT_CALL(*renderer_, OnStop(_))
-        .WillOnce(DoAll(OnStop(), Return()))
-        .RetiresOnSaturation();
-
     renderer_->Stop(NewWaitableClosure());
     WaitForClosure();
   }
@@ -265,7 +240,7 @@ class VideoRendererBaseTest : public ::testing::Test {
   }
 
   // Fixture members.
-  scoped_refptr<MockVideoRendererBase> renderer_;
+  scoped_refptr<VideoRendererBase> renderer_;
   scoped_refptr<MockVideoDecoder> decoder_;
   StrictMock<MockFilterHost> host_;
   MockStatisticsCallback stats_callback_object_;
@@ -297,7 +272,7 @@ class VideoRendererBaseTest : public ::testing::Test {
   }
 
   void FinishSeeking(int64 timestamp) {
-    EXPECT_CALL(*renderer_, OnFrameAvailable());
+    EXPECT_CALL(*this, PaintCBWasCalled());
     EXPECT_TRUE(seeking_);
 
     // Satisfy the read requests.  The callback must be executed in order
@@ -342,28 +317,7 @@ class VideoRendererBaseTest : public ::testing::Test {
   DISALLOW_COPY_AND_ASSIGN(VideoRendererBaseTest);
 };
 
-// Test initialization where the subclass failed for some reason.
-TEST_F(VideoRendererBaseTest, Initialize_Failed) {
-  InSequence s;
-
-  // We expect the video size to be set.
-  EXPECT_CALL(host_, SetNaturalVideoSize(kNaturalSize));
-
-  // Our subclass will fail when asked to initialize.
-  EXPECT_CALL(*renderer_, OnInitialize(_))
-      .WillOnce(Return(false));
-
-  // We expect to receive an error.
-  EXPECT_CALL(host_, SetError(PIPELINE_ERROR_INITIALIZATION_FAILED));
-
-  // Initialize, we expect to have no reads.
-  renderer_->Initialize(decoder_,
-                        NewExpectedClosure(), NewStatisticsCallback());
-  EXPECT_EQ(0u, read_queue_.size());
-}
-
-// Test successful initialization and preroll.
-TEST_F(VideoRendererBaseTest, Initialize_Successful) {
+TEST_F(VideoRendererBaseTest, Initialize) {
   Initialize();
   ExpectCurrentTimestamp(0);
   Shutdown();
@@ -383,7 +337,7 @@ TEST_F(VideoRendererBaseTest, EndOfStream) {
   // frames as we go along.
   //
   // Put the gmock expectation here to avoid racing with the rendering thread.
-  EXPECT_CALL(*renderer_, OnFrameAvailable())
+  EXPECT_CALL(*this, PaintCBWasCalled())
       .Times(limits::kMaxVideoFrames - 1);
   for (int i = 1; i < limits::kMaxVideoFrames; ++i) {
     RenderFrame(kFrameDuration * i);
diff --git a/media/media.gyp b/media/media.gyp
index 27b9dd2..64f5648 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -210,8 +210,6 @@
         'filters/in_memory_url_protocol.h',
         'filters/null_audio_renderer.cc',
         'filters/null_audio_renderer.h',
-        'filters/null_video_renderer.cc',
-        'filters/null_video_renderer.h',
         'filters/reference_audio_renderer.cc',
         'filters/reference_audio_renderer.h',
         'filters/video_renderer_base.cc',
@@ -735,8 +733,6 @@
             'tools/player_wtl/seek.h',
             'tools/player_wtl/resource.h',
             'tools/player_wtl/view.h',
-            'tools/player_wtl/wtl_renderer.cc',
-            'tools/player_wtl/wtl_renderer.h',
           ],
           'msvs_settings': {
             'VCLinkerTool': {
diff --git a/media/tools/player_wtl/movie.cc b/media/tools/player_wtl/movie.cc
index b6bd2f8..043932e 100644
--- a/media/tools/player_wtl/movie.cc
+++ b/media/tools/player_wtl/movie.cc
@@ -18,7 +18,7 @@
 #include "media/filters/file_data_source_factory.h"
 #include "media/filters/null_audio_renderer.h"
 #include "media/filters/reference_audio_renderer.h"
-#include "media/tools/player_wtl/wtl_renderer.h"
+#include "media/filters/video_renderer_base.h"
 
 using media::FFmpegAudioDecoder;
 using media::FFmpegDemuxerFactory;
@@ -58,7 +58,7 @@ void Movie::SetFrameBuffer(HBITMAP hbmp, HWND hwnd) {
   movie_hwnd_ = hwnd;
 }
 
-bool Movie::Open(const wchar_t* url, WtlVideoRenderer* video_renderer) {
+bool Movie::Open(const wchar_t* url, VideoRendererBase* video_renderer) {
   // Close previous movie.
   if (pipeline_) {
     Close();
diff --git a/media/tools/player_wtl/movie.h b/media/tools/player_wtl/movie.h
index cd24c88..0b407bf 100644
--- a/media/tools/player_wtl/movie.h
+++ b/media/tools/player_wtl/movie.h
@@ -15,11 +15,11 @@
 
 class AudioManager;
 template <typename T> struct DefaultSingletonTraits;
-class WtlVideoRenderer;
 
 namespace media {
 
 class PipelineImpl;
+class VideoRendererBase;
 
 class Movie {
  public:
@@ -27,7 +27,7 @@ class Movie {
   static Movie* GetInstance();
 
   // Open a movie.
-  bool Open(const wchar_t* url, WtlVideoRenderer* video_renderer);
+  bool Open(const wchar_t* url, VideoRendererBase* video_renderer);
 
   // Set playback rate.
   void Play(float rate);
diff --git a/media/tools/player_wtl/view.h b/media/tools/player_wtl/view.h
index f89f093..ee2c580 100644
--- a/media/tools/player_wtl/view.h
+++ b/media/tools/player_wtl/view.h
@@ -9,12 +9,13 @@
 #include <process.h>
 #include <string.h>
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "media/base/video_frame.h"
 #include "media/base/yuv_convert.h"
+#include "media/filters/video_renderer_base.h"
 #include "media/tools/player_wtl/movie.h"
 #include "media/tools/player_wtl/player_wtl.h"
-#include "media/tools/player_wtl/wtl_renderer.h"
 
 // Fetchs current time as milliseconds.
 // Returns as double for high duration and precision.
@@ -42,11 +43,17 @@ class WtlVideoWindow : public CScrollWindowImpl<WtlVideoWindow> {
     view_size_ = 2;  // Normal size.
     view_rotate_ = media::ROTATE_0;
     view_filter_ = media::FILTER_NONE;
-    renderer_ = new WtlVideoRenderer(this);
+    renderer_ = new media::VideoRendererBase(base::Bind(
+        &WtlVideoWindow::InvalidateWrapper, base::Unretained(this)));
     last_frame_ = NULL;
     hbmp_ = NULL;
   }
 
+  // Drops the bool return so we can use base::Bind().
+  void InvalidateWrapper() {
+    Invalidate();
+  }
+
   BOOL PreTranslateMessage(MSG* /*msg*/)  {
     return FALSE;
   }
@@ -228,7 +235,11 @@ class WtlVideoWindow : public CScrollWindowImpl<WtlVideoWindow> {
     if (!bmp_.IsNull()) {
       scoped_refptr<media::VideoFrame> frame;
       renderer_->GetCurrentFrame(&frame);
-      if (frame.get()) {
+      if (frame) {
+        // Size the window the first time we get a frame.
+        if (!last_frame_)
+          SetSize(frame->width(), frame->height());
+
         base::TimeDelta frame_timestamp = frame->GetTimestamp();
         if (frame != last_frame_ || frame_timestamp != last_timestamp_) {
           last_frame_ = frame;
@@ -356,9 +367,10 @@ class WtlVideoWindow : public CScrollWindowImpl<WtlVideoWindow> {
     hbmp_ = hbmp;
   }
 
-  CBitmap bmp_;  // Used by mainfrm.h.
-  SIZE size_;  // Used by WtlVideoWindow.
-  scoped_refptr<WtlVideoRenderer> renderer_;  // Used by WtlVideoWindow.
+  // Following member variables are accessed by CMainFrame.
+  CBitmap bmp_;
+  SIZE size_;
+  scoped_refptr<media::VideoRendererBase> renderer_;
 
  private:
   HBITMAP hbmp_;  // For Images
diff --git a/media/tools/player_wtl/wtl_renderer.cc b/media/tools/player_wtl/wtl_renderer.cc
deleted file mode 100644
index 9e1589f..0000000
--- a/media/tools/player_wtl/wtl_renderer.cc
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright (c) 2011 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 "media/tools/player_wtl/wtl_renderer.h"
-
-#include "media/tools/player_wtl/view.h"
-
-WtlVideoRenderer::WtlVideoRenderer(WtlVideoWindow* window)
-    : window_(window) {
-}
-
-WtlVideoRenderer::~WtlVideoRenderer() {}
-
-bool WtlVideoRenderer::OnInitialize(media::VideoDecoder* decoder) {
-  window_->SetSize(
-      decoder->natural_size().width(), decoder->natural_size().height());
-  return true;
-}
-
-void WtlVideoRenderer::OnStop(const base::Closure& callback) {
-  if (!callback.is_null())
-    callback.Run();
-}
-
-void WtlVideoRenderer::OnFrameAvailable() {
-  window_->Invalidate();
-}
diff --git a/media/tools/player_wtl/wtl_renderer.h b/media/tools/player_wtl/wtl_renderer.h
deleted file mode 100644
index 8e504b8..0000000
--- a/media/tools/player_wtl/wtl_renderer.h
+++ /dev/null
@@ -1,33 +0,0 @@
-// Copyright (c) 2011 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.
-
-#ifndef MEDIA_TOOLS_PLAYER_WTL_WTL_RENDERER_H_
-#define MEDIA_TOOLS_PLAYER_WTL_WTL_RENDERER_H_
-
-#include "media/filters/video_renderer_base.h"
-
-class WtlVideoWindow;
-
-// Video renderer for media player.
-class WtlVideoRenderer : public media::VideoRendererBase {
- public:
-  explicit WtlVideoRenderer(WtlVideoWindow* window);
-
- protected:
-  // VideoRendererBase implementation.
-  virtual bool OnInitialize(media::VideoDecoder* decoder);
-  virtual void OnStop(const base::Closure& callback);
-  virtual void OnFrameAvailable();
-
- private:
-  // Only allow to be deleted by reference counting.
-  friend class scoped_refptr<WtlVideoRenderer>;
-  virtual ~WtlVideoRenderer();
-
-  WtlVideoWindow* window_;
-
-  DISALLOW_COPY_AND_ASSIGN(WtlVideoRenderer);
-};
-
-#endif  // MEDIA_TOOLS_PLAYER_WTL_WTL_RENDERER_H_
diff --git a/media/tools/player_x11/gl_video_renderer.cc b/media/tools/player_x11/gl_video_renderer.cc
index 3ce1f5f..fc36991 100644
--- a/media/tools/player_x11/gl_video_renderer.cc
+++ b/media/tools/player_x11/gl_video_renderer.cc
@@ -13,23 +13,6 @@
 #include "media/base/yuv_convert.h"
 #include "ui/gfx/gl/gl_implementation.h"
 
-GlVideoRenderer::GlVideoRenderer(Display* display, Window window,
-                                 MessageLoop* main_message_loop)
-    : display_(display),
-      window_(window),
-      gl_context_(NULL),
-      main_message_loop_(main_message_loop) {
-}
-
-GlVideoRenderer::~GlVideoRenderer() {}
-
-void GlVideoRenderer::OnStop(const base::Closure& callback) {
-  glXMakeCurrent(display_, 0, NULL);
-  glXDestroyContext(display_, gl_context_);
-  if (!callback.is_null())
-    callback.Run();
-}
-
 static GLXContext InitGLContext(Display* display, Window window) {
   // Some versions of NVIDIA's GL libGL.so include a broken version of
   // dlopen/dlsym, and so linking it into chrome breaks it. So we dynamically
@@ -123,16 +106,57 @@ static const char kFragmentShader[] =
 // Buffer size for compile errors.
 static const unsigned int kErrorSize = 4096;
 
-bool GlVideoRenderer::OnInitialize(media::VideoDecoder* decoder) {
+GlVideoRenderer::GlVideoRenderer(Display* display, Window window)
+    : display_(display),
+      window_(window),
+      gl_context_(NULL) {
+}
+
+GlVideoRenderer::~GlVideoRenderer() {
+  glXMakeCurrent(display_, 0, NULL);
+  glXDestroyContext(display_, gl_context_);
+}
+
+void GlVideoRenderer::Paint(media::VideoFrame* video_frame) {
+  if (!gl_context_)
+    Initialize(video_frame->width(), video_frame->height());
+
+  // Convert YUV frame to RGB.
+  DCHECK(video_frame->format() == media::VideoFrame::YV12 ||
+         video_frame->format() == media::VideoFrame::YV16);
+  DCHECK(video_frame->stride(media::VideoFrame::kUPlane) ==
+         video_frame->stride(media::VideoFrame::kVPlane));
+  DCHECK(video_frame->planes() == media::VideoFrame::kNumYUVPlanes);
+
+  if (glXGetCurrentContext() != gl_context_ ||
+      glXGetCurrentDrawable() != window_) {
+    glXMakeCurrent(display_, window_, gl_context_);
+  }
+  for (unsigned int i = 0; i < media::VideoFrame::kNumYUVPlanes; ++i) {
+    unsigned int width = (i == media::VideoFrame::kYPlane) ?
+        video_frame->width() : video_frame->width() / 2;
+    unsigned int height = (i == media::VideoFrame::kYPlane ||
+                           video_frame->format() == media::VideoFrame::YV16) ?
+        video_frame->height() : video_frame->height() / 2;
+    glActiveTexture(GL_TEXTURE0 + i);
+    glPixelStorei(GL_UNPACK_ROW_LENGTH, video_frame->stride(i));
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_LUMINANCE, width, height, 0,
+                 GL_LUMINANCE, GL_UNSIGNED_BYTE, video_frame->data(i));
+  }
+
+  glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
+  glXSwapBuffers(display_, window_);
+}
+
+void GlVideoRenderer::Initialize(int width, int height) {
+  CHECK(!gl_context_);
   LOG(INFO) << "Initializing GL Renderer...";
 
   // Resize the window to fit that of the video.
-  XResizeWindow(display_, window_, decoder->natural_size().width(),
-                decoder->natural_size().height());
+  XResizeWindow(display_, window_, width, height);
 
   gl_context_ = InitGLContext(display_, window_);
-  if (!gl_context_)
-    return false;
+  CHECK(gl_context_) << "Failed to initialize GL context";
 
   // Create 3 textures, one for each plane, and bind them to different
   // texture units.
@@ -224,51 +248,4 @@ bool GlVideoRenderer::OnInitialize(media::VideoDecoder* decoder) {
   // We are getting called on a thread. Release the context so that it can be
   // made current on the main thread.
   glXMakeCurrent(display_, 0, NULL);
-
-  return true;
-}
-
-void GlVideoRenderer::OnFrameAvailable() {
-  main_message_loop_->PostTask(FROM_HERE,
-      base::Bind(&GlVideoRenderer::PaintOnMainThread, this));
-}
-
-void GlVideoRenderer::PaintOnMainThread() {
-  DCHECK_EQ(main_message_loop_, MessageLoop::current());
-
-  scoped_refptr<media::VideoFrame> video_frame;
-  GetCurrentFrame(&video_frame);
-
-  if (!video_frame) {
-    // TODO(jiesun): Use color fill rather than create black frame then scale.
-    PutCurrentFrame(video_frame);
-    return;
-  }
-
-  // Convert YUV frame to RGB.
-  DCHECK(video_frame->format() == media::VideoFrame::YV12 ||
-         video_frame->format() == media::VideoFrame::YV16);
-  DCHECK(video_frame->stride(media::VideoFrame::kUPlane) ==
-         video_frame->stride(media::VideoFrame::kVPlane));
-  DCHECK(video_frame->planes() == media::VideoFrame::kNumYUVPlanes);
-
-  if (glXGetCurrentContext() != gl_context_ ||
-      glXGetCurrentDrawable() != window_) {
-    glXMakeCurrent(display_, window_, gl_context_);
-  }
-  for (unsigned int i = 0; i < media::VideoFrame::kNumYUVPlanes; ++i) {
-    unsigned int width = (i == media::VideoFrame::kYPlane) ?
-        video_frame->width() : video_frame->width() / 2;
-    unsigned int height = (i == media::VideoFrame::kYPlane ||
-                           video_frame->format() == media::VideoFrame::YV16) ?
-        video_frame->height() : video_frame->height() / 2;
-    glActiveTexture(GL_TEXTURE0 + i);
-    glPixelStorei(GL_UNPACK_ROW_LENGTH, video_frame->stride(i));
-    glTexImage2D(GL_TEXTURE_2D, 0, GL_LUMINANCE, width, height, 0,
-                 GL_LUMINANCE, GL_UNSIGNED_BYTE, video_frame->data(i));
-  }
-  PutCurrentFrame(video_frame);
-
-  glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
-  glXSwapBuffers(display_, window_);
 }
diff --git a/media/tools/player_x11/gl_video_renderer.h b/media/tools/player_x11/gl_video_renderer.h
index 211747c..2ecdc0b 100644
--- a/media/tools/player_x11/gl_video_renderer.h
+++ b/media/tools/player_x11/gl_video_renderer.h
@@ -5,30 +5,25 @@
 #ifndef MEDIA_TOOLS_PLAYER_X11_GL_VIDEO_RENDERER_H_
 #define MEDIA_TOOLS_PLAYER_X11_GL_VIDEO_RENDERER_H_
 
-#include "base/compiler_specific.h"
-#include "base/memory/scoped_ptr.h"
-#include "media/filters/video_renderer_base.h"
+#include "base/basictypes.h"
 #include "ui/gfx/gl/gl_bindings.h"
 
 class MessageLoop;
 
-class GlVideoRenderer : public media::VideoRendererBase {
+namespace media {
+class VideoFrame;
+}
+
+class GlVideoRenderer {
  public:
-  GlVideoRenderer(Display* display, Window window,
-                  MessageLoop* main_message_loop);
+  GlVideoRenderer(Display* display, Window window);
+  ~GlVideoRenderer();
 
- protected:
-  // VideoRendererBase implementation.
-  virtual bool OnInitialize(media::VideoDecoder* decoder) OVERRIDE;
-  virtual void OnStop(const base::Closure& callback) OVERRIDE;
-  virtual void OnFrameAvailable() OVERRIDE;
+  void Paint(media::VideoFrame* video_frame);
 
  private:
-  // Only allow to be deleted by reference counting.
-  friend class scoped_refptr<GlVideoRenderer>;
-  virtual ~GlVideoRenderer();
-
-  void PaintOnMainThread();
+  // Initializes GL rendering for the given dimensions.
+  void Initialize(int width, int height);
 
   Display* display_;
   Window window_;
@@ -39,8 +34,6 @@ class GlVideoRenderer : public media::VideoRendererBase {
   // 3 textures, one for each plane.
   GLuint textures_[3];
 
-  MessageLoop* main_message_loop_;
-
   DISALLOW_COPY_AND_ASSIGN(GlVideoRenderer);
 };
 
diff --git a/media/tools/player_x11/player_x11.cc b/media/tools/player_x11/player_x11.cc
index d563328..8c58dc4 100644
--- a/media/tools/player_x11/player_x11.cc
+++ b/media/tools/player_x11/player_x11.cc
@@ -22,12 +22,14 @@
 #include "media/base/media_switches.h"
 #include "media/base/message_loop_factory_impl.h"
 #include "media/base/pipeline_impl.h"
+#include "media/base/video_frame.h"
 #include "media/filters/ffmpeg_audio_decoder.h"
 #include "media/filters/ffmpeg_demuxer_factory.h"
 #include "media/filters/ffmpeg_video_decoder.h"
 #include "media/filters/file_data_source_factory.h"
 #include "media/filters/null_audio_renderer.h"
 #include "media/filters/reference_audio_renderer.h"
+#include "media/filters/video_renderer_base.h"
 #include "media/tools/player_x11/gl_video_renderer.h"
 #include "media/tools/player_x11/x11_video_renderer.h"
 
@@ -37,6 +39,8 @@ static bool g_running = false;
 
 AudioManager* g_audio_manager = NULL;
 
+media::VideoRendererBase* g_video_renderer = NULL;
+
 class MessageLoopQuitter {
  public:
   explicit MessageLoopQuitter(MessageLoop* loop) : loop_(loop) {}
@@ -74,9 +78,24 @@ bool InitX11() {
   return true;
 }
 
+typedef base::Callback<void(media::VideoFrame*)> PaintCB;
+void Paint(MessageLoop* message_loop, const PaintCB& paint_cb) {
+  if (message_loop != MessageLoop::current()) {
+    message_loop->PostTask(FROM_HERE, base::Bind(
+        &Paint, message_loop, paint_cb));
+    return;
+  }
+
+  scoped_refptr<media::VideoFrame> video_frame;
+  g_video_renderer->GetCurrentFrame(&video_frame);
+  if (video_frame)
+    paint_cb.Run(video_frame);
+  g_video_renderer->PutCurrentFrame(video_frame);
+}
+
 bool InitPipeline(MessageLoop* message_loop,
                   const char* filename,
-                  bool use_gl,
+                  const PaintCB& paint_cb,
                   bool enable_audio,
                   scoped_refptr<media::PipelineImpl>* pipeline,
                   MessageLoop* paint_message_loop,
@@ -98,13 +117,10 @@ bool InitPipeline(MessageLoop* message_loop,
   collection->AddVideoDecoder(new media::FFmpegVideoDecoder(
       message_loop_factory->GetMessageLoop("VideoDecoderThread")));
 
-  if (use_gl) {
-    collection->AddVideoRenderer(
-        new GlVideoRenderer(g_display, g_window, paint_message_loop));
-  } else {
-    collection->AddVideoRenderer(
-        new X11VideoRenderer(g_display, g_window, paint_message_loop));
-  }
+  // Create our video renderer and save a reference to it for painting.
+  g_video_renderer = new media::VideoRendererBase(base::Bind(
+      &Paint, paint_message_loop, paint_cb));
+  collection->AddVideoRenderer(g_video_renderer);
 
   if (enable_audio) {
     collection->AddAudioRenderer(
@@ -200,6 +216,8 @@ void PeriodicalUpdate(
 }
 
 int main(int argc, char** argv) {
+  base::AtExitManager at_exit;
+
   scoped_refptr<AudioManager> audio_manager(AudioManager::Create());
   g_audio_manager = audio_manager;
 
@@ -241,7 +259,6 @@ int main(int argc, char** argv) {
     return 1;
 
   // Initialize the pipeline thread and the pipeline.
-  base::AtExitManager at_exit;
   scoped_ptr<media::MessageLoopFactory> message_loop_factory(
       new media::MessageLoopFactoryImpl());
   scoped_ptr<base::Thread> thread;
@@ -249,8 +266,21 @@ int main(int argc, char** argv) {
   MessageLoop message_loop;
   thread.reset(new base::Thread("PipelineThread"));
   thread->Start();
+
+  // Create both our renderers but only bind the one we plan on using.
+  X11VideoRenderer x11_renderer(g_display, g_window);
+  GlVideoRenderer gl_renderer(g_display, g_window);
+  PaintCB paint_cb;
+  if (use_gl) {
+    paint_cb = base::Bind(
+        &GlVideoRenderer::Paint, base::Unretained(&gl_renderer));
+  } else {
+    paint_cb = base::Bind(
+        &X11VideoRenderer::Paint, base::Unretained(&x11_renderer));
+  }
+
   if (InitPipeline(thread->message_loop(), filename.c_str(),
-                   use_gl, enable_audio, &pipeline, &message_loop,
+                   paint_cb, enable_audio, &pipeline, &message_loop,
                    message_loop_factory.get())) {
     // Main loop of the application.
     g_running = true;
diff --git a/media/tools/player_x11/x11_video_renderer.cc b/media/tools/player_x11/x11_video_renderer.cc
index 70191ee..9cde66d 100644
--- a/media/tools/player_x11/x11_video_renderer.cc
+++ b/media/tools/player_x11/x11_video_renderer.cc
@@ -68,87 +68,30 @@ static XRenderPictFormat* GetRenderARGB32Format(Display* dpy) {
   return pictformat;
 }
 
-X11VideoRenderer::X11VideoRenderer(Display* display, Window window,
-                                   MessageLoop* main_message_loop)
+X11VideoRenderer::X11VideoRenderer(Display* display, Window window)
     : display_(display),
       window_(window),
       image_(NULL),
       picture_(0),
-      use_render_(false),
-      main_message_loop_(main_message_loop) {
+      use_render_(false) {
 }
 
-X11VideoRenderer::~X11VideoRenderer() {}
-
-void X11VideoRenderer::OnStop(const base::Closure& callback) {
+X11VideoRenderer::~X11VideoRenderer() {
   if (image_)
     XDestroyImage(image_);
-  XRenderFreePicture(display_, picture_);
-  if (!callback.is_null())
-    callback.Run();
-}
-
-bool X11VideoRenderer::OnInitialize(media::VideoDecoder* decoder) {
-  LOG(INFO) << "Initializing X11 Renderer...";
-
-  // Resize the window to fit that of the video.
-  int width = decoder->natural_size().width();
-  int height = decoder->natural_size().height();
-  XResizeWindow(display_, window_, width, height);
-
-  // Allocate an XImage for caching RGB result.
-  image_ = CreateImage(display_, width, height);
-
-  // Testing XRender support. We'll use the very basic of XRender
-  // so if it presents it is already good enough. We don't need
-  // to check its version.
-  int dummy;
-  use_render_ = XRenderQueryExtension(display_, &dummy, &dummy);
-
-  if (use_render_) {
-    LOG(INFO) << "Using XRender extension.";
-
-    // If we are using XRender, we'll create a picture representing the
-    // window.
-    XWindowAttributes attr;
-    XGetWindowAttributes(display_, window_, &attr);
-
-    XRenderPictFormat* pictformat = XRenderFindVisualFormat(
-        display_,
-        attr.visual);
-    CHECK(pictformat) << "XRender does not support default visual";
-
-    picture_ = XRenderCreatePicture(display_, window_, pictformat, 0, NULL);
-    CHECK(picture_) << "Backing picture not created";
-  }
-
-  return true;
-}
-
-void X11VideoRenderer::OnFrameAvailable() {
-  main_message_loop_->PostTask(FROM_HERE,
-      base::Bind(&X11VideoRenderer::PaintOnMainThread, this));
+  if (use_render_)
+    XRenderFreePicture(display_, picture_);
 }
 
-void X11VideoRenderer::PaintOnMainThread() {
-  DCHECK_EQ(main_message_loop_, MessageLoop::current());
-
-  scoped_refptr<media::VideoFrame> video_frame;
-  GetCurrentFrame(&video_frame);
-  if (!video_frame) {
-    // TODO(jiesun): Use color fill rather than create black frame then scale.
-    PutCurrentFrame(video_frame);
-    return;
-  }
-
+void X11VideoRenderer::Paint(media::VideoFrame* video_frame) {
   int width = video_frame->width();
   int height = video_frame->height();
 
-  // Check if we need to re-allocate our XImage.
+  if (!image_)
+    Initialize(width, height);
+
+  // Check if we need to reallocate our XImage.
   if (image_->width != width || image_->height != height) {
-    LOG(INFO) << "Detection resolution change: "
-              << image_->width << "x" << image_->height << " -> "
-              << width << "x" << height;
     XDestroyImage(image_);
     image_ = CreateImage(display_, width, height);
   }
@@ -174,7 +117,6 @@ void X11VideoRenderer::PaintOnMainThread() {
                            video_frame->stride(media::VideoFrame::kUPlane),
                            image_->bytes_per_line,
                            yuv_type);
-  PutCurrentFrame(video_frame);
 
   if (use_render_) {
     // If XRender is used, we'll upload the image to a pixmap. And then
@@ -234,3 +176,35 @@ void X11VideoRenderer::PaintOnMainThread() {
   XFlush(display_);
   XFreeGC(display_, gc);
 }
+
+void X11VideoRenderer::Initialize(int width, int height) {
+  CHECK(!image_);
+  LOG(INFO) << "Initializing X11 Renderer...";
+
+  // Resize the window to fit that of the video.
+  XResizeWindow(display_, window_, width, height);
+  image_ = CreateImage(display_, width, height);
+
+  // Testing XRender support. We'll use the very basic of XRender
+  // so if it presents it is already good enough. We don't need
+  // to check its version.
+  int dummy;
+  use_render_ = XRenderQueryExtension(display_, &dummy, &dummy);
+
+  if (use_render_) {
+    LOG(INFO) << "Using XRender extension.";
+
+    // If we are using XRender, we'll create a picture representing the
+    // window.
+    XWindowAttributes attr;
+    XGetWindowAttributes(display_, window_, &attr);
+
+    XRenderPictFormat* pictformat = XRenderFindVisualFormat(
+        display_,
+        attr.visual);
+    CHECK(pictformat) << "XRender does not support default visual";
+
+    picture_ = XRenderCreatePicture(display_, window_, pictformat, 0, NULL);
+    CHECK(picture_) << "Backing picture not created";
+  }
+}
diff --git a/media/tools/player_x11/x11_video_renderer.h b/media/tools/player_x11/x11_video_renderer.h
index 667b2cf..1ce5b62 100644
--- a/media/tools/player_x11/x11_video_renderer.h
+++ b/media/tools/player_x11/x11_video_renderer.h
@@ -7,29 +7,24 @@
 
 #include <X11/Xlib.h>
 
-#include "base/compiler_specific.h"
-#include "base/memory/scoped_ptr.h"
-#include "media/filters/video_renderer_base.h"
+#include "base/basictypes.h"
 
 class MessageLoop;
 
-class X11VideoRenderer : public media::VideoRendererBase {
+namespace media {
+class VideoFrame;
+}
+
+class X11VideoRenderer {
  public:
-  X11VideoRenderer(Display* display, Window window,
-                   MessageLoop* main_message_loop);
+  X11VideoRenderer(Display* display, Window window);
+  ~X11VideoRenderer();
 
- protected:
-  // VideoRendererBase implementation.
-  virtual bool OnInitialize(media::VideoDecoder* decoder) OVERRIDE;
-  virtual void OnStop(const base::Closure& callback) OVERRIDE;
-  virtual void OnFrameAvailable() OVERRIDE;
+  void Paint(media::VideoFrame* video_frame);
 
  private:
-  // Only allow to be deleted by reference counting.
-  friend class scoped_refptr<X11VideoRenderer>;
-  virtual ~X11VideoRenderer();
-
-  void PaintOnMainThread();
+  // Initializes X11 rendering for the given dimensions.
+  void Initialize(int width, int height);
 
   Display* display_;
   Window window_;
@@ -43,8 +38,6 @@ class X11VideoRenderer : public media::VideoRendererBase {
 
   bool use_render_;
 
-  MessageLoop* main_message_loop_;
-
   DISALLOW_COPY_AND_ASSIGN(X11VideoRenderer);
 };