Remove VideoDecoder from the Filter heirarchy.

This is the second step to move VideoDecoder out of Filter hierarchy (See r128289). VideoDecoder is not a Filter any more and Seek()/Pause()/Play() methods are removed from its definition and all implementations.

BUG=108340
TEST=media_unittest,content_unittest,normal html5 video playback

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

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

10 files changed, 136 insertions, 130 deletions

diff --git a/media/base/filter_collection.cc b/media/base/filter_collection.cc
index 537dd60..205da15 100644
--- a/media/base/filter_collection.cc
+++ b/media/base/filter_collection.cc
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "media/base/audio_decoder.h"
+#include "media/base/video_decoder.h"
 
 namespace media {
 
diff --git a/media/base/filter_collection.h b/media/base/filter_collection.h
index 8d8591d..0f5e320 100644
--- a/media/base/filter_collection.h
+++ b/media/base/filter_collection.h
@@ -14,6 +14,7 @@
 namespace media {
 
 class AudioDecoder;
+class VideoDecoder;
 
 // This is a collection of Filter objects used to form a media playback
 // pipeline. See src/media/base/pipeline.h for more information.
diff --git a/media/base/filters.cc b/media/base/filters.cc
index 5682be3..d5bd6d7 100644
--- a/media/base/filters.cc
+++ b/media/base/filters.cc
@@ -57,34 +57,4 @@ void Filter::Seek(base::TimeDelta time, const PipelineStatusCB& callback) {
 void Filter::OnAudioRendererDisabled() {
 }
 
-VideoDecoder::VideoDecoder() {}
-
-VideoDecoder::~VideoDecoder() {}
-
-// TODO(xhwang): Remove the following four functions when VideoDecoder is not a
-// Filter any more. See bug: http://crbug.com/108340
-void VideoDecoder::Play(const base::Closure& /* callback */) {
-  LOG(FATAL) << "VideoDecoder::Play is not supposed to be called.";
-}
-
-void VideoDecoder::Pause(const base::Closure& /* callback */) {
-  LOG(FATAL) << "VideoDecoder::Pause is not supposed to be called.";
-}
-
-void VideoDecoder::Seek(base::TimeDelta /* time */,
-                        const PipelineStatusCB& /* callback */) {
-  LOG(FATAL) << "VideoDecoder::Seek is not supposed to be called.";
-}
-
-FilterHost* VideoDecoder::host() {
-  LOG(FATAL) << "VideoDecoder::host is not supposed to be called.";
-  return NULL;
-}
-
-bool VideoDecoder::HasAlpha() const {
-  return false;
-}
-
-void VideoDecoder::PrepareForShutdownHack() {}
-
 }  // namespace media
diff --git a/media/base/filters.h b/media/base/filters.h
index 0126995..c96d82f 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -30,11 +30,9 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/time.h"
-#include "media/base/channel_layout.h"
 #include "media/base/media_export.h"
 #include "media/base/pipeline_status.h"
 #include "media/base/video_frame.h"
-#include "ui/gfx/size.h"
 
 namespace media {
 
@@ -44,6 +42,7 @@ class Decoder;
 class DemuxerStream;
 class Filter;
 class FilterHost;
+class VideoDecoder;
 
 class MEDIA_EXPORT Filter : public base::RefCountedThreadSafe<Filter> {
  public:
@@ -107,77 +106,6 @@ class MEDIA_EXPORT Filter : public base::RefCountedThreadSafe<Filter> {
   DISALLOW_COPY_AND_ASSIGN(Filter);
 };
 
-class MEDIA_EXPORT VideoDecoder : public Filter {
- public:
-  // Status codes for read operations on VideoDecoder.
-  enum DecoderStatus {
-    kOk,  // Everything went as planned.
-    kDecodeError,  // Decoding error happened.
-    kDecryptError  // Decrypting error happened.
-  };
-
-  // Initialize a VideoDecoder with the given DemuxerStream, executing the
-  // callback upon completion.
-  // statistics_cb is used to update global pipeline statistics.
-  virtual void Initialize(DemuxerStream* stream,
-                          const PipelineStatusCB& status_cb,
-                          const StatisticsCB& statistics_cb) = 0;
-
-  // Requests a frame to be decoded. The status of the decoder and decoded frame
-  // are returned via the provided callback. Only one read may be in flight at
-  // any given time.
-  //
-  // Implementations guarantee that the callback will not be called from within
-  // this method.
-  //
-  // If the returned status is not kOk, some error has occurred in the video
-  // decoder. In this case, the returned frame should always be NULL.
-  //
-  // Otherwise, the video decoder is in good shape. In this case, Non-NULL
-  // frames contain decoded video data or may indicate the end of the stream.
-  // NULL video frames indicate an aborted read. This can happen if the
-  // DemuxerStream gets flushed and doesn't have any more data to return.
-  typedef base::Callback<void(DecoderStatus, scoped_refptr<VideoFrame>)> ReadCB;
-  virtual void Read(const ReadCB& read_cb) = 0;
-
-  // Returns the natural width and height of decoded video in pixels.
-  //
-  // Clients should NOT rely on these values to remain constant. Instead, use
-  // the width/height from decoded video frames themselves.
-  //
-  // TODO(scherkus): why not rely on prerolling and decoding a single frame to
-  // get dimensions?
-  virtual const gfx::Size& natural_size() = 0;
-
-  // Returns true if the output format has an alpha channel. Most formats do not
-  // have alpha so the default is false. Override and return true for decoders
-  // that return formats with an alpha channel.
-  virtual bool HasAlpha() const;
-
-  // Prepare decoder for shutdown.  This is a HACK needed because
-  // PipelineImpl::Stop() goes through a Pause/Flush/Stop dance to all its
-  // filters, waiting for each state transition to complete before starting the
-  // next, but WebMediaPlayerImpl::Destroy() holds the renderer loop hostage for
-  // the duration.  Default implementation does nothing; derived decoders may
-  // override as needed.  http://crbug.com/110228 tracks removing this.
-  virtual void PrepareForShutdownHack();
-
- protected:
-  VideoDecoder();
-  virtual ~VideoDecoder();
-
- private:
-  // These functions will be removed later. Declare here to make sure they are
-  // not called from VideoDecoder interface anymore.
-  // TODO(xhwang): Remove them when VideoDecoder is not a Filter any more.
-  // See bug: http://crbug.com/108340
-  virtual void Play(const base::Closure& callback) OVERRIDE;
-  virtual void Pause(const base::Closure& callback) OVERRIDE;
-  virtual void Seek(base::TimeDelta time,
-                    const PipelineStatusCB& callback) OVERRIDE;
-  virtual FilterHost* host() OVERRIDE;
-};
-
 class MEDIA_EXPORT VideoRenderer : public Filter {
  public:
   // Used to update the pipeline's clock time. The parameter is the time that
diff --git a/media/base/mock_filters.cc b/media/base/mock_filters.cc
index 258e396..00347cd 100644
--- a/media/base/mock_filters.cc
+++ b/media/base/mock_filters.cc
@@ -84,10 +84,6 @@ scoped_ptr<FilterCollection> MockFilterCollection::Create() {
   return collection.Pass();
 }
 
-void RunFilterCallback(::testing::Unused, const base::Closure& closure) {
-  closure.Run();
-}
-
 void RunPipelineStatusCB(const PipelineStatusCB& status_cb) {
   status_cb.Run(PIPELINE_OK);
 }
@@ -107,7 +103,7 @@ void RunPipelineStatusCB4(::testing::Unused, const PipelineStatusCB& status_cb,
   status_cb.Run(PIPELINE_OK);
 }
 
-void RunStopFilterCallback(const base::Closure& closure) {
+void RunClosure(const base::Closure& closure) {
   closure.Run();
 }
 
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 409d09d..a5060d3 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -22,6 +22,7 @@
 #include "media/base/filters.h"
 #include "media/base/filter_collection.h"
 #include "media/base/pipeline.h"
+#include "media/base/video_decoder.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -146,18 +147,13 @@ class MockVideoDecoder : public VideoDecoder {
  public:
   MockVideoDecoder();
 
-  // Filter implementation.
-  MOCK_METHOD1(Flush, void(const base::Closure& callback));
-  MOCK_METHOD1(Stop, void(const base::Closure& callback));
-  MOCK_METHOD1(SetPlaybackRate, void(float playback_rate));
-  MOCK_METHOD2(Seek, void(base::TimeDelta time, const PipelineStatusCB& cb));
-  MOCK_METHOD0(OnAudioRendererDisabled, void());
-
   // VideoDecoder implementation.
-  MOCK_METHOD3(Initialize, void(DemuxerStream* stream,
-                                const PipelineStatusCB& status_cb,
-                                const StatisticsCB& statistics_cb));
-  MOCK_METHOD1(Read, void(const ReadCB& read_cb));
+  MOCK_METHOD3(Initialize, void(const scoped_refptr<DemuxerStream>&,
+                                const PipelineStatusCB&,
+                                const StatisticsCB&));
+  MOCK_METHOD1(Read, void(const ReadCB&));
+  MOCK_METHOD1(Reset, void(const base::Closure&));
+  MOCK_METHOD1(Stop, void(const base::Closure&));
   MOCK_METHOD0(natural_size, const gfx::Size&());
   MOCK_CONST_METHOD0(HasAlpha, bool());
 
@@ -274,9 +270,8 @@ class MockFilterCollection {
 };
 
 // Helper gmock functions that immediately executes and destroys the
-// Closure on behalf of the provided filter.  Can be used when mocking
+// Closure on behalf of the provided filter. Can be used when mocking
 // the Initialize() and Seek() methods.
-void RunFilterCallback(::testing::Unused, const base::Closure& closure);
 void RunPipelineStatusCB(const PipelineStatusCB& status_cb);
 void RunPipelineStatusCB2(::testing::Unused, const PipelineStatusCB& status_cb);
 void RunPipelineStatusCB3(::testing::Unused, const PipelineStatusCB& status_cb,
@@ -284,8 +279,8 @@ void RunPipelineStatusCB3(::testing::Unused, const PipelineStatusCB& status_cb,
 void RunPipelineStatusCB4(::testing::Unused, const PipelineStatusCB& status_cb,
                           ::testing::Unused, ::testing::Unused);
 // Helper gmock function that immediately executes the Closure on behalf of the
-// provided filter.  Can be used when mocking the Stop() method.
-void RunStopFilterCallback(const base::Closure& closure);
+// provided filter. Can be used when mocking the Stop() method.
+void RunClosure(const base::Closure& closure);
 
 // Helper gmock action that calls SetError() on behalf of the provided filter.
 ACTION_P2(SetError, filter, error) {
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 719f579..c231910 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -21,6 +21,7 @@
 #include "media/base/filter_collection.h"
 #include "media/base/filters.h"
 #include "media/base/media_log.h"
+#include "media/base/video_decoder.h"
 
 namespace media {
 
diff --git a/media/base/pipeline_unittest.cc b/media/base/pipeline_unittest.cc
index a18c55f..dc11d8c 100644
--- a/media/base/pipeline_unittest.cc
+++ b/media/base/pipeline_unittest.cc
@@ -113,7 +113,7 @@ class PipelineTest : public ::testing::Test {
                         Invoke(&RunPipelineStatusCB2)));
     EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->demuxer(), Stop(_))
-        .WillOnce(Invoke(&RunStopFilterCallback));
+        .WillOnce(Invoke(&RunClosure));
 
     // Demuxer properties.
     EXPECT_CALL(*mocks_->demuxer(), GetBitrate())
@@ -145,7 +145,7 @@ class PipelineTest : public ::testing::Test {
   }
 
   // Sets up expectations to allow the video decoder to initialize.
-  void InitializeVideoDecoder(MockDemuxerStream* stream) {
+  void InitializeVideoDecoder(const scoped_refptr<DemuxerStream>& stream) {
     EXPECT_CALL(*mocks_->video_decoder(),
                 Initialize(stream, _, _))
         .WillOnce(Invoke(&RunPipelineStatusCB3));
@@ -167,7 +167,7 @@ class PipelineTest : public ::testing::Test {
                 Seek(mocks_->demuxer()->GetStartTime(), _))
         .WillOnce(Invoke(&RunPipelineStatusCB2));
     EXPECT_CALL(*mocks_->video_renderer(), Stop(_))
-        .WillOnce(Invoke(&RunStopFilterCallback));
+        .WillOnce(Invoke(&RunClosure));
   }
 
   // Sets up expectations to allow the audio renderer to initialize.
@@ -187,7 +187,7 @@ class PipelineTest : public ::testing::Test {
     EXPECT_CALL(*mocks_->audio_renderer(), Seek(base::TimeDelta(), _))
         .WillOnce(Invoke(&RunPipelineStatusCB2));
     EXPECT_CALL(*mocks_->audio_renderer(), Stop(_))
-        .WillOnce(Invoke(&RunStopFilterCallback));
+        .WillOnce(Invoke(&RunClosure));
   }
 
   // Sets up expectations on the callback and initializes the pipeline.  Called
@@ -304,7 +304,7 @@ TEST_F(PipelineTest, NeverInitializes) {
   // Don't execute the callback passed into Initialize().
   EXPECT_CALL(*mocks_->demuxer(), Initialize(_, _));
   EXPECT_CALL(*mocks_->demuxer(), Stop(_))
-      .WillOnce(Invoke(&RunStopFilterCallback));
+      .WillOnce(Invoke(&RunClosure));
 
   // This test hangs during initialization by never calling
   // InitializationComplete().  StrictMock<> will ensure that the callback is
@@ -346,7 +346,7 @@ TEST_F(PipelineTest, URLNotFound) {
   EXPECT_CALL(*mocks_->demuxer(), Initialize(_, _))
       .WillOnce(InitializeDemuxerWithError(PIPELINE_ERROR_URL_NOT_FOUND));
   EXPECT_CALL(*mocks_->demuxer(), Stop(_))
-      .WillOnce(Invoke(&RunStopFilterCallback));
+      .WillOnce(Invoke(&RunClosure));
 
   InitializePipeline(PIPELINE_ERROR_URL_NOT_FOUND);
   EXPECT_FALSE(pipeline_->IsInitialized());
@@ -356,7 +356,7 @@ TEST_F(PipelineTest, NoStreams) {
   EXPECT_CALL(*mocks_->demuxer(), Initialize(_, _))
       .WillOnce(Invoke(&RunPipelineStatusCB2));
   EXPECT_CALL(*mocks_->demuxer(), Stop(_))
-      .WillOnce(Invoke(&RunStopFilterCallback));
+      .WillOnce(Invoke(&RunClosure));
 
   InitializePipeline(PIPELINE_ERROR_COULD_NOT_RENDER);
   EXPECT_FALSE(pipeline_->IsInitialized());
diff --git a/media/base/video_decoder.cc b/media/base/video_decoder.cc
new file mode 100644
index 0000000..b5100b8
--- /dev/null
+++ b/media/base/video_decoder.cc
@@ -0,0 +1,19 @@
+// Copyright (c) 2012 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/base/video_decoder.h"
+
+namespace media {
+
+VideoDecoder::VideoDecoder() {}
+
+VideoDecoder::~VideoDecoder() {}
+
+bool VideoDecoder::HasAlpha() const {
+  return false;
+}
+
+void VideoDecoder::PrepareForShutdownHack() {}
+
+}  // namespace media
diff --git a/media/base/video_decoder.h b/media/base/video_decoder.h
new file mode 100644
index 0000000..61b626e
--- /dev/null
+++ b/media/base/video_decoder.h
@@ -0,0 +1,95 @@
+// Copyright (c) 2012 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_BASE_VIDEO_DECODER_H_
+#define MEDIA_BASE_VIDEO_DECODER_H_
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "media/base/pipeline_status.h"
+#include "media/base/media_export.h"
+#include "ui/gfx/size.h"
+
+namespace media {
+
+class Buffer;
+class DemuxerStream;
+class VideoFrame;
+
+class MEDIA_EXPORT VideoDecoder
+    : public base::RefCountedThreadSafe<VideoDecoder> {
+ public:
+  // Status codes for read operations on VideoDecoder.
+  enum DecoderStatus {
+    kOk,  // Everything went as planned.
+    kDecodeError,  // Decoding error happened.
+    kDecryptError  // Decrypting error happened.
+  };
+
+  // Initialize a VideoDecoder with the given DemuxerStream, executing the
+  // callback upon completion.
+  // statistics_cb is used to update global pipeline statistics.
+  virtual void Initialize(const scoped_refptr<DemuxerStream>& stream,
+                          const PipelineStatusCB& status_cb,
+                          const StatisticsCB& statistics_cb) = 0;
+
+  // Requests a frame to be decoded. The status of the decoder and decoded frame
+  // are returned via the provided callback. Only one read may be in flight at
+  // any given time.
+  //
+  // Implementations guarantee that the callback will not be called from within
+  // this method.
+  //
+  // If the returned status is not kOk, some error has occurred in the video
+  // decoder. In this case, the returned frame should always be NULL.
+  //
+  // Otherwise, the video decoder is in good shape. In this case, Non-NULL
+  // frames contain decoded video data or may indicate the end of the stream.
+  // NULL video frames indicate an aborted read. This can happen if the
+  // DemuxerStream gets flushed and doesn't have any more data to return.
+  typedef base::Callback<void(DecoderStatus, scoped_refptr<VideoFrame>)> ReadCB;
+  virtual void Read(const ReadCB& read_cb) = 0;
+
+  // Reset decoder state, fulfilling all pending ReadCB and dropping extra
+  // queued decoded data. After this call, the decoder is back to an initialized
+  // clean state.
+  virtual void Reset(const base::Closure& closure) = 0;
+
+  // Stop decoder and set it to an uninitialized state. Note that a VideoDecoder
+  // should/could not be re-initialized after it has been stopped.
+  virtual void Stop(const base::Closure& closure) = 0;
+
+  // Returns the natural width and height of decoded video in pixels.
+  //
+  // Clients should NOT rely on these values to remain constant. Instead, use
+  // the width/height from decoded video frames themselves.
+  //
+  // TODO(scherkus): why not rely on prerolling and decoding a single frame to
+  // get dimensions?
+  virtual const gfx::Size& natural_size() = 0;
+
+  // Returns true if the output format has an alpha channel. Most formats do not
+  // have alpha so the default is false. Override and return true for decoders
+  // that return formats with an alpha channel.
+  virtual bool HasAlpha() const;
+
+  // Prepare decoder for shutdown.  This is a HACK needed because
+  // PipelineImpl::Stop() goes through a Pause/Flush/Stop dance to all its
+  // filters, waiting for each state transition to complete before starting the
+  // next, but WebMediaPlayerImpl::Destroy() holds the renderer loop hostage for
+  // the duration.  Default implementation does nothing; derived decoders may
+  // override as needed.  http://crbug.com/110228 tracks removing this.
+  virtual void PrepareForShutdownHack();
+
+ protected:
+  friend class base::RefCountedThreadSafe<VideoDecoder>;
+  virtual ~VideoDecoder();
+  VideoDecoder();
+
+  DISALLOW_COPY_AND_ASSIGN(VideoDecoder);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_VIDEO_DECODER_H_