Move OnDecoderInitDone() from decoder to pipeline thread.

media/filters/audio_renderer_impl.h

 // 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.
 
 // Audio rendering unit utilizing an AudioRendererSink to output data.
 //
 // This class lives inside three threads during it's lifetime, namely:
 // 1. Render thread.
 //    This object is created on the render thread.
 // 2. Pipeline thread
 //    Initialize() is called here with the audio format.
 //    Play/Pause/Preroll() also happens here.
 // 3. Audio thread created by the AudioRendererSink.
 //    Render() is called here where audio data is decoded into raw PCM data.
 //
 // AudioRendererImpl talks to an AudioRendererAlgorithm that takes care of
 // queueing audio data and stretching/shrinking audio data when playback rate !=
 // 1.0 or 0.0.
 
 #ifndef MEDIA_FILTERS_AUDIO_RENDERER_IMPL_H_
 #define MEDIA_FILTERS_AUDIO_RENDERER_IMPL_H_
 
 #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"
 #include "media/base/buffers.h"
 #include "media/filters/audio_renderer_algorithm.h"
 
 namespace media {
 
 class MEDIA_EXPORT AudioRendererImpl
     : public AudioRenderer,
@@ skipping 70 matching lines @@
 
   // Get the playback rate of |algorithm_|.
   float GetPlaybackRate();
 
   // Convert number of bytes to duration of time using information about the
   // number of channels, sample rate and sample bits.
   base::TimeDelta ConvertToDuration(int bytes);
 
   // 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);
 
   // Methods called on pipeline thread ----------------------------------------
   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;
 
   // Helper method that schedules an asynchronous read from the decoder and
   // increments |pending_reads_|.
   //
   // Safe to call from any thread.
   void ScheduleRead_Locked();
 
@@ skipping 10 matching lines @@
   // Called when |decoder_| initialization completes.
   // |demuxer_stream| & |decoders| are used if initialization failed and
   // InitializeNextDecoder() needs to be called again.
   void OnDecoderInitDone(const scoped_refptr<DemuxerStream>& demuxer_stream,
                          scoped_ptr<AudioDecoderList> decoders,
                          PipelineStatus status);
 
   // Audio decoder.
   scoped_refptr<AudioDecoder> decoder_;
 
-  // Algorithm for scaling audio.
+  // 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_;
 
-  base::Lock lock_;
-
   // Simple state tracking variable.
   enum State {
     kUninitialized,
     kPaused,
     kPrerolling,
     kPlaying,
     kStopped,
     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_|.
   bool pending_read_;
 
   // Keeps track of whether we received and rendered the end of stream buffer.
   bool received_end_of_stream_;
   bool rendered_end_of_stream_;
 
   // The timestamp of the last frame (i.e. furthest in the future) buffered as
   // well as the current time that takes current playback delay into account.
   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.
   //
   // As a result we cannot use it to find when stream ends. If we just ignore
   // buffered data we will notify host that stream ended before it is actually
   // did so, I've seen it done ~140ms too early when playing ~150ms file.
   //
   // Instead of trying to invent OS-specific solution for each and every OS we
   // are supporting, use simple workaround: every time we fill the buffer we
   // remember when it should stop playing, and do not assume that buffer is
   // empty till that time. Workaround is not bulletproof, as we don't exactly
   // know when that particular data would start playing, but it is much better
   // 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);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_FILTERS_AUDIO_RENDERER_IMPL_H_