Always fully fill PulseAudio's requested buffer. Allow larger initial requests.

media/audio/pulse/pulse_output.cc

 // 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.
 
 #include "media/audio/pulse/pulse_output.h"
 
 #include <pulse/pulseaudio.h>
 
 #include "base/message_loop.h"
 #include "media/audio/audio_manager_base.h"
@@ skipping 135 matching lines @@
   }
   pa_stream_ = pa_stream_new(
       pa_context_, "Playback", &pa_sample_specifications, map);
   RETURN_ON_FAILURE(pa_stream_, "Failed to create PulseAudio stream.");
   pa_stream_set_state_callback(pa_stream_, &StreamNotifyCallback, this);
 
   // Even though we start the stream corked below, PulseAudio will issue one
   // stream request after setup.  FulfillWriteRequest() must fulfill the write.
   pa_stream_set_write_callback(pa_stream_, &StreamRequestCallback, this);
 
-  // Tell pulse audio we only want callbacks of a certain size.
+  // Pulse is very finicky with the small buffer sizes used by Chrome.  The
+  // settings below are mostly found through trial and error.  Essentially we
+  // want Pulse to auto size its internal buffers, but call us back nearly every
+  // |minreq| bytes.  |tlength| should be a multiple of |minreq|; too low and
+  // Pulse will issue callbacks way too fast, too high and we don't get
+  // callbacks frequently enough.
   pa_buffer_attr pa_buffer_attributes;
-  pa_buffer_attributes.maxlength = params_.GetBytesPerBuffer();
+  pa_buffer_attributes.maxlength = static_cast<uint32_t>(-1);
   pa_buffer_attributes.minreq = params_.GetBytesPerBuffer();
-  pa_buffer_attributes.prebuf = params_.GetBytesPerBuffer();
-  pa_buffer_attributes.tlength = params_.GetBytesPerBuffer();
+  pa_buffer_attributes.prebuf = static_cast<uint32_t>(-1);
+  pa_buffer_attributes.tlength = params_.GetBytesPerBuffer() * 3;
   pa_buffer_attributes.fragsize = static_cast<uint32_t>(-1);
 
-  // Connect playback stream.
-  // TODO(dalecurtis): Pulse tends to want really large buffer sizes if we are
-  // not using the native sample rate.  We should always open the stream with
-  // PA_STREAM_FIX_RATE and ensure this is true.
+  // Connect playback stream.  Like pa_buffer_attr, the pa_stream_flags have a
+  // huge impact on the performance of the stream and were chosen through trial
+  // and error.
   RETURN_ON_FAILURE(
       pa_stream_connect_playback(
           pa_stream_, NULL, &pa_buffer_attributes,
           static_cast<pa_stream_flags_t>(
+              PA_STREAM_FIX_RATE | PA_STREAM_INTERPOLATE_TIMING |

[Sam Edwards, 2013/05/22 23:25:03]

All,

I realize I'm a bit late to the party here (as of this writing, this code has
been moved to pulse_util.cc) but PA_STREAM_FIX_RATE is improper here. This
forces the stream's sample rate to match the sink's sample rate, which isn't
necessarily the same as info->sample_spec.rate.

When this happens, PulseAudio begins playing back the audio at an improper
sample rate, leading to e.g.
https://code.google.com/p/webrtc/issues/detail?id=1587

Thanks,
Sam

               PA_STREAM_ADJUST_LATENCY | PA_STREAM_AUTO_TIMING_UPDATE |
               PA_STREAM_NOT_MONOTONIC | PA_STREAM_START_CORKED),
           NULL, NULL) == 0,
       "Failed to connect PulseAudio stream.");
 
   // Wait for the stream to be ready.
   while (true) {
     pa_stream_state_t stream_state = pa_stream_get_state(pa_stream_);
     RETURN_ON_FAILURE(
         PA_STREAM_IS_GOOD(stream_state), "Invalid PulseAudio stream state.");
@@ skipping 49 matching lines @@
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
 
   Reset();
 
   // 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::FulfillWriteRequest(size_t requested_bytes) {
-  CHECK_EQ(requested_bytes, static_cast<size_t>(params_.GetBytesPerBuffer()));
-
-  int frames_filled = 0;
-  if (source_callback_) {
-    uint32 hardware_delay = pulse::GetHardwareLatencyInBytes(
-        pa_stream_, params_.sample_rate(),
-        params_.GetBytesPerFrame());
-    frames_filled = source_callback_->OnMoreData(
-        audio_bus_.get(), AudioBuffersState(0, hardware_delay));
-  }
-
-  // Zero any unfilled data so it plays back as silence.
-  if (frames_filled < audio_bus_->frames()) {
-    audio_bus_->ZeroFramesPartial(
-        frames_filled, audio_bus_->frames() - frames_filled);
-  }
-
-  // PulseAudio won't always be able to provide a buffer large enough, so we may
-  // need to request multiple buffers and fill them individually.
-  int current_frame = 0;
-  size_t bytes_remaining = requested_bytes;
+  int bytes_remaining = requested_bytes;
   while (bytes_remaining > 0) {
     void* buffer = NULL;
-    size_t bytes_to_fill = bytes_remaining;
+    size_t bytes_to_fill = params_.GetBytesPerBuffer();
     CHECK_GE(pa_stream_begin_write(pa_stream_, &buffer, &bytes_to_fill), 0);
+    CHECK_EQ(bytes_to_fill, static_cast<size_t>(params_.GetBytesPerBuffer()));
 
-    // In case PulseAudio gives us a bigger buffer than we want, cap our size.
-    bytes_to_fill = std::min(
-        std::min(bytes_remaining, bytes_to_fill),
-        static_cast<size_t>(params_.GetBytesPerBuffer()));
+    int frames_filled = 0;
+    if (source_callback_) {
+      uint32 hardware_delay = pulse::GetHardwareLatencyInBytes(
+          pa_stream_, params_.sample_rate(),
+          params_.GetBytesPerFrame());
+      source_callback_->WaitTillDataReady();
+      frames_filled = source_callback_->OnMoreData(
+          audio_bus_.get(), AudioBuffersState(0, hardware_delay));
+    }
 
-    int frames_to_fill = bytes_to_fill / params_.GetBytesPerFrame();;
+    // Zero any unfilled data so it plays back as silence.
+    if (frames_filled < audio_bus_->frames()) {
+      audio_bus_->ZeroFramesPartial(
+          frames_filled, audio_bus_->frames() - frames_filled);
+    }
 
     // Note: If this ever changes to output raw float the data must be clipped
     // and sanitized since it may come from an untrusted source such as NaCl.
-    audio_bus_->ToInterleavedPartial(
-        current_frame, frames_to_fill, params_.bits_per_sample() / 8, buffer);
+    audio_bus_->ToInterleaved(
+        audio_bus_->frames(), params_.bits_per_sample() / 8, buffer);
     media::AdjustVolume(buffer, bytes_to_fill, params_.channels(),
                         params_.bits_per_sample() / 8, volume_);
 
     if (pa_stream_write(pa_stream_, buffer, bytes_to_fill, NULL, 0LL,
                         PA_SEEK_RELATIVE) < 0) {
       if (source_callback_) {
         source_callback_->OnError(this, pa_context_errno(pa_context_));
       }
     }
 
     bytes_remaining -= bytes_to_fill;
-    current_frame = frames_to_fill;
   }
 }
 
 void PulseAudioOutputStream::Start(AudioSourceCallback* callback) {
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
   CHECK(callback);
   CHECK(pa_stream_);
 
   AutoPulseLock auto_lock(pa_mainloop_);
 
@@ skipping 12 matching lines @@
   WaitForOperationCompletion(pa_mainloop_, operation);
 }
 
 void PulseAudioOutputStream::Stop() {
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
 
   // Cork (pause) the stream.  Waiting for the main loop lock will ensure
   // outstanding callbacks have completed.
   AutoPulseLock auto_lock(pa_mainloop_);
 
+  // Set |source_callback_| to NULL so all FulfillWriteRequest() calls which may

[DaleCurtis, 2013/03/01 00:08:17]

Also discovered some shutdown hangs here due to WaitTilDataReady() hanging for
1.5seconds multiple times.

[no longer working on chromium, 2013/03/01 20:44:13]

makes sense.

+  // occur while waiting on the flush and cork exit immediately.
+  source_callback_ = NULL;
+
   // Flush the stream prior to cork, doing so after will cause hangs.  Write
   // callbacks are suspended while inside pa_threaded_mainloop_lock() so this
   // is all thread safe.
   pa_operation* operation = pa_stream_flush(
       pa_stream_, &pulse::StreamSuccessCallback, pa_mainloop_);
   WaitForOperationCompletion(pa_mainloop_, operation);
 
   operation = pa_stream_cork(pa_stream_, 1, &pulse::StreamSuccessCallback,
                              pa_mainloop_);
   WaitForOperationCompletion(pa_mainloop_, operation);
-
-  source_callback_ = NULL;
 }
 
 void PulseAudioOutputStream::SetVolume(double volume) {
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
 
   volume_ = static_cast<float>(volume);
 }
 
 void PulseAudioOutputStream::GetVolume(double* volume) {
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
 
   *volume = volume_;
 }
 
 }  // namespace media