Remove refcounting of VideoDecodeEngine

VideoDecodeEngine doesn't need refcounting, all decode engines live on one
thread so there isn't a problem posting task to a random thread. Also
the stop is asynchronous, we can always until the stop is done before we
destroy the object and doesn't need refcounting to destroy it.

TEST=media_unittests

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

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

11 files changed, 54 insertions, 42 deletions

diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index eb57493..f43d90b 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -432,7 +432,7 @@ void FFmpegVideoDecoder::FlushBuffers() {
 
 void FFmpegVideoDecoder::SetVideoDecodeEngineForTest(
     VideoDecodeEngine* engine) {
-  decode_engine_ = engine;
+  decode_engine_.reset(engine);
 }
 
 // static
diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h
index e898266..8a9e5e5 100644
--- a/media/filters/ffmpeg_video_decoder.h
+++ b/media/filters/ffmpeg_video_decoder.h
@@ -121,7 +121,7 @@ class FFmpegVideoDecoder : public VideoDecoder,
   TimeTuple last_pts_;
   scoped_ptr<AVRational> time_base_;  // Pointer to avoid needing full type.
   DecoderState state_;
-  scoped_refptr<VideoDecodeEngine> decode_engine_;
+  scoped_ptr<VideoDecodeEngine> decode_engine_;
 
   scoped_ptr<FilterCallback> initialize_callback_;
   scoped_ptr<FilterCallback> uninitialize_callback_;
diff --git a/media/filters/omx_video_decoder.cc b/media/filters/omx_video_decoder.cc
index 0a6d9910..c450b43 100644
--- a/media/filters/omx_video_decoder.cc
+++ b/media/filters/omx_video_decoder.cc
@@ -254,3 +254,7 @@ void OmxVideoDecoder::DemuxCompleteTask(Buffer* buffer) {
 }
 
 }  // namespace media
+
+// Disable refcounting for the decode engine because it only lives on the
+// video decoder thread.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(media::VideoDecodeEngine);
diff --git a/media/filters/omx_video_decoder.h b/media/filters/omx_video_decoder.h
index 26efc38..c82d930 100644
--- a/media/filters/omx_video_decoder.h
+++ b/media/filters/omx_video_decoder.h
@@ -17,6 +17,7 @@ class MessageLoop;
 namespace media {
 
 class Buffer;
+class OmxVideoDecodeEngine;
 class VideoFrame;
 
 class OmxVideoDecoder : public VideoDecoder,
@@ -53,7 +54,7 @@ class OmxVideoDecoder : public VideoDecoder,
 
   // Pointer to the demuxer stream that will feed us compressed buffers.
   scoped_refptr<DemuxerStream> demuxer_stream_;
-  scoped_refptr<VideoDecodeEngine> omx_engine_;
+  scoped_ptr<VideoDecodeEngine> omx_engine_;
   MediaFormat media_format_;
   size_t width_;
   size_t height_;
diff --git a/media/mf/mft_h264_decoder_example.cc b/media/mf/mft_h264_decoder_example.cc
index 25c2947..4f5cd64 100644
--- a/media/mf/mft_h264_decoder_example.cc
+++ b/media/mf/mft_h264_decoder_example.cc
@@ -151,7 +151,7 @@ class MftH264DecoderHandler
     info_.stream_info_ = stream_info;
   }
   virtual void OnEmptyBufferCallback(scoped_refptr<Buffer> buffer) {
-    if (reader_ && decoder_.get()) {
+    if (reader_ && decoder_) {
       scoped_refptr<DataBuffer> input;
       reader_->Read(&input);
       if (!input->IsEndOfStream())
@@ -169,8 +169,8 @@ class MftH264DecoderHandler
   virtual void SetReader(FFmpegFileReader* reader) {
     reader_ = reader;
   }
-  virtual void SetDecoder(scoped_refptr<MftH264Decoder> decoder) {
-    decoder_ = decoder;
+  virtual void SetDecoder(MftH264Decoder* decoder) {
+    decoder_= decoder;
   }
   virtual void DecodeSingleFrame() {
     scoped_refptr<VideoFrame> frame;
@@ -185,7 +185,7 @@ class MftH264DecoderHandler
   int frames_read_;
   int frames_decoded_;
   FFmpegFileReader* reader_;
-  scoped_refptr<MftH264Decoder> decoder_;
+  MftH264Decoder* decoder_;
 };
 
 class RenderToWindowHandler : public MftH264DecoderHandler {
@@ -295,7 +295,7 @@ static int Run(bool use_dxva, bool render, const std::string& input_file) {
     }
   }
 
-  scoped_refptr<MftH264Decoder> mft(new MftH264Decoder(use_dxva));
+  scoped_ptr<MftH264Decoder> mft(new MftH264Decoder(use_dxva));
   if (!mft.get()) {
     LOG(ERROR) << "Failed to create fake MFT";
     return -1;
@@ -306,7 +306,7 @@ static int Run(bool use_dxva, bool render, const std::string& input_file) {
     handler = new RenderToWindowHandler(window, MessageLoop::current());
   else
     handler = new MftH264DecoderHandler();
-  handler->SetDecoder(mft);
+  handler->SetDecoder(mft.get());
   handler->SetReader(reader.get());
   if (!handler.get()) {
     LOG(ERROR) << "FAiled to create handler";
diff --git a/media/mf/test/mft_h264_decoder_unittest.cc b/media/mf/test/mft_h264_decoder_unittest.cc
index c14a3e41..e805127 100644
--- a/media/mf/test/mft_h264_decoder_unittest.cc
+++ b/media/mf/test/mft_h264_decoder_unittest.cc
@@ -128,7 +128,7 @@ class SimpleMftH264DecoderHandler : public VideoDecodeEngine::EventHandler {
     info_.stream_info_ = stream_info;
   }
   virtual void OnEmptyBufferCallback(scoped_refptr<Buffer> buffer) {
-    if (reader_.get() && decoder_.get()) {
+    if (reader_.get() && decoder_) {
       empty_buffer_callback_count_++;
       scoped_refptr<DataBuffer> input;
       reader_->ReadCallback(&input);
@@ -142,7 +142,7 @@ class SimpleMftH264DecoderHandler : public VideoDecodeEngine::EventHandler {
   void SetReader(scoped_refptr<BaseMftReader> reader) {
     reader_ = reader;
   }
-  void SetDecoder(scoped_refptr<MftH264Decoder> decoder) {
+  void SetDecoder(MftH264Decoder* decoder) {
     decoder_ = decoder;
   }
 
@@ -154,7 +154,7 @@ class SimpleMftH264DecoderHandler : public VideoDecodeEngine::EventHandler {
   int fill_buffer_callback_count_;
   VideoCodecInfo info_;
   scoped_refptr<BaseMftReader> reader_;
-  scoped_refptr<MftH264Decoder> decoder_;
+  MftH264Decoder* decoder_;
   scoped_refptr<VideoFrame> current_frame_;
 };
 
@@ -165,7 +165,7 @@ TEST_F(MftH264DecoderTest, LibraryInit) {
 }
 
 TEST_F(MftH264DecoderTest, DecoderUninitializedAtFirst) {
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(true));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(true));
   ASSERT_TRUE(decoder.get());
   EXPECT_EQ(MftH264Decoder::kUninitialized, decoder->state());
 }
@@ -174,7 +174,7 @@ TEST_F(MftH264DecoderTest, DecoderInitMissingArgs) {
   VideoCodecConfig config;
   config.width_ = 800;
   config.height_ = 600;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(NULL, NULL, config);
   EXPECT_EQ(MftH264Decoder::kUninitialized, decoder->state());
@@ -186,7 +186,7 @@ TEST_F(MftH264DecoderTest, DecoderInitNoDxva) {
   VideoCodecConfig config;
   config.width_ = 800;
   config.height_ = 600;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(1, handler.init_count_);
@@ -200,7 +200,7 @@ TEST_F(MftH264DecoderTest, DecoderInitDxva) {
   VideoCodecConfig config;
   config.width_ = 800;
   config.height_ = 600;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(true));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(true));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(1, handler.init_count_);
@@ -214,7 +214,7 @@ TEST_F(MftH264DecoderTest, DecoderUninit) {
   VideoCodecConfig config;
   config.width_ = 800;
   config.height_ = 600;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
@@ -229,7 +229,7 @@ TEST_F(MftH264DecoderTest, UninitBeforeInit) {
   VideoCodecConfig config;
   config.width_ = 800;
   config.height_ = 600;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Uninitialize();
   EXPECT_EQ(0, handler.uninit_count_);
@@ -241,7 +241,7 @@ TEST_F(MftH264DecoderTest, InitWithNegativeDimensions) {
   VideoCodecConfig config;
   config.width_ = -123;
   config.height_ = -456;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
@@ -256,7 +256,7 @@ TEST_F(MftH264DecoderTest, InitWithTooHighDimensions) {
   VideoCodecConfig config;
   config.width_ = kDecoderMaxWidth + 1;
   config.height_ = kDecoderMaxHeight + 1;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
@@ -271,7 +271,7 @@ TEST_F(MftH264DecoderTest, DrainOnEmptyBuffer) {
   VideoCodecConfig config;
   config.width_ = 1024;
   config.height_ = 768;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
@@ -300,12 +300,12 @@ TEST_F(MftH264DecoderTest, NoOutputOnGarbageInput) {
   VideoCodecConfig config;
   config.width_ = 1024;
   config.height_ = 768;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
   handler.SetReader(reader);
-  handler.SetDecoder(decoder);
+  handler.SetDecoder(decoder.get());
   while (MftH264Decoder::kStopped != decoder->state()) {
     scoped_refptr<VideoFrame> frame;
     decoder->FillThisBuffer(frame);
@@ -328,7 +328,7 @@ TEST_F(MftH264DecoderTest, FlushAtStart) {
   VideoCodecConfig config;
   config.width_ = 1024;
   config.height_ = 768;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
@@ -348,12 +348,12 @@ TEST_F(MftH264DecoderTest, NoFlushAtStopped) {
   VideoCodecConfig config;
   config.width_ = 1024;
   config.height_ = 768;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(false));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(false));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
   handler.SetReader(reader);
-  handler.SetDecoder(decoder);
+  handler.SetDecoder(decoder.get());
   while (MftH264Decoder::kStopped != decoder->state()) {
     scoped_refptr<VideoFrame> frame;
     decoder->FillThisBuffer(frame);
@@ -391,12 +391,12 @@ void DecodeValidVideo(const std::string& filename, int num_frames, bool dxva) {
   VideoCodecConfig config;
   config.width_ = 1;
   config.height_ = 1;
-  scoped_refptr<MftH264Decoder> decoder(new MftH264Decoder(dxva));
+  scoped_ptr<MftH264Decoder> decoder(new MftH264Decoder(dxva));
   ASSERT_TRUE(decoder.get());
   decoder->Initialize(&loop, &handler, config);
   EXPECT_EQ(MftH264Decoder::kNormal, decoder->state());
   handler.SetReader(reader);
-  handler.SetDecoder(decoder);
+  handler.SetDecoder(decoder.get());
   while (MftH264Decoder::kStopped != decoder->state()) {
     scoped_refptr<VideoFrame> frame;
     decoder->FillThisBuffer(frame);
diff --git a/media/tools/omx_test/omx_test.cc b/media/tools/omx_test/omx_test.cc
index e48251c..f970c45 100644
--- a/media/tools/omx_test/omx_test.cc
+++ b/media/tools/omx_test/omx_test.cc
@@ -188,7 +188,7 @@ class TestApp : public base::RefCountedThreadSafe<TestApp>,
     config.opaque_context_ = NULL;
     config.width_ = av_stream_->codec->width;
     config.height_ = av_stream_->codec->height;
-    engine_ = new OmxVideoDecodeEngine();
+    engine_.reset(new OmxVideoDecodeEngine());
     engine_->Initialize(&message_loop_, this, config);
 
     // Execute the message loop so that we can run tasks on it. This call
@@ -220,7 +220,7 @@ class TestApp : public base::RefCountedThreadSafe<TestApp>,
     printf("\n");
   }
 
-  scoped_refptr<OmxVideoDecodeEngine> engine_;
+  scoped_ptr<OmxVideoDecodeEngine> engine_;
   MessageLoop message_loop_;
   scoped_ptr<AVStream> av_stream_;
   scoped_ptr<FileReader> file_reader_;
diff --git a/media/video/ffmpeg_video_decode_engine.cc b/media/video/ffmpeg_video_decode_engine.cc
index e71cb6f..e1908b6 100644
--- a/media/video/ffmpeg_video_decode_engine.cc
+++ b/media/video/ffmpeg_video_decode_engine.cc
@@ -357,3 +357,7 @@ VideoFrame::Format FFmpegVideoDecodeEngine::GetSurfaceFormat() const {
 }
 
 }  // namespace media
+
+// Disable refcounting for this object because this object only lives
+// on the video decoder thread and there's no need to refcount it.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(media::FFmpegVideoDecodeEngine);
diff --git a/media/video/ffmpeg_video_decode_engine_unittest.cc b/media/video/ffmpeg_video_decode_engine_unittest.cc
index 73ac917..b7856d8 100644
--- a/media/video/ffmpeg_video_decode_engine_unittest.cc
+++ b/media/video/ffmpeg_video_decode_engine_unittest.cc
@@ -57,7 +57,7 @@ class FFmpegVideoDecodeEngineTest : public testing::Test,
     // Initialize MockFFmpeg.
     MockFFmpeg::set(&mock_ffmpeg_);
 
-    test_engine_ = new FFmpegVideoDecodeEngine();
+    test_engine_.reset(new FFmpegVideoDecodeEngine());
     test_engine_->SetCodecContextForTest(&codec_context_);
 
     VideoFrame::CreateFrame(VideoFrame::YV12,
@@ -69,7 +69,7 @@ class FFmpegVideoDecodeEngineTest : public testing::Test,
   }
 
   ~FFmpegVideoDecodeEngineTest() {
-    test_engine_ = NULL;
+    test_engine_.reset();
     MockFFmpeg::set(NULL);
   }
 
@@ -112,7 +112,7 @@ class FFmpegVideoDecodeEngineTest : public testing::Test,
   VideoCodecConfig config_;
   VideoCodecInfo info_;
  protected:
-  scoped_refptr<FFmpegVideoDecodeEngine> test_engine_;
+  scoped_ptr<FFmpegVideoDecodeEngine> test_engine_;
   scoped_array<uint8_t> frame_buffer_;
   StrictMock<MockFFmpeg> mock_ffmpeg_;
 
diff --git a/media/video/omx_video_decode_engine.cc b/media/video/omx_video_decode_engine.cc
index b6d7fea..c893d7d 100644
--- a/media/video/omx_video_decode_engine.cc
+++ b/media/video/omx_video_decode_engine.cc
@@ -7,12 +7,11 @@
 //
 // THREADING SEMANTICS
 //
-// This class is created by VideoDecoderImpl and lives on the thread
-// that VideoDecoderImpl lives. This class is given the message loop
+// This class is created by OmxVideoDecoder and lives on the thread
+// that it lives. This class is given the message loop
 // for the above thread. The OMX callbacks are guaranteed to be
-// executed on the hosting message loop. This essentially means that
-// all methods in this class are executed on the same thread as
-// VideoDecoderImpl. Because of that there's no need for locking anywhere.
+// executed on the hosting message loop. Because of that there's no need
+// for locking anywhere.
 
 #include "media/video/omx_video_decode_engine.h"
 
@@ -1337,3 +1336,7 @@ OMX_ERRORTYPE OmxVideoDecodeEngine::FillBufferCallback(
 }
 
 }  // namespace media
+
+// Disable refcounting for this object because this object only lives
+// on the video decoder thread and there's no need to refcount it.
+DISABLE_RUNNABLE_METHOD_REFCOUNT(media::OmxVideoDecodeEngine);
diff --git a/media/video/video_decode_engine.h b/media/video/video_decode_engine.h
index d2cf7b2..9c84f11 100644
--- a/media/video/video_decode_engine.h
+++ b/media/video/video_decode_engine.h
@@ -68,7 +68,7 @@ struct VideoCodecInfo {
   VideoStreamInfo stream_info_;
 };
 
-class VideoDecodeEngine : public base::RefCountedThreadSafe<VideoDecodeEngine> {
+class VideoDecodeEngine {
  public:
   struct EventHandler {
    public:
@@ -83,7 +83,6 @@ class VideoDecodeEngine : public base::RefCountedThreadSafe<VideoDecodeEngine> {
     virtual void OnFillBufferCallback(scoped_refptr<VideoFrame> frame) = 0;
   };
 
-  VideoDecodeEngine() {}
   virtual ~VideoDecodeEngine() {}
 
   // Initialized the engine with specified configuration. |message_loop| could
@@ -103,8 +102,9 @@ class VideoDecodeEngine : public base::RefCountedThreadSafe<VideoDecodeEngine> {
   // could be itself. ) then call EventHandler::OnFlushDone().
   virtual void Flush() = 0;
 
-  // Used in openmax to InitialReadBuffers().
-  virtual void Seek() = 0;  // TODO(jiesun): Do we need this?
+  // This method is used as a signal for the decode engine to prefoll and
+  // issue read requests after Flush() us made.
+  virtual void Seek() = 0;
 
   // Buffer exchange method for input and output stream.
   // These functions and callbacks could be used in two scenarios for both