Simplify AlsaPcmOutputStream and AudioManagerLinux. Code was thread-safe, but now

media/audio/linux/alsa_output.cc

 // Copyright (c) 2011 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.
 //
 // THREAD SAFETY
 //
-// The AlsaPcmOutputStream object's internal state is accessed by two threads:
-//
+// AlsaPcmOutputStream object is *not* thread-safe -- we assume that
 //   client thread - creates the object and calls the public APIs.
 //   message loop thread - executes all the internal tasks including querying
 //      the data source for more data, writing to the alsa device, and closing
-//      the alsa device.  It does *not* handle opening the device though.
-//
-// The class is designed so that most operations that read/modify the object's
-// internal state are done on the message loop thread.  The exception is data
-// conatined in the |shared_data_| structure.  Data in this structure needs to
-// be accessed by both threads, and should only be accessed when the
-// |shared_data_.lock_| is held.
-//
-// All member variables that are not in |shared_data_| are created/destroyed on
-// the |message_loop_|.  This allows safe access to them from any task posted to
-// |message_loop_|.  The values in |shared_data_| are considered to be read-only
-// signals by tasks posted to |message_loop_| (see the SEMANTICS of
-// |shared_data_| section below).  Because of these two constraints, tasks can,
-// and must, be coded to be safe in the face of a changing |shared_data_|.
-//
-//
-// SEMANTICS OF |shared_data_|
-//
-// Though |shared_data_| is accessable by both threads, the code has been
-// structured so that all mutations to |shared_data_| are only done in the
-// client thread.  The message loop thread only ever reads the shared data.
-//
-// This reduces the need for critical sections because the public API code can
-// assume that no mutations occur to the |shared_data_| between queries.
-//
-// On the message loop side, tasks have been coded such that they can
-// operate safely regardless of when state changes happen to |shared_data_|.
-// Code that is sensitive to the timing of state changes are delegated to the
-// |shared_data_| object so they can executed while holding
-// |shared_data_.lock_|.
+//      the alsa device.
+// is actually the same thread.
 //
 //
 // SEMANTICS OF CloseTask()
 //
 // The CloseTask() is responsible for cleaning up any resources that were
-// acquired after a successful Open().  After a CloseTask() has executed,
-// scheduling of reads should stop.  Currently scheduled tasks may run, but
-// they should not attempt to access any of the internal structures beyond
-// querying the |stop_stream_| flag and no-oping themselves.  This will
-// guarantee that eventually no more tasks will be posted into the message
-// loop, and the AlsaPcmOutputStream will be able to delete itself.
+// acquired after a successful Open().  CloseTask() would revoke any
+// scheduled outstanding runnable methods.
 //
 //
 // SEMANTICS OF ERROR STATES
 //
-// The object has two distinct error states: |shared_data_.state_| == kInError
-// and |stop_stream_|.  The |shared_data_.state_| state is only settable
-// by the client thread, and thus cannot be used to signal when the ALSA device
-// fails due to a hardware (or other low-level) event.  The |stop_stream_|
-// variable is only accessed by the message loop thread; it is used to indicate
+// The object has two distinct error states: |state_| == kInError
+// and |stop_stream_|.  The |stop_stream_| variable is used to indicate
 // that the playback_handle should no longer be used either because of a
-// hardware/low-level event, or because the CloseTask() has been run.
+// hardware/low-level event.
 //
-// When |shared_data_.state_| == kInError, all public API functions will fail
+// When |state_| == kInError, all public API functions will fail
 // with an error (Start() will call the OnError() function on the callback
 // immediately), or no-op themselves with the exception of Close().  Even if an
 // error state has been entered, if Open() has previously returned successfully,
 // Close() must be called to cleanup the ALSA devices and release resources.
 //
 // When |stop_stream_| is set, no more commands will be made against the
 // ALSA device, and playback will effectively stop.  From the client's point of
 // view, it will seem that the device has just clogged and stopped requesting
 // data.
 
@@ skipping 135 matching lines @@
 // Since we expect to only be able to wake up with a resolution of
 // kSleepErrorMilliseconds, double that for our minimum required latency.
 const uint32 AlsaPcmOutputStream::kMinLatencyMicros =
     kSleepErrorMilliseconds * 2 * 1000;
 
 AlsaPcmOutputStream::AlsaPcmOutputStream(const std::string& device_name,
                                          AudioParameters params,
                                          AlsaWrapper* wrapper,
                                          AudioManagerLinux* manager,
                                          MessageLoop* message_loop)
-    : shared_data_(MessageLoop::current()),
-      requested_device_name_(device_name),
+    : requested_device_name_(device_name),
       pcm_format_(alsa_util::BitsToFormat(params.bits_per_sample)),
       channels_(params.channels),
       sample_rate_(params.sample_rate),
       bytes_per_sample_(params.bits_per_sample / 8),
       bytes_per_frame_(channels_ * params.bits_per_sample / 8),
       should_downmix_(false),
       packet_size_(params.GetPacketSize()),
       micros_per_packet_(FramesToMicros(
           params.samples_per_packet, sample_rate_)),
       latency_micros_(std::max(AlsaPcmOutputStream::kMinLatencyMicros,
                                micros_per_packet_ * 2)),
       bytes_per_output_frame_(bytes_per_frame_),
       alsa_buffer_frames_(0),
       stop_stream_(false),
       wrapper_(wrapper),
       manager_(manager),
       playback_handle_(NULL),
       frames_per_packet_(packet_size_ / bytes_per_frame_),
-      client_thread_loop_(MessageLoop::current()),
-      message_loop_(message_loop) {
+      message_loop_(message_loop),
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
+      state_(kCreated),
+      volume_(1.0f),
+      source_callback_(NULL) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // Sanity check input values.
   if ((params.sample_rate > kAlsaMaxSampleRate) || (params.sample_rate <= 0)) {
     LOG(WARNING) << "Unsupported audio frequency.";
-    shared_data_.TransitionTo(kInError);
+    TransitionTo(kInError);
   }
 
   if (AudioParameters::AUDIO_PCM_LINEAR != params.format &&
       AudioParameters::AUDIO_PCM_LOW_LATENCY != params.format) {
     LOG(WARNING) << "Unsupported audio format";
-    shared_data_.TransitionTo(kInError);
+    TransitionTo(kInError);
   }
 
   if (pcm_format_ == SND_PCM_FORMAT_UNKNOWN) {
     LOG(WARNING) << "Unsupported bits per sample: " << params.bits_per_sample;
-    shared_data_.TransitionTo(kInError);
+    TransitionTo(kInError);
   }
 }
 
 AlsaPcmOutputStream::~AlsaPcmOutputStream() {
-  InternalState state = shared_data_.state();
-  DCHECK(state == kCreated || state == kIsClosed || state == kInError);
+  InternalState current_state = state();
+  DCHECK(current_state == kCreated ||
+         current_state == kIsClosed ||
+         current_state == kInError);
 
   // TODO(ajwong): Ensure that CloseTask has been called and the
   // playback handle released by DCHECKing that playback_handle_ is NULL.
   // Currently, because of Bug 18217, there is a race condition on destruction
   // where the stream is not always stopped and closed, causing this to fail.
 }
 
 bool AlsaPcmOutputStream::Open() {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  if (shared_data_.state() == kInError) {
+  if (state() == kInError) {
     return false;
   }
 
-  if (!shared_data_.CanTransitionTo(kIsOpened)) {
-    NOTREACHED() << "Invalid state: " << shared_data_.state();
+  if (!CanTransitionTo(kIsOpened)) {
+    NOTREACHED() << "Invalid state: " << state();
     return false;
   }
 
   // We do not need to check if the transition was successful because
   // CanTransitionTo() was checked above, and it is assumed that this
   // object's public API is only called on one thread so the state cannot
   // transition out from under us.
-  shared_data_.TransitionTo(kIsOpened);
+  TransitionTo(kIsOpened);
   message_loop_->PostTask(
       FROM_HERE,
-      NewRunnableMethod(this, &AlsaPcmOutputStream::OpenTask));
+      method_factory_.NewRunnableMethod(&AlsaPcmOutputStream::OpenTask));
 
   return true;
 }
 
 void AlsaPcmOutputStream::Close() {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // Sanity check that the transition occurs correctly.  It is safe to
   // continue anyways because all operations for closing are idempotent.
-  if (shared_data_.TransitionTo(kIsClosed) != kIsClosed) {
+  if (TransitionTo(kIsClosed) != kIsClosed) {
     NOTREACHED() << "Unable to transition Closed.";
   }
 
   message_loop_->PostTask(
       FROM_HERE,
-      NewRunnableMethod(this, &AlsaPcmOutputStream::CloseTask));
-
-  // Signal to the manager that we're closed and can be removed.  Since
-  // we just posted a CloseTask to the message loop, we won't be deleted
-  // immediately, but it will happen soon afterwards.
-  manager()->ReleaseOutputStream(this);
+      method_factory_.NewRunnableMethod(&AlsaPcmOutputStream::CloseTask));
 }
 
 void AlsaPcmOutputStream::Start(AudioSourceCallback* callback) {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   CHECK(callback);
 
-  shared_data_.set_source_callback(callback);
+  set_source_callback(callback);
 
   // Only post the task if we can enter the playing state.
-  if (shared_data_.TransitionTo(kIsPlaying) == kIsPlaying) {
+  if (TransitionTo(kIsPlaying) == kIsPlaying) {
     message_loop_->PostTask(
         FROM_HERE,
-        NewRunnableMethod(this, &AlsaPcmOutputStream::StartTask));
+        method_factory_.NewRunnableMethod(&AlsaPcmOutputStream::StartTask));
   }
 }
 
 void AlsaPcmOutputStream::Stop() {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
   // Reset the callback, so that it is not called anymore.
-  shared_data_.set_source_callback(NULL);
+  set_source_callback(NULL);
 
-  shared_data_.TransitionTo(kIsStopped);
+  TransitionTo(kIsStopped);
 }
 
 void AlsaPcmOutputStream::SetVolume(double volume) {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  shared_data_.set_volume(static_cast<float>(volume));
+  volume_ = static_cast<float>(volume);
 }
 
 void AlsaPcmOutputStream::GetVolume(double* volume) {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  *volume = shared_data_.volume();
+  *volume = volume_;
 }
 
 void AlsaPcmOutputStream::OpenTask() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   // Try to open the device.
   if (requested_device_name_ == kAutoSelectDevice) {
     playback_handle_ = AutoSelectDevice(latency_micros_);
     if (playback_handle_)
       VLOG(1) << "Auto-selected device: " << device_name_;
@@ skipping 30 matching lines @@
   }
 }
 
 void AlsaPcmOutputStream::StartTask() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   if (stop_stream_) {
     return;
   }
 
-  if (shared_data_.state() != kIsPlaying) {
+  if (state() != kIsPlaying) {
     return;
   }
 
   // Before starting, the buffer might have audio from previous user of this
   // device.
   buffer_->Clear();
 
   // When starting again, drop all packets in the device and prepare it again
   // incase we are restarting from a pause state and need to flush old data.
   int error = wrapper_->PcmDrop(playback_handle_);
@@ skipping 11 matching lines @@
                << wrapper_->PcmName(playback_handle_) << "): "
                << wrapper_->StrError(error);
     stop_stream_ = true;
     return;
   }
 
   ScheduleNextWrite(false);
 }
 
 void AlsaPcmOutputStream::CloseTask() {
-  // NOTE: Keep this function idempotent to handle errors that might cause
-  // multiple CloseTasks to be posted.
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   // Shutdown the audio device.
   if (playback_handle_ &&
       alsa_util::CloseDevice(wrapper_, playback_handle_) < 0) {
     LOG(WARNING) << "Unable to close audio device. Leaking handle.";
   }
   playback_handle_ = NULL;
 
   // Release the buffer.
   buffer_.reset();
 
   // Signal anything that might already be scheduled to stop.
-  stop_stream_ = true;
+  stop_stream_ = true;  // Not necessary in production, but unit tests
+                        // uses the flag to verify that stream was closed.
+  method_factory_.RevokeAll();
+
+  // Signal to the manager that we're closed and can be removed.
+  // Should be last call in the method as it deletes "this".
+  manager_->ReleaseOutputStream(this);
 }
 
 void AlsaPcmOutputStream::BufferPacket(bool* source_exhausted) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   // If stopped, simulate a 0-lengthed packet.
   if (stop_stream_) {
     buffer_->Clear();
     *source_exhausted = true;
     return;
   }
 
   *source_exhausted = false;
 
   // Request more data if we have capacity.
   if (buffer_->forward_capacity() > buffer_->forward_bytes()) {
     // Before making a request to source for data we need to determine the
     // delay (in bytes) for the requested data to be played.
 
     uint32 buffer_delay = buffer_->forward_bytes() * bytes_per_frame_ /
         bytes_per_output_frame_;
 
     uint32 hardware_delay = GetCurrentDelay() * bytes_per_frame_;
 
     scoped_refptr<media::DataBuffer> packet =
         new media::DataBuffer(packet_size_);
-    size_t packet_size =
-        shared_data_.OnMoreData(
-            this, packet->GetWritableData(), packet->GetBufferSize(),
-            AudioBuffersState(buffer_delay, hardware_delay));
+    size_t packet_size = RunDataCallback(packet->GetWritableData(),
+                                         packet->GetBufferSize(),
+                                         AudioBuffersState(buffer_delay,
+                                                           hardware_delay));
     CHECK(packet_size <= packet->GetBufferSize()) <<
         "Data source overran buffer.";
 
     // This should not happen, but in case it does, drop any trailing bytes
     // that aren't large enough to make a frame.  Without this, packet writing
     // may stall because the last few bytes in the packet may never get used by
     // WritePacket.
     DCHECK(packet_size % bytes_per_frame_ == 0);
     packet_size = (packet_size / bytes_per_frame_) * bytes_per_frame_;
 
     if (should_downmix_) {
       if (media::FoldChannels(packet->GetWritableData(),
                               packet_size,
                               channels_,
                               bytes_per_sample_,
-                              shared_data_.volume())) {
+                              volume_)) {
         // Adjust packet size for downmix.
         packet_size =
             packet_size / bytes_per_frame_ * bytes_per_output_frame_;
       } else {
         LOG(ERROR) << "Folding failed";
       }
     } else {
       // TODO(ajwong): Handle other channel orderings.
 
       // Handle channel order for 5.0 audio.
@@ skipping 15 matching lines @@
           Swizzle51Layout(packet->GetWritableData(), packet_size);
         } else if (bytes_per_sample_ == 4) {
           Swizzle51Layout(packet->GetWritableData(), packet_size);
         }
       }
 
       media::AdjustVolume(packet->GetWritableData(),
                           packet_size,
                           channels_,
                           bytes_per_sample_,
-                          shared_data_.volume());
+                          volume_);
     }
 
     if (packet_size > 0) {
       packet->SetDataSize(packet_size);
       // Add the packet to the buffer.
       buffer_->Append(packet);
     } else {
       *source_exhausted = true;
     }
   }
 }
 
 void AlsaPcmOutputStream::WritePacket() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   // If the device is in error, just eat the bytes.
   if (stop_stream_) {
     buffer_->Clear();
     return;
   }
 
-  if (shared_data_.state() == kIsStopped) {
+  if (state() == kIsStopped) {
     return;
   }
 
   CHECK_EQ(buffer_->forward_bytes() % bytes_per_output_frame_, 0u);
 
   const uint8* buffer_data;
   size_t buffer_size;
   if (buffer_->GetCurrentChunk(&buffer_data, &buffer_size)) {
     buffer_size = buffer_size - (buffer_size % bytes_per_output_frame_);
     snd_pcm_sframes_t frames = buffer_size / bytes_per_output_frame_;
@@ skipping 11 matching lines @@
                                             frames_written,
                                             kPcmRecoverIsSilent);
     }
 
     if (frames_written < 0) {
       // TODO(ajwong): Is EAGAIN the only error we want to except from stopping
       // the pcm playback?
       if (frames_written != -EAGAIN) {
         LOG(ERROR) << "Failed to write to pcm device: "
                    << wrapper_->StrError(frames_written);
-        shared_data_.OnError(this, frames_written);
+        RunErrorCallback(frames_written);
         stop_stream_ = true;
       }
     } else {
       if (frames_written > frames) {
         LOG(WARNING)
             << "snd_pcm_writei() has written more frame that we asked.";
         frames_written = frames;
       }
 
       // Seek forward in the buffer after we've written some data to ALSA.
@@ skipping 10 matching lines @@
   }
 }
 
 void AlsaPcmOutputStream::WriteTask() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   if (stop_stream_) {
     return;
   }
 
-  if (shared_data_.state() == kIsStopped) {
+  if (state() == kIsStopped) {
     return;
   }
 
   bool source_exhausted;
   BufferPacket(&source_exhausted);
   WritePacket();
 
   ScheduleNextWrite(source_exhausted);
 }
 
@@ skipping 24 matching lines @@
   } else {
     next_fill_time_ms -= kSleepErrorMilliseconds;
   }
 
   // Avoid busy looping if the data source is exhausted.
   if (source_exhausted) {
     next_fill_time_ms = std::max(next_fill_time_ms, kNoDataSleepMilliseconds);
   }
 
   // Only schedule more reads/writes if we are still in the playing state.
-  if (shared_data_.state() == kIsPlaying) {
+  if (state() == kIsPlaying) {
     if (next_fill_time_ms == 0) {
       message_loop_->PostTask(
           FROM_HERE,
-          NewRunnableMethod(this, &AlsaPcmOutputStream::WriteTask));
+          method_factory_.NewRunnableMethod(&AlsaPcmOutputStream::WriteTask));
     } else {
       // TODO(ajwong): Measure the reliability of the delay interval.  Use
       // base/metrics/histogram.h.
       message_loop_->PostDelayedTask(
           FROM_HERE,
-          NewRunnableMethod(this, &AlsaPcmOutputStream::WriteTask),
+          method_factory_.NewRunnableMethod(&AlsaPcmOutputStream::WriteTask),
           next_fill_time_ms);
     }
   }
 }
 
 uint32 AlsaPcmOutputStream::FramesToMicros(uint32 frames, uint32 sample_rate) {
   return frames * base::Time::kMicrosecondsPerSecond / sample_rate;
 }
 
 uint32 AlsaPcmOutputStream::FramesToMillis(uint32 frames, uint32 sample_rate) {
@@ skipping 156 matching lines @@
                                               default_channels, sample_rate_,
                                               pcm_format_, latency)) != NULL) {
     return handle;
   }
 
   // Unable to open any device.
   device_name_.clear();
   return NULL;
 }
 
-AudioManagerLinux* AlsaPcmOutputStream::manager() {
-  DCHECK_EQ(MessageLoop::current(), client_thread_loop_);
-  return manager_;
-}
-
-AlsaPcmOutputStream::SharedData::SharedData(
-    MessageLoop* state_transition_loop)
-    : state_(kCreated),
-      volume_(1.0f),
-      source_callback_(NULL),
-      state_transition_loop_(state_transition_loop) {
-}
-
-bool AlsaPcmOutputStream::SharedData::CanTransitionTo(InternalState to) {
-  base::AutoLock l(lock_);
-  return CanTransitionTo_Locked(to);
-}
-
-bool AlsaPcmOutputStream::SharedData::CanTransitionTo_Locked(
-    InternalState to) {
-  lock_.AssertAcquired();
-
+bool AlsaPcmOutputStream::CanTransitionTo(InternalState to) {
   switch (state_) {
     case kCreated:
       return to == kIsOpened || to == kIsClosed || to == kInError;
 
     case kIsOpened:
       return to == kIsPlaying || to == kIsStopped ||
           to == kIsClosed || to == kInError;
 
     case kIsPlaying:
       return to == kIsPlaying || to == kIsStopped ||
           to == kIsClosed || to == kInError;
 
     case kIsStopped:
       return to == kIsPlaying || to == kIsStopped ||
           to == kIsClosed || to == kInError;
 
     case kInError:
       return to == kIsClosed || to == kInError;
 
     case kIsClosed:
     default:
       return false;
   }
 }
 
 AlsaPcmOutputStream::InternalState
-AlsaPcmOutputStream::SharedData::TransitionTo(InternalState to) {
-  DCHECK_EQ(MessageLoop::current(), state_transition_loop_);
+AlsaPcmOutputStream::TransitionTo(InternalState to) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  base::AutoLock l(lock_);
-  if (!CanTransitionTo_Locked(to)) {
+  if (!CanTransitionTo(to)) {
     NOTREACHED() << "Cannot transition from: " << state_ << " to: " << to;
     state_ = kInError;
   } else {
     state_ = to;
   }
   return state_;
 }
 
-AlsaPcmOutputStream::InternalState AlsaPcmOutputStream::SharedData::state() {
-  base::AutoLock l(lock_);
+AlsaPcmOutputStream::InternalState AlsaPcmOutputStream::state() {
   return state_;
 }
 
-float AlsaPcmOutputStream::SharedData::volume() {
-  base::AutoLock l(lock_);
-  return volume_;
-}
-
-void AlsaPcmOutputStream::SharedData::set_volume(float v) {
-  base::AutoLock l(lock_);
-  volume_ = v;
-}
-
-uint32 AlsaPcmOutputStream::SharedData::OnMoreData(
-    AudioOutputStream* stream, uint8* dest, uint32 max_size,
-    AudioBuffersState buffers_state) {
-  base::AutoLock l(lock_);
+uint32 AlsaPcmOutputStream::RunDataCallback(uint8* dest,
+                                            uint32 max_size,
+                                            AudioBuffersState buffers_state) {
   if (source_callback_) {
-    return source_callback_->OnMoreData(stream, dest, max_size, buffers_state);
+    return source_callback_->OnMoreData(this, dest, max_size, buffers_state);
   }
 
   return 0;
 }
 
-void AlsaPcmOutputStream::SharedData::OnError(AudioOutputStream* stream,
-                                              int code) {
-  base::AutoLock l(lock_);
+void AlsaPcmOutputStream::RunErrorCallback(int code) {
   if (source_callback_) {
-    source_callback_->OnError(stream, code);
+    source_callback_->OnError(this, code);
   }
 }
 
 // Changes the AudioSourceCallback to proxy calls to.  Pass in NULL to
 // release ownership of the currently registered callback.
-void AlsaPcmOutputStream::SharedData::set_source_callback(
-    AudioSourceCallback* callback) {
-  DCHECK_EQ(MessageLoop::current(), state_transition_loop_);
-  base::AutoLock l(lock_);
+void AlsaPcmOutputStream::set_source_callback(AudioSourceCallback* callback) {
+  DCHECK_EQ(MessageLoop::current(), message_loop_);
   source_callback_ = callback;
 }