Remove FFmpegDemuxer::current_timestamp_ and always pass AVSEEK_FLAG_BACKWARD when seeking.

Previously we falsely assumed that AVSEEK_FLAG_BACKWARD was needed when seeking backwards in the media file.  Turns out all this flag does is seek to a point in the file that is less-than-or-equal to the desired timestamp -- regardless of the current position in the file!

Furthermore, the value of current_timestamp_ was never guaranteed to be valid as poorly muxed files tend to contain AV_NOPTS_VALUE packets.  This all resulted in non-deterministic seeking behaviour depending on whether current_timestamp_ was valid or contained AV_NOPTS_VALUE.

BUG=48722
TEST=covered by layout tests

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

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

3 files changed, 11 insertions, 17 deletions

diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 5366ffa..951e6b5 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -105,15 +105,16 @@ bool FFmpegDemuxerStream::HasPendingReads() {
   return !read_queue_.empty();
 }
 
-base::TimeDelta FFmpegDemuxerStream::EnqueuePacket(AVPacket* packet) {
+void FFmpegDemuxerStream::EnqueuePacket(AVPacket* packet) {
   DCHECK_EQ(MessageLoop::current(), demuxer_->message_loop());
   base::TimeDelta timestamp =
       ConvertStreamTimestamp(stream_->time_base, packet->pts);
   base::TimeDelta duration =
       ConvertStreamTimestamp(stream_->time_base, packet->duration);
+
   if (stopped_) {
     NOTREACHED() << "Attempted to enqueue packet on a stopped stream";
-    return timestamp;
+    return;
   }
 
   // Convert if the packet if there is bitstream filter.
@@ -127,11 +128,11 @@ base::TimeDelta FFmpegDemuxerStream::EnqueuePacket(AVPacket* packet) {
       new AVPacketBuffer(packet, timestamp, duration);
   if (!buffer) {
     NOTREACHED() << "Unable to allocate AVPacketBuffer";
-    return timestamp;
+    return;
   }
   buffer_queue_.push_back(buffer);
   FulfillPendingRead();
-  return timestamp;
+  return;
 }
 
 void FFmpegDemuxerStream::FlushBuffers() {
@@ -494,11 +495,8 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time, FilterCallback* callback) {
     return;
   }
 
-  // Seek backwards if requested timestamp is behind FFmpeg's current time.
-  int flags = 0;
-  if (time <= current_timestamp_) {
-    flags |= AVSEEK_FLAG_BACKWARD;
-  }
+  // Always seek to a timestamp less than or equal to the desired timestamp.
+  int flags = AVSEEK_FLAG_BACKWARD;
 
   // Explicitly set the behavior of Ogg to be able to seek to any frame.
   if (!strcmp("ogg", format_context_->iformat->name))
@@ -554,7 +552,7 @@ void FFmpegDemuxer::DemuxTask() {
       // other codecs.  It is safe to call this function even if the packet does
       // not refer to inner memory from FFmpeg.
       av_dup_packet(packet.get());
-      current_timestamp_ = demuxer_stream->EnqueuePacket(packet.release());
+      demuxer_stream->EnqueuePacket(packet.release());
     }
   }
 
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index c43bd20..5f6d29f 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -60,9 +60,8 @@ class FFmpegDemuxerStream : public DemuxerStream, public AVStreamProvider {
   // Safe to call on any thread.
   virtual bool HasPendingReads();
 
-  // Enqueues and takes ownership over the given AVPacket, returns the timestamp
-  // of the enqueued packet.
-  virtual base::TimeDelta EnqueuePacket(AVPacket* packet);
+  // Enqueues and takes ownership over the given AVPacket.
+  virtual void EnqueuePacket(AVPacket* packet);
 
   // Signals to empty the buffer queue and mark next packet as discontinuous.
   virtual void FlushBuffers();
@@ -197,9 +196,6 @@ class FFmpegDemuxer : public Demuxer,
   // FFmpeg context handle.
   AVFormatContext* format_context_;
 
-  // Latest timestamp read on the demuxer thread.
-  base::TimeDelta current_timestamp_;
-
   // Two vector of streams:
   //   - |streams_| is indexed for the Demuxer interface GetStream(), which only
   //     contains supported streams and no NULL entries.
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index c6c8e74..ea43509 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -441,7 +441,7 @@ TEST_F(FFmpegDemuxerTest, Seek) {
 
   // Expected values.
   const int64 kExpectedTimestamp = 1234;
-  const int64 kExpectedFlags = 0;
+  const int64 kExpectedFlags = AVSEEK_FLAG_BACKWARD;
 
   // Ignore all AVFreePacket() calls.  We check this via valgrind.
   EXPECT_CALL(*MockFFmpeg::get(), AVFreePacket(_)).Times(AnyNumber());