Better seeking for <video> and <audio>

There two problems surrouding the seek feature for
<video> and <audio>:
1. After each seek, the time goes back and forth until
   stable, so user has to "fight" the time bar for
   seeking.
2. When playing an audio file, sounds plays a litte bit
   and then the play position rewinded and plays again.

The cause of above problems:
1. There are demuxed packets and decoded buffers inside
   decoders and renderers. When seek is requested only
   demuxer flush its buffers, decoders and renderers
   still plays a little bit of the old buffer, thus
   updating the time incorrectly.
2. When playing a media file, WebKit does:
   1. load()
   2. pause()
   3. seek(0.0f)
   4. play()
   Since load() does prerolling internally, there were
   some decoded buffers in the renderers, and when
   seek(0.0f) is fired the play position reset to time
   0 and decode starts from there again, the internal
   buffer queue in the renderer would then look like:

   | 0.0s | | 0.1s | | 0.2s | | 0.0s | | 0.1s | ...

   Thus playback at the beginning goes back and forth.

To solve the seek problems, here's what is done in this CL:
1. All decoders and renderers should receive the seek signal.
2. When seek signal is received, discard all buffers and
   schedule read again.

With this CL, we can now scrub an <audio> tag with the timebar.
The downside of this fix is that we don't have prerolling
(notice that due to how WebKit starts playing back we didn't
preroll anyway...), this should be fixed in another CL.

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

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

10 files changed, 205 insertions, 71 deletions

diff --git a/media/base/media_posix.cc b/media/base/media_posix.cc
index cd6bbe3..9a964dd 100644
--- a/media/base/media_posix.cc
+++ b/media/base/media_posix.cc
@@ -70,6 +70,11 @@ AVCodec* avcodec_find_decoder(enum CodecID id) {
   return avcodec_find_decoder_ptr(id);
 }
 
+void (*avcodec_flush_buffers_ptr)(AVCodecContext *avctx) = NULL;
+void avcodec_flush_buffers(AVCodecContext *avctx) {
+  avcodec_flush_buffers_ptr(avctx);
+}
+
 void (*avcodec_init_ptr)(void) = NULL;
 void avcodec_init(void) {
   avcodec_init_ptr();
@@ -230,6 +235,9 @@ bool InitializeMediaLibrary(const FilePath& module_dir) {
   avcodec_find_decoder_ptr =
       reinterpret_cast<AVCodec* (*)(enum CodecID)>(
           dlsym(libs[FILE_LIBAVCODEC], "avcodec_find_decoder"));
+  avcodec_flush_buffers_ptr =
+      reinterpret_cast<void (*)(AVCodecContext*)>(
+          dlsym(libs[FILE_LIBAVCODEC], "avcodec_flush_buffers"));
   avcodec_init_ptr =
       reinterpret_cast<void (*)(void)>(
           dlsym(libs[FILE_LIBAVCODEC], "avcodec_init"));
@@ -282,6 +290,7 @@ bool InitializeMediaLibrary(const FilePath& module_dir) {
       avcodec_decode_audio3_ptr &&
       avcodec_decode_video2_ptr &&
       avcodec_find_decoder_ptr &&
+      avcodec_flush_buffers_ptr &&
       avcodec_init_ptr &&
       avcodec_open_ptr &&
       avcodec_thread_init_ptr &&
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 88c3537..49ffb98 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -488,6 +488,15 @@ void PipelineThread::SeekTask(base::TimeDelta time,
        ++iter) {
     (*iter)->media_filter()->Seek(time);
   }
+
+  // TODO(hclam): we should set the time when the above seek operations were all
+  // successful and first frame/packet at the desired time is decoded. I'm
+  // setting the time here because once we do the callback the user can ask for
+  // current time immediately, which is the old time. In order to get rid this
+  // little glitch, we either assume the seek was successful and time is updated
+  // immediately here or we set time and do callback when we have new
+  // frames/packets.
+  SetTime(time);
   if (seek_callback) {
     seek_callback->Run(true);
     delete seek_callback;
diff --git a/media/filters/audio_renderer_base.cc b/media/filters/audio_renderer_base.cc
index 8466c2e..d838981 100644
--- a/media/filters/audio_renderer_base.cc
+++ b/media/filters/audio_renderer_base.cc
@@ -42,6 +42,22 @@ void AudioRendererBase::Stop() {
   stopped_ = true;
 }
 
+void AudioRendererBase::Seek(base::TimeDelta time) {
+  AutoLock auto_lock(lock_);
+  last_fill_buffer_time_ = base::TimeDelta();
+
+  // Clear the queue of decoded packets and release the buffers. Fire as many
+  // reads as buffers released. It is safe to schedule reads here because
+  // demuxer and decoders should have received the seek signal.
+  // TODO(hclam): we should preform prerolling again after each seek to avoid
+  // glitch or clicking of audio.
+  while (!queue_.empty()) {
+    queue_.front()->Release();
+    queue_.pop_front();
+    ScheduleRead();
+  }
+}
+
 bool AudioRendererBase::Initialize(AudioDecoder* decoder) {
   DCHECK(decoder);
   decoder_ = decoder;
@@ -82,84 +98,107 @@ void AudioRendererBase::OnReadComplete(Buffer* buffer_in) {
 // TODO(scherkus): clean up FillBuffer().. it's overly complex!!
 size_t AudioRendererBase::FillBuffer(uint8* dest, size_t dest_len,
                                      float rate) {
-  // Update the pipeline's time if it was set last time.
-  if (last_fill_buffer_time_.InMicroseconds() > 0) {
-    host_->SetTime(last_fill_buffer_time_);
-    last_fill_buffer_time_ = base::TimeDelta();
-  }
-
   size_t buffers_released = 0;
   size_t dest_written = 0;
+  base::TimeDelta last_fill_buffer_time;
+  {
+    AutoLock auto_lock(lock_);
 
-  AutoLock auto_lock(lock_);
-  // Loop until the buffer has been filled.
-  while (dest_len > 0 && !queue_.empty()) {
-    Buffer* buffer = queue_.front();
-
-    // Determine how much to copy.
-    const uint8* data = buffer->GetData() + data_offset_;
-    size_t data_len = buffer->GetDataSize() - data_offset_;
-
-    // New scaled packet size aligned to 16 to ensure its on a
-    // channel/sample boundary.  Only guaranteed to works for power of 2
-    // number of channels and sample size.
-    size_t scaled_data_len = (rate <= 0.0f) ? 0 :
-      static_cast<size_t>(data_len / rate) & ~15;
-    if (scaled_data_len > dest_len) {
-      data_len = (data_len * dest_len / scaled_data_len) & ~15;
-      scaled_data_len = dest_len;
-    }
+    // Save a local copy of last fill buffer time and reset the member.
+    last_fill_buffer_time = last_fill_buffer_time_;
+    last_fill_buffer_time_ = base::TimeDelta();
 
-    if (rate >= 1.0f) {  // Speed up.
-      memcpy(dest, data, scaled_data_len);
-    } else if (rate >= 0.5) {  // Slow down.
-      memcpy(dest, data, data_len);
-      memcpy(dest + data_len, data, scaled_data_len - data_len);
-    } else {  // Pause.
-      memset(dest, 0, data_len);
-    }
-    dest += scaled_data_len;
-    dest_len -= scaled_data_len;
-    dest_written += scaled_data_len;
-
-    data_offset_ += data_len;
-
-    if (rate == 0.0f)
-      return 0;
-
-    // Check to see if we're finished with the front buffer.
-    if (buffer->GetDataSize() - data_offset_ < 16) {
-      // Update the time.  If this is the last buffer in the queue, we'll
-      // drop out of the loop before len == 0, so we need to always update
-      // the time here.
-      if (buffer->GetTimestamp().InMicroseconds() > 0) {
-        last_fill_buffer_time_ = buffer->GetTimestamp() + buffer->GetDuration();
+    // Loop until the buffer has been filled.
+    while (dest_len > 0 && !queue_.empty()) {
+      Buffer* buffer = queue_.front();
+
+      // Determine how much to copy.
+      const uint8* data = buffer->GetData() + data_offset_;
+      size_t data_len = buffer->GetDataSize() - data_offset_;
+
+      // New scaled packet size aligned to 16 to ensure it's on a
+      // channel/sample boundary.  Only guaranteed to work for power of 2
+      // number of channels and sample size.
+      size_t scaled_data_len = (rate <= 0.0f) ? 0 :
+          static_cast<size_t>(data_len / rate) & ~15;
+      if (scaled_data_len > dest_len) {
+        data_len = (data_len * dest_len / scaled_data_len) & ~15;
+        scaled_data_len = dest_len;
+      }
+
+      // Handle playback rate in three different cases:
+      // 1. If rate >= 1.0
+      //    Speed up the playback, we copy partial amount of decoded samples
+      //    into target buffer.
+      // 2. If 0.5 <= rate < 1.0
+      //    Slow down the playback, duplicate the decoded samples to fill a
+      //    larger size of target buffer.
+      // 3. If rate < 0.5
+      //    Playback is too slow, simply mute the audio.
+      // TODO(hclam): the logic for handling playback rate is too complex and
+      // is not careful enough. I should do some bounds checking and even better
+      // replace this with a better/clearer implementation.
+      if (rate >= 1.0f) {
+        memcpy(dest, data, scaled_data_len);
+      } else if (rate >= 0.5) {
+        memcpy(dest, data, data_len);
+        memcpy(dest + data_len, data, scaled_data_len - data_len);
+      } else {
+        memset(dest, 0, data_len);
+      }
+      dest += scaled_data_len;
+      dest_len -= scaled_data_len;
+      dest_written += scaled_data_len;
+
+      data_offset_ += data_len;
+
+      if (rate == 0.0f) {
+        dest_written = 0;
+        break;
       }
 
-      // Dequeue the buffer.
-      queue_.pop_front();
-      buffer->Release();
-      ++buffers_released;
-
-      // Reset our offset into the front buffer.
-      data_offset_ = 0;
-    } else {
-      // If we're done with the read, compute the time.
-      // Integer divide so multiply before divide to work properly.
-      int64 us_written = (buffer->GetDuration().InMicroseconds() *
-                          data_offset_) / buffer->GetDataSize();
-
-      if (buffer->GetTimestamp().InMicroseconds() > 0) {
-        last_fill_buffer_time_ = buffer->GetTimestamp() +
-            base::TimeDelta::FromMicroseconds(us_written);
+      // Check to see if we're finished with the front buffer.
+      if (buffer->GetDataSize() - data_offset_ < 16) {
+        // Update the time.  If this is the last buffer in the queue, we'll
+        // drop out of the loop before len == 0, so we need to always update
+        // the time here.
+        if (buffer->GetTimestamp().InMicroseconds() > 0) {
+          last_fill_buffer_time_ = buffer->GetTimestamp() +
+                                   buffer->GetDuration();
+        }
+
+        // Dequeue the buffer.
+        queue_.pop_front();
+        buffer->Release();
+        ++buffers_released;
+
+        // Reset our offset into the front buffer.
+        data_offset_ = 0;
+      } else {
+        // If we're done with the read, compute the time.
+        // Integer divide so multiply before divide to work properly.
+        int64 us_written = (buffer->GetDuration().InMicroseconds() *
+                            data_offset_) / buffer->GetDataSize();
+
+        if (buffer->GetTimestamp().InMicroseconds() > 0) {
+          last_fill_buffer_time_ =
+              buffer->GetTimestamp() +
+              base::TimeDelta::FromMicroseconds(us_written);
+        }
       }
     }
+
+    // If we've released any buffers, read more buffers from the decoder.
+    for (size_t i = 0; i < buffers_released; ++i) {
+      ScheduleRead();
+    }
   }
 
-  // If we've released any buffers, read more buffers from the decoder.
-  for (size_t i = 0; i < buffers_released; ++i) {
-    ScheduleRead();
+  // Update the pipeline's time if it was set last time.
+  if (last_fill_buffer_time.InMicroseconds() > 0) {
+    host_->SetTime(last_fill_buffer_time);
   }
+
   return dest_written;
 }
 
diff --git a/media/filters/audio_renderer_base.h b/media/filters/audio_renderer_base.h
index 5536a5d..a772853 100644
--- a/media/filters/audio_renderer_base.h
+++ b/media/filters/audio_renderer_base.h
@@ -32,6 +32,8 @@ class AudioRendererBase : public AudioRenderer {
   // MediaFilter implementation.
   virtual void Stop();
 
+  virtual void Seek(base::TimeDelta time);
+
   // AudioRenderer implementation.
   virtual bool Initialize(AudioDecoder* decoder);
 
diff --git a/media/filters/decoder_base.h b/media/filters/decoder_base.h
index 17309e6..38bd387 100644
--- a/media/filters/decoder_base.h
+++ b/media/filters/decoder_base.h
@@ -35,11 +35,33 @@ class DecoderBase : public Decoder {
       }
       DiscardQueues();
     }
+
     // Because decode_thread_ is a scoped_ptr this will destroy the thread,
     // if there was one, which causes it to be shut down in an orderly way.
     decode_thread_.reset();
   }
 
+  virtual void Seek(base::TimeDelta time) {
+    // Delegate to the subclass first.
+    OnSeek(time);
+    {
+      AutoLock auto_lock(lock_);
+
+      // Flush the result queue.
+      result_queue_.clear();
+
+      // Flush the input queue. This will trigger more reads from the demuxer.
+      input_queue_.clear();
+
+      // Turn on the seeking flag so that we can discard buffers until a
+      // discontinuous buffer is received.
+      seeking_ = true;
+    }
+    // ScheduleProcessTask to trigger more reads and keep the process loop
+    // rolling.
+    ScheduleProcessTask();
+  }
+
   // Decoder implementation.
   virtual bool Initialize(DemuxerStream* demuxer_stream) {
     demuxer_stream_ = demuxer_stream;
@@ -76,7 +98,18 @@ class DecoderBase : public Decoder {
   void OnReadComplete(Buffer* buffer) {
     AutoLock auto_lock(lock_);
     if (IsRunning()) {
-      input_queue_.push_back(buffer);
+      // Once the |seeking_| flag is set we ignore every buffers here
+      // until we receive a discontinuous buffer and we will turn off the
+      // |seeking_| flag.
+      if (buffer->IsDiscontinuous()) {
+        // TODO(hclam): put a DCHECK here to assert |seeking_| being true.
+        // I cannot do this now because seek operation is not fully
+        // asynchronous. There may be pending seek requests even before the
+        // previous was finished.
+        seeking_ = false;
+      }
+      if (!seeking_)
+        input_queue_.push_back(buffer);
       --pending_reads_;
       ScheduleProcessTask();
     }
@@ -92,7 +125,8 @@ class DecoderBase : public Decoder {
         demuxer_stream_(NULL),
         decode_thread_(thread_name ? new base::Thread(thread_name) : NULL),
         pending_reads_(0),
-        process_task_(NULL) {
+        process_task_(NULL),
+        seeking_(false) {
   }
 
   virtual ~DecoderBase() {
@@ -119,10 +153,14 @@ class DecoderBase : public Decoder {
   virtual bool OnInitialize(DemuxerStream* demuxer_stream) = 0;
 
   // Method that may be implemented by the derived class if desired.  It will
-  // be called from within the MediaFilter::Stop method prior to stopping the
+  // be called from within the MediaFilter::Stop() method prior to stopping the
   // base class.
   virtual void OnStop() {}
 
+  // Derived class can implement this method and perform seeking logic prior
+  // to the base class.
+  virtual void OnSeek(base::TimeDelta time) {}
+
   // Method that must be implemented by the derived class.  If the decode
   // operation produces one or more outputs, the derived class should call
   // the EnequeueResult() method from within this method.
@@ -278,6 +316,11 @@ class DecoderBase : public Decoder {
   typedef std::deque<ReadCallback*> ReadQueue;
   ReadQueue read_queue_;
 
+  // An internal state of the decoder that indicates that are waiting for seek
+  // to complete. We expect to receive a discontinuous frame/packet from the
+  // demuxer to signal that seeking is completed.
+  bool seeking_;
+
   DISALLOW_COPY_AND_ASSIGN(DecoderBase);
 };
 
diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc
index 808904c7..b2bad80 100644
--- a/media/filters/ffmpeg_audio_decoder.cc
+++ b/media/filters/ffmpeg_audio_decoder.cc
@@ -74,6 +74,12 @@ void FFmpegAudioDecoder::OnStop() {
 }
 
 void FFmpegAudioDecoder::OnDecode(Buffer* input) {
+  // Check for discontinuous buffer. If we receive a discontinuous buffer here,
+  // flush the internal buffer of FFmpeg.
+  if (input->IsDiscontinuous()) {
+    avcodec_flush_buffers(codec_context_);
+  }
+
   // Due to FFmpeg API changes we no longer have const read-only pointers.
   AVPacket packet;
   av_init_packet(&packet);
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 201bd95..085bfcc 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -195,6 +195,10 @@ void FFmpegDemuxer::Stop() {
 }
 
 void FFmpegDemuxer::Seek(base::TimeDelta time) {
+  // TODO(hclam): by returning from this method, it is assumed that the seek
+  // operation is completed and filters behind the demuxer is good to issue
+  // more reads, but we are posting a task here, which makes the seek operation
+  // asynchronous, should change how seek works to make it fully asynchronous.
   thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(this, &FFmpegDemuxer::SeekTask, time));
 }
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index b80d215..3ec8c6d 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -71,10 +71,22 @@ bool FFmpegVideoDecoder::OnInitialize(DemuxerStream* demuxer_stream) {
   return true;
 }
 
+void FFmpegVideoDecoder::OnSeek(base::TimeDelta time) {
+  // Everything in the time queue is invalid, clear the queue.
+  while (!time_queue_.empty())
+    time_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_);
+  }
+
   // Queue the incoming timestamp.
   TimeTuple times;
   times.timestamp = buffer->GetTimestamp();
diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h
index 7f20054e..6da44ee 100644
--- a/media/filters/ffmpeg_video_decoder.h
+++ b/media/filters/ffmpeg_video_decoder.h
@@ -26,6 +26,8 @@ class FFmpegVideoDecoder : public DecoderBase<VideoDecoder, VideoFrame> {
 
   virtual bool OnInitialize(DemuxerStream* demuxer_stream);
 
+  virtual void OnSeek(base::TimeDelta time);
+
   virtual void OnDecode(Buffer* buffer);
 
  private:
diff --git a/media/filters/video_thread.cc b/media/filters/video_thread.cc
index 50f02e6..fd88757 100644
--- a/media/filters/video_thread.cc
+++ b/media/filters/video_thread.cc
@@ -75,7 +75,15 @@ void VideoThread::SetPlaybackRate(float playback_rate) {
 }
 
 void VideoThread::Seek(base::TimeDelta time) {
-  // Do nothing.
+  AutoLock auto_lock(lock_);
+  // We need the first frame in |frames_| to run the VideoThread main loop, but
+  // we don't need decoded frames after the first frame since we are at a new
+  // time. We should get some new frames so issue reads to compensate for those
+  // discarded.
+  while (frames_.size() > 1) {
+    frames_.pop_back();
+    ScheduleRead();
+  }
 }
 
 bool VideoThread::Initialize(VideoDecoder* decoder) {