Simplify VideoDecodeEngine interface by making everything synchronous.

media/filters/ffmpeg_video_decoder.cc

 // 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/ffmpeg_video_decoder.h"
 
 #include "base/bind.h"
-#include "base/callback.h"
 #include "base/message_loop.h"
-#include "base/task.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/filter_host.h"
 #include "media/base/limits.h"
+#include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
 #include "media/ffmpeg/ffmpeg_common.h"
 #include "media/video/ffmpeg_video_decode_engine.h"
 
 namespace media {
 
 FFmpegVideoDecoder::FFmpegVideoDecoder(MessageLoop* message_loop)
     : message_loop_(message_loop),
-      state_(kUnInitialized),
+      state_(kUninitialized),
       decode_engine_(new FFmpegVideoDecodeEngine()) {
 }
 
 FFmpegVideoDecoder::~FFmpegVideoDecoder() {}
 
 void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
                                     const base::Closure& callback,
                                     const StatisticsCallback& stats_callback) {
   if (MessageLoop::current() != message_loop_) {
-    message_loop_->PostTask(
-        FROM_HERE,
-        base::Bind(&FFmpegVideoDecoder::Initialize, this,
-                   make_scoped_refptr(demuxer_stream),
-                   callback, stats_callback));
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &FFmpegVideoDecoder::Initialize, this,
+        make_scoped_refptr(demuxer_stream), callback, stats_callback));
     return;
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(!demuxer_stream_);
-  DCHECK(initialize_callback_.is_null());
 
   if (!demuxer_stream) {
     host()->SetError(PIPELINE_ERROR_DECODE);
     callback.Run();
     return;
   }
 
   demuxer_stream_ = demuxer_stream;
-  initialize_callback_ = callback;
   statistics_callback_ = stats_callback;
 
   const VideoDecoderConfig& config = demuxer_stream->video_decoder_config();
 
   pts_stream_.Initialize(GetFrameDuration(config));
 
   natural_size_ = config.natural_size();
   if (natural_size_.width() > Limits::kMaxDimension ||
       natural_size_.height() > Limits::kMaxDimension ||
       natural_size_.GetArea() > Limits::kMaxCanvas) {
-    OnInitializeComplete(false);
+    host()->SetError(PIPELINE_ERROR_DECODE);
+    callback.Run();

[acolwell GONE FROM CHROMIUM, 2011/10/28 17:38:47]

I'd like to see callback converted to a PipelineStatusCB. Do it now or add a
TODO so this can happen in another CL.

[scherkus (not reviewing), 2011/11/01 04:18:26]

TODO added in filters.h

there are a few related filters.h TODOs I want to do (scoped_refptr<> for
buffers) so I'll tackle them in a future CL

     return;
   }
 
-  state_ = kInitializing;
-  decode_engine_->Initialize(this, config);
-}
+  if (!decode_engine_->Initialize(config)) {
+    host()->SetError(PIPELINE_ERROR_DECODE);
+    callback.Run();
+    return;
+  }
 
-void FFmpegVideoDecoder::OnInitializeComplete(bool success) {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(!initialize_callback_.is_null());
-
-  if (success) {
-    state_ = kNormal;
-  } else {
-    host()->SetError(PIPELINE_ERROR_DECODE);
-  }
-  ResetAndRunCB(&initialize_callback_);
+  state_ = kNormal;
+  callback.Run();
 }
 
 void FFmpegVideoDecoder::Stop(const base::Closure& callback) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &FFmpegVideoDecoder::Stop, this, callback));
     return;
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(uninitialize_callback_.is_null());
 
-  uninitialize_callback_ = callback;
-  if (state_ != kUnInitialized)
-    decode_engine_->Uninitialize();
-  else
-    OnUninitializeComplete();
+  decode_engine_->Uninitialize();
+  state_ = kUninitialized;
+  callback.Run();
 }
 
-void FFmpegVideoDecoder::OnUninitializeComplete() {
+void FFmpegVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
+  if (MessageLoop::current() != message_loop_) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &FFmpegVideoDecoder::Seek, this, time, cb));
+    return;
+  }
+
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(!uninitialize_callback_.is_null());
 
-  state_ = kStopped;
-
-  ResetAndRunCB(&uninitialize_callback_);
+  pts_stream_.Seek(time);
+  cb.Run(PIPELINE_OK);
 }
 
 void FFmpegVideoDecoder::Pause(const base::Closure& callback) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &FFmpegVideoDecoder::Pause, this, callback));
     return;
   }
 
-  state_ = kPausing;
   callback.Run();
 }
 
 void FFmpegVideoDecoder::Flush(const base::Closure& callback) {
   if (MessageLoop::current() != message_loop_) {
     message_loop_->PostTask(FROM_HERE, base::Bind(
         &FFmpegVideoDecoder::Flush, this, callback));
     return;
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(flush_callback_.is_null());
-
-  state_ = kFlushing;
-
-  FlushBuffers();
-
-  flush_callback_ = callback;
 
   decode_engine_->Flush();
+  pts_stream_.Flush();
+  state_ = kNormal;
+  callback.Run();
 }
 
-void FFmpegVideoDecoder::OnFlushComplete() {
+void FFmpegVideoDecoder::DoDecodeBuffer(const scoped_refptr<Buffer>& buffer) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(!flush_callback_.is_null());
-
-  // Everything in the presentation time queue is invalid, clear the queue.
-  pts_stream_.Flush();
-
-  // Mark flush operation had been done.
-  state_ = kNormal;
-
-  ResetAndRunCB(&flush_callback_);
-}
-
-void FFmpegVideoDecoder::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
-  if (MessageLoop::current() != message_loop_) {
-     message_loop_->PostTask(FROM_HERE,
-                             base::Bind(&FFmpegVideoDecoder::Seek, this,
-                                               time, cb));
-     return;
-  }
-
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(seek_cb_.is_null());
-
-  pts_stream_.Seek(time);
-  seek_cb_ = cb;
-  decode_engine_->Seek();
-}
-
-void FFmpegVideoDecoder::OnSeekComplete() {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK(!seek_cb_.is_null());
-
-  ResetAndRunCB(&seek_cb_, PIPELINE_OK);
-}
-
-void FFmpegVideoDecoder::OnError() {
-  VideoFrameReady(NULL);
-}
-
-void FFmpegVideoDecoder::OnReadComplete(Buffer* buffer_in) {
-  scoped_refptr<Buffer> buffer(buffer_in);
-  message_loop_->PostTask(FROM_HERE, base::Bind(
-      &FFmpegVideoDecoder::OnReadCompleteTask, this, buffer));
-}
-
-void FFmpegVideoDecoder::OnReadCompleteTask(scoped_refptr<Buffer> buffer) {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK_NE(state_, kStopped);  // because of Flush() before Stop().
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kDecodeFinished);
 
   // During decode, because reads are issued asynchronously, it is possible to
   // receive multiple end of stream buffers since each read is acked. When the
   // first end of stream buffer is read, FFmpeg may still have frames queued
   // up in the decoder so we need to go through the decode loop until it stops
   // giving sensible data.  After that, the decoder should output empty
   // frames.  There are three states the decoder can be in:
   //
   //   kNormal: This is the starting state. Buffers are decoded. Decode errors
   //            are discarded.
   //   kFlushCodec: There isn't any more input data. Call avcodec_decode_video2
   //                until no more data is returned to flush out remaining
   //                frames. The input buffer is ignored at this point.
   //   kDecodeFinished: All calls return empty frames.
   //
   // These are the possible state transitions.
   //
   // kNormal -> kFlushCodec:
   //     When buffer->IsEndOfStream() is first true.
   // kNormal -> kDecodeFinished:
   //     A catastrophic failure occurs, and decoding needs to stop.
   // kFlushCodec -> kDecodeFinished:
   //     When avcodec_decode_video2() returns 0 data or errors out.
   // (any state) -> kNormal:
-  //     Any time buffer->IsDiscontinuous() is true.
+  //     Any time Flush() is called.
 
   // Transition to kFlushCodec on the first end of stream buffer.
   if (state_ == kNormal && buffer->IsEndOfStream()) {
     state_ = kFlushCodec;
   }
 
   // Push all incoming timestamps into the priority queue as long as we have
   // not yet received an end of stream buffer.  It is important that this line
   // stay below the state transition into kFlushCodec done above.
   if (state_ == kNormal) {
     pts_stream_.EnqueuePts(buffer.get());
   }
 
-  // Otherwise, attempt to decode a single frame.
-  decode_engine_->ConsumeVideoSample(buffer);
+  // XXXXXXXXXXXXXXX
+  // This will cause every frame to get allocated.
+  scoped_refptr<VideoFrame> video_frame;

[scherkus (not reviewing), 2011/10/28 16:54:04]

I'm not entirely sure if I should attempt to keep frames from being re-allocated

It would mean continuing to do a little bit of buffer recycling inside of
VideoRendererBase so the support is there today I just need to hold onto
VideoFrame pointers inside FFmpegVideoDecoder

[acolwell GONE FROM CHROMIUM, 2011/10/28 17:38:47]

I think we should remove the buffer recycling for now and add a new
implementation back in if we need to.

On 2011/10/28 16:54:04, scherkus wrote:

[scherkus (not reviewing), 2011/11/01 04:18:26]

as discussed offline it looks like there was no way out of it!

I changed it to one-read-request-at-a-time -- let me know if you have thoughts
otherwise

+
+  if (!decode_engine_->Decode(buffer, &video_frame)) {
+    state_ = kDecodeFinished;
+    host()->SetError(PIPELINE_ERROR_DECODE);
+    return;
+  }
+
+  // Any successful decode counts!
+  if (buffer->GetDataSize()) {
+    PipelineStatistics statistics;
+    statistics.video_bytes_decoded = buffer->GetDataSize();
+    statistics_callback_.Run(statistics);
+  }
+
+  // If we didn't get a frame then we've either completely finished decoding or
+  // we need more data.
+  if (!video_frame) {
+    if (state_ == kFlushCodec) {
+      state_ = kDecodeFinished;
+      VideoFrameReady(VideoFrame::CreateEmptyFrame());
+      return;
+    }
+
+    ReadFromDemuxerStream();
+    return;
+  }
+
+  // If we got a frame make sure its timestamp is correct before sending it off.
+  pts_stream_.UpdatePtsAndDuration(video_frame.get());
+  video_frame->SetTimestamp(pts_stream_.current_pts());
+  video_frame->SetDuration(pts_stream_.current_duration());
+
+  VideoFrameReady(video_frame);
 }
 
 void FFmpegVideoDecoder::ProduceVideoFrame(
     scoped_refptr<VideoFrame> video_frame) {
   if (MessageLoop::current() != message_loop_) {
-    if (state_ != kStopped) {
-      message_loop_->PostTask(FROM_HERE, base::Bind(
-          &FFmpegVideoDecoder::ProduceVideoFrame, this, video_frame));
-    }
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        &FFmpegVideoDecoder::ProduceVideoFrame, this, video_frame));
     return;
   }
 
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  // Synchronized flushing before stop should prevent this.
-  DCHECK_NE(state_, kStopped);
-
-  // If the decoding is finished, we just always return empty frames.
-  if (state_ == kDecodeFinished) {
-    // Signal VideoRenderer the end of the stream event.
-    VideoFrameReady(VideoFrame::CreateEmptyFrame());
-
-    // Fall through, because we still need to keep record of this frame.
+  // This can happen during shutdown after Stop() has been called.
+  if (state_ == kUninitialized) {
+    return;
   }
 
-  // Notify decode engine the available of new frame.
-  decode_engine_->ProduceVideoFrame(video_frame);
-}
+  // Return empty frames if decoding has finished.
+  if (state_ == kDecodeFinished) {
+    VideoFrameReady(VideoFrame::CreateEmptyFrame());
+    return;
+  }
 
-void FFmpegVideoDecoder::ConsumeVideoFrame(
-    scoped_refptr<VideoFrame> video_frame,
-    const PipelineStatistics& statistics) {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK_NE(state_, kStopped);
+  // XXX: Queue frame?
+  // XXXXXXXXXXXXX right now we drop video_frame on the ground, causing all
+  // frame to be re-allocated on the fly.

[scherkus (not reviewing), 2011/10/28 16:54:04]

this is where we could either:
1) cache the video frames locally or if we wanted to get fancy
2) use currying to carry the video_frame object via a Read() to DemuxerStream

I'm hesitant of (2) since I'm not 100% sure how object lifetimes would work out

[acolwell GONE FROM CHROMIUM, 2011/10/28 17:38:47]

Remove the video_frame pointer for now. I want to remove all of the frame
recycling code. Like I said before I think this can be best handled with a
factory and this code would never need to know that buffer recycling is going
on.

On 2011/10/28 16:54:04, scherkus wrote:

[scherkus (not reviewing), 2011/11/01 04:18:26]

Done.

 
-  statistics_callback_.Run(statistics);
-
-  if (video_frame.get()) {
-    if (kPausing == state_ || kFlushing == state_) {
-      frame_queue_flushed_.push_back(video_frame);
-      if (kFlushing == state_)
-        FlushBuffers();
-      return;
-    }
-
-    // If we actually got data back, enqueue a frame.
-    pts_stream_.UpdatePtsAndDuration(video_frame.get());
-
-    video_frame->SetTimestamp(pts_stream_.current_pts());
-    video_frame->SetDuration(pts_stream_.current_duration());
-
-    VideoFrameReady(video_frame);
-  } else {
-    // When in kFlushCodec, any errored decode, or a 0-lengthed frame,
-    // is taken as a signal to stop decoding.
-    if (state_ == kFlushCodec) {
-      state_ = kDecodeFinished;
-
-      // Signal VideoRenderer the end of the stream event.
-      VideoFrameReady(VideoFrame::CreateEmptyFrame());
-    }
-  }
-}
-
-void FFmpegVideoDecoder::ProduceVideoSample(
-    scoped_refptr<Buffer> buffer) {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-  DCHECK_NE(state_, kStopped);
-
-  demuxer_stream_->Read(base::Bind(&FFmpegVideoDecoder::OnReadComplete,
-                                   this));
+  ReadFromDemuxerStream();
 }
 
 gfx::Size FFmpegVideoDecoder::natural_size() {
   return natural_size_;
 }
 
-void FFmpegVideoDecoder::FlushBuffers() {
-  while (!frame_queue_flushed_.empty()) {
-    scoped_refptr<VideoFrame> video_frame;
-    video_frame = frame_queue_flushed_.front();
-    frame_queue_flushed_.pop_front();
+void FFmpegVideoDecoder::ReadFromDemuxerStream() {
+  DCHECK_NE(state_, kUninitialized);
+  DCHECK_NE(state_, kDecodeFinished);
 
-    // Return frames back to the decode engine.
-    decode_engine_->ProduceVideoFrame(video_frame);
-  }
+  demuxer_stream_->Read(base::Bind(&FFmpegVideoDecoder::DecodeBuffer, this));
 }
 
-void FFmpegVideoDecoder::SetVideoDecodeEngineForTest(
-    VideoDecodeEngine* engine) {
-  decode_engine_.reset(engine);
+void FFmpegVideoDecoder::DecodeBuffer(Buffer* buffer) {
+  // TODO(scherkus): change DemuxerStream::Read() to use scoped_refptr<> for
+  // callback.
+  message_loop_->PostTask(FROM_HERE, base::Bind(
+      &FFmpegVideoDecoder::DoDecodeBuffer, this, make_scoped_refptr(buffer)));
 }
 
 }  // namespace media