Clean-up and bug fixes in AudioOutputController:

media/audio/audio_output_controller.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.
 
 #ifndef MEDIA_AUDIO_AUDIO_OUTPUT_CONTROLLER_H_
 #define MEDIA_AUDIO_AUDIO_OUTPUT_CONTROLLER_H_
 
+#include "base/atomic_ref_count.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
-#include "base/synchronization/lock.h"
 #include "media/audio/audio_buffers_state.h"
 #include "media/audio/audio_io.h"
 #include "media/audio/audio_manager.h"
 #include "media/audio/audio_source_diverter.h"
 #include "media/audio/simple_sources.h"
 #include "media/base/media_export.h"
 
-namespace base {
-class WaitableEvent;
-}  // namespace base
-
 class MessageLoop;
 
 // An AudioOutputController controls an AudioOutputStream and provides data
 // to this output stream. It has an important function that it executes
 // audio operations like play, pause, stop, etc. on a separate thread,
 // namely the audio manager thread.
 //
 // All the public methods of AudioOutputController are non-blocking.
 // The actual operations are performed on the audio manager thread.
 //
-// Here is a state diagram for the AudioOutputController:
+// Here is a state transition diagram for the AudioOutputController:
 //
-//             .----------------------->  [ Closed / Error ]  <------.
-//             |                                   ^                 |
-//             |                                   |                 |
-//        [ Created ]  -->  [ Starting ]  -->  [ Playing ]  -->  [ Paused ]
-//             ^                 |                 ^                |  ^
-//             |                 |                 |                |  |
-//             |                 |                 `----------------'  |
-//             |                 V                                     |
-//             |        [ PausedWhenStarting ] ------------------------'
-//             |
-//       *[  Empty  ]
+//   *[ Empty ]  -->  [ Created ]  -->  [ Starting ]  -->  [ Playing ]  --.
+//        |                |               |    ^               |         |
+//        |                |               |    |               |         |
+//        |                |               |    |               v         |
+//        |                |               |    `---------  [ Paused ]    |
+//        |                |               |                    |         |
+//        |                v               v                    |         |
+//        `----------->  [      Closed       ]  <-------------------------'
 //
 // * Initial state
 //
-// At any time after reaching the Created state but before Closed / Error, the
-// AudioOutputController may be notified of a device change via OnDeviceChange()
-// and transition to the Recreating state.  If OnDeviceChange() completes
-// successfully the state will transition back to an equivalent pre-call state.
-// E.g., if the state was Paused or PausedWhenStarting, the new state will be
-// Created, since these states are all functionally equivalent and require a
-// Play() call to continue to the next state.
+// At any time after reaching the Created state but before Closed, the
+// AudioOutputController may be notified of a device change via
+// OnDeviceChange().  As the OnDeviceChange() is processed, state transitions
+// will occur, ultimately ending up in an equivalent pre-call state.  E.g., if
+// the state was Paused, the new state will be Created, since these states are
+// all functionally equivalent and require a Play() call to continue to the next
+// state.
 //
 // The AudioOutputStream can request data from the AudioOutputController via the
 // AudioSourceCallback interface. AudioOutputController uses the SyncReader
 // passed to it via construction to synchronously fulfill this read request.
 //
 // Since AudioOutputController uses AudioManager's message loop the controller
 // uses WeakPtr to allow safe cancellation of pending tasks.
 //
 
 namespace media {
@@ skipping 93 matching lines @@
   // AudioSourceDiverter implementation.
   virtual const AudioParameters& GetAudioParameters() OVERRIDE;
   virtual void StartDiverting(AudioOutputStream* to_stream) OVERRIDE;
   virtual void StopDiverting() OVERRIDE;
 
  protected:
   // Internal state of the source.
   enum State {
     kEmpty,
     kCreated,
+    kStarting,
     kPlaying,
-    kStarting,
-    kPausedWhenStarting,
     kPaused,
     kClosed,
     kError,
-    kRecreating,
   };
 
   friend class base::RefCountedThreadSafe<AudioOutputController>;
   virtual ~AudioOutputController();
 
  private:
   // We are polling sync reader if data became available.
   static const int kPollNumAttempts;
   static const int kPollPauseInMilliseconds;
 
   AudioOutputController(AudioManager* audio_manager, EventHandler* handler,
                         const AudioParameters& params, SyncReader* sync_reader);
 
   // The following methods are executed on the audio manager thread.
-  void DoCreate();
+  void DoCreate(bool is_for_device_change);
   void DoPlay();
   void PollAndStartIfDataReady();
   void DoPause();
   void DoFlush();
   void DoClose();
   void DoSetVolume(double volume);
   void DoReportError(int code);
   void DoStartDiverting(AudioOutputStream* to_stream);
   void DoStopDiverting();
 
-  // Helper method that starts physical stream.
+  // Helper methods that start/stop physical stream.
   void StartStream();
+  void StopStream();
 
   // Helper method that stops, closes, and NULLs |*stream_|.
-  // Signals event when done if it is not NULL.
-  void DoStopCloseAndClearStream(base::WaitableEvent *done);
+  void DoStopCloseAndClearStream();
+
+  // Sanity-check that entry/exit to OnMoreIOData() by the hardware audio thread
+  // happens only between AudioOutputStream::Start() and Stop().
+  void AllowEntryToOnMoreIOData();
+  void DisallowEntryToOnMoreIOData();
 
   AudioManager* const audio_manager_;
   const AudioParameters params_;
-
-  // |handler_| may be called only if |state_| is not kClosed.
-  EventHandler* handler_;
+  EventHandler* const handler_;
 
   // Note: It's important to invalidate the weak pointers whenever stream_ is
   // changed.  See comment for weak_this_.
   AudioOutputStream* stream_;
 
   // When non-NULL, audio is being diverted to this stream.
   AudioOutputStream* diverting_to_stream_;
 
   // The current volume of the audio stream.
   double volume_;
 
   // |state_| is written on the audio manager thread and is read on the
   // hardware audio thread. These operations need to be locked. But lock
   // is not required for reading on the audio manager thread.
   State state_;
 
-  // The |lock_| must be acquired whenever we access |state_| from a thread
-  // other than the audio manager thread.
-  base::Lock lock_;
+  // Binary semaphore, used to ensure that only one thread enters the
+  // OnMoreIOData() method, and only when it is valid to do so.  This is for
+  // sanity-checking the behavior of platform implementations of
+  // AudioOutputStream.  In other words, multiple contention is not expected,
+  // nor in the design here.
+  base::AtomicRefCount num_allowed_io_;
 
   // SyncReader is used only in low latency mode for synchronous reading.
-  SyncReader* sync_reader_;
+  SyncReader* const sync_reader_;
 
   // The message loop of audio manager thread that this object runs on.
-  scoped_refptr<base::MessageLoopProxy> message_loop_;
+  const scoped_refptr<base::MessageLoopProxy> message_loop_;
 
   // When starting stream we wait for data to become available.
   // Number of times left.
   int number_polling_attempts_left_;
 
   // Used to auto-cancel the delayed tasks that are created to poll for data
   // (when starting-up a stream).
   base::WeakPtrFactory<AudioOutputController> weak_this_;
 
   DISALLOW_COPY_AND_ASSIGN(AudioOutputController);
 };
 
 }  // namespace media
 
 #endif  // MEDIA_AUDIO_AUDIO_OUTPUT_CONTROLLER_H_