Move pending seek cancellation logic from ChunkDemuxerStream to ChunkDemuxer.

media/filters/chunk_demuxer.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/filters/chunk_demuxer.h"
 
 #include <algorithm>
 #include <deque>
 #include <limits>
 
@@ skipping 187 matching lines @@
   typedef std::deque<scoped_refptr<StreamParserBuffer> > BufferQueue;
   typedef std::deque<ReadCB> ReadCBQueue;
   typedef std::deque<base::Closure> ClosureQueue;
 
   ChunkDemuxerStream(const AudioDecoderConfig& audio_config,
                      const LogCB& log_cb);
   ChunkDemuxerStream(const VideoDecoderConfig& video_config,
                      const LogCB& log_cb);
   virtual ~ChunkDemuxerStream();
 
-  void StartWaitingForSeek();
+  void AbortReads();
   void Seek(TimeDelta time);
-  void CancelPendingSeek();
   bool IsSeekWaitingForData() const;
 
   // Add buffers to this stream.  Buffers are stored in SourceBufferStreams,
   // which handle ordering and overlap resolution.
   // Returns true if buffers were successfully added.
   bool Append(const StreamParser::BufferQueue& buffers);
 
   // Signal to the stream that duration has changed to |duration|.
   void OnSetDuration(TimeDelta duration);
 
@@ skipping 17 matching lines @@
 
   // DemuxerStream methods.
   virtual void Read(const ReadCB& read_cb) OVERRIDE;
   virtual Type type() OVERRIDE;
   virtual void EnableBitstreamConverter() OVERRIDE;
   virtual AudioDecoderConfig audio_decoder_config() OVERRIDE;
   virtual VideoDecoderConfig video_decoder_config() OVERRIDE;
 
  private:
   enum State {
+    UNINITIALIZED,
     RETURNING_DATA_FOR_READS,
-    WAITING_FOR_SEEK,
-    CANCELED,
+    RETURNING_ABORT_FOR_READS,
     SHUTDOWN,
   };
 
   // Assigns |state_| to |state|
   void ChangeState_Locked(State state);
 
   // Adds the callback to |read_cbs_| so it can be called later when we
   // have data.
   void DeferRead_Locked(const ReadCB& read_cb);
 
@@ skipping 15 matching lines @@
   mutable base::Lock lock_;
   State state_;
   ReadCBQueue read_cbs_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(ChunkDemuxerStream);
 };
 
 ChunkDemuxerStream::ChunkDemuxerStream(const AudioDecoderConfig& audio_config,
                                        const LogCB& log_cb)
     : type_(AUDIO),
-      state_(RETURNING_DATA_FOR_READS) {
+      state_(UNINITIALIZED) {
   stream_.reset(new SourceBufferStream(audio_config, log_cb));
 }
 
 ChunkDemuxerStream::ChunkDemuxerStream(const VideoDecoderConfig& video_config,
                                        const LogCB& log_cb)
     : type_(VIDEO),
-      state_(RETURNING_DATA_FOR_READS) {
+      state_(UNINITIALIZED) {
   stream_.reset(new SourceBufferStream(video_config, log_cb));
 }
 
-void ChunkDemuxerStream::StartWaitingForSeek() {
-  DVLOG(1) << "ChunkDemuxerStream::StartWaitingForSeek()";
+void ChunkDemuxerStream::AbortReads() {
+  DVLOG(1) << "ChunkDemuxerStream::AbortReads()";
   ReadCBQueue read_cbs;
   {
     base::AutoLock auto_lock(lock_);
-    ChangeState_Locked(WAITING_FOR_SEEK);
+    ChangeState_Locked(RETURNING_ABORT_FOR_READS);
     std::swap(read_cbs_, read_cbs);
   }
 
   for (ReadCBQueue::iterator it = read_cbs.begin(); it != read_cbs.end(); ++it)
     it->Run(kAborted, NULL);
 }
 
 void ChunkDemuxerStream::Seek(TimeDelta time) {
   base::AutoLock auto_lock(lock_);
-
   DCHECK(read_cbs_.empty());
-
-  // Ignore seek requests when canceled.
-  if (state_ == CANCELED)
-    return;
+  DCHECK(state_ == UNINITIALIZED || state_ == RETURNING_ABORT_FOR_READS);
 
   stream_->Seek(time);
-
-  if (state_ == WAITING_FOR_SEEK)
-    ChangeState_Locked(RETURNING_DATA_FOR_READS);
-}
-
-void ChunkDemuxerStream::CancelPendingSeek() {
-  DVLOG(1) << "ChunkDemuxerStream::CancelPendingSeek()";
-  ReadCBQueue read_cbs;
-  {
-    base::AutoLock auto_lock(lock_);
-    ChangeState_Locked(CANCELED);
-    std::swap(read_cbs_, read_cbs);
-  }
-
-  for (ReadCBQueue::iterator it = read_cbs.begin(); it != read_cbs.end(); ++it)
-    it->Run(kAborted, NULL);
+  ChangeState_Locked(RETURNING_DATA_FOR_READS);
 }
 
 bool ChunkDemuxerStream::IsSeekWaitingForData() const {
   base::AutoLock auto_lock(lock_);
   return stream_->IsSeekPending();
 }
 
 void ChunkDemuxerStream::OnNewMediaSegment(TimeDelta start_timestamp) {
   base::AutoLock auto_lock(lock_);
   stream_->OnNewMediaSegment(start_timestamp);
@@ skipping 96 matching lines @@
     message_loop_proxy->PostTask(FROM_HERE, base::Bind(
         &RunOnMessageLoop, read_cb, message_loop_proxy, status, buffer));
     return;
   }
 
   read_cb.Run(status, buffer);
 }
 
 // DemuxerStream methods.
 void ChunkDemuxerStream::Read(const ReadCB& read_cb) {
+  base::AutoLock auto_lock(lock_);
+  DCHECK_NE(state_, UNINITIALIZED);
+
   DemuxerStream::Status status = kOk;
   scoped_refptr<StreamParserBuffer> buffer;
-  {
-    base::AutoLock auto_lock(lock_);
-    if (!read_cbs_.empty() || !GetNextBuffer_Locked(&status, &buffer)) {
-      DeferRead_Locked(read_cb);
-      return;
-    }
+
+  if (!read_cbs_.empty() || !GetNextBuffer_Locked(&status, &buffer)) {
+    DeferRead_Locked(read_cb);
+    return;
   }
 
   base::MessageLoopProxy::current()->PostTask(FROM_HERE, base::Bind(
       read_cb, status, buffer));
 }
 
 DemuxerStream::Type ChunkDemuxerStream::type() { return type_; }
 
 void ChunkDemuxerStream::EnableBitstreamConverter() {}
 
@@ skipping 43 matching lines @@
     read_cbs_.pop_front();
   }
 }
 
 bool ChunkDemuxerStream::GetNextBuffer_Locked(
     DemuxerStream::Status* status,
     scoped_refptr<StreamParserBuffer>* buffer) {
   lock_.AssertAcquired();
 
   switch (state_) {
+    case UNINITIALIZED:
+      NOTREACHED();
+      return false;
+      break;

[xhwang, 2013/06/21 20:44:13]

nit: no need to break after return.

[acolwell GONE FROM CHROMIUM, 2013/06/21 22:08:38]

Done.

     case RETURNING_DATA_FOR_READS:
       switch (stream_->GetNextBuffer(buffer)) {
         case SourceBufferStream::kSuccess:
           *status = DemuxerStream::kOk;
           return true;
         case SourceBufferStream::kNeedBuffer:
           return false;
         case SourceBufferStream::kEndOfStream:
           *status = DemuxerStream::kOk;
           *buffer = StreamParserBuffer::CreateEOSBuffer();
           return true;
         case SourceBufferStream::kConfigChange:
           DVLOG(2) << "Config change reported to ChunkDemuxerStream.";
           *status = kConfigChanged;
           *buffer = NULL;
           return true;
       }
       break;
-    case CANCELED:
-    case WAITING_FOR_SEEK:
+    case RETURNING_ABORT_FOR_READS:
       // Null buffers should be returned in this state since we are waiting
       // for a seek. Any buffers in the SourceBuffer should NOT be returned
       // because they are associated with the seek.
       DCHECK(read_cbs_.empty());
       *status = DemuxerStream::kAborted;
       *buffer = NULL;
       return true;
     case SHUTDOWN:
       DCHECK(read_cbs_.empty());
       *status = DemuxerStream::kOk;
       *buffer = StreamParserBuffer::CreateEOSBuffer();
       return true;
   }
 
   NOTREACHED();
   return false;
 }
 
 ChunkDemuxer::ChunkDemuxer(const base::Closure& open_cb,
                            const NeedKeyCB& need_key_cb,
                            const AddTextTrackCB& add_text_track_cb,
                            const LogCB& log_cb)
     : state_(WAITING_FOR_INIT),
+      cancel_next_seek_(false),
       host_(NULL),
       open_cb_(open_cb),
       need_key_cb_(need_key_cb),
       add_text_track_cb_(add_text_track_cb),
       log_cb_(log_cb),
       duration_(kNoTimestamp()),
       user_specified_duration_(-1) {
   DCHECK(!open_cb_.is_null());
   DCHECK(!need_key_cb_.is_null());
 }
@@ skipping 18 matching lines @@
 
 void ChunkDemuxer::Stop(const base::Closure& callback) {
   DVLOG(1) << "Stop()";
   Shutdown();
   callback.Run();
 }
 
 void ChunkDemuxer::Seek(TimeDelta time, const PipelineStatusCB& cb) {
   DVLOG(1) << "Seek(" << time.InSecondsF() << ")";
   DCHECK(time >= TimeDelta());
+
+  base::AutoLock auto_lock(lock_);
   DCHECK(seek_cb_.is_null());
 
-  PipelineStatus status = PIPELINE_ERROR_INVALID_STATE;
-  base::AutoLock auto_lock(lock_);
-
   seek_cb_ = BindToCurrentLoop(cb);
-  if (state_ == INITIALIZED || state_ == ENDED) {
-    if (audio_)
-      audio_->Seek(time);
-
-    if (video_)
-      video_->Seek(time);
-
-    if (IsSeekWaitingForData_Locked()) {
-      DVLOG(1) << "Seek() : waiting for more data to arrive.";
-      return;
-    }
-
-    status = PIPELINE_OK;
+  if (state_ != INITIALIZED && state_ != ENDED) {
+    base::ResetAndReturn(&seek_cb_).Run(PIPELINE_ERROR_INVALID_STATE);
+    return;
   }
 
-  base::ResetAndReturn(&seek_cb_).Run(status);
+  if (cancel_next_seek_) {
+    cancel_next_seek_ = false;
+    base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
+    return;
+  }
+
+  if (audio_)
+    audio_->Seek(time);
+
+  if (video_)
+    video_->Seek(time);
+
+  if (IsSeekWaitingForData_Locked()) {
+    DVLOG(1) << "Seek() : waiting for more data to arrive.";
+    return;
+  }
+
+  base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 void ChunkDemuxer::OnAudioRendererDisabled() {
   base::AutoLock auto_lock(lock_);
   audio_->Shutdown();
   disabled_audio_ = audio_.Pass();
 }
 
 // Demuxer implementation.
 DemuxerStream* ChunkDemuxer::GetStream(DemuxerStream::Type type) {
   base::AutoLock auto_lock(lock_);
   if (type == DemuxerStream::VIDEO)
     return video_.get();
 
   if (type == DemuxerStream::AUDIO)
     return audio_.get();
 
   return NULL;
 }
 
 TimeDelta ChunkDemuxer::GetStartTime() const {
   return TimeDelta();
 }
 
 void ChunkDemuxer::StartWaitingForSeek() {
   DVLOG(1) << "StartWaitingForSeek()";
   base::AutoLock auto_lock(lock_);
   DCHECK(state_ == INITIALIZED || state_ == ENDED || state_ == SHUTDOWN);
+  DCHECK(seek_cb_.is_null());
 
   if (state_ == SHUTDOWN)
     return;
 
   if (audio_)
-    audio_->StartWaitingForSeek();
+    audio_->AbortReads();
 
   if (video_)
-    video_->StartWaitingForSeek();
+    video_->AbortReads();
+
+  cancel_next_seek_ = false;

[xhwang, 2013/06/21 21:21:36]

Can |cancel_next_seek_| be true here? If so, does that mean the caller didn't
wait for the previous Seek() to be cancelled to call StartWaitingForSeek(),
which shouldn't happen?

[acolwell GONE FROM CHROMIUM, 2013/06/21 22:08:38]

This is needed to handle the case where the initial preroll gets cancelled. In
that case there is no Seek() call in flight. We are just waiting for the
pipeline initialization to complete. Once it completes, WMPI initiates the new
seek and calls StartWaitingForSeek(). In this case we want to clear the
cancellation state because we actually want to pay attention to the next Seek().

 }
 
 void ChunkDemuxer::CancelPendingSeek() {
   base::AutoLock auto_lock(lock_);
-  DCHECK(seek_cb_.is_null() != IsSeekWaitingForData_Locked());
+  DCHECK_NE(state_, INITIALIZING);
+  DCHECK(seek_cb_.is_null() || IsSeekWaitingForData_Locked());

[acolwell GONE FROM CHROMIUM, 2013/06/21 16:33:33]

Changed this back to the original DCHECK because there is an implicit seek on
the initial preroll. I added a unit test to make sure this case is covered.

+
+  if (cancel_next_seek_)
+    return;
 
   if (audio_)
-    audio_->CancelPendingSeek();
+    audio_->AbortReads();
 
   if (video_)
-    video_->CancelPendingSeek();
+    video_->AbortReads();
 
-  if (!seek_cb_.is_null())
-    base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
+  if (seek_cb_.is_null()) {
+    cancel_next_seek_ = true;
+    return;
+  }
+
+  base::ResetAndReturn(&seek_cb_).Run(PIPELINE_OK);
 }
 
 ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
                                          const std::string& type,
                                          std::vector<std::string>& codecs) {
   DCHECK_GT(codecs.size(), 0u);
   base::AutoLock auto_lock(lock_);
 
   if ((state_ != WAITING_FOR_INIT && state_ != INITIALIZING) || IsValidId(id))
     return kReachedIdLimit;
@@ skipping 390 matching lines @@
   }
 
   if (duration != TimeDelta() && duration_ == kNoTimestamp())
     UpdateDuration(duration);
 
   // Wait until all streams have initialized.
   if ((!source_id_audio_.empty() && !audio_) ||
       (!source_id_video_.empty() && !video_))
     return;
 
+  TimeDelta start_time = GetStartTime();

[acolwell GONE FROM CHROMIUM, 2013/06/21 16:33:33]

This is just to make it clear what these initial seeks are for.

   if (audio_)
-    audio_->Seek(TimeDelta());
+    audio_->Seek(start_time);
 
   if (video_)
-    video_->Seek(TimeDelta());
+    video_->Seek(start_time);
 
   if (duration_ == kNoTimestamp())
     duration_ = kInfiniteDuration();
 
   // The demuxer is now initialized after the |start_timestamp_| was set.
   ChangeState_Locked(INITIALIZED);
   base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 bool ChunkDemuxer::OnNewConfigs(bool has_audio, bool has_video,
@@ skipping 174 matching lines @@
 
 Ranges<TimeDelta> ChunkDemuxer::GetBufferedRanges() const {
   if (audio_ && !video_)
     return audio_->GetBufferedRanges(duration_);
   else if (!audio_ && video_)
     return video_->GetBufferedRanges(duration_);
   return ComputeIntersection();
 }
 
 }  // namespace media