Fix race condition in media Pipeline::GetMediaTime() after seeking.

media/base/pipeline_impl.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_impl.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 940 matching lines @@
   DCHECK(demuxer);
   DCHECK(renderer);
   DCHECK(client);
   DCHECK(!seek_cb.is_null());
 
   DCHECK(!client_);
   DCHECK(seek_cb_.is_null());
   client_ = client;
   seek_cb_ = seek_cb;
   last_media_time_ = base::TimeDelta();
+  seek_time_ = kNoTimestamp;
 
   std::unique_ptr<TextRenderer> text_renderer;
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableInbandTextTracks)) {
     text_renderer.reset(new TextRenderer(
         media_task_runner_,
         BindToCurrentLoop(base::Bind(&PipelineImpl::OnAddTextTrack,
                                      weak_factory_.GetWeakPtr()))));
   }
 
@@ skipping 61 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!seek_cb.is_null());
 
   if (!IsRunning()) {
     DLOG(ERROR) << "Media pipeline isn't running. Ignoring Seek().";
     return;
   }
 
   DCHECK(seek_cb_.is_null());
   seek_cb_ = seek_cb;
+  seek_time_ = time;
   last_media_time_ = base::TimeDelta();
   media_task_runner_->PostTask(
       FROM_HERE, base::Bind(&RendererWrapper::Seek,
                             base::Unretained(renderer_wrapper_.get()), time));
 }
 
 void PipelineImpl::Suspend(const PipelineStatusCB& suspend_cb) {
   DVLOG(2) << __func__;
   DCHECK(!suspend_cb.is_null());
 
@@ skipping 10 matching lines @@
                           base::TimeDelta time,
                           const PipelineStatusCB& seek_cb) {
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(renderer);
   DCHECK(!seek_cb.is_null());
 
   DCHECK(IsRunning());
   DCHECK(seek_cb_.is_null());
   seek_cb_ = seek_cb;
+  seek_time_ = time;
   last_media_time_ = base::TimeDelta();
 
   media_task_runner_->PostTask(
       FROM_HERE, base::Bind(&RendererWrapper::Resume,
                             base::Unretained(renderer_wrapper_.get()),
                             base::Passed(&renderer), time));
 }
 
 bool PipelineImpl::IsRunning() const {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 34 matching lines @@
   volume_ = volume;
   media_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RendererWrapper::SetVolume,
                  base::Unretained(renderer_wrapper_.get()), volume_));
 }
 
 base::TimeDelta PipelineImpl::GetMediaTime() const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  base::TimeDelta media_time = renderer_wrapper_->GetMediaTime();
+  // Don't trust renderer time during a pending seek. Renderer may return
+  // pre-seek time which may corrupt |last_media_time_| used for clamping.
+  base::TimeDelta media_time = seek_time_ != kNoTimestamp
+                                   ? seek_time_
+                                   : renderer_wrapper_->GetMediaTime();
 
   // Clamp current media time to the last reported value, this prevents higher
   // level clients from seeing time go backwards based on inaccurate or spurious
   // delay values reported to the AudioClock.
   //
   // It is expected that such events are transient and will be recovered as
   // rendering continues over time.
   if (media_time < last_media_time_) {
     DVLOG(2) << __func__ << ": actual=" << media_time
              << " clamped=" << last_media_time_;
     return last_media_time_;
   }
 
   DVLOG(3) << __func__ << ": " << media_time.InMilliseconds() << " ms";

[sandersd (OOO until July 31), 2017/01/04 21:49:26]

Probably best to not set this until the seek completes, since it is conceivable
to have a renderer that starts playback earlier than the target seek time (eg.
fastSeek()).

It looks like all the current impls are monotonic starting from the seek time,
so I don't think this can reintroduce a time-going-backwards bug.

[chcunningham, 2017/01/04 22:33:47]

Done.

   last_media_time_ = media_time;
   return last_media_time_;
 }
 
 Ranges<base::TimeDelta> PipelineImpl::GetBufferedTimeRanges() const {
   DCHECK(thread_checker_.CalledOnValidThread());
   return renderer_wrapper_->GetBufferedTimeRanges();
 }
 
 base::TimeDelta PipelineImpl::GetMediaDuration() const {
@@ skipping 143 matching lines @@
 
   DCHECK(client_);
   client_->OnVideoOpacityChange(opaque);
 }
 
 void PipelineImpl::OnSeekDone() {
   DVLOG(3) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(IsRunning());
 
+  seek_time_ = kNoTimestamp;
+
   DCHECK(!seek_cb_.is_null());
   base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 void PipelineImpl::OnSuspendDone() {
   DVLOG(3) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(IsRunning());
 
   DCHECK(!suspend_cb_.is_null());
   base::ResetAndReturn(&suspend_cb_).Run(PIPELINE_OK);
 }
 
 }  // namespace media