Refactor FFmpegVideoDecoder::OnDecode and unittest and add state tracking.

The decoder needs a concept of state to know when it should just stop attempting to decode.  This is because ffmpeg will sometimes gives spurious frames back, which plays badly with the time presentation timestamp calculation logic.

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

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

6 files changed, 645 insertions, 128 deletions

diff --git a/media/base/buffers.h b/media/base/buffers.h
index 6fa0cfe..def573e 100644
--- a/media/base/buffers.h
+++ b/media/base/buffers.h
@@ -132,6 +132,7 @@ struct VideoSurface {
   // http://www.fourcc.org/rgb.php
   // http://www.fourcc.org/yuv.php
   enum Format {
+    INVALID,     // Invalid format value.  Used for error reporting.
     RGB555,      // 16bpp RGB packed 5:5:5
     RGB565,      // 16bpp RGB packed 5:6:5
     RGB24,       // 24bpp RGB packed 8:8:8
diff --git a/media/base/mock_ffmpeg.cc b/media/base/mock_ffmpeg.cc
index b1de0c4..8a397de 100644
--- a/media/base/mock_ffmpeg.cc
+++ b/media/base/mock_ffmpeg.cc
@@ -62,22 +62,17 @@ int avcodec_thread_init(AVCodecContext* avctx, int threads) {
 }
 
 void avcodec_flush_buffers(AVCodecContext* avctx) {
-  NOTREACHED();
+  return media::MockFFmpeg::get()->AVCodecFlushBuffers(avctx);
 }
 
 AVFrame* avcodec_alloc_frame() {
-  NOTREACHED();
-  return NULL;
+  return media::MockFFmpeg::get()->AVCodecAllocFrame();
 }
 
 int avcodec_decode_video2(AVCodecContext* avctx, AVFrame* picture,
                           int* got_picture_ptr, AVPacket* avpkt) {
-  NOTREACHED();
-  return 0;
-}
-
-void av_init_packet(AVPacket* pkt) {
-  NOTREACHED();
+  return media::MockFFmpeg::get()->
+      AVCodecDecodeVideo2(avctx, picture, got_picture_ptr, avpkt);
 }
 
 int av_open_input_file(AVFormatContext** format, const char* filename,
@@ -100,14 +95,28 @@ int64 av_rescale_q(int64 a, AVRational bq, AVRational cq) {
   return a * num / den;
 }
 
-void av_free_packet(AVPacket* packet) {
-  media::MockFFmpeg::get()->AVFreePacket(packet);
+int av_read_frame(AVFormatContext* format, AVPacket* packet) {
+  return media::MockFFmpeg::get()->AVReadFrame(format, packet);
+}
+
+int av_seek_frame(AVFormatContext *format, int stream_index, int64_t timestamp,
+                  int flags) {
+  return media::MockFFmpeg::get()->AVSeekFrame(format, stream_index, timestamp,
+                                               flags);
+}
+
+void av_init_packet(AVPacket* pkt) {
+  return media::MockFFmpeg::get()->AVInitPacket(pkt);
 }
 
 int av_new_packet(AVPacket* packet, int size) {
   return media::MockFFmpeg::get()->AVNewPacket(packet, size);
 }
 
+void av_free_packet(AVPacket* packet) {
+  media::MockFFmpeg::get()->AVFreePacket(packet);
+}
+
 void av_free(void* ptr) {
   // Freeing NULL pointers are valid, but they aren't interesting from a mock
   // perspective.
@@ -116,16 +125,6 @@ void av_free(void* ptr) {
   }
 }
 
-int av_read_frame(AVFormatContext* format, AVPacket* packet) {
-  return media::MockFFmpeg::get()->AVReadFrame(format, packet);
-}
-
-int av_seek_frame(AVFormatContext *format, int stream_index, int64_t timestamp,
-                  int flags) {
-  return media::MockFFmpeg::get()->AVSeekFrame(format, stream_index, timestamp,
-                                               flags);
-}
-
 }  // extern "C"
 
 }  // namespace media
diff --git a/media/base/mock_ffmpeg.h b/media/base/mock_ffmpeg.h
index 597898e..70f34e3 100644
--- a/media/base/mock_ffmpeg.h
+++ b/media/base/mock_ffmpeg.h
@@ -14,11 +14,16 @@ namespace media {
 class MockFFmpeg {
  public:
   MockFFmpeg();
-  ~MockFFmpeg();
+  virtual ~MockFFmpeg();
 
   MOCK_METHOD1(AVCodecFindDecoder, AVCodec*(enum CodecID id));
   MOCK_METHOD2(AVCodecOpen, int(AVCodecContext* avctx, AVCodec* codec));
   MOCK_METHOD2(AVCodecThreadInit, int(AVCodecContext* avctx, int threads));
+  MOCK_METHOD1(AVCodecFlushBuffers, void(AVCodecContext* avctx));
+  MOCK_METHOD0(AVCodecAllocFrame, AVFrame*());
+  MOCK_METHOD4(AVCodecDecodeVideo2,
+               int(AVCodecContext* avctx, AVFrame* picture,
+                   int* got_picture_ptr, AVPacket* avpkt));
 
   MOCK_METHOD5(AVOpenInputFile, int(AVFormatContext** format,
                                     const char* filename,
@@ -32,9 +37,10 @@ class MockFFmpeg {
                                 int64_t timestamp,
                                 int flags));
 
+  MOCK_METHOD1(AVInitPacket, void(AVPacket* pkt));
   MOCK_METHOD2(AVNewPacket, int(AVPacket* packet, int size));
-  MOCK_METHOD1(AVFree, void(void* ptr));
   MOCK_METHOD1(AVFreePacket, void(AVPacket* packet));
+  MOCK_METHOD1(AVFree, void(void* ptr));
 
   // Used for verifying check points during tests.
   MOCK_METHOD1(CheckPoint, void(int id));
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index 4a943dd..115f97b 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -7,6 +7,19 @@
 #include "media/filters/ffmpeg_demuxer.h"
 #include "media/filters/ffmpeg_video_decoder.h"
 
+namespace {
+
+const AVRational kMicrosBase = { 1, base::Time::kMicrosecondsPerSecond };
+
+// TODO(ajwong): Move this into a utility function file and dedup with
+// FFmpegDemuxer ConvertTimestamp.
+base::TimeDelta ConvertTimestamp(const AVRational& time_base, int64 timestamp) {
+  int64 microseconds = av_rescale_q(timestamp, time_base, kMicrosBase);
+  return base::TimeDelta::FromMicroseconds(microseconds);
+}
+
+}  // namespace
+
 namespace media {
 
 // Always try to use two threads for video decoding.  There is little reason
@@ -26,7 +39,10 @@ static const int kDecodeThreads = 2;
 FFmpegVideoDecoder::FFmpegVideoDecoder()
     : DecoderBase<VideoDecoder, VideoFrame>("VideoDecoderThread"),
       width_(0),
-      height_(0) {
+      height_(0),
+      time_base_(new AVRational()),
+      state_(kNormal),
+      codec_context_(NULL) {
 }
 
 FFmpegVideoDecoder::~FFmpegVideoDecoder() {
@@ -49,6 +65,7 @@ bool FFmpegVideoDecoder::OnInitialize(DemuxerStream* demuxer_stream) {
 
   width_ = av_stream->codec->width;
   height_ = av_stream->codec->height;
+  *time_base_ = av_stream->time_base;
 
   media_format_.SetAsString(MediaFormat::kMimeType,
                             mime_type::kUncompressedVideo);
@@ -76,110 +93,104 @@ bool FFmpegVideoDecoder::OnInitialize(DemuxerStream* demuxer_stream) {
 }
 
 void FFmpegVideoDecoder::OnSeek(base::TimeDelta time) {
-  // Everything in the time queue is invalid, clear the queue.
-  while (!time_queue_.empty())
-    time_queue_.pop();
+  // Everything in the presentation time queue is invalid, clear the queue.
+  while (!pts_queue_.empty())
+    pts_queue_.pop();
 }
 
 void FFmpegVideoDecoder::OnDecode(Buffer* buffer) {
-  // Check for end of stream.
-  // TODO(scherkus): check for end of stream.
-
-  // Check for discontinuous buffer. If we receive a discontinuous buffer here,
-  // flush the internal buffer of FFmpeg.
-  if (buffer->IsDiscontinuous()) {
-    avcodec_flush_buffers(codec_context_);
+  // During decode, because reads are issued asynchronously, it is possible to
+  // recieve 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.
+  //
+  // If the decoding is finished, we just always return empty frames.
+  if (state_ == kDecodeFinished) {
+    EnqueueEmptyFrame();
+    return;
   }
 
-  // Create a packet for input data.
-  // Due to FFmpeg API changes we no longer have const read-only pointers.
-  AVPacket packet;
-  av_init_packet(&packet);
-  packet.data = const_cast<uint8*>(buffer->GetData());
-  packet.size = buffer->GetDataSize();
-
-  // We don't allocate AVFrame on the stack since different versions of FFmpeg
-  // may change the size of AVFrame, causing stack corruption.  The solution is
-  // to let FFmpeg allocate the structure via avcodec_alloc_frame().
-  int decoded = 0;
-  scoped_ptr_malloc<AVFrame, ScopedPtrAVFree> yuv_frame(avcodec_alloc_frame());
-  int result = avcodec_decode_video2(codec_context_, yuv_frame.get(), &decoded,
-                                     &packet);
-
-  // Log the problem if we can't decode a video frame.
-  if (result < 0) {
-    LOG(INFO) << "Error decoding a video frame with timestamp: "
-              << buffer->GetTimestamp().InMicroseconds() << " us"
-              << " , duration: "
-              << buffer->GetDuration().InMicroseconds() << " us"
-              << " , packet size: "
-              << buffer->GetDataSize() << " bytes";
+  // Transition to kFlushCodec on the first end of stream buffer.
+  if (state_ == kNormal && buffer->IsEndOfStream()) {
+    state_ = kFlushCodec;
   }
 
-  // We queue all incoming timestamps as long as decoding succeeded and it's not
-  // end of stream.
-  if (!buffer->IsEndOfStream()) {
-    TimeTuple times;
-    times.timestamp = buffer->GetTimestamp();
-    times.duration = buffer->GetDuration();
-    time_queue_.push(times);
+  // 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.
+  //
+  // TODO(ajwong): This push logic, along with the pop logic below needs to
+  // be reevaluated to correctly handle decode errors.
+  if (state_ == kNormal) {
+    pts_queue_.push(buffer->GetTimestamp());
   }
 
-  // Check for a decoded frame instead of checking the return value of
-  // avcodec_decode_video(). We don't need to stop the pipeline on
-  // decode errors.
-  if (decoded == 0) {
-    // Three conditions to meet to declare end of stream for this decoder:
-    // 1. FFmpeg didn't read anything.
-    // 2. FFmpeg didn't output anything.
-    // 3. An end of stream buffer is received.
-    if (result == 0 && buffer->IsEndOfStream()) {
-      // Create an empty video frame and queue it.
-      scoped_refptr<VideoFrame> video_frame;
-      VideoFrameImpl::CreateEmptyFrame(&video_frame);
-      EnqueueResult(video_frame);
-    }
-    return;
-  }
+  // Otherwise, attempt to decode a single frame.
+  scoped_ptr_malloc<AVFrame, ScopedPtrAVFree> yuv_frame(avcodec_alloc_frame());
+  if (DecodeFrame(*buffer, codec_context_, yuv_frame.get())) {
+    last_pts_ = FindPtsAndDuration(*time_base_,
+                                   pts_queue_,
+                                   last_pts_,
+                                   yuv_frame.get());
 
-  // J (Motion JPEG) versions of YUV are full range 0..255.
-  // Regular (MPEG) YUV is 16..240.
-  // For now we will ignore the distinction and treat them the same.
+    // Pop off a pts on a successful decode since we are "using up" one
+    // timestamp.
+    //
+    // TODO(ajwong): Do we need to pop off a pts when avcodec_decode_video2()
+    // returns < 0?  The rationale is that when get_picture_ptr == 0, we skip
+    // popping a pts because no frame was produced.  However, when
+    // avcodec_decode_video2() returns false, it is a decode error, which
+    // if it means a frame is dropped, may require us to pop one more time.
+    if (!pts_queue_.empty()) {
+      pts_queue_.pop();
+    } else {
+      NOTREACHED() << "Attempting to decode more frames than were input.";
+    }
 
-  VideoSurface::Format surface_format;
-  switch (codec_context_->pix_fmt) {
-    case PIX_FMT_YUV420P:
-    case PIX_FMT_YUVJ420P:
-      surface_format = VideoSurface::YV12;
-      break;
-    case PIX_FMT_YUV422P:
-    case PIX_FMT_YUVJ422P:
-      surface_format = VideoSurface::YV16;
-      break;
-    default:
-      // TODO(scherkus): More formats here?
-      NOTREACHED();
-      host_->Error(PIPELINE_ERROR_DECODE);
-      return;
-  }
-  if (!EnqueueVideoFrame(surface_format, yuv_frame.get())) {
-    host_->Error(PIPELINE_ERROR_DECODE);
+    if (!EnqueueVideoFrame(
+            GetSurfaceFormat(*codec_context_), last_pts_, yuv_frame.get())) {
+      // On an EnqueueEmptyFrame error, error out the whole pipeline and
+      // set the state to kDecodeFinished.
+      SignalPipelineError();
+    }
+  } 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;
+      EnqueueEmptyFrame();
+    }
   }
 }
 
 bool FFmpegVideoDecoder::EnqueueVideoFrame(VideoSurface::Format surface_format,
+                                           const TimeTuple& time,
                                            const AVFrame* frame) {
-  // Dequeue the next time tuple and create a VideoFrame object with
-  // that timestamp and duration.
-  TimeTuple time = time_queue_.top();
-  time_queue_.pop();
-
   scoped_refptr<VideoFrame> video_frame;
   VideoFrameImpl::CreateFrame(surface_format, width_, height_,
                               time.timestamp, time.duration, &video_frame);
   if (!video_frame) {
     return false;
   }
+
   // Copy the frame data since FFmpeg reuses internal buffers for AVFrame
   // output, meaning the data is only valid until the next
   // avcodec_decode_video() call.
@@ -221,4 +232,123 @@ void FFmpegVideoDecoder::CopyPlane(size_t plane,
   }
 }
 
+void FFmpegVideoDecoder::EnqueueEmptyFrame() {
+  scoped_refptr<VideoFrame> video_frame;
+  VideoFrameImpl::CreateEmptyFrame(&video_frame);
+  EnqueueResult(video_frame);
+}
+
+bool FFmpegVideoDecoder::DecodeFrame(const Buffer& buffer,
+                                     AVCodecContext* codec_context,
+                                     AVFrame* yuv_frame) {
+  // Check for discontinuous buffer. If we receive a discontinuous buffer here,
+  // flush the internal buffer of FFmpeg.
+  if (buffer.IsDiscontinuous()) {
+    avcodec_flush_buffers(codec_context);
+  }
+
+  // Create a packet for input data.
+  // Due to FFmpeg API changes we no longer have const read-only pointers.
+  AVPacket packet;
+  av_init_packet(&packet);
+  packet.data = const_cast<uint8*>(buffer.GetData());
+  packet.size = buffer.GetDataSize();
+
+  // We don't allocate AVFrame on the stack since different versions of FFmpeg
+  // may change the size of AVFrame, causing stack corruption.  The solution is
+  // to let FFmpeg allocate the structure via avcodec_alloc_frame().
+  int frame_decoded = 0;
+  int result =
+      avcodec_decode_video2(codec_context, yuv_frame, &frame_decoded, &packet);
+
+  // Log the problem if we can't decode a video frame and exit early.
+  if (result < 0) {
+    LOG(INFO) << "Error decoding a video frame with timestamp: "
+              << buffer.GetTimestamp().InMicroseconds() << " us"
+              << " , duration: "
+              << buffer.GetDuration().InMicroseconds() << " us"
+              << " , packet size: "
+              << buffer.GetDataSize() << " bytes";
+    return false;
+  }
+
+  // If frame_decoded == 0, then no frame was produced.
+  return frame_decoded != 0;
+}
+
+FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoder::FindPtsAndDuration(
+    const AVRational& time_base,
+    const TimeQueue& pts_queue,
+    const TimeTuple& last_pts,
+    const AVFrame* frame) {
+  TimeTuple pts;
+
+  // Default repeat_pict to 0 because if there is no frame information,
+  // we just assume the frame only plays for one time_base.
+  int repeat_pict = 0;
+
+  // First search the AVFrame for the pts. This is the most authoritative.
+  // Make a special exclusion for the value frame->pts == 0.  Though this
+  // is technically a valid value, it seems a number of ffmpeg codecs will
+  // mistakenly always set frame->pts to 0.
+  //
+  // Oh, and we have to cast AV_NOPTS_VALUE since it ends up becoming unsigned
+  // because the value they use doesn't fit in a signed 64-bit number which
+  // produces a signedness comparison warning on gcc.
+  if (frame &&
+      (frame->pts != static_cast<int64_t>(AV_NOPTS_VALUE)) &&
+      (frame->pts != 0)) {
+    pts.timestamp = ConvertTimestamp(time_base, frame->pts);
+    repeat_pict = frame->repeat_pict;
+  } else if (!pts_queue.empty()) {
+    // If the frame did not have pts, try to get the pts from the
+    // |pts_queue_|.
+    pts.timestamp = pts_queue.top();
+  } else {
+    // Unable to read the pts from anywhere. Time to guess.
+    pts.timestamp = last_pts.timestamp + last_pts.duration;
+  }
+
+  // Fill in the duration while accounting for repeated frames.
+  //
+  // TODO(ajwong): Make sure this formula is correct.
+  pts.duration = ConvertTimestamp(time_base, 1 + repeat_pict);
+
+  return pts;
+}
+
+VideoSurface::Format FFmpegVideoDecoder::GetSurfaceFormat(
+    const AVCodecContext& codec_context) {
+  // J (Motion JPEG) versions of YUV are full range 0..255.
+  // Regular (MPEG) YUV is 16..240.
+  // For now we will ignore the distinction and treat them the same.
+  switch (codec_context.pix_fmt) {
+    case PIX_FMT_YUV420P:
+    case PIX_FMT_YUVJ420P:
+      return VideoSurface::YV12;
+      break;
+    case PIX_FMT_YUV422P:
+    case PIX_FMT_YUVJ422P:
+      return VideoSurface::YV16;
+      break;
+    default:
+      // TODO(scherkus): More formats here?
+      return VideoSurface::INVALID;
+  }
+}
+
+void FFmpegVideoDecoder::SignalPipelineError() {
+  host_->Error(PIPELINE_ERROR_DECODE);
+  state_ = kDecodeFinished;
+}
+
+// static
+bool FFmpegVideoDecoder::PtsHeapOrdering::operator()(
+    const base::TimeDelta& lhs,
+    const base::TimeDelta& rhs) const {
+  // std::priority_queue is a max-heap. We want lower timestamps to show up
+  // first so reverse the natural less-than comparison.
+  return rhs < lhs;
+}
+
 }  // namespace
diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h
index 6da44ee..17ff53b 100644
--- a/media/filters/ffmpeg_video_decoder.h
+++ b/media/filters/ffmpeg_video_decoder.h
@@ -9,10 +9,12 @@
 
 #include "media/base/factory.h"
 #include "media/filters/decoder_base.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 
 // FFmpeg types.
 struct AVCodecContext;
 struct AVFrame;
+struct AVRational;
 
 namespace media {
 
@@ -32,17 +34,18 @@ class FFmpegVideoDecoder : public DecoderBase<VideoDecoder, VideoFrame> {
 
  private:
   friend class FilterFactoryImpl0<FFmpegVideoDecoder>;
-  FFmpegVideoDecoder();
-  virtual ~FFmpegVideoDecoder();
-
-  bool EnqueueVideoFrame(VideoSurface::Format surface_format,
-                         const AVFrame* frame);
-
-  void CopyPlane(size_t plane, const VideoSurface& surface,
-                 const AVFrame* frame);
-
-  size_t width_;
-  size_t height_;
+  friend class DecoderPrivateMock;
+  friend class FFmpegVideoDecoderTest;
+  FRIEND_TEST(FFmpegVideoDecoderTest, DecodeFrame_0ByteFrame);
+  FRIEND_TEST(FFmpegVideoDecoderTest, DecodeFrame_DecodeError);
+  FRIEND_TEST(FFmpegVideoDecoderTest, DecodeFrame_DiscontinuousBuffer);
+  FRIEND_TEST(FFmpegVideoDecoderTest, DecodeFrame_Normal);
+  FRIEND_TEST(FFmpegVideoDecoderTest, FindPtsAndDuration);
+  FRIEND_TEST(FFmpegVideoDecoderTest, GetSurfaceFormat);
+  FRIEND_TEST(FFmpegVideoDecoderTest, OnDecode_EnqueueVideoFrameError);
+  FRIEND_TEST(FFmpegVideoDecoderTest, OnDecode_FinishEnqueuesEmptyFrames);
+  FRIEND_TEST(FFmpegVideoDecoderTest, OnDecode_TestStateTransition);
+  FRIEND_TEST(FFmpegVideoDecoderTest, TimeQueue_Ordering);
 
   // FFmpeg outputs packets in decode timestamp (dts) order, which may not
   // always be in presentation timestamp (pts) order.  Therefore, when Process
@@ -57,20 +60,77 @@ class FFmpegVideoDecoder : public DecoderBase<VideoDecoder, VideoFrame> {
   //    4             4             3  <--- copying timestamp 4 and 6 would be
   //    5             6             4  <-'  incorrect, which is why we sort and
   //    6             5             5       queue incoming timestamps
+  //
+  // The TimeQueue is used to reorder these types.
+  struct PtsHeapOrdering {
+    bool operator()(const base::TimeDelta& lhs,
+                    const base::TimeDelta& rhs) const;
+  };
+  typedef std::priority_queue<base::TimeDelta,
+                              std::vector<base::TimeDelta>,
+                              PtsHeapOrdering> TimeQueue;
 
-  // A queue entry that holds a timestamp and a duration.
+  // The TimeTuple struct is used to hold the needed timestamp data needed for
+  // enqueuing a video frame.
   struct TimeTuple {
     base::TimeDelta timestamp;
     base::TimeDelta duration;
-
-    bool operator<(const TimeTuple& other) const {
-      return timestamp >= other.timestamp;
-    }
   };
 
+  FFmpegVideoDecoder();
+  virtual ~FFmpegVideoDecoder();
+
+  virtual bool EnqueueVideoFrame(VideoSurface::Format surface_format,
+                                 const TimeTuple& time,
+                                 const AVFrame* frame);
+
+  // Create an empty video frame and queue it.
+  virtual void EnqueueEmptyFrame();
+
+  virtual void CopyPlane(size_t plane, const VideoSurface& surface,
+                         const AVFrame* frame);
+
+  // Converts a AVCodecContext |pix_fmt| to a VideoSurface::Format.
+  virtual VideoSurface::Format GetSurfaceFormat(
+      const AVCodecContext& codec_context);
+
+  // Decodes one frame of video with the given buffer.  Returns false if there
+  // was a decode error, or a zero-byte frame was produced.
+  virtual bool DecodeFrame(const Buffer& buffer, AVCodecContext* codec_context,
+                           AVFrame* yuv_frame);
+
+  // Attempt to get the PTS and Duration for this frame by examining the time
+  // info provided via packet stream (stored in |pts_queue|), or the info
+  // writen into the AVFrame itself.  If no data is available in either, then
+  // attempt to generate a best guess of the pts based on the last known pts.
+  //
+  // Data inside the AVFrame (if provided) is trusted the most, followed
+  // by data from the packet stream.  Estimation based on the |last_pts| is
+  // reserved as a last-ditch effort.
+  virtual TimeTuple FindPtsAndDuration(const AVRational& time_base,
+                                       const TimeQueue& pts_queue,
+                                       const TimeTuple& last_pts,
+                                       const AVFrame* frame);
+
+  // Signals the pipeline that a decode error occurs, and moves the decoder
+  // into the kDecodeFinished state.
+  virtual void SignalPipelineError();
+
+  size_t width_;
+  size_t height_;
+
   // A priority queue of presentation TimeTuples.
-  typedef std::priority_queue<TimeTuple> TimeQueue;
-  TimeQueue time_queue_;
+  TimeQueue pts_queue_;
+  TimeTuple last_pts_;
+  scoped_ptr<AVRational> time_base_;  // Pointer to avoid needing full type.
+
+  enum DecoderState {
+    kNormal,
+    kFlushCodec,
+    kDecodeFinished,
+  };
+
+  DecoderState state_;
 
   AVCodecContext* codec_context_;
 
diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc
index 4d11e15..7d350e6 100644
--- a/media/filters/ffmpeg_video_decoder_unittest.cc
+++ b/media/filters/ffmpeg_video_decoder_unittest.cc
@@ -5,6 +5,7 @@
 #include <deque>
 
 #include "base/singleton.h"
+#include "media/base/data_buffer.h"
 #include "media/base/filters.h"
 #include "media/base/mock_ffmpeg.h"
 #include "media/base/mock_filter_host.h"
@@ -13,8 +14,12 @@
 #include "media/filters/ffmpeg_video_decoder.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
+using ::testing::DoAll;
 using ::testing::Return;
 using ::testing::ReturnNull;
+using ::testing::SetArgumentPointee;
+using ::testing::StrictMock;
 
 namespace media {
 
@@ -29,9 +34,26 @@ class MockDemuxerStream : public DemuxerStream, public AVStreamProvider {
   MOCK_METHOD0(GetAVStream, AVStream*());
 };
 
-}  // namespace media
-
-namespace media {
+// Class that just mocks the private functions.
+class DecoderPrivateMock : public FFmpegVideoDecoder {
+ public:
+  MOCK_METHOD3(EnqueueVideoFrame, bool(VideoSurface::Format surface_format,
+                                       const TimeTuple& time,
+                                       const AVFrame* frame));
+  MOCK_METHOD0(EnqueueEmptyFrame, void());
+  MOCK_METHOD3(CopyPlane, void(size_t plane, const VideoSurface& surface,
+                               const AVFrame* frame));
+  MOCK_METHOD1(GetSurfaceFormat,
+               VideoSurface::Format(const AVCodecContext& codec_context));
+  MOCK_METHOD3(DecodeFrame, bool(const Buffer& buffer,
+                                 AVCodecContext* codec_context,
+                                 AVFrame* yuv_frame));
+  MOCK_METHOD4(FindPtsAndDuration, TimeTuple(const AVRational& time_base,
+                                             const TimeQueue& pts_queue,
+                                             const TimeTuple& last_pts,
+                                             const AVFrame* frame));
+  MOCK_METHOD0(SignalPipelineError, void());
+};
 
 // Fixture class to facilitate writing tests.  Takes care of setting up the
 // FFmpeg, pipeline and filter host mocks.
@@ -39,6 +61,8 @@ class FFmpegVideoDecoderTest : public testing::Test {
  protected:
   static const int kWidth;
   static const int kHeight;
+  static const FFmpegVideoDecoder::TimeTuple kTestPts1;
+  static const FFmpegVideoDecoder::TimeTuple kTestPts2;
 
   FFmpegVideoDecoderTest() {
     MediaFormat media_format;
@@ -59,9 +83,14 @@ class FFmpegVideoDecoderTest : public testing::Test {
     memset(&stream_, 0, sizeof(stream_));
     memset(&codec_context_, 0, sizeof(codec_context_));
     memset(&codec_, 0, sizeof(codec_));
+    memset(&yuv_frame_, 0, sizeof(yuv_frame_));
+
     stream_.codec = &codec_context_;
     codec_context_.width = kWidth;
     codec_context_.height = kHeight;
+    buffer_ = new DataBuffer();
+    buffer_->GetWritableData(1);
+    end_of_stream_buffer_ = new DataBuffer();
 
     // Initialize MockFFmpeg.
     MockFFmpeg::set(&mock_ffmpeg_);
@@ -81,12 +110,15 @@ class FFmpegVideoDecoderTest : public testing::Test {
   scoped_ptr<MockPipeline> pipeline_;
   scoped_ptr<MockFilterHost<VideoDecoder> > filter_host_;
   scoped_refptr<MockDemuxerStream> demuxer_;
+  scoped_refptr<DataBuffer> buffer_;
+  scoped_refptr<DataBuffer> end_of_stream_buffer_;
 
   // FFmpeg fixtures.
   AVStream stream_;
   AVCodecContext codec_context_;
   AVCodec codec_;
-  MockFFmpeg mock_ffmpeg_;
+  AVFrame yuv_frame_;
+  StrictMock<MockFFmpeg> mock_ffmpeg_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FFmpegVideoDecoderTest);
@@ -94,6 +126,12 @@ class FFmpegVideoDecoderTest : public testing::Test {
 
 const int FFmpegVideoDecoderTest::kWidth = 1280;
 const int FFmpegVideoDecoderTest::kHeight = 720;
+const FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoderTest::kTestPts1 =
+    { base::TimeDelta::FromMicroseconds(123),
+      base::TimeDelta::FromMicroseconds(50) };
+const FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoderTest::kTestPts2 =
+    { base::TimeDelta::FromMicroseconds(456),
+      base::TimeDelta::FromMicroseconds(60) };
 
 TEST(FFmpegVideoDecoderFactoryTest, Create) {
   // Should only accept video/x-ffmpeg mime type.
@@ -206,4 +244,287 @@ TEST_F(FFmpegVideoDecoderTest, Initialize_Successful) {
   EXPECT_EQ(kHeight, height);
 }
 
+TEST_F(FFmpegVideoDecoderTest, DecodeFrame_Normal) {
+  // Expect a bunch of avcodec calls.
+  EXPECT_CALL(mock_ffmpeg_, AVInitPacket(_));
+  EXPECT_CALL(mock_ffmpeg_,
+              AVCodecDecodeVideo2(&codec_context_, &yuv_frame_, _, _))
+      .WillOnce(DoAll(SetArgumentPointee<2>(1),  // Simulate 1 byte frame.
+                      Return(0)));
+
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+  EXPECT_TRUE(decoder->DecodeFrame(*buffer_, &codec_context_, &yuv_frame_));
+}
+
+TEST_F(FFmpegVideoDecoderTest, DecodeFrame_DiscontinuousBuffer) {
+  buffer_->SetDiscontinuous(true);
+
+  // Expect a bunch of avcodec calls.
+  EXPECT_CALL(mock_ffmpeg_, AVCodecFlushBuffers(&codec_context_));
+  EXPECT_CALL(mock_ffmpeg_, AVInitPacket(_));
+  EXPECT_CALL(mock_ffmpeg_,
+              AVCodecDecodeVideo2(&codec_context_, &yuv_frame_, _, _))
+      .WillOnce(DoAll(SetArgumentPointee<2>(1),  // Simulate 1 byte frame.
+                      Return(0)));
+
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+  EXPECT_TRUE(decoder->DecodeFrame(*buffer_, &codec_context_, &yuv_frame_));
+}
+
+TEST_F(FFmpegVideoDecoderTest, DecodeFrame_0ByteFrame) {
+  // Expect a bunch of avcodec calls.
+  EXPECT_CALL(mock_ffmpeg_, AVInitPacket(_));
+  EXPECT_CALL(mock_ffmpeg_,
+              AVCodecDecodeVideo2(&codec_context_, &yuv_frame_, _, _))
+      .WillOnce(DoAll(SetArgumentPointee<2>(0),  // Simulate 0 byte frame.
+                      Return(0)));
+
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+  EXPECT_FALSE(decoder->DecodeFrame(*buffer_, &codec_context_, &yuv_frame_));
+}
+
+TEST_F(FFmpegVideoDecoderTest, DecodeFrame_DecodeError) {
+  // Expect a bunch of avcodec calls.
+  EXPECT_CALL(mock_ffmpeg_, AVInitPacket(_));
+  EXPECT_CALL(mock_ffmpeg_,
+              AVCodecDecodeVideo2(&codec_context_, &yuv_frame_, _, _))
+      .WillOnce(Return(-1));
+
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+  EXPECT_FALSE(decoder->DecodeFrame(*buffer_, &codec_context_, &yuv_frame_));
+}
+
+TEST_F(FFmpegVideoDecoderTest, GetSurfaceFormat) {
+  AVCodecContext context;
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+
+  // YV12 formats.
+  context.pix_fmt = PIX_FMT_YUV420P;
+  EXPECT_EQ(VideoSurface::YV12, decoder->GetSurfaceFormat(context));
+  context.pix_fmt =  PIX_FMT_YUVJ420P;
+  EXPECT_EQ(VideoSurface::YV12, decoder->GetSurfaceFormat(context));
+
+  // YV16 formats.
+  context.pix_fmt = PIX_FMT_YUV422P;
+  EXPECT_EQ(VideoSurface::YV16, decoder->GetSurfaceFormat(context));
+  context.pix_fmt = PIX_FMT_YUVJ422P;
+  EXPECT_EQ(VideoSurface::YV16, decoder->GetSurfaceFormat(context));
+
+  // Invalid value.
+  context.pix_fmt = PIX_FMT_NONE;
+  EXPECT_EQ(VideoSurface::INVALID, decoder->GetSurfaceFormat(context));
+}
+
+TEST_F(FFmpegVideoDecoderTest, FindPtsAndDuration) {
+  // Start with an empty timestamp queue.
+  FFmpegVideoDecoder::TimeQueue pts_queue;
+  scoped_refptr<FFmpegVideoDecoder> decoder = new FFmpegVideoDecoder();
+
+  // Use 1/2 second for simple results.  Thus, calculated Durations should be
+  // 500000 microseconds.
+  AVRational time_base = {1, 2};
+
+  // Setup the last known pts to be at 100 microseconds with a have 16
+  // duration.
+  FFmpegVideoDecoder::TimeTuple last_pts;
+  last_pts.timestamp = base::TimeDelta::FromMicroseconds(100);
+  last_pts.duration = base::TimeDelta::FromMicroseconds(16);
+
+  // Simulate an uninitialized yuv_frame.
+  yuv_frame_.pts = AV_NOPTS_VALUE;
+  FFmpegVideoDecoder::TimeTuple result_pts =
+      decoder->FindPtsAndDuration(time_base, pts_queue, last_pts, &yuv_frame_);
+  EXPECT_EQ(116, result_pts.timestamp.InMicroseconds());
+  EXPECT_EQ(500000, result_pts.duration.InMicroseconds());
+
+  // Test that providing no frame has the same result as an uninitialized
+  // frame.
+  result_pts = decoder->FindPtsAndDuration(time_base,
+                                           pts_queue,
+                                           last_pts,
+                                           NULL);
+  EXPECT_EQ(116, result_pts.timestamp.InMicroseconds());
+  EXPECT_EQ(500000, result_pts.duration.InMicroseconds());
+
+  // Test that having pts == 0 in the frame also behaves like the pts is not 
+  // provided.  This is because FFmpeg set the pts to zero when there is no
+  // data for the frame, which means that value is useless to us.
+  yuv_frame_.pts = 0;
+  result_pts =
+      decoder->FindPtsAndDuration(time_base, pts_queue, last_pts, &yuv_frame_);
+  EXPECT_EQ(116, result_pts.timestamp.InMicroseconds());
+  EXPECT_EQ(500000, result_pts.duration.InMicroseconds());
+
+  // Add a pts to the timequeue and make sure it overrides estimation.
+  pts_queue.push(base::TimeDelta::FromMicroseconds(123));
+  result_pts = decoder->FindPtsAndDuration(time_base,
+                                           pts_queue,
+                                           last_pts,
+                                           &yuv_frame_);
+  EXPECT_EQ(123, result_pts.timestamp.InMicroseconds());
+  EXPECT_EQ(500000, result_pts.duration.InMicroseconds());
+
+  // Add a pts into the frame and make sure it overrides the timequeue.
+  yuv_frame_.pts = 333;
+  yuv_frame_.repeat_pict = 2;
+  result_pts = decoder->FindPtsAndDuration(time_base,
+                                           pts_queue,
+                                           last_pts,
+                                           &yuv_frame_);
+  EXPECT_EQ(166500000, result_pts.timestamp.InMicroseconds());
+  EXPECT_EQ(1500000, result_pts.duration.InMicroseconds());
+}
+
+TEST_F(FFmpegVideoDecoderTest, OnDecode_TestStateTransition) {
+  // Simulates a input sequence of three buffers, and six decode requests to
+  // exercise the state transitions, and bookkeeping logic of OnDecode.
+  //
+  // We try verify the folowing:
+  //   1) Non-EoS buffer timestamps are pushed into the pts_queue.
+  //   2) Timestamps are popped for each decoded frame.
+  //   3) The last_pts_ is updated for each decoded frame.
+  //   4) kDecodeFinished is never left regardless of what kind of buffer is
+  //      given.
+  //   5) All state transitions happen as expected.
+  scoped_refptr<DecoderPrivateMock> mock_decoder =
+      new StrictMock<DecoderPrivateMock>();
+
+  // Setup decoder to buffer one frame, decode one frame, fail one frame,
+  // decode one more, and then fail the last one to end decoding.
+  EXPECT_CALL(*mock_decoder, DecodeFrame(_, _, _))
+      .WillOnce(Return(false))
+      .WillOnce(DoAll(SetArgumentPointee<2>(yuv_frame_), Return(true)))
+      .WillOnce(Return(false))
+      .WillOnce(DoAll(SetArgumentPointee<2>(yuv_frame_), Return(true)))
+      .WillOnce(DoAll(SetArgumentPointee<2>(yuv_frame_), Return(true)))
+      .WillOnce(Return(false));
+  EXPECT_CALL(*mock_decoder, GetSurfaceFormat(_))
+      .Times(3)
+      .WillRepeatedly(Return(VideoSurface::YV16));
+  EXPECT_CALL(*mock_decoder, EnqueueVideoFrame(_, _, _))
+      .Times(3)
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_decoder, FindPtsAndDuration(_, _, _, _))
+      .WillOnce(Return(kTestPts1))
+      .WillOnce(Return(kTestPts2))
+      .WillOnce(Return(kTestPts1));
+  EXPECT_CALL(*mock_decoder, EnqueueEmptyFrame())
+      .Times(1);
+
+  // Setup FFmpeg expectations for frame allocations.  We do
+  // 6 decodes in this test.
+  EXPECT_CALL(mock_ffmpeg_, AVCodecAllocFrame())
+      .Times(6)
+      .WillRepeatedly(Return(&yuv_frame_));
+  EXPECT_CALL(mock_ffmpeg_, AVFree(&yuv_frame_))
+      .Times(6);
+
+  // Setup initial state and check that it is sane.
+  mock_decoder->codec_context_ = &codec_context_;
+  ASSERT_EQ(FFmpegVideoDecoder::kNormal, mock_decoder->state_);
+  ASSERT_TRUE(base::TimeDelta() == mock_decoder->last_pts_.timestamp);
+  ASSERT_TRUE(base::TimeDelta() == mock_decoder->last_pts_.duration);
+
+  // Decode once, which should simulate a buffering call.
+  mock_decoder->OnDecode(buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kNormal, mock_decoder->state_);
+  ASSERT_TRUE(base::TimeDelta() == mock_decoder->last_pts_.timestamp);
+  ASSERT_TRUE(base::TimeDelta() == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(1, mock_decoder->pts_queue_.size());
+
+  // Decode a second time, which should yield the first frame.
+  mock_decoder->OnDecode(buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kNormal, mock_decoder->state_);
+  EXPECT_TRUE(kTestPts1.timestamp == mock_decoder->last_pts_.timestamp);
+  EXPECT_TRUE(kTestPts1.duration == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(1, mock_decoder->pts_queue_.size());
+
+  // Decode a third time, with a regular buffer.  The decode will error
+  // out, but the state should be the same.
+  mock_decoder->OnDecode(buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kNormal, mock_decoder->state_);
+  EXPECT_TRUE(kTestPts1.timestamp == mock_decoder->last_pts_.timestamp);
+  EXPECT_TRUE(kTestPts1.duration == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(2, mock_decoder->pts_queue_.size());
+
+  // Decode a fourth time, with an end of stream buffer.  This should
+  // yield the second frame, and stay in flushing mode.
+  mock_decoder->OnDecode(end_of_stream_buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kFlushCodec, mock_decoder->state_);
+  EXPECT_TRUE(kTestPts2.timestamp == mock_decoder->last_pts_.timestamp);
+  EXPECT_TRUE(kTestPts2.duration == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(1, mock_decoder->pts_queue_.size());
+
+  // Decode a fifth time with an end of stream buffer.  this should
+  // yield the third frame.
+  mock_decoder->OnDecode(end_of_stream_buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kFlushCodec, mock_decoder->state_);
+  EXPECT_TRUE(kTestPts1.timestamp == mock_decoder->last_pts_.timestamp);
+  EXPECT_TRUE(kTestPts1.duration == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(0, mock_decoder->pts_queue_.size());
+
+  // Decode a sixth time with an end of stream buffer.  This should
+  // Move into kDecodeFinished.
+  mock_decoder->OnDecode(end_of_stream_buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kDecodeFinished, mock_decoder->state_);
+  EXPECT_TRUE(kTestPts1.timestamp == mock_decoder->last_pts_.timestamp);
+  EXPECT_TRUE(kTestPts1.duration == mock_decoder->last_pts_.duration);
+  EXPECT_EQ(0, mock_decoder->pts_queue_.size());
+}
+
+TEST_F(FFmpegVideoDecoderTest, OnDecode_EnqueueVideoFrameError) {
+  scoped_refptr<DecoderPrivateMock> mock_decoder =
+      new StrictMock<DecoderPrivateMock>();
+
+  // Setup decoder to decode one frame, but then fail on enqueue.
+  EXPECT_CALL(*mock_decoder, DecodeFrame(_, _, _))
+      .WillOnce(DoAll(SetArgumentPointee<2>(yuv_frame_), Return(true)));
+  EXPECT_CALL(*mock_decoder, GetSurfaceFormat(_))
+      .WillOnce(Return(VideoSurface::YV16));
+  EXPECT_CALL(*mock_decoder, EnqueueVideoFrame(_, _, _))
+      .WillOnce(Return(false));
+  EXPECT_CALL(*mock_decoder, FindPtsAndDuration(_, _, _, _))
+      .WillOnce(Return(kTestPts1));
+  EXPECT_CALL(*mock_decoder, SignalPipelineError());
+
+  // Setup FFmpeg expectations for frame allocations.
+  EXPECT_CALL(mock_ffmpeg_, AVCodecAllocFrame())
+      .WillOnce(Return(&yuv_frame_));
+  EXPECT_CALL(mock_ffmpeg_, AVFree(&yuv_frame_));
+
+  // Attempt the decode.
+  buffer_->GetWritableData(1);
+  mock_decoder->codec_context_ = &codec_context_;
+  mock_decoder->OnDecode(buffer_);
+}
+
+TEST_F(FFmpegVideoDecoderTest, OnDecode_FinishEnqueuesEmptyFrames) {
+  scoped_refptr<DecoderPrivateMock> mock_decoder =
+      new StrictMock<DecoderPrivateMock>();
+
+  // Move the decoder into the finished state for this test.
+  mock_decoder->state_ = FFmpegVideoDecoder::kDecodeFinished;
+
+  // Expect 2 calls, make two calls.
+  EXPECT_CALL(*mock_decoder, EnqueueEmptyFrame()).Times(3);
+  mock_decoder->OnDecode(NULL);
+  mock_decoder->OnDecode(buffer_);
+  mock_decoder->OnDecode(end_of_stream_buffer_);
+  EXPECT_EQ(FFmpegVideoDecoder::kDecodeFinished, mock_decoder->state_);
+}
+
+TEST_F(FFmpegVideoDecoderTest, TimeQueue_Ordering) {
+  FFmpegVideoDecoder::TimeQueue queue;
+  queue.push(kTestPts1.timestamp);
+  queue.push(kTestPts2.timestamp);
+  queue.push(kTestPts1.timestamp);
+
+  EXPECT_TRUE(kTestPts1.timestamp == queue.top());
+  queue.pop();
+  EXPECT_TRUE(kTestPts1.timestamp == queue.top());
+  queue.pop();
+  EXPECT_TRUE(kTestPts2.timestamp == queue.top());
+  queue.pop();
+}
+
 }  // namespace media