Move AudioDecoder initialization into AudioRenderer.

media/filters/audio_renderer_impl.cc

Index: media/filters/audio_renderer_impl.cc
diff --git a/media/filters/audio_renderer_impl.cc b/media/filters/audio_renderer_impl.cc
index 94ee20f7145fb35e5f650eab18461af55c7cc5b7..deed856fa848df32cb175fe476397a009086ae24 100644
--- a/media/filters/audio_renderer_impl.cc
+++ b/media/filters/audio_renderer_impl.cc
@@ -11,6 +11,7 @@
 #include "base/callback_helpers.h"
 #include "base/logging.h"
 #include "media/audio/audio_util.h"
+#include "media/base/demuxer_stream.h"
 
 namespace media {
 
@@ -22,10 +23,8 @@ AudioRendererImpl::AudioRendererImpl(media::AudioRendererSink* sink)
       audio_time_buffered_(kNoTimestamp()),
       current_time_(kNoTimestamp()),
       bytes_per_frame_(0),
-      bytes_per_second_(0),
       stopped_(false),
       sink_(sink),
-      is_initialized_(false),
       underflow_disabled_(false) {
 }
 
@@ -49,7 +48,6 @@ void AudioRendererImpl::Play(const base::Closure& callback) {
 
 void AudioRendererImpl::DoPlay() {
   earliest_end_time_ = base::Time::Now();
-  DCHECK(sink_.get());

[Ami GONE FROM CHROMIUM, 2012/09/07 14:40:02]

this is just random cleanup?

   sink_->Play();
 }
 
@@ -73,7 +71,6 @@ void AudioRendererImpl::Pause(const base::Closure& callback) {
 }
 
 void AudioRendererImpl::DoPause() {
-  DCHECK(sink_.get());
   sink_->Pause(false);
 }
 
@@ -83,9 +80,7 @@ void AudioRendererImpl::Flush(const base::Closure& callback) {
 
 void AudioRendererImpl::Stop(const base::Closure& callback) {
   if (!stopped_) {
-    DCHECK(sink_.get());
     sink_->Stop();
-
     stopped_ = true;
   }
   {
@@ -128,36 +123,82 @@ void AudioRendererImpl::Preroll(base::TimeDelta time,
   sink_->Pause(true);
 }
 
-void AudioRendererImpl::Initialize(const scoped_refptr<AudioDecoder>& decoder,
+void AudioRendererImpl::Initialize(const scoped_refptr<DemuxerStream>& stream,
+                                   const AudioDecoderList& decoders,
                                    const PipelineStatusCB& init_cb,
+                                   const StatisticsCB& statistics_cb,
                                    const base::Closure& underflow_cb,
                                    const TimeCB& time_cb,
                                    const base::Closure& ended_cb,
                                    const base::Closure& disabled_cb,
                                    const PipelineStatusCB& error_cb) {
-  DCHECK(decoder);
+  base::AutoLock l(lock_);

[Ami GONE FROM CHROMIUM, 2012/09/07 14:40:02]

"l" is not stylish

+  DCHECK(stream);
+  DCHECK(!decoders.empty());
+  DCHECK_EQ(stream->type(), DemuxerStream::AUDIO);
   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_);
-  decoder_ = decoder;
+  init_cb_ = init_cb;
+  statistics_cb_ = statistics_cb;

[Ami GONE FROM CHROMIUM, 2012/09/07 14:40:02]

This is only used during the init dance, right?
Why not Bind it into the init calls, instead?

   underflow_cb_ = underflow_cb;
   time_cb_ = time_cb;
   ended_cb_ = ended_cb;
   disabled_cb_ = disabled_cb;
   error_cb_ = error_cb;
 
-  // Create a callback so our algorithm can request more reads.
-  base::Closure cb = base::Bind(&AudioRendererImpl::ScheduleRead_Locked, this);
+  scoped_ptr<AudioDecoderList> decoder_list(new AudioDecoderList(decoders));
+  InitializeNextDecoder(stream, decoder_list.Pass());
+}
 
-  // Construct the algorithm.
-  algorithm_.reset(new AudioRendererAlgorithm());
+void AudioRendererImpl::InitializeNextDecoder(
+    const scoped_refptr<DemuxerStream>& demuxer_stream,
+    scoped_ptr<AudioDecoderList> decoders) {
+  lock_.AssertAcquired();

[Ami GONE FROM CHROMIUM, 2012/09/07 14:40:02]

this locking is making me very sad.
If you dropped the support for multiple decoders could you drop the locking
de-sad me?
It's kind of crazy to have all this code here considering it'll never other than
exactly one decoder to chew on.

+  DCHECK(!decoders->empty());
+
+  scoped_refptr<AudioDecoder> decoder = decoders->front();

[Ami GONE FROM CHROMIUM, 2012/09/07 14:40:02]

put it straight into decoder_ instead?

+  decoders->pop_front();
+
+  DCHECK(decoder);
+  decoder_ = decoder;
+
+  base::AutoUnlock auto_unlock(lock_);
+  decoder->Initialize(
+      demuxer_stream,
+      base::Bind(&AudioRendererImpl::OnDecoderInitDone, this,
+                 demuxer_stream,
+                 base::Passed(&decoders)),
+      statistics_cb_);
+}
+
+void AudioRendererImpl::OnDecoderInitDone(
+    const scoped_refptr<DemuxerStream>& demuxer_stream,
+    scoped_ptr<AudioDecoderList> decoders,
+    PipelineStatus status) {
+  base::AutoLock auto_lock(lock_);
+
+  if (stopped_ || state_ == kStopped)
+    return;
+
+  if (!decoders->empty() && status == DECODER_ERROR_NOT_SUPPORTED) {
+    InitializeNextDecoder(demuxer_stream, decoders.Pass());
+    return;
+  }
+
+  if (status != PIPELINE_OK) {
+    base::ResetAndReturn(&init_cb_).Run(status);
+    return;
+  }
+
+  // We're all good! Continue initializing the rest of the audio renderer based
+  // on the decoder format.
 
-  // Initialize our algorithm with media properties, initial playback rate,
-  // and a callback to request more reads from the data source.
   ChannelLayout channel_layout = decoder_->channel_layout();
   int channels = ChannelLayoutToChannelCount(channel_layout);
   int bits_per_channel = decoder_->bits_per_channel();
@@ -165,15 +206,15 @@ void AudioRendererImpl::Initialize(const scoped_refptr<AudioDecoder>& decoder,
   // TODO(vrk): Add method to AudioDecoder to compute bytes per frame.
   bytes_per_frame_ = channels * bits_per_channel / 8;
 
-  bool config_ok = algorithm_->ValidateConfig(channels, sample_rate,

[scherkus (not reviewing), 2012/09/07 13:59:54]

this was some miscellaneous cleanup

-                                              bits_per_channel);
-  if (!config_ok || is_initialized_) {
-    init_cb.Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
+  algorithm_.reset(new AudioRendererAlgorithm());
+  if (!algorithm_->ValidateConfig(channels, sample_rate, bits_per_channel)) {
+    base::ResetAndReturn(&init_cb_).Run(PIPELINE_ERROR_INITIALIZATION_FAILED);
     return;
   }
 
-  if (config_ok)
-    algorithm_->Initialize(channels, sample_rate, bits_per_channel, 0.0f, cb);
+  algorithm_->Initialize(
+      channels, sample_rate, bits_per_channel, 0.0f,
+      base::Bind(&AudioRendererImpl::ScheduleRead_Locked, this));
 
   // We use the AUDIO_PCM_LINEAR flag because AUDIO_PCM_LOW_LATENCY
   // does not currently support all the sample-rates that we require.
@@ -183,19 +224,11 @@ void AudioRendererImpl::Initialize(const scoped_refptr<AudioDecoder>& decoder,
       AudioParameters::AUDIO_PCM_LINEAR, channel_layout, sample_rate,
       bits_per_channel, GetHighLatencyOutputBufferSize(sample_rate));
 
-  bytes_per_second_ = audio_parameters_.GetBytesPerSecond();
-
-  DCHECK(sink_.get());
-  DCHECK(!is_initialized_);
-
   sink_->Initialize(audio_parameters_, this);
-
   sink_->Start();
-  is_initialized_ = true;
 
-  // Finally, execute the start callback.
   state_ = kPaused;
-  init_cb.Run(PIPELINE_OK);
+  base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK);
 }
 
 void AudioRendererImpl::ResumeAfterUnderflow(bool buffer_more_audio) {
@@ -481,11 +514,10 @@ void AudioRendererImpl::UpdateEarliestEndTime(int bytes_filled,
 }
 
 base::TimeDelta AudioRendererImpl::ConvertToDuration(int bytes) {
-  if (bytes_per_second_) {
-    return base::TimeDelta::FromMicroseconds(
-        base::Time::kMicrosecondsPerSecond * bytes / bytes_per_second_);
-  }
-  return base::TimeDelta();
+  int bytes_per_second = audio_parameters_.GetBytesPerSecond();
+  CHECK(bytes_per_second);

[scherkus (not reviewing), 2012/09/07 13:59:54]

calling algorithm_->ValidateConfig() should prevent this from becoming true

+  return base::TimeDelta::FromMicroseconds(
+      base::Time::kMicrosecondsPerSecond * bytes / bytes_per_second);
 }
 
 void AudioRendererImpl::OnRenderError() {
@@ -493,7 +525,6 @@ void AudioRendererImpl::OnRenderError() {
 }
 
 void AudioRendererImpl::DisableUnderflowForTesting() {
-  DCHECK(!is_initialized_);
   underflow_disabled_ = true;
 }