Suppress slider thumb jumping around during seeking

BUG=19396
TEST=Open a video, seek to any position, the thumb should stops a bit and then goes forward from there.

Preventing the slider thumb from jumping around after seek
by freezing the clock until we get a valid time update from
the audio renderer.

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

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

9 files changed, 91 insertions, 16 deletions

diff --git a/media/base/buffers.cc b/media/base/buffers.cc
new file mode 100644
index 0000000..4247e39
--- /dev/null
+++ b/media/base/buffers.cc
@@ -0,0 +1,13 @@
+// Copyright (c) 2009 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/base/buffers.h"
+
+namespace media {
+
+// static
+const base::TimeDelta StreamSample::kInvalidTimestamp =
+    base::TimeDelta::FromMicroseconds(kint64min);
+
+}  // namespace media
diff --git a/media/base/buffers.h b/media/base/buffers.h
index 2b4f25b..27ad130 100644
--- a/media/base/buffers.h
+++ b/media/base/buffers.h
@@ -35,6 +35,9 @@ namespace media {
 
 class StreamSample : public base::RefCountedThreadSafe<StreamSample> {
  public:
+  // Constant timestamp value to indicate an invalid or missing timestamp.
+  static const base::TimeDelta kInvalidTimestamp;
+
   // Returns the timestamp of this buffer in microseconds.
   base::TimeDelta GetTimestamp() const {
     return timestamp_;
diff --git a/media/base/clock_impl.cc b/media/base/clock_impl.cc
index cc46568..b853742 100644
--- a/media/base/clock_impl.cc
+++ b/media/base/clock_impl.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/logging.h"
+#include "media/base/buffers.h"
 #include "media/base/clock_impl.h"
 
 namespace media {
@@ -32,6 +33,10 @@ base::TimeDelta ClockImpl::Pause() {
 }
 
 void ClockImpl::SetTime(const base::TimeDelta& time) {
+  if (time == StreamSample::kInvalidTimestamp) {
+    NOTREACHED();
+    return;
+  }
   if (playing_) {
     reference_ = time_provider_();
   }
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 61f6344..270347c 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -74,6 +74,7 @@ void DecrementCounter(Lock* lock, ConditionVariable* cond_var, int* count) {
 PipelineImpl::PipelineImpl(MessageLoop* message_loop)
     : message_loop_(message_loop),
       clock_(&base::Time::Now),
+      waiting_for_clock_update_(false),
       state_(kCreated),
       remaining_transitions_(0) {
   ResetState();
@@ -292,6 +293,7 @@ void PipelineImpl::ResetState() {
   volume_           = 1.0f;
   playback_rate_    = 0.0f;
   error_            = PIPELINE_OK;
+  waiting_for_clock_update_ = false;
   clock_.SetTime(kZero);
   rendered_mime_types_.clear();
 }
@@ -346,6 +348,17 @@ base::TimeDelta PipelineImpl::GetTime() const {
 void PipelineImpl::SetTime(base::TimeDelta time) {
   DCHECK(IsRunning());
   AutoLock auto_lock(lock_);
+
+  // If we were waiting for a valid timestamp and such timestamp arrives, we
+  // need to clear the flag for waiting and start the clock.
+  if (waiting_for_clock_update_) {
+    if (time < clock_.Elapsed())
+      return;
+    waiting_for_clock_update_ = false;
+    clock_.SetTime(time);
+    clock_.Play();
+    return;
+  }
   clock_.SetTime(time);
 }
 
@@ -607,7 +620,10 @@ void PipelineImpl::ErrorChangedTask(PipelineError error) {
 
 void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-  clock_.SetPlaybackRate(playback_rate);
+  {
+    AutoLock auto_lock(lock_);
+    clock_.SetPlaybackRate(playback_rate);
+  }
   for (FilterVector::iterator iter = filters_.begin();
        iter != filters_.end();
        ++iter) {
@@ -652,7 +668,12 @@ void PipelineImpl::SeekTask(base::TimeDelta time,
   remaining_transitions_ = filters_.size();
 
   // Kick off seeking!
-  clock_.Pause();
+  {
+    AutoLock auto_lock(lock_);
+    // If we are waiting for a clock update, the clock hasn't been played yet.
+    if (!waiting_for_clock_update_)
+      clock_.Pause();
+  }
   filters_.front()->Pause(
       NewCallback(this, &PipelineImpl::OnFilterStateTransition));
 }
@@ -688,10 +709,14 @@ void PipelineImpl::NotifyEndedTask() {
 void PipelineImpl::BroadcastMessageTask(FilterMessage message) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  // TODO(kylep): This is a horribly ugly hack, but we have no better way to log
-  // that audio is not and will not be working.
-  if (message == media::kMsgDisableAudio)
+  // TODO(kylep): This is a horribly ugly hack, but we have no better way to
+  // log that audio is not and will not be working.
+  if (message == media::kMsgDisableAudio) {
+    // |rendered_mime_types_| is read through public methods so we need to lock
+    // this variable.
+    AutoLock auto_lock(lock_);
     rendered_mime_types_.erase(mime_type::kMajorTypeAudio);
+  }
 
   // Broadcast the message to all filters.
   for (FilterVector::iterator iter = filters_.begin();
@@ -722,9 +747,8 @@ void PipelineImpl::FilterStateTransitionTask() {
   if (--remaining_transitions_ == 0) {
     state_ = FindNextState(state_);
     if (state_ == kSeeking) {
+      AutoLock auto_lock(lock_);
       clock_.SetTime(seek_timestamp_);
-    } else if (state_ == kStarting) {
-      clock_.Play();
     }
 
     if (StateTransitionsToStarted(state_)) {
@@ -755,6 +779,15 @@ void PipelineImpl::FilterStateTransitionTask() {
 
     // Finally, reset our seeking timestamp back to zero.
     seek_timestamp_ = base::TimeDelta();
+
+    AutoLock auto_lock(lock_);
+    // We use audio stream to update the clock. So if there is such a stream,
+    // we pause the clock until we receive a valid timestamp.
+    waiting_for_clock_update_ =
+        rendered_mime_types_.find(mime_type::kMajorTypeAudio) !=
+        rendered_mime_types_.end();
+    if (!waiting_for_clock_update_)
+      clock_.Play();
   } else {
     NOTREACHED();
   }
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 383969d..c448106 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -308,6 +308,11 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // by filters.
   ClockImpl clock_;
 
+  // If this value is set to true, then |clock_| is paused and we are waiting
+  // for an update of the clock greater than or equal to the elapsed time to
+  // start the clock.
+  bool waiting_for_clock_update_;
+
   // Status of the pipeline.  Initialized to PIPELINE_OK which indicates that
   // the pipeline is operating correctly. Any other value indicates that the
   // pipeline is stopped or is stopping.  Clients can call the Stop() method to
diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc
index e103093..5bbe4e5 100644
--- a/media/filters/ffmpeg_audio_decoder.cc
+++ b/media/filters/ffmpeg_audio_decoder.cc
@@ -76,7 +76,7 @@ bool FFmpegAudioDecoder::OnInitialize(DemuxerStream* demuxer_stream) {
 
 void FFmpegAudioDecoder::OnSeek(base::TimeDelta time) {
   avcodec_flush_buffers(codec_context_);
-  estimated_next_timestamp_ = base::TimeDelta();
+  estimated_next_timestamp_ = StreamSample::kInvalidTimestamp;
 }
 
 void FFmpegAudioDecoder::OnStop() {
@@ -114,26 +114,27 @@ void FFmpegAudioDecoder::OnDecode(Buffer* input) {
 
     // Determine the duration if the demuxer couldn't figure it out, otherwise
     // copy it over.
-    if (input->GetDuration().InMicroseconds() == 0) {
+    if (input->GetDuration().ToInternalValue() == 0) {
       result_buffer->SetDuration(CalculateDuration(output_buffer_size));
     } else {
+      DCHECK(input->GetDuration() != StreamSample::kInvalidTimestamp);
       result_buffer->SetDuration(input->GetDuration());
     }
 
     // Use our estimate for the timestamp if |input| does not have one.
     // Otherwise, copy over the timestamp.
-    if (input->GetTimestamp().InMicroseconds() == 0) {
+    if (input->GetTimestamp() == StreamSample::kInvalidTimestamp) {
       result_buffer->SetTimestamp(estimated_next_timestamp_);
     } else {
       result_buffer->SetTimestamp(input->GetTimestamp());
     }
 
     // Only use the timestamp of |result_buffer| to estimate the next timestamp
-    // if it is valid (i.e. greater than 0).  Otherwise the error will stack
-    // together and we will get a series of incorrect timestamps.  In this case,
-    // this will maintain a series of zero timestamps.
-    // TODO(hclam): We should use another invalid value other than 0.
-    if (result_buffer->GetTimestamp().InMicroseconds() > 0) {
+    // if it is valid (i.e. != StreamSample::kInvalidTimestamp).  Otherwise the
+    // error will stack together and we will get a series of incorrect
+    // timestamps.  In this case, this will maintain a series of zero
+    // timestamps.
+    if (result_buffer->GetTimestamp() != StreamSample::kInvalidTimestamp) {
       // Update our estimated timestamp for the next packet.
       estimated_next_timestamp_ = result_buffer->GetTimestamp() +
           result_buffer->GetDuration();
@@ -143,6 +144,16 @@ void FFmpegAudioDecoder::OnDecode(Buffer* input) {
     return;
   }
 
+  // We can get a positive result but no decoded data.  This is ok because this
+  // this can be a marker packet that only contains timestamp.  In this case we
+  // save the timestamp for later use.
+  if (result && !input->IsEndOfStream() &&
+      input->GetTimestamp() != StreamSample::kInvalidTimestamp &&
+      input->GetDuration() != StreamSample::kInvalidTimestamp) {
+    estimated_next_timestamp_ = input->GetTimestamp() + input->GetDuration();
+    return;
+  }
+
   // Three conditions to meet to declare end of stream for this decoder:
   // 1. FFmpeg didn't read anything.
   // 2. FFmpeg didn't output anything.
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 4a1431b..74c7bad 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -207,6 +207,8 @@ void FFmpegDemuxerStream::FulfillPendingRead() {
 }
 
 base::TimeDelta FFmpegDemuxerStream::ConvertTimestamp(int64 timestamp) {
+  if (timestamp == AV_NOPTS_VALUE)
+    return StreamSample::kInvalidTimestamp;
   AVRational time_base = { 1, base::Time::kMicrosecondsPerSecond };
   int64 microseconds = av_rescale_q(timestamp, stream_->time_base, time_base);
   return base::TimeDelta::FromMicroseconds(microseconds);
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index ef64562..8d54c8b 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -306,6 +306,8 @@ FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoder::FindPtsAndDuration(
     // |pts_queue_|.
     pts.timestamp = pts_queue.top();
   } else {
+    DCHECK(last_pts.timestamp != StreamSample::kInvalidTimestamp);
+    DCHECK(last_pts.duration != StreamSample::kInvalidTimestamp);
     // Unable to read the pts from anywhere. Time to guess.
     pts.timestamp = last_pts.timestamp + last_pts.duration;
   }
diff --git a/media/media.gyp b/media/media.gyp
index 9599890..768abc8 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -50,8 +50,9 @@
         'audio/win/audio_output_win.cc',
         'audio/win/waveout_output_win.cc',
         'audio/win/waveout_output_win.h',
-        'base/buffer_queue.h',
         'base/buffer_queue.cc',
+        'base/buffer_queue.h',
+        'base/buffers.cc',
         'base/buffers.h',
         'base/clock.h',
         'base/clock_impl.cc',