Move OnDecoderInitDone() from decoder to pipeline thread.

media/filters/audio_renderer_impl.h

Index: media/filters/audio_renderer_impl.h
diff --git a/media/filters/audio_renderer_impl.h b/media/filters/audio_renderer_impl.h
index 47a498b4079ee5f9d59f5fea28c4dbc15ca62980..b3767d01149a037c285833233fb2d3a62e371d01 100644
--- a/media/filters/audio_renderer_impl.h
+++ b/media/filters/audio_renderer_impl.h
@@ -23,6 +23,7 @@
 #include <deque>
 
 #include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
 #include "media/base/audio_decoder.h"
 #include "media/base/audio_renderer.h"
 #include "media/base/audio_renderer_sink.h"
@@ -113,6 +114,7 @@ class MEDIA_EXPORT AudioRendererImpl
 
   // Estimate earliest time when current buffer can stop playing.
   void UpdateEarliestEndTime(int bytes_filled,
+                             float playback_rate,
                              base::TimeDelta request_delay,
                              base::Time time_now);
 
@@ -120,7 +122,8 @@ class MEDIA_EXPORT AudioRendererImpl
   void DoPlay();
   void DoPause();
 
-  // media::AudioRendererSink::RenderCallback implementation.
+  // media::AudioRendererSink::RenderCallback implementation.  Called on the
+  // AudioDevice thread.
   virtual int Render(AudioBus* audio_bus,
                      int audio_delay_milliseconds) OVERRIDE;
   virtual void OnRenderError() OVERRIDE;
@@ -151,11 +154,41 @@ class MEDIA_EXPORT AudioRendererImpl
   // Audio decoder.
   scoped_refptr<AudioDecoder> decoder_;
 
-  // Algorithm for scaling audio.
-  scoped_ptr<AudioRendererAlgorithm> algorithm_;
+  // The sink (destination) for rendered audio.  |sink_| must only be accessed
+  // on the pipeline thread (verify with |pipeline_thread_checker_|).  |sink_|
+  // must never be called under |lock_| or the 3-way thread bridge between the
+  // audio, pipeline, and decoder threads may dead lock.

[Ami GONE FROM CHROMIUM, 2012/11/03 00:11:45]

deadlock is one word.

[DaleCurtis, 2012/11/03 01:32:47]

Done.

+  scoped_refptr<media::AudioRendererSink> sink_;
+
+  // Ensures certain methods are always called on the pipeline thread.
+  base::ThreadChecker pipeline_thread_checker_;
+
+  // AudioParameters constructed during Initialize() based on |decoder_|.
+  AudioParameters audio_parameters_;
+
+  // Callbacks provided during Initialize().
+  PipelineStatusCB init_cb_;
+  StatisticsCB statistics_cb_;
+  base::Closure underflow_cb_;
+  TimeCB time_cb_;
+  base::Closure ended_cb_;
+  base::Closure disabled_cb_;
+  PipelineStatusCB error_cb_;
+
+  // Callback provided to Pause(...).

[Ami GONE FROM CHROMIUM, 2012/11/03 00:11:45]

ellipses here and below aren't useful

[DaleCurtis, 2012/11/03 01:32:47]

Done.

+  base::Closure pause_cb_;
 
+  // Callback provided to Preroll(...).
+  PipelineStatusCB preroll_cb_;
+
+  // After Initialize() has completed, all variables below must be accessed
+  // under |lock_|. ------------------------------------------------------------
   base::Lock lock_;
 
+  // Algorithm for scaling audio.  Access must be guarded by |lock_| if not on
+  // the pipeline thread.

[Ami GONE FROM CHROMIUM, 2012/11/03 00:11:45]

this is a very codesmelly comment.
Why is it OK to access this concurrently under lock from one thread and not
under lock on the pipeline thread?

[DaleCurtis, 2012/11/03 01:32:47]

Removed.

+  scoped_ptr<AudioRendererAlgorithm> algorithm_;
+
   // Simple state tracking variable.
   enum State {
     kUninitialized,
@@ -166,6 +199,8 @@ class MEDIA_EXPORT AudioRendererImpl
     kUnderflow,
     kRebuffering,
   };
+
+  // |state_| access must always be guarded by |lock_|

[Ami GONE FROM CHROMIUM, 2012/11/03 00:11:45]

redundant to l.184?

[DaleCurtis, 2012/11/03 01:32:47]

Done.

   State state_;
 
   // Keep track of our outstanding read to |decoder_|.
@@ -180,29 +215,8 @@ class MEDIA_EXPORT AudioRendererImpl
   base::TimeDelta audio_time_buffered_;
   base::TimeDelta current_time_;
 
-  PipelineStatusCB init_cb_;
-  StatisticsCB statistics_cb_;
-
-  // Filter callbacks.
-  base::Closure pause_cb_;
-  PipelineStatusCB preroll_cb_;
-
-  base::Closure underflow_cb_;
-  TimeCB time_cb_;
-  base::Closure ended_cb_;
-  base::Closure disabled_cb_;
-  PipelineStatusCB error_cb_;
-
   base::TimeDelta preroll_timestamp_;
 
-  uint32 bytes_per_frame_;
-
-  // A flag that indicates this filter is called to stop.
-  bool stopped_;
-
-  // The sink (destination) for rendered audio.
-  scoped_refptr<media::AudioRendererSink> sink_;
-
   // We're supposed to know amount of audio data OS or hardware buffered, but
   // that is not always so -- on my Linux box
   // AudioBuffersState::hardware_delay_bytes never reaches 0.
@@ -219,14 +233,14 @@ class MEDIA_EXPORT AudioRendererImpl
   // than nothing.
   base::Time earliest_end_time_;
 
-  AudioParameters audio_parameters_;
-
   bool underflow_disabled_;
 
   // True if the renderer receives a buffer with kAborted status during preroll,
   // false otherwise. This flag is cleared on the next Preroll() call.
   bool preroll_aborted_;
 
+  // End variables which must be accessed under |lock_|. ----------------------
+
   DISALLOW_COPY_AND_ASSIGN(AudioRendererImpl);
 };