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

media/audio/pulse/pulse_output.cc

Index: media/audio/pulse/pulse_output.cc
diff --git a/media/audio/pulse/pulse_output.cc b/media/audio/pulse/pulse_output.cc
index 7cf66cb10c801ba9f7de12c9aae90d1e7e06dc40..1d969f605851eeff2ebece86d37694cd610a9ef6 100644
--- a/media/audio/pulse/pulse_output.cc
+++ b/media/audio/pulse/pulse_output.cc
@@ -153,12 +153,14 @@ bool PulseAudioOutputStream::Open() {
   // 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.
@@ -238,43 +240,33 @@ void PulseAudioOutputStream::Close() {
 }
 
 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.

+
+    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));
+    }
 
-    // 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_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_);
 
@@ -286,7 +278,6 @@ void PulseAudioOutputStream::FulfillWriteRequest(size_t requested_bytes) {
     }
 
     bytes_remaining -= bytes_to_fill;
-    current_frame = frames_to_fill;
   }
 }