Fix ChunkDemuxer seek deadlock

media/filters/audio_renderer_base.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/audio_renderer_base.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 49 matching lines @@
   }
   if (!callback.is_null()) {
     callback.Run();
   }
 }
 
 void AudioRendererBase::Seek(base::TimeDelta time, const FilterStatusCB& cb) {
   base::AutoLock auto_lock(lock_);
   DCHECK_EQ(kPaused, state_);
   DCHECK(!pending_read_) << "Pending read must complete before seeking";
+  DCHECK(pause_callback_.is_null());
   DCHECK(seek_cb_.is_null());
   state_ = kSeeking;
   seek_cb_ = cb;
   seek_timestamp_ = time;
 
   // Throw away everything and schedule our reads.
   last_fill_buffer_time_ = base::TimeDelta();
   recieved_end_of_stream_ = false;
   rendered_end_of_stream_ = false;
 
@@ skipping 64 matching lines @@
   CHECK(pending_read_);
   pending_read_ = false;
 
   // TODO(scherkus): this happens due to a race, primarily because Stop() is a
   // synchronous call when it should be asynchronous and accept a callback.
   // Refer to http://crbug.com/16059
   if (state_ == kStopped) {
     return;
   }
 
-  // Don't enqueue an end-of-stream buffer because it has no data, otherwise
-  // discard decoded audio data until we reach our desired seek timestamp.
-  if (buffer->IsEndOfStream()) {
-    recieved_end_of_stream_ = true;
+  if (buffer) {

[Ami GONE FROM CHROMIUM, 2012/01/27 23:44:58]

that you can't reverse the test and early-return (b/c of the code from l.175
downwards) smells wrong.

I wonder whether this and the next big if block can be unified into a switch
(state).  Want to take a crack at it?

[acolwell GONE FROM CHROMIUM, 2012/01/29 03:00:41]

Done. Cleaned things up nicely and uncovered an unused state!

+    // Don't enqueue an end-of-stream buffer because it has no data, otherwise
+    // discard decoded audio data until we reach our desired seek timestamp.
+    if (buffer->IsEndOfStream()) {
+      recieved_end_of_stream_ = true;
 
-    // Transition to kPlaying if we are currently handling an underflow since no
-    // more data will be arriving.
-    if (state_ == kUnderflow || state_ == kRebuffering)
-      state_ = kPlaying;
-  } else if (state_ == kSeeking && !buffer->IsEndOfStream() &&
-             (buffer->GetTimestamp() + buffer->GetDuration()) <
-                 seek_timestamp_) {
-    ScheduleRead_Locked();
-  } else {
-    // Note: Calling this may schedule more reads.
-    algorithm_->EnqueueBuffer(buffer);
+      // Transition to kPlaying if we are currently handling an underflow since
+      // no more data will be arriving.
+      if (state_ == kUnderflow || state_ == kRebuffering)
+        state_ = kPlaying;
+    } else if (state_ == kSeeking && !buffer->IsEndOfStream() &&
+               (buffer->GetTimestamp() + buffer->GetDuration()) <
+               seek_timestamp_) {
+      ScheduleRead_Locked();
+    } else {
+      // Note: Calling this may schedule more reads.
+      algorithm_->EnqueueBuffer(buffer);
+    }
   }
 
   // Check for our preroll complete condition.
   if (state_ == kSeeking) {
     DCHECK(!seek_cb_.is_null());
-    if (algorithm_->IsQueueFull() || recieved_end_of_stream_) {
-      // Transition into paused whether we have data in |algorithm_| or not.
-      // FillBuffer() will play silence if there's nothing to fill.
+
+    // Transition to paused if we have enough data or have reached the end of
+    // the stream. A NULL buffer also triggers the transition because it
+    // indicates that another seek will happen right after we complete this one
+    // so there is no point in continuing to preroll.
+    if (algorithm_->IsQueueFull() || recieved_end_of_stream_ || !buffer) {
       state_ = kPaused;
       ResetAndRunCB(&seek_cb_, PIPELINE_OK);
     }
   } else if (state_ == kPaused && !pending_read_) {
     // No more pending read!  We're now officially "paused".
     if (!pause_callback_.is_null()) {
       pause_callback_.Run();
       pause_callback_.Reset();
     }
   }
@@ skipping 93 matching lines @@
   base::AutoLock auto_lock(lock_);
   algorithm_->SetPlaybackRate(playback_rate);
 }
 
 float AudioRendererBase::GetPlaybackRate() {
   base::AutoLock auto_lock(lock_);
   return algorithm_->playback_rate();
 }
 
 }  // namespace media