[PATCH] Allow pulseaudio writes in larger chunks

[David Henningsson]

The previous patch (posted several months ago) for improving pulsesink 
writes was rejected, due to causing underruns when playing wma files. 
I've been able to reproduce that error here, and have come up with a new 
version of the patch which does not underruns in that case.

In short, the behavioural difference is
  - without the patch, waiting and write size were both never above segsize
  - with the old patch, both waiting and write size could grow up to 
bufsize.
  - with the new patch, waiting is done up to segsize. However when 
waiting is done, it writes as much as it can.

Also, the previous patch was blamed to "ignore the latency-time" of the 
sink. This statement is false, at least for the new version of the 
patch. The latency is controlled by setting tlength and minreq 
parameters of pa_buffer_attr, which in turn controls the behaviour of 
pa_stream_writable_size.

Let me go through the patch quickly for you:

 > diff --git a/ext/pulse/pulsesink.c b/ext/pulse/pulsesink.c
 > index c9f0b58..b90ebe3 100644
 > --- a/ext/pulse/pulsesink.c
 > +++ b/ext/pulse/pulsesink.c
 > @@ -1430,9 +1430,7 @@ gst_pulseringbuffer_commit (GstRingBuffer * 
buf, guint64 * sample,
 >
 >      towrite = out_samples * bps;
 >
 > -    /* Only ever write segsize bytes at once. This will
 > -     * also limit the PA shm buffer to segsize
 > -     */
 > +    /* Wait for at least segsize bytes to become available */
 >      if (towrite > buf->spec.segsize)
 >        towrite = buf->spec.segsize;

Just change the comment to adhere to the new behaviour.

 > @@ -1481,7 +1479,7 @@ gst_pulseringbuffer_commit (GstRingBuffer * 
buf, guint64 * sample,
 >             goto uncork_failed;
 >         }
 >
 > -        /* we can't write a single byte, wait a bit */
 > +        /* we can't write segsize bytes, wait a bit */
 >          GST_LOG_OBJECT (psink, "waiting for free space");
 >          pa_threaded_mainloop_wait (mainloop);

Comment clarification.

 > @@ -1489,14 +1487,10 @@ gst_pulseringbuffer_commit (GstRingBuffer * 
buf, guint64 * sample,
 >            goto was_paused;
 >        }
 >
 > -      /* make sure we only buffer up latency-time samples */
 > -      if (pbuf->m_writable > buf->spec.segsize) {
 > -        /* limit buffering to latency-time value */
 > -        pbuf->m_writable = buf->spec.segsize;
 > -
 > -        GST_LOG_OBJECT (psink, "Limiting buffering to %" > 
G_GSIZE_FORMAT,
 > -            pbuf->m_writable);
 > -      }

So this is what's essentially wrong - there is no extra "buffering" 
being done here (despite the comment), as the samples are already in a 
buffer when they're coming into this function. This limit is what 
worsens performance, so remove it.

 > +      /* Recalculate what we can write in the next chunk */
 > +      towrite = out_samples * bps;
 > +      if (pbuf->m_writable > towrite)
 > +          pbuf->m_writable = towrite;
 >
 >        GST_LOG_OBJECT (psink, "requesting %" G_GSIZE_FORMAT " bytes of "
 >            "shared memory", pbuf->m_writable);

Instead limit what we write to what's left to write in this round.

 > @@ -1510,14 +1504,9 @@ gst_pulseringbuffer_commit (GstRingBuffer * 
buf, guint64 * sample,
 >        GST_LOG_OBJECT (psink, "got %" G_GSIZE_FORMAT " bytes of 
shared memory",
 >            pbuf->m_writable);
 >
 > -      /* Just to make sure that we didn't get more than requested */
 > -      if (pbuf->m_writable > buf->spec.segsize) {
 > -        /* limit buffering to latency-time value */
 > -        pbuf->m_writable = buf->spec.segsize;
 > -      }
 >      }

The same write limit stuff is being done twice, remove it here as well.

 >
 > -    if (pbuf->m_writable < towrite)
 > +    if (towrite > pbuf->m_writable)
 >        towrite = pbuf->m_writable;
 >      avail = towrite / bps;

Just more readable/common semantic IMO.

-- 
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic