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.
+  // Tell pulse audio we generally want callbacks of a certain size.  Pulse
+  // typically wants larger buffers up front but will honor our maxlength and
+  // minreq settings once it reaches steady state.
   pa_buffer_attr pa_buffer_attributes;
   pa_buffer_attributes.maxlength = params_.GetBytesPerBuffer();
   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 = static_cast<uint32_t>(-1);
   pa_buffer_attributes.fragsize = static_cast<uint32_t>(-1);

[no longer working on chromium, 2013/02/28 08:56:09]

by looking at
http://freedesktop.org/software/pulseaudio/doxygen/structpa__buffer__attr.html
it seems fragsize, prebuf and tlength can affect the latency of Pulse when
PA_STREAM_ADJUST_LATENCY is set. If I understand correctly, setting them to -1
means we let Pulse buffer up enough data and compromise its latency for good
audio.

Do you know if flash and audio tag are always trying to open the streams using
the default buffer size (512)?

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

Hmm I retested this with multiple streams and Pulse went berserk (super fast
callbacks a la http://crbug.com/116435).  After some trial and error this seems
to work reliably without a latency increase (as measured by audio_latency_perf).

I needed to adjust some of the flags to make this work though.  Without
PA_STREAM_INTERPOLATE_TIMING, the latency value was on the order of seconds (!)
when using a tlength value this small.

Please patch this in locally with your other Pulse patches and see how it
sounds.

 
   // 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.
   RETURN_ON_FAILURE(
       pa_stream_connect_playback(
           pa_stream_, NULL, &pa_buffer_attributes,
           static_cast<pa_stream_flags_t>(
               PA_STREAM_ADJUST_LATENCY | PA_STREAM_AUTO_TIMING_UPDATE |
@@ skipping 58 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()));

[no longer working on chromium, 2013/02/28 08:56:09]

I agree these checks might be dangerous if we support different buffer sizes.
Could you elaborate why we want to check it instead of handling it?

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

Mostly because I don't think we'll need to since we're requesting a buffer equal
in size to our minreq.  As such I'd rather not over complicate the code at this
time.  We'll notice immediately in crash reports if we need to handle this or
not.

 
-    // 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 39 matching lines @@
   volume_ = static_cast<float>(volume);
 }
 
 void PulseAudioOutputStream::GetVolume(double* volume) {
   DCHECK(manager_->GetMessageLoop()->BelongsToCurrentThread());
 
   *volume = volume_;
 }
 
 }  // namespace media