Pipeline: Call clock_->SetTime() right before StartPlayingFrom().

media/base/pipeline.cc

Index: media/base/pipeline.cc
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc
index 1c7507c57f56279472d45a61374445233db5b826..6da0553769c80466aeb9276f81c76459ec6bd364 100644
--- a/media/base/pipeline.cc
+++ b/media/base/pipeline.cc
@@ -55,6 +55,12 @@ Pipeline::Pipeline(
   media_log_->AddEvent(media_log_->CreatePipelineStateChangedEvent(kCreated));
   media_log_->AddEvent(
       media_log_->CreateEvent(MediaLogEvent::PIPELINE_CREATED));
+  {
+    base::AutoLock auto_lock(lock_);

[scherkus (not reviewing), 2014/07/08 18:37:20]

we shouldn't have any synchronization issues inside the ctor

also .. what does the Clock ctor do? does it make sense to have it start at 0,0?

[xhwang, 2014/07/08 19:32:16]

Done.
The media_time_ defaults to base::TimeDelta(). The max_time_ defaults to
kNoTimeStamp(). It seems the Clock actually uses the info about whether the
max_time_ has been set to calculate Eclapsed(). I think these can be simplified
but it definitely deserves a separate CL.

+    // We do not want to start the clock running. We only want to set the
+    // base media time so our timestamp calculations will be correct.
+    clock_->SetTime(base::TimeDelta(), base::TimeDelta());
+  }
 }
 
 Pipeline::~Pipeline() {
@@ -373,12 +379,6 @@ void Pipeline::StateTransitionTask(PipelineStatus status) {
       // state.
       if (filter_collection_) {
         filter_collection_.reset();
-        {
-          base::AutoLock l(lock_);
-          // We do not want to start the clock running. We only want to set the
-          // base media time so our timestamp calculations will be correct.
-          clock_->SetTime(base::TimeDelta(), base::TimeDelta());
-        }
         if (!audio_renderer_ && !video_renderer_) {
           ErrorChangedTask(PIPELINE_ERROR_COULD_NOT_RENDER);
           return;
@@ -400,6 +400,11 @@ void Pipeline::StateTransitionTask(PipelineStatus status) {
 
       base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 
+      {
+        base::AutoLock auto_lock(lock_);
+        clock_->SetTime(start_timestamp_, start_timestamp_);
+      }
+
       if (audio_renderer_)
         audio_renderer_->StartPlayingFrom(start_timestamp_);
       if (video_renderer_)
@@ -441,6 +446,10 @@ void Pipeline::DoSeek(
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!pending_callbacks_.get());
   SerialRunner::Queue bound_fns;
+  {
+    base::AutoLock auto_lock(lock_);
+    PauseClockAndStopRendering_Locked();
+  }
 
   // Pause.
   if (text_renderer_) {
@@ -683,12 +692,6 @@ void Pipeline::SeekTask(TimeDelta time, const PipelineStatusCB& seek_cb) {
   text_ended_ = false;
   start_timestamp_ = time;
 
-  // Kick off seeking!
-  {
-    base::AutoLock auto_lock(lock_);
-    PauseClockAndStopRendering_Locked();
-    clock_->SetTime(time, time);
-  }
   DoSeek(time, base::Bind(
       &Pipeline::OnStateTransition, base::Unretained(this)));
 }