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();
+  pending_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;
+  pending_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 64 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 = pending_seek_time_ == kNoTimestamp

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

Nit: Swap order.

[chcunningham, 2017/01/04 21:15:46]

Done.

+                                   ? renderer_wrapper_->GetMediaTime()
+                                   : pending_seek_time_;
 
   // 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_;
@@ skipping 157 matching lines @@
 
   DCHECK(client_);
   client_->OnVideoOpacityChange(opaque);
 }
 
 void PipelineImpl::OnSeekDone() {
   DVLOG(3) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(IsRunning());
 
+  pending_seek_time_ = kNoTimestamp;

[chcunningham, 2017/01/04 21:01:37]

Should I also be clearing this in PipelineImp::Resume()? I notice
last_media_time_ is cleared there, but its not obvious why.

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

Resume() should also set the seek time, I believe that OnSeekDone() completes
both operations.

[chcunningham, 2017/01/04 21:15:46]

Done.

+
   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