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)

[DaleCurtis, 2014/01/22 18:43:26]

This still needs to hit the stop_cb_ run below.

[rileya (GONE FROM CHROMIUM), 2014/01/22 18:58:52]

Fixed (assuming there's no issue with posting the callback while we are holding
the lock?)

+      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_));
+    } else {
+      base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED);
+    }
     return;
+  } else {

[DaleCurtis, 2014/01/22 18:43:26]

Why add the else {, the block above returns early? 

[rileya (GONE FROM CHROMIUM), 2014/01/22 18:58:52]

I needed to scope the whole block for the AutoLock anyway, so I thought it
looked nicer to add the else. Removed it.

+    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