Reland: Add Stop() to AudioDecoder. (https://codereview.chromium.org/126793002/)

media/filters/audio_renderer_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/filters/audio_renderer_impl.h"
 
 #include <math.h>
 
 #include <algorithm>
 
@@ skipping 129 matching lines @@
 }
 
 void AudioRendererImpl::DoFlush_Locked() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   lock_.AssertAcquired();
 
   DCHECK(!pending_read_);
   DCHECK_EQ(state_, kPaused);
 
   if (decrypting_demuxer_stream_) {
-    decrypting_demuxer_stream_->Reset(BindToCurrentLoop(
-        base::Bind(&AudioRendererImpl::ResetDecoder, weak_this_)));
+    decrypting_demuxer_stream_->Reset(
+        base::Bind(&AudioRendererImpl::ResetDecoder, weak_this_));
     return;
   }
 
   ResetDecoder();
 }
 
 void AudioRendererImpl::ResetDecoder() {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  decoder_->Reset(BindToCurrentLoop(
-      base::Bind(&AudioRendererImpl::ResetDecoderDone, weak_this_)));
+  decoder_->Reset(base::Bind(&AudioRendererImpl::ResetDecoderDone, weak_this_));
 }
 
 void AudioRendererImpl::ResetDecoderDone() {
-  base::AutoLock auto_lock(lock_);
-  if (state_ == kStopped)
-    return;
+  DCHECK(task_runner_->BelongsToCurrentThread());
+  {
+    base::AutoLock auto_lock(lock_);
+    if (state_ == kStopped)
+      return;
 
-  DCHECK_EQ(state_, kPaused);
-  DCHECK(!flush_cb_.is_null());
+    DCHECK_EQ(state_, kPaused);
+    DCHECK(!flush_cb_.is_null());
 
-  audio_time_buffered_ = kNoTimestamp();
-  current_time_ = kNoTimestamp();
-  received_end_of_stream_ = false;
-  rendered_end_of_stream_ = false;
-  preroll_aborted_ = false;
+    audio_time_buffered_ = kNoTimestamp();
+    current_time_ = kNoTimestamp();
+    received_end_of_stream_ = false;
+    rendered_end_of_stream_ = false;
+    preroll_aborted_ = false;
 
-  earliest_end_time_ = now_cb_.Run();
-  splicer_->Reset();
-  algorithm_->FlushBuffers();
-
+    earliest_end_time_ = now_cb_.Run();
+    splicer_->Reset();
+    algorithm_->FlushBuffers();
+  }
   base::ResetAndReturn(&flush_cb_).Run();
 }
 
 void AudioRendererImpl::Stop(const base::Closure& callback) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(!callback.is_null());
+  DCHECK(stop_cb_.is_null());
+
+  stop_cb_ = callback;
 
   // TODO(scherkus): Consider invalidating |weak_factory_| and replacing
   // task-running guards that check |state_| with DCHECK().
 
+  {
+    base::AutoLock auto_lock(lock_);
+    if (state_ == kInitializing) {
+      decoder_selector_->Abort();
+      return;
+    }
+
+    if (state_ == kStopped)
+      return;
+
+    ChangeState_Locked(kStopped);
+    algorithm_.reset();
+    underflow_cb_.Reset();
+    time_cb_.Reset();
+    flush_cb_.Reset();
+  }
+
   if (sink_) {
     sink_->Stop();
     sink_ = NULL;
   }
 
-  {
-    base::AutoLock auto_lock(lock_);
-    ChangeState_Locked(kStopped);
-    algorithm_.reset(NULL);
-    init_cb_.Reset();
-    underflow_cb_.Reset();
-    time_cb_.Reset();
-    flush_cb_.Reset();
+  if (decoder_) {
+    decoder_->Stop(base::ResetAndReturn(&stop_cb_));
+    return;
   }
 
-  callback.Run();
+  task_runner_->PostTask(FROM_HERE, base::ResetAndReturn(&stop_cb_));
 }
 
 void AudioRendererImpl::Preroll(base::TimeDelta time,
                                 const PipelineStatusCB& cb) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   base::AutoLock auto_lock(lock_);
   DCHECK(!sink_playing_);
   DCHECK_EQ(state_, kPaused);
   DCHECK(!pending_read_) << "Pending read must complete before seeking";
@@ skipping 20 matching lines @@
   DCHECK(!init_cb.is_null());
   DCHECK(!statistics_cb.is_null());
   DCHECK(!underflow_cb.is_null());
   DCHECK(!time_cb.is_null());
   DCHECK(!ended_cb.is_null());
   DCHECK(!disabled_cb.is_null());
   DCHECK(!error_cb.is_null());
   DCHECK_EQ(kUninitialized, state_);
   DCHECK(sink_);
 
+  state_ = kInitializing;
+
   weak_this_ = weak_factory_.GetWeakPtr();
   init_cb_ = init_cb;
   statistics_cb_ = statistics_cb;
   underflow_cb_ = underflow_cb;
   time_cb_ = time_cb;
   ended_cb_ = ended_cb;
   disabled_cb_ = disabled_cb;
   error_cb_ = error_cb;
 
   decoder_selector_->SelectAudioDecoder(
       stream,
       statistics_cb,
       base::Bind(&AudioRendererImpl::OnDecoderSelected, weak_this_));
 }
 
 void AudioRendererImpl::OnDecoderSelected(
     scoped_ptr<AudioDecoder> decoder,
     scoped_ptr<DecryptingDemuxerStream> decrypting_demuxer_stream) {
   DCHECK(task_runner_->BelongsToCurrentThread());
 
-  base::AutoLock auto_lock(lock_);
   scoped_ptr<AudioDecoderSelector> deleter(decoder_selector_.Pass());
 
-  if (state_ == kStopped) {
-    DCHECK(!sink_);
+  if (!decoder) {
+    {
+      base::AutoLock auto_lock(lock_);
+      ChangeState_Locked(kUninitialized);
+    }
+    // Stop() called during initialization.
+    if (!stop_cb_.is_null()) {
+      base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_ABORT);
+      Stop(base::ResetAndReturn(&stop_cb_));

[rileya (GONE FROM CHROMIUM), 2014/01/22 17:35:28]

We originally held the lock for the entirety of this function, but since Stop()
also grabs the lock we have to release it before we get here. This resulted in
rearranging this function a fair bit, let me know if you see a better way to lay
out this function's scoping/flow...

+    } else {
+      base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED);
+    }
     return;
+  } else {
+    base::AutoLock auto_lock(lock_);
+    decoder_ = decoder.Pass();
+    decrypting_demuxer_stream_ = decrypting_demuxer_stream.Pass();
+
+    int sample_rate = decoder_->samples_per_second();
+
+    // The actual buffer size is controlled via the size of the AudioBus
+    // provided to Render(), so just choose something reasonable here for looks.
+    int buffer_size = decoder_->samples_per_second() / 100;
+    audio_parameters_ = AudioParameters(
+        AudioParameters::AUDIO_PCM_LOW_LATENCY, decoder_->channel_layout(),
+        sample_rate, decoder_->bits_per_channel(), buffer_size);
+    if (!audio_parameters_.IsValid()) {
+      ChangeState_Locked(kUninitialized);
+      base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
+      return;
+    }
+
+    splicer_.reset(new AudioSplicer(sample_rate));
+
+    // We're all good! Continue initializing the rest of the audio renderer
+    // based on the decoder format.
+    algorithm_.reset(new AudioRendererAlgorithm());
+    algorithm_->Initialize(0, audio_parameters_);
+
+    ChangeState_Locked(kPaused);
+
+    HistogramRendererEvent(INITIALIZED);
+
+    {
+      base::AutoUnlock auto_unlock(lock_);
+      sink_->Initialize(audio_parameters_, weak_this_.get());
+      sink_->Start();
+
+      // Some sinks play on start...
+      sink_->Pause();
+    }
+
+    DCHECK(!sink_playing_);
+
+    base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
   }
-
-  if (!decoder) {
-    base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED);
-    return;
-  }
-
-  decoder_ = decoder.Pass();
-  decrypting_demuxer_stream_ = decrypting_demuxer_stream.Pass();
-
-  int sample_rate = decoder_->samples_per_second();
-
-  // The actual buffer size is controlled via the size of the AudioBus provided
-  // to Render(), so just choose something reasonable here for looks.
-  int buffer_size = decoder_->samples_per_second() / 100;
-  audio_parameters_ = AudioParameters(
-      AudioParameters::AUDIO_PCM_LOW_LATENCY, decoder_->channel_layout(),
-      sample_rate, decoder_->bits_per_channel(), buffer_size);
-  if (!audio_parameters_.IsValid()) {
-    base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
-    return;
-  }
-
-  splicer_.reset(new AudioSplicer(sample_rate));
-
-  // We're all good! Continue initializing the rest of the audio renderer based
-  // on the decoder format.
-  algorithm_.reset(new AudioRendererAlgorithm());
-  algorithm_->Initialize(0, audio_parameters_);
-
-  ChangeState_Locked(kPaused);
-
-  HistogramRendererEvent(INITIALIZED);
-
-  {
-    base::AutoUnlock auto_unlock(lock_);
-    sink_->Initialize(audio_parameters_, weak_this_.get());
-    sink_->Start();
-
-    // Some sinks play on start...
-    sink_->Pause();
-  }
-
-  DCHECK(!sink_playing_);
-
-  base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void AudioRendererImpl::ResumeAfterUnderflow() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   base::AutoLock auto_lock(lock_);
   if (state_ == kUnderflow) {
     // The "!preroll_aborted_" is a hack. If preroll is aborted, then we
     // shouldn't even reach the kUnderflow state to begin with. But for now
     // we're just making sure that the audio buffer capacity (i.e. the
     // number of bytes that need to be buffered for preroll to complete)
@@ skipping 89 matching lines @@
       if (!buffer->frame_count())
         return true;
     }
 
     if (state_ != kUninitialized && state_ != kStopped)
       algorithm_->EnqueueBuffer(buffer);
   }
 
   switch (state_) {
     case kUninitialized:
+    case kInitializing:
     case kFlushing:
       NOTREACHED();
       return false;
 
     case kPaused:
       DCHECK(!pending_read_);
       return false;
 
     case kPrerolling:
       if (!buffer->end_of_stream() && !algorithm_->IsQueueFull())
@@ skipping 32 matching lines @@
 
   pending_read_ = true;
   decoder_->Read(base::Bind(&AudioRendererImpl::DecodedAudioReady, weak_this_));
 }
 
 bool AudioRendererImpl::CanRead_Locked() {
   lock_.AssertAcquired();
 
   switch (state_) {
     case kUninitialized:
+    case kInitializing:
     case kPaused:
     case kFlushing:
     case kStopped:
       return false;
 
     case kPrerolling:
     case kPlaying:
     case kUnderflow:
     case kRebuffering:
       break;
@@ skipping 169 matching lines @@
 void AudioRendererImpl::DisableUnderflowForTesting() {
   underflow_disabled_ = true;
 }
 
 void AudioRendererImpl::HandleAbortedReadOrDecodeError(bool is_decode_error) {
   lock_.AssertAcquired();
 
   PipelineStatus status = is_decode_error ? PIPELINE_ERROR_DECODE : PIPELINE_OK;
   switch (state_) {
     case kUninitialized:
+    case kInitializing:
       NOTREACHED();
       return;
     case kPaused:
       if (status != PIPELINE_OK)
         error_cb_.Run(status);
       return;
     case kFlushing:
       ChangeState_Locked(kPaused);
 
       if (status == PIPELINE_OK) {
@@ skipping 20 matching lines @@
   }
 }
 
 void AudioRendererImpl::ChangeState_Locked(State new_state) {
   DVLOG(1) << __FUNCTION__ << " : " << state_ << " -> " << new_state;
   lock_.AssertAcquired();
   state_ = new_state;
 }
 
 }  // namespace media