Re: [pulseaudio-discuss] underrun behavior with alsa-plugins

2011-04-20 Thread David Henningsson

On 2011-04-19 18:12, Maarten Lankhorst wrote:

Hi all,

For wine I was investigating a bug with pulseaudio, it seems
alsa-plugins' pulse driver ignores underruns.


Hmm, doesn't wine come with a native pulse driver these days?


This is probably because
an underrun will force you to call snd_pcm_prepare, this will destroy
the stream and set up a similar new one again.


...whereas the proper way to resolve the underrun in the pulse plugin is 
just to feed it with more data, which is likely what would happen if you 
do not report the underrun.



This causes more
susceptibility to underruns, so that code was disabled.


Takashi added some kind of option so you can turn the reporting of 
underruns on again, if that helps.



However if I
force underruns to occur,the state will stay running and it appears
there is still some data left in the buffer, so sound stalls entirely.


This is nothing I've heard of.


The latency gets updated to something like 0x7bdX which
looks suspiciously much like a pointer to me, which may be a bug. If I
instead make pulse_prepare call pulse_start on underrun, it's handled
properly and sound will continue to work.

So my questions are.
1. Is the weird latency value update a bug?


I guess so, but what sense would it make to read the latency if you're 
in an underrun condition - and btw, exactly what latency variable is this?



Digging it up I can only
assume it's a bug in src/stream.c , but I haven't figured out why yet.
Tested with pulseaudio.c
2. Any comments on this patch for alsa-plugins?


IIRC, by setting handle_underrun to 1, you're reopening bugs of 
broken/skipping audio for mpg123 and other programs, which get stuck in 
an infinite loop of write one sample, force start the stream, write 
more samples, asynchronusly get an underrun, drop the stream and drop 
samples already written.


There should be threads on at least alsa-devel about this, from a year 
back or so.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] underrun behavior with alsa-plugins

2011-04-20 Thread Maarten Lankhorst

Hi David,

Op 20-04-11 09:33, David Henningsson schreef:

On 2011-04-19 18:12, Maarten Lankhorst wrote:

Hi all,

For wine I was investigating a bug with pulseaudio, it seems
alsa-plugins' pulse driver ignores underruns.


Hmm, doesn't wine come with a native pulse driver these days?
Nope, but distros patch in a buggy winepulse driver, wine is working on 
rearchitecting its driver model, so that a pulseaudio driver might be 
added after that, but the current winepulse driver was a bad 
copy/replace job of the wrong sound driver. :)



This is probably because
an underrun will force you to call snd_pcm_prepare, this will destroy
the stream and set up a similar new one again.


...whereas the proper way to resolve the underrun in the pulse plugin is
just to feed it with more data, which is likely what would happen if you
do not report the underrun.


This causes more
susceptibility to underruns, so that code was disabled.


Takashi added some kind of option so you can turn the reporting of
underruns on again, if that helps.

If you read my patch, that's what it does.


However if I
force underruns to occur,the state will stay running and it appears
there is still some data left in the buffer, so sound stalls entirely.


This is nothing I've heard of.
Sadly all too common here, if I feed data and it underruns, it may 
appear to be running but is stalled entirely.



The latency gets updated to something like 0x7bdX which
looks suspiciously much like a pointer to me, which may be a bug. If I
instead make pulse_prepare call pulse_start on underrun, it's handled
properly and sound will continue to work.

So my questions are.
1. Is the weird latency value update a bug?


I guess so, but what sense would it make to read the latency if you're
in an underrun condition - and btw, exactly what latency variable is this?
In a response to a latency event I printf the latency inside the 
callback, with pa_stream_get_latency.



Digging it up I can only
assume it's a bug in src/stream.c , but I haven't figured out why yet.
Tested with pulseaudio.c
2. Any comments on this patch for alsa-plugins?


IIRC, by setting handle_underrun to 1, you're reopening bugs of
broken/skipping audio for mpg123 and other programs, which get stuck in
an infinite loop of write one sample, force start the stream, write
more samples, asynchronusly get an underrun, drop the stream and drop
samples already written.

There should be threads on at least alsa-devel about this, from a year
back or so.
This is exactly what my patch was addressing, I was fixing that bug by 
handling underrun correctly. snd_pcm_prepare() is called when an 
underrun occurs, so instead I make it only restart the stream if 
underrun. Not handling underruns at all seems to allow it to stall on 
xrun, while claiming to be running. This sometimes appears to happen in 
other programs too though, like mplayer. Could it be because of the 
weird latency value?


Cheers,
Maarten
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] underrun behavior with alsa-plugins

2011-04-20 Thread David Henningsson

On 2011-04-20 12:09, Maarten Lankhorst wrote:

Hi David,

Op 20-04-11 09:33, David Henningsson schreef:

On 2011-04-19 18:12, Maarten Lankhorst wrote:

Hi all,

For wine I was investigating a bug with pulseaudio, it seems
alsa-plugins' pulse driver ignores underruns.


Hmm, doesn't wine come with a native pulse driver these days?

Nope, but distros patch in a buggy winepulse driver, wine is working on
rearchitecting its driver model, so that a pulseaudio driver might be
added after that, but the current winepulse driver was a bad
copy/replace job of the wrong sound driver. :)


This is probably because
an underrun will force you to call snd_pcm_prepare, this will destroy
the stream and set up a similar new one again.


...whereas the proper way to resolve the underrun in the pulse plugin is
just to feed it with more data, which is likely what would happen if you
do not report the underrun.


This causes more
susceptibility to underruns, so that code was disabled.


Takashi added some kind of option so you can turn the reporting of
underruns on again, if that helps.

If you read my patch, that's what it does.


However if I
force underruns to occur,the state will stay running and it appears
there is still some data left in the buffer, so sound stalls entirely.


This is nothing I've heard of.

Sadly all too common here, if I feed data and it underruns, it may
appear to be running but is stalled entirely.


Could you provide a test case where this occurs? (Pulseaudio/ALSA 
version, client applications/libraries, etc)



The latency gets updated to something like 0x7bdX which
looks suspiciously much like a pointer to me, which may be a bug. If I
instead make pulse_prepare call pulse_start on underrun, it's handled
properly and sound will continue to work.

So my questions are.
1. Is the weird latency value update a bug?


I guess so, but what sense would it make to read the latency if you're
in an underrun condition - and btw, exactly what latency variable is
this?

In a response to a latency event I printf the latency inside the
callback, with pa_stream_get_latency.


Digging it up I can only
assume it's a bug in src/stream.c , but I haven't figured out why yet.
Tested with pulseaudio.c
2. Any comments on this patch for alsa-plugins?


IIRC, by setting handle_underrun to 1, you're reopening bugs of
broken/skipping audio for mpg123 and other programs, which get stuck in
an infinite loop of write one sample, force start the stream, write
more samples, asynchronusly get an underrun, drop the stream and drop
samples already written.

There should be threads on at least alsa-devel about this, from a year
back or so.

This is exactly what my patch was addressing, I was fixing that bug by
handling underrun correctly. snd_pcm_prepare() is called when an
underrun occurs, so instead I make it only restart the stream if
underrun. Not handling underruns at all seems to allow it to stall on
xrun, while claiming to be running. This sometimes appears to happen in
other programs too though, like mplayer. Could it be because of the
weird latency value?


Ok, thanks for the clarifications. I've taken a closer look at your 
patch now and have the following comments:


When you're calling pulse_start instead of continuing in pulse_prepare - 
pulse_start will call uncork/trigger, won't that just cause another 
underrun? Would it perhaps be better to just return without doing anything?


mpg123 was used as a test case for the original bug, and mpg123 calls 
snd_pcm_drop on an underrun, so you will be regressing mpg123 by 
changing handle_underrun to 1. (Now of course we could fix mpg123, but I 
don't know if there are a lot of other programs out there doing similar 
things?)


My main point is that the underrun is often obsolete when the message 
reached the application, because more data has already been written, 
therefore reporting it to the app does more harm than good.
At least until the underrun callback (and PA protocol) supports sending 
the position of underrun, together with the underrun message. If we had 
that position, we could compare that with the current write pointer to 
determine whether the underrun is actually obsolete or not.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss


Re: [pulseaudio-discuss] underrun behavior with alsa-plugins

2011-04-20 Thread Maarten Lankhorst

Hey,

Op 20-04-11 15:18, David Henningsson schreef:

However if I
force underruns to occur,the state will stay running and it appears
there is still some data left in the buffer, so sound stalls entirely.


This is nothing I've heard of.

Sadly all too common here, if I feed data and it underruns, it may
appear to be running but is stalled entirely.


Could you provide a test case where this occurs? (Pulseaudio/ALSA
version, client applications/libraries, etc)


Well this could be a basic test version I suppose, although it's not 
fair since it's guaranteed to underrun and doesn't work with dmix since 
it specifies period sizes exactly.



Digging it up I can only
assume it's a bug in src/stream.c , but I haven't figured out why yet.
Tested with pulseaudio.c
2. Any comments on this patch for alsa-plugins?


IIRC, by setting handle_underrun to 1, you're reopening bugs of
broken/skipping audio for mpg123 and other programs, which get stuck in
an infinite loop of write one sample, force start the stream, write
more samples, asynchronusly get an underrun, drop the stream and drop
samples already written.

There should be threads on at least alsa-devel about this, from a year
back or so.

This is exactly what my patch was addressing, I was fixing that bug by
handling underrun correctly. snd_pcm_prepare() is called when an
underrun occurs, so instead I make it only restart the stream if
underrun. Not handling underruns at all seems to allow it to stall on
xrun, while claiming to be running. This sometimes appears to happen in
other programs too though, like mplayer. Could it be because of the
weird latency value?


Ok, thanks for the clarifications. I've taken a closer look at your
patch now and have the following comments:

When you're calling pulse_start instead of continuing in pulse_prepare -
pulse_start will call uncork/trigger, won't that just cause another
underrun? Would it perhaps be better to just return without doing anything?

mpg123 was used as a test case for the original bug, and mpg123 calls
snd_pcm_drop on an underrun, so you will be regressing mpg123 by
changing handle_underrun to 1. (Now of course we could fix mpg123, but I
don't know if there are a lot of other programs out there doing similar
things?)

My main point is that the underrun is often obsolete when the message
reached the application, because more data has already been written,
therefore reporting it to the app does more harm than good.
At least until the underrun callback (and PA protocol) supports sending
the position of underrun, together with the underrun message. If we had
that position, we could compare that with the current write pointer to
determine whether the underrun is actually obsolete or not.
Well, this is a dumb unfair test program that forcibly underruns, 
probably should work if you change default to hw:0, but I think dmix 
doesn't allow you to set random period sizes. If I try this with the 
pulseaudio default plugin, it will break.


With my patch:
~$ ./a.out
Written: 2047 - available: 2047
Successfully underrun
Written: 960 - available: 960
Successfully underrun
Written: 1890 - available: 1890
Successfully underrun

Without:

~$ ./a.out 2/dev/null
Written: 2047 - available: 2047
Written: 0 - available: 0
Written: 0 - available: 0

Note: not any form of error checking is done here, just cobbled this 
dumb program together for demonstration purposes. :)


Cheers,
Maarten
#include stdio.h
#include unistd.h
#include asoundlib.h

int main() {
unsigned t = 1, rate = 44100;
char silence[4096] = {};
snd_pcm_hw_params_t *hw_parms;
snd_pcm_sw_params_t *sw_parms;
snd_pcm_t *pcm;

snd_pcm_hw_params_alloca(hw_parms);
snd_pcm_sw_params_alloca(sw_parms);

if (snd_pcm_open(pcm, default, SND_PCM_STREAM_PLAYBACK, SND_PCM_NONBLOCK)  0) return 1;
snd_pcm_hw_params_any(pcm, hw_parms);
snd_pcm_hw_params_set_period_size(pcm, hw_parms, 512, 0);
snd_pcm_hw_params_set_periods(pcm, hw_parms, 4, 0);
snd_pcm_hw_params_set_format(pcm, hw_parms, SND_PCM_FORMAT_S16_LE);
snd_pcm_hw_params_set_rate_near(pcm, hw_parms, rate, NULL);
snd_pcm_hw_params_set_channels(pcm, hw_parms, 1);
snd_pcm_hw_params_set_access(pcm, hw_parms, SND_PCM_ACCESS_RW_INTERLEAVED);
snd_pcm_hw_params(pcm, hw_parms);
snd_pcm_prepare(pcm);
snd_pcm_start(pcm);
snd_pcm_writei(pcm, silence, sizeof(silence)/2);

while (1) {
snd_pcm_uframes_t avail = snd_pcm_avail_update(pcm);
int state = snd_pcm_state(pcm);
if (state == SND_PCM_STATE_XRUN) {
printf(Successfully underrun\n);
avail = sizeof(silence)/2;
snd_pcm_prepare(pcm);
snd_pcm_writei(pcm, silence, avail);
} else if (state == SND_PCM_STATE_RUNNING) {
long ret = snd_pcm_writei(pcm, silence, avail);
printf(Written: %li - available: %lu\n, ret, avail);
usleep(10);
}
usleep(1);
}
}
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de

[pulseaudio-discuss] underrun behavior with alsa-plugins

2011-04-19 Thread Maarten Lankhorst

Hi all,

For wine I was investigating a bug with pulseaudio, it seems 
alsa-plugins' pulse driver ignores underruns. This is probably because 
an underrun will force you to call snd_pcm_prepare, this will destroy 
the stream and set up a similar new one again. This causes more 
susceptibility to underruns, so that code was disabled. However if I 
force underruns to occur, the state will stay running and it appears 
there is still some data left in the buffer, so sound stalls entirely. 
The latency gets updated to something like 0x7bdX which 
looks suspiciously much like a pointer to me, which may be a bug. If I 
instead make pulse_prepare call pulse_start on underrun, it's handled 
properly and sound will continue to work.


So my questions are.
1. Is the weird latency value update a bug? Digging it up I can only 
assume it's a bug in src/stream.c , but I haven't figured out why yet. 
Tested with pulseaudio.c

2. Any comments on this patch for alsa-plugins?

Cheers,
Maarten


xrun.patch
Description: application/mbox
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss