Cancel pending seek in ChunkDemuxer when a new seek occurs

This makes it so playback doesn't stall if one seeks into an unbuffered
region then seeks back into a buffered region.

BUG=132935


Review URL: https://chromiumcodereview.appspot.com/10835044

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150489 0039d316-1c4b-4281-b951-d872f2087c98

media/filters/chunk_demuxer.cc

@@ -173,6 +173,7 @@ class ChunkDemuxerStream : public DemuxerStream {
 
   void StartWaitingForSeek();
   void Seek(TimeDelta time);
+  void CancelPendingSeek();
   bool IsSeekPending() const;
 
   // Add buffers to this stream.  Buffers are stored in SourceBufferStreams,
@@ -214,6 +215,7 @@ class ChunkDemuxerStream : public DemuxerStream {
   enum State {
     RETURNING_DATA_FOR_READS,
     WAITING_FOR_SEEK,
+    CANCELED,
     SHUTDOWN,
   };
 
@@ -280,12 +282,29 @@ void ChunkDemuxerStream::Seek(TimeDelta time) {
 
   DCHECK(read_cbs_.empty());
 
+  // Ignore seek requests when canceled.
+  if (state_ == CANCELED)
+    return;
+
   stream_->Seek(time);
 
   if (state_ == WAITING_FOR_SEEK)
     ChangeState_Locked(RETURNING_DATA_FOR_READS);
 }
 
+void ChunkDemuxerStream::CancelPendingSeek() {
+  DVLOG(1) << "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);
+}
+
 bool ChunkDemuxerStream::IsSeekPending() const {
   base::AutoLock auto_lock(lock_);
   return stream_->IsSeekPending();
@@ -490,6 +509,7 @@ bool ChunkDemuxerStream::GetNextBuffer_Locked(
           return true;
       }
       break;
+    case CANCELED:
     case WAITING_FOR_SEEK:
       // Null buffers should be returned in this state since we are waiting
       // for a seek. Any buffers in the SourceBuffer should NOT be returned
@@ -540,6 +560,7 @@ void ChunkDemuxer::Stop(const base::Closure& callback) {
 void ChunkDemuxer::Seek(TimeDelta time, const PipelineStatusCB& cb) {
   DVLOG(1) << "Seek(" << time.InSecondsF() << ")";
   DCHECK(time >= start_time_);
+  DCHECK(seek_cb_.is_null());
 
   PipelineStatus status = PIPELINE_ERROR_INVALID_STATE;
   {
@@ -605,6 +626,24 @@ void ChunkDemuxer::StartWaitingForSeek() {
   ChangeState_Locked(INITIALIZED);
 }
 
+void ChunkDemuxer::CancelPendingSeek() {
+  PipelineStatusCB cb;
+  {
+    base::AutoLock auto_lock(lock_);
+    if (IsSeekPending_Locked() && !seek_cb_.is_null()) {
+      std::swap(cb, seek_cb_);
+    }
+    if (audio_)
+      audio_->CancelPendingSeek();
+
+    if (video_)
+      video_->CancelPendingSeek();
+  }
+
+  if (!cb.is_null())
+    cb.Run(PIPELINE_OK);
+}
+
 ChunkDemuxer::Status ChunkDemuxer::AddId(const std::string& id,
                                          const std::string& type,
                                          std::vector<std::string>& codecs) {

media/filters/chunk_demuxer.h

@@ -47,6 +47,7 @@ class MEDIA_EXPORT ChunkDemuxer : public Demuxer {
 
   // Methods used by an external object to control this demuxer.
   void StartWaitingForSeek();
+  void CancelPendingSeek();
 
   // Registers a new |id| to use for AppendData() calls. |type| indicates
   // the MIME type for the data that we intend to append for this ID.

media/filters/chunk_demuxer_unittest.cc

@@ -86,6 +86,14 @@ static void OnReadDone(const base::TimeDelta& expected_time,
   *called = true;
 }
 
+static void OnReadDone_AbortExpected(
+    bool* called, DemuxerStream::Status status,
+    const scoped_refptr<DecoderBuffer>& buffer) {
+  EXPECT_EQ(status, DemuxerStream::kAborted);
+  EXPECT_EQ(NULL, buffer.get());
+  *called = true;
+}
+
 static void OnReadDone_EOSExpected(bool* called,
                                    DemuxerStream::Status status,
                                    const scoped_refptr<DecoderBuffer>& buffer) {
@@ -1489,6 +1497,79 @@ TEST_F(ChunkDemuxerTest, TestRemoveId) {
   GenerateSingleStreamExpectedReads(0, 4, video, kVideoBlockDuration);
 }
 
+TEST_F(ChunkDemuxerTest, TestSeekCanceled) {
+  ASSERT_TRUE(InitDemuxer(true, true, false));
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  // Append cluster at the beginning of the stream.
+  scoped_ptr<Cluster> start_cluster(GenerateCluster(0, 4));
+  ASSERT_TRUE(AppendData(start_cluster->data(), start_cluster->size()));
+
+  // Seek to an unbuffered region.
+  demuxer_->StartWaitingForSeek();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(50),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+  // Attempt to read in unbuffered area; should not fulfill the read.
+  bool audio_read_done = false;
+  bool video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone_AbortExpected, &audio_read_done));
+  video->Read(base::Bind(&OnReadDone_AbortExpected, &video_read_done));
+  EXPECT_FALSE(audio_read_done);
+  EXPECT_FALSE(video_read_done);
+
+  // Now cancel the pending seek, which should flush the reads with empty
+  // buffers.
+  demuxer_->CancelPendingSeek();
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+
+  // A seek back to the buffered region should succeed.
+  demuxer_->StartWaitingForSeek();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(0),
+                 NewExpectedStatusCB(PIPELINE_OK));
+  GenerateExpectedReads(0, 4, audio, video);
+}
+
+TEST_F(ChunkDemuxerTest, TestSeekCanceledWhileWaitingForSeek) {
+  ASSERT_TRUE(InitDemuxer(true, true, false));
+
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+
+  // Append cluster at the beginning of the stream.
+  scoped_ptr<Cluster> start_cluster(GenerateCluster(0, 4));
+  ASSERT_TRUE(AppendData(start_cluster->data(), start_cluster->size()));
+
+  // Start waiting for a seek.
+  demuxer_->StartWaitingForSeek();
+
+  // Now cancel the upcoming seek to an unbuffered region.
+  demuxer_->CancelPendingSeek();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(50),
+                 NewExpectedStatusCB(PIPELINE_OK));
+
+  // Read requests should be fulfilled with empty buffers.
+  bool audio_read_done = false;
+  bool video_read_done = false;
+  audio->Read(base::Bind(&OnReadDone_AbortExpected, &audio_read_done));
+  video->Read(base::Bind(&OnReadDone_AbortExpected, &video_read_done));
+  EXPECT_TRUE(audio_read_done);
+  EXPECT_TRUE(video_read_done);
+
+  // A seek back to the buffered region should succeed.
+  demuxer_->StartWaitingForSeek();
+  demuxer_->Seek(base::TimeDelta::FromSeconds(0),
+                 NewExpectedStatusCB(PIPELINE_OK));
+  GenerateExpectedReads(0, 4, audio, video);
+}
+
 // Test that Seek() successfully seeks to all source IDs.
 TEST_F(ChunkDemuxerTest, TestSeekAudioAndVideoSources) {
   std::string audio_id = "audio1";

webkit/media/webmediaplayer_impl.cc

@@ -313,6 +313,7 @@ void WebMediaPlayerImpl::seek(float seconds) {
   if (seeking_) {
     pending_seek_ = true;
     pending_seek_seconds_ = seconds;
+    proxy_->DemuxerCancelPendingSeek();
     return;
   }
 

webkit/media/webmediaplayer_proxy.cc

@@ -188,6 +188,11 @@ void WebMediaPlayerProxy::DemuxerStartWaitingForSeek() {
     chunk_demuxer_->StartWaitingForSeek();
 }
 
+void WebMediaPlayerProxy::DemuxerCancelPendingSeek() {
+  if (chunk_demuxer_.get())
+    chunk_demuxer_->CancelPendingSeek();
+}
+
 media::ChunkDemuxer::Status WebMediaPlayerProxy::DemuxerAddId(
     const std::string& id,
     const std::string& type,

webkit/media/webmediaplayer_proxy.h

@@ -88,6 +88,7 @@ class WebMediaPlayerProxy
 
   // Methods for Demuxer communication.
   void DemuxerStartWaitingForSeek();
+  void DemuxerCancelPendingSeek();
   media::ChunkDemuxer::Status DemuxerAddId(const std::string& id,
                                            const std::string& type,
                                            std::vector<std::string>& codecs);