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

media/base/pipeline.cc

 // Copyright (c) 2012 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/pipeline.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 37 matching lines @@
       audio_ended_(false),
       video_ended_(false),
       text_ended_(false),
       audio_buffering_state_(BUFFERING_HAVE_NOTHING),
       video_buffering_state_(BUFFERING_HAVE_NOTHING),
       demuxer_(NULL),
       underflow_disabled_for_testing_(false) {
   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() {
   DCHECK(thread_checker_.CalledOnValidThread())
       << "Pipeline must be destroyed on same thread that created it";
   DCHECK(!running_) << "Stop() must complete before destroying object";
   DCHECK(stop_cb_.is_null());
   DCHECK(seek_cb_.is_null());
 
   media_log_->AddEvent(
@@ skipping 298 matching lines @@
       return InitializeAudioRenderer(done_cb);
 
     case kInitVideoRenderer:
       return InitializeVideoRenderer(done_cb);
 
     case kPlaying:
       // Finish initial start sequence the first time we enter the playing
       // 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;
         }
 
         {
           PipelineMetadata metadata;
           metadata.has_audio = audio_renderer_;
           metadata.has_video = video_renderer_;
           metadata.timeline_offset = demuxer_->GetTimelineOffset();
           DemuxerStream* stream = demuxer_->GetStream(DemuxerStream::VIDEO);
           if (stream) {
             metadata.natural_size =
                 stream->video_decoder_config().natural_size();
           }
           metadata_cb_.Run(metadata);
         }
       }
 
       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_)
         video_renderer_->StartPlayingFrom(start_timestamp_);
       if (text_renderer_)
         text_renderer_->StartPlaying();
 
       PlaybackRateChangedTask(GetPlaybackRate());
       VolumeChangedTask(GetVolume());
       return;
@@ skipping 21 matching lines @@
   DCHECK_EQ(*video_buffering_state, BUFFERING_HAVE_NOTHING);
 }
 #endif
 
 void Pipeline::DoSeek(
     base::TimeDelta seek_timestamp,
     const PipelineStatusCB& done_cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!pending_callbacks_.get());
   SerialRunner::Queue bound_fns;
+  {
+    base::AutoLock auto_lock(lock_);
+    PauseClockAndStopRendering_Locked();
+  }
 
   // Pause.
   if (text_renderer_) {
     bound_fns.Push(base::Bind(
         &TextRenderer::Pause, base::Unretained(text_renderer_.get())));
   }
 
   // Flush.
   if (audio_renderer_) {
     bound_fns.Push(base::Bind(
@@ skipping 222 matching lines @@
 
   DCHECK(seek_cb_.is_null());
 
   SetState(kSeeking);
   seek_cb_ = seek_cb;
   audio_ended_ = false;
   video_ended_ = false;
   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)));
 }
 
 void Pipeline::DoAudioRendererEnded() {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   if (state_ != kPlaying)
     return;
 
@@ skipping 208 matching lines @@
 void Pipeline::StartClockIfWaitingForTimeUpdate_Locked() {
   lock_.AssertAcquired();
   if (clock_state_ != CLOCK_WAITING_FOR_AUDIO_TIME_UPDATE)
     return;
 
   clock_state_ = CLOCK_PLAYING;
   clock_->Play();
 }
 
 }  // namespace media