PulseAudio Sound Playback on Linux

media/audio/linux/pulse_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.
+
+#include "media/audio/linux/pulse_output.h"
+
+#include "media/audio/linux/audio_manager_linux.h"
+#include "media/base/data_buffer.h"
+#include "media/base/seekable_buffer.h"
+
+void PulseAudioStateCallback(pa_context* c, void* userdata) {

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

All these callback methods should be static (to avoid cluttering up global
namespace!)

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: more descriptive variable name than "c"?

[slock, 2011/08/08 20:30:15]

Done.

[slock, 2011/08/08 20:30:15]

Done.

+  // TODO(slock): Cover the rest of the states and integrate this state with the
+  // InternalState system.
+  pa_context_state_t state;
+  int* pa_context_ready = (int*)userdata;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

static_cast instead of C-style cast

[slock, 2011/08/08 20:30:15]

Done.

+  state = pa_context_get_state(c);

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

pa_context_state_t state = ... 
and remove line 14

[slock, 2011/08/08 20:30:15]

Done.

+  switch(state) {
+    default:
+      break;
+    case PA_CONTEXT_FAILED:
+      *pa_context_ready = 3;
+      break;
+    case PA_CONTEXT_TERMINATED:
+      *pa_context_ready = 2;
+      break;
+    case PA_CONTEXT_READY:
+      *pa_context_ready = 1;
+      break;
+  }
+}
+
+void WriteCallback(pa_stream* s, size_t length, void* userdata) {

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

So actually, I think you can make this a static private member function of the
PulseAudioOutputStream class.

Then you could pass in "this" for the user data, and you should be able to make
BufferPacketInClient() a private method?

I *think* that'll work, try it and see!

[slock, 2011/08/08 20:30:15]

Done.  That DID work, awesome.  That's huge.  Should I have known that that
would work?

+  PulseAudioOutputStream* stream_ptr =
+      static_cast<PulseAudioOutputStream*>(userdata);
+
+  // Request data from upstream if necessary.
+  while (stream_ptr->client_buffer_->forward_bytes() < length &&
+         !stream_ptr->source_exhausted_)

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: {} around body of loop (multi-line condition)

[slock, 2011/08/08 20:30:15]

Done.

+    stream_ptr->BufferPacketInClient();
+
+  // Get data to write.
+  uint8 read_data[length];

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

variable-length arrays are no good! scoped_array instead

[slock, 2011/08/08 20:30:15]

Done.

+  stream_ptr->client_buffer_->Read(read_data, length);
+  // Write to stream.
+  pa_stream_write(s, read_data, length, NULL, 0LL, PA_SEEK_RELATIVE);

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: more descriptive variable name than "s"?

[slock, 2011/08/08 20:30:15]

Done.

+}
+
+pa_sample_format_t BitsToFormat(int bits_per_sample) {

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

static

[slock, 2011/08/08 20:30:15]

Done.

+  switch(bits_per_sample) {
+    // Unsupported sample formats shown for reference.  I am assuming we want
+    // signed and little endian because that is what we gave to ALSA.
+    case 8:
+      return PA_SAMPLE_U8;
+      // Also 8-bits: PA_SAMPLE_ALAW and PA_SAMPLE_ULAW
+    case 16:
+      return PA_SAMPLE_S16LE;
+      // Also 16-bits: PA_SAMPLE_S16BE (big endian).
+    case 24:
+      return PA_SAMPLE_S24LE;
+      // Also 24-bits: PA_SAMPLE_S24BE (big endian).
+    // Other cases: PA_SAMPLE_24_32LE (in LSBs of 32-bit field, little endian),
+    //          and PA_SAMPLE_24_32BE (in LSBs of 32-bit field, big endian),
+    case 32:
+      return PA_SAMPLE_S32LE;
+      // Also 32-bits: PA_SAMPLE_S32BE (big endian),
+      //               PA_SAMPLE_FLOAT32LE (floating point little endian),
+      //           and PA_SAMPLE_FLOAT32BE (floating point big endian).
+    default:
+      return PA_SAMPLE_INVALID;
+  }
+}
+
+PulseAudioOutputStream::PulseAudioOutputStream(const AudioParameters& params,
+                                               AudioManagerLinux* manager)
+      : client_buffer_(NULL),
+        source_exhausted_(false),
+        channel_layout_(params.channel_layout),
+        sample_format_(BitsToFormat(params.bits_per_sample)),
+        sample_rate_(params.sample_rate),
+        bytes_per_sample_(params.bits_per_sample / 8),
+        bytes_per_frame_(params.channels * params.bits_per_sample / 8),
+        should_downmix_(false),
+        should_swizzle_(false),
+        packet_size_(params.GetPacketSize()),
+        stop_stream_(false),
+        manager_(manager),
+        pa_mainloop_(NULL),
+        pa_mainloop_api_(NULL),
+        pa_context_(NULL),
+        playback_handle_(NULL),
+        frames_per_packet_(packet_size_ / bytes_per_frame_),
+        state_(kCreated),
+        volume_(1.0f),
+        source_callback_(NULL) {
+  // TODO(slock): Sanity check input values.
+}
+
+PulseAudioOutputStream::~PulseAudioOutputStream() {
+  // TODO(slock): Nothing to be done but state work.

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Delete comment

Also, here you *do* want to free your resources, stop the stream, etc...

[slock, 2011/08/08 20:30:15]

Done.  Note that AudioManagerLinux requires the runflow this::Close(), then
AudioManagerLinux::ReleaseOutputStream(this), and finally
this::~PulseAudioOutputStream.  Therefore, resources are freed in Close() and
the destructor is empty.

+}
+
+bool PulseAudioOutputStream::Open() {
+  // TODO(slock): Possibly move most of this to a OpenPlaybackDevice function in
+  // a new class 'pulse_util', like alsa_util.
+
+  if (state() == kInError)

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

delete; state transitions are not implemented

[slock, 2011/08/08 20:30:15]

Done.

+    return false;
+
+  // TODO(slock): Implement state transitions.

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

delete TODO

[slock, 2011/08/08 20:30:15]

Done.

+
+  // Create a mainloop API and connect to the default server.
+  pa_mainloop_ = pa_mainloop_new();

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Never deleted; needs a call to pa_mainloop_free

[slock, 2011/08/08 20:30:15]

Done.

+  pa_mainloop_api_ = pa_mainloop_get_api(pa_mainloop_);
+  pa_context_ = pa_context_new(pa_mainloop_api_, "Chromium");

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Never deleted; I believe you need to call pa_context_unref

[slock, 2011/08/08 20:30:15]

Done.

+  pa_context_connect(pa_context_, NULL, PA_CONTEXT_NOFLAGS, NULL);
+
+  // Wait until PulseAudio is ready.
+  int pa_context_ready = 0;
+  pa_context_set_state_callback(pa_context_, &PulseAudioStateCallback,
+                                &pa_context_ready);
+  while (pa_context_ready == 0 ){

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: while (!pa_context_ready) and no {}

[slock, 2011/08/08 20:30:15]

Done.

+    pa_mainloop_iterate(pa_mainloop_, 1, NULL);
+  }
+

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Handle error condition (pa_context_ready != PA_CONTEXT_READY) and return false?

[slock, 2011/08/08 20:30:15]

Done.

+  // Set sample specifications and open playback stream.
+  pa_sample_specs_ = new pa_sample_spec;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

probably better to stack-allocate instead of creating new

[slock, 2011/08/08 20:30:15]

Done.

+  pa_sample_specs_->format = sample_format_;
+  pa_sample_specs_->rate = sample_rate_;
+  pa_sample_specs_->channels = ChannelLayoutToChannelCount(channel_layout_);
+  playback_handle_ = pa_stream_new(pa_context_, "Playback",

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Need to delete (I believe pa_stream_unref)

[slock, 2011/08/08 20:30:15]

Done.

+                                   pa_sample_specs_, NULL);
+
+  // Initialize client buffer.
+  bytes_per_output_frame_ = bytes_per_frame_;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

bytes_per_output_frame_ is an unnecessary field; remove

[slock, 2011/08/08 20:30:15]

Done.

+  uint32 output_packet_size = frames_per_packet_ * bytes_per_output_frame_;
+  client_buffer_ = new media::SeekableBuffer(0, output_packet_size);
+
+  // Set write callback.
+  pa_stream_set_write_callback(playback_handle_, WriteCallback,

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: put & before "WriteCallback" for clarity/consistency

[slock, 2011/08/08 20:30:15]

Done.

+                               this);

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: move "this" to line above

[slock, 2011/08/08 20:30:15]

Done.

+
+  // Set server side buffer attributes and connect playback stream.
+  // TODO(slock): Figure out what these values should actually be, recommended
+  // values from PulseAudio's documentation for now.

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

link to documentation where it gives these recommendations?

[slock, 2011/08/08 20:30:15]

Done, but the url plus the "//" and indentation is more than 80 characters.

+  pa_buffer_attributes_ = new pa_buffer_attr;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

stack-allocate instead of creating new

[slock, 2011/08/08 20:30:15]

Done.

+  pa_buffer_attributes_->maxlength = (uint32_t)-1;
+  pa_buffer_attributes_->tlength = output_packet_size;
+  pa_buffer_attributes_->prebuf = (uint32_t)-1;
+  pa_buffer_attributes_->minreq = (uint32_t)-1;
+  pa_buffer_attributes_->fragsize = (uint32_t)-1;
+  pa_stream_connect_playback(playback_handle_, NULL,

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

indentation

[slock, 2011/08/08 20:30:15]

Done.

+                                         pa_buffer_attributes_,
+                                         (pa_stream_flags_t)
+                                         (PA_STREAM_INTERPOLATE_TIMING |
+                                         PA_STREAM_ADJUST_LATENCY |
+                                         PA_STREAM_AUTO_TIMING_UPDATE),
+                                         NULL, NULL);
+
+  // Finish initializing the stream if the device was opened successfully.

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Comment doesn't seem to match the block below?

[slock, 2011/08/08 20:30:15]

Done.

+  if (playback_handle_ == NULL) {

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: if (!playback_handle_) and no {}

[slock, 2011/08/08 20:30:15]

Done.

+    stop_stream_ = true;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

What does this accomplish? Also, wouldn't you want to return false here?

[slock, 2011/08/08 20:30:15]

Done.

+  }
+
+  return true;
+}
+
+void PulseAudioOutputStream::Close() {
+  // Close the device.
+  pa_stream_disconnect(playback_handle_);
+
+  // Release stuff.
+  delete pa_sample_specs_;
+  delete pa_buffer_attributes_;
+  delete client_buffer_;
+
+  // Stop everything.
+  stop_stream_ = true;
+
+  // Signal to the manager that we're closed and can be removed.
+  // This should be the last call in the function as it deletes "this".
+  manager_->ReleaseOutputStream(this);
+}
+
+void PulseAudioOutputStream::BufferPacketInClient() {
+  // If stopped, simulate a 0-length packet.
+  if (stop_stream_) {

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Is this ever reached?

BufferPacketInClient() is only called by WriteCallback
WriteCallback will be triggered synchronously from ClientBufferLoop()'s
pa_mainloop_iterate

stop_stream_ is never modified in between that time, I don't think.

[slock, 2011/08/08 20:30:15]

I'll remove this for now, but this is part of the larger problem of
Start()/ClientBufferLoop() blocking everything else.

+    client_buffer_->Clear();
+    source_exhausted_ = true;
+    return;
+  }
+
+  source_exhausted_ = false;
+
+  // Request more data if we have more capacity.
+  if (client_buffer_->forward_capacity() > client_buffer_->forward_bytes()) {
+
+    // Before making request to source for data we need to determine the delay
+    // (in bytes) for the requested data to be played.
+    uint32 buffer_delay = client_buffer_->forward_bytes();
+    pa_usec_t pa_latency_micros;
+    int negative;

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Is it okay for negative to be unused in hardware_delay calculation?

[slock, 2011/08/08 20:30:15]

No, but its never been negative that I know of.  I'll need to decide what
latency to use in this case.  This will involve more research.

+    pa_stream_get_latency(playback_handle_, &pa_latency_micros, &negative);
+    uint32 hardware_delay = MicrosToBytes(pa_latency_micros, sample_rate_,

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: Change "Micros" to "Microseconds"

[slock, 2011/08/08 20:30:15]

Done.

+                                          bytes_per_frame_);
+    scoped_refptr<media::DataBuffer> packet =
+        new media::DataBuffer(packet_size_);
+    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.  TODO(slocK): Ensure that this is relevant here, it might

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

"WritePacket" doesn't exist

[slock, 2011/08/08 20:30:15]

Done.  This wasn't relevant anyway because I don't write by the packet.

+    // not be.
+    DCHECK(packet_size % bytes_per_frame_ == 0);
+    packet_size = (packet_size / bytes_per_frame_) * bytes_per_frame_;
+
+    // TODO(slock): Swizzling, downmixing, and volume adjusting.
+
+    if (packet_size > 0) {
+      packet->SetDataSize(packet_size);
+      // Add the packet to the buffer.
+      client_buffer_->Append(packet);
+    } else

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

nit: This time you *should* have {} around the else block :)

[slock, 2011/08/08 20:30:15]

Done.

+      source_exhausted_ = true;
+  }
+}
+
+void PulseAudioOutputStream::ClientBufferLoop() {
+  while(!stop_stream_ && !source_exhausted_) {
+    // As long as the stream is active, we should be buffering packets if need
+    // be and writing packets if need be.  These are asynchronous processes.
+    // This loop buffers packets and the PulseAudio mainloop writes them.
+    // BufferPacket() only actually buffers under certain circumstances and
+    // pa_mainloop_iterate() only calls WriteCallback under certain
+    // circumstances, but the loop marches on in either case.
+    pa_mainloop_iterate(pa_mainloop_, 1, NULL);
+  }
+}
+
+void PulseAudioOutputStream::Start(AudioSourceCallback* callback) {
+  CHECK(callback);
+  set_source_callback(callback);

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

inline this method

[slock, 2011/08/08 20:30:15]

Done.

+
+  // Clear buffer, it might still have data in it.
+  client_buffer_->Clear();
+  source_exhausted_ = false;
+
+  // Start playing.
+  ClientBufferLoop();
+}
+
+void PulseAudioOutputStream::Stop() {
+  // Nothing to be done because InternalState not implemented.

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

Instead of trying to "port" the Alsa output stream behavior, you should read and
implement the AudioOutputStream interface. The description for stop says: "Stops
playing audio. Effect might not be instantaneous as the hardware might have
locked audio data that is processing."

This is relevant/necessary for your class, independent of state transitions.

[slock, 2011/08/08 20:30:15]

Done.

+  // TODO(slock): Implement state transitions.
+}
+
+void PulseAudioOutputStream::SetVolume(double volume) {
+  volume_ = static_cast<float>(volume);

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

TODO make necessary calls to PulseAudio to actually set volume?

[slock, 2011/08/08 20:30:15]

Done.  I actually implemented this, but its not tested.

+}
+
+void PulseAudioOutputStream::GetVolume(double* volume) {
+  *volume = volume_;
+}
+
+bool PulseAudioOutputStream::CanTransitionTo(InternalState to) {
+  // TODO(slock): Not implemented.
+  return false;
+}
+
+PulseAudioOutputStream::InternalState
+PulseAudioOutputStream::TransitionTo(InternalState to) {
+  // TODO(slock): Not implemented.
+  return state_;
+}
+
+PulseAudioOutputStream::InternalState PulseAudioOutputStream::state() {
+  return state_;
+}
+
+uint32 PulseAudioOutputStream::RunDataCallback(
+    uint8* dest, uint32 max_size, AudioBuffersState buffers_state) {
+  if (source_callback_)
+    return source_callback_->OnMoreData(this, dest, max_size, buffers_state);
+
+  return 0;
+}
+
+void PulseAudioOutputStream::RunErrorCallback(int code) {
+  NOTIMPLEMENTED();
+}
+
+size_t PulseAudioOutputStream::MicrosToBytes(uint32 micros, uint32 sample_rate,

[vrk (LEFT CHROMIUM), 2011/08/05 15:02:26]

file-static function?

[slock, 2011/08/08 20:30:15]

Done.

+                                             size_t bytes_per_frame) {
+  return micros * sample_rate * bytes_per_frame /
+      base::Time::kMicrosecondsPerSecond;
+}
+
+void PulseAudioOutputStream::set_source_callback(
+    AudioSourceCallback* callback) {
+  source_callback_= callback;
+}