[pulseaudio-discuss] [PATCH] pulse: Fix invalid buffer pointer return value

[David Henningsson]

Colin Guthrie wrote:
> 'Twas brillig, and David Henningsson at 16/01/10 07:07 did gyre and gimble:
>> David Henningsson wrote:
>>> Lennart Poettering wrote:
>>>> On Sat, 09.01.10 10:00, David Henningsson (launchpad.web at epost.diwic.se) wrote:
>>>>
>>>>> The pulse ALSA plugin has been known, for a while, to not work properly,
>>>>> causing underruns, hangs etc. I sat down yesterday trying to figure it
>>>>> out, and I'm pretty certain this patch improves the situation, but I
>>>>> don't mind getting some help testing it before it is committed
>>>>> upstream.
>>>> I am kinda convinced that the actual fix for this issue is this
>>>> patch. Could you check that?
>>>>
>>>> http://git.0pointer.de/?p=pulseaudio.git;a=patch;h=8d356659e69556fa25d0579a66084f820683e2b8
>> I have now checked it with the latest version of pulseaudio and without
>> my patch posted earlier, I still experience hangs. That and positive
>> feedback given from https://bugs.launchpad.net/bugs/485488 makes me
>> believe that the patch is ready for inclusion upstream (it is already in
>> Ubuntu, thanks Dan).
>>
>> Now onto the next problem. If an underrun occurs in the pulseaudio
>> front-end, it is delivered asynchonously to an underrun callback. This
>> sometimes happens after more buffers are sent (through
>> pa_stream_write()). So these new buffers will end the underrun in
>> parallell with the processing of the underrun in the client, making the
>> underrun obsolete. After all, seen from the client program's
>> perspective, if I write a buffer and it returns underrun (-EPIPE), I
>> assume that all other buffers I've written, which returned successfully,
>> have already been played back.
>>
>> The best action in this case should be to ignore the underrun, if I can
>> determine that the above case is true. Now, how do I do that? I have
>> tried to look at pa_stream_get_timing_info() [1], but I can't find a
>> reliable way to distinguish valid underruns from obsolete ones.
>>
>> // David
>>
>> [1] which needs to be updated, and I can't use pa_wait_operation from
>> the callback so I'll have to use a second callback for the timing info...
> 
> 
> Just pinging this one up. I forgot I'd applied this to my own personal
> alsa-plugins build and only noticed it when I updated to 1.0.23 upstream
> version.
> 
> I've not seen any specific problems with this but also not sure what it
> fixes.

It fixes (at least) the mpg123 startup race (which lead to dropped
buffers on an underrun).

> Is this still applied in Ubuntu 

Daniel Chen volunteered to do some testing and he found no regressions,
so he applied my "remove underrun handling" patch, and AFAIK it is still
applied.

I haven't heard of any regressions so far, so I believe it should be
pretty safe to upstream.

> and should we get further review from
> Lennart when he's got a mo (which is sadly not often in recent months)
> about the issue with a view to getting it upstreamed?

Lennart's, anyone else's opinion, and upstreaming, is very welcome. If
you feel I should do something to help this happen, go ahead and tell
me. :-)

// David