[alsa-devel] [PATCH] Two patches for Alsa-plugins (pulse)
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
* If pulseaudio gets an underrun, the normal way to end that underrun is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset. * In addition, underrun signals are delivered asynchronously from pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
// David
2010/6/24 David Henningsson launchpad.web@epost.diwic.se
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
// David
Are there any test program which can reproduce this condition ?
any program similar to alsa-time-test.c which PA developer use to test the alsa-driver but alsa-time-test.c aborted when I specify to use the pulse device
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
thanks,
Takashi
On 2010-06-24 08:42, Takashi Iwai wrote:
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
Thanks!
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it configurable. Just so I understand you right, exactly how do you expect the user / application to configure it?
// David
'Twas brillig, and David Henningsson at 25/06/10 11:41 did gyre and gimble:
Sure, I can do that - it sounds like a good idea to have it configurable. Just so I understand you right, exactly how do you expect the user / application to configure it?
At a guess I'd say some option in the ~/.asoundrc when using the pulse plugin... I've never looked at how this is done tho', so could be off base :D
Col
At Fri, 25 Jun 2010 12:41:46 +0200, David Henningsson wrote:
On 2010-06-24 08:42, Takashi Iwai wrote:
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
Thanks!
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it configurable. Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of alsa-plugins/pulse/pcm_pulse.c, add the code like below:
int handle_underrun = 0; ...
snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }
Takashi
At Mon, 05 Jul 2010 08:06:45 +0200, I wrote:
At Fri, 25 Jun 2010 12:41:46 +0200, David Henningsson wrote:
On 2010-06-24 08:42, Takashi Iwai wrote:
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
Thanks!
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it configurable. Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of alsa-plugins/pulse/pcm_pulse.c, add the code like below:
int handle_underrun = 0; ...
snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }
FYI, I modified the patch and applied to git tree right now.
thanks,
Takashi
2010/7/9 Takashi Iwai tiwai@suse.de
At Mon, 05 Jul 2010 08:06:45 +0200, I wrote:
At Fri, 25 Jun 2010 12:41:46 +0200, David Henningsson wrote:
On 2010-06-24 08:42, Takashi Iwai wrote:
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this
April)
and are working well enough to have people asking us to backport
them to
Karmic. Today I got an email, asking for the upstream status of one
of
these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken
logic:
This patch improves recovering from underruns, and prevents hangs
inside
snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
Thanks!
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that
underrun
is to feed it with more buffers. This is different from the ALSA way
of
dealing with underruns, which requires hardware buffer pointers to
be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way
to
pulseaudio when the underrun is reported, making the underrun
obsolete.
Unfortunately, there is currently no known way to determine whether
this
is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it
configurable.
Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of alsa-plugins/pulse/pcm_pulse.c, add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
thanks,
Takashi
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
2010/7/9 Takashi Iwai tiwai@suse.de
At Mon, 05 Jul 2010 08:06:45 +0200, I wrote:
At Fri, 25 Jun 2010 12:41:46 +0200, David Henningsson wrote:
On 2010-06-24 08:42, Takashi Iwai wrote:
At Wed, 23 Jun 2010 21:57:01 +0200, David Henningsson wrote:
These two patches are being used in Ubuntu Lucid (released this
April)
and are working well enough to have people asking us to backport
them to
Karmic. Today I got an email, asking for the upstream status of one
of
these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken
logic:
This patch improves recovering from underruns, and prevents hangs
inside
snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
This one looks OK. Applied now.
Thanks!
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that
underrun
is to feed it with more buffers. This is different from the ALSA way
of
dealing with underruns, which requires hardware buffer pointers to
be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way
to
pulseaudio when the underrun is reported, making the underrun
obsolete.
Unfortunately, there is currently no known way to determine whether
this
is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it
configurable.
Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of alsa-plugins/pulse/pcm_pulse.c, add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
thanks,
Takashi
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
Takashi
2010-07-09 18:23, Takashi Iwai skrev:
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
> The second one (Do not report underruns to the ALSA layer) is more a > change of behavior, which could be questioned. The arguments for > removing that code are these: > > * If pulseaudio gets an underrun, the normal way to end that
underrun
> is to feed it with more buffers. This is different from the ALSA way
of
> dealing with underruns, which requires hardware buffer pointers to
be reset.
> * In addition, underrun signals are delivered asynchronously from > pulseaudio. This means that there might be more buffers on the way
to
> pulseaudio when the underrun is reported, making the underrun
obsolete.
> Unfortunately, there is currently no known way to determine whether
this
> is the case or not.
I think this helps for normal use-cases, so it'd be good to apply. But, I prefer having a runtime option for such a behavior change (although the default value can be the new behavior).
Care to add it?
Sure, I can do that - it sounds like a good idea to have it
configurable.
Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of alsa-plugins/pulse/pcm_pulse.c, add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
Thanks! I've been on holiday this week but thought I would do it in the following week. Only nice to see it being done :-)
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
// David
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
2010-07-09 18:23, Takashi Iwai skrev:
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
>> The second one (Do not report underruns to the ALSA layer) is more
a
>> change of behavior, which could be questioned. The arguments for >> removing that code are these: >> >> * If pulseaudio gets an underrun, the normal way to end that
underrun
>> is to feed it with more buffers. This is different from the ALSA
way
of
>> dealing with underruns, which requires hardware buffer pointers to
be reset.
>> * In addition, underrun signals are delivered asynchronously from >> pulseaudio. This means that there might be more buffers on the way
to
>> pulseaudio when the underrun is reported, making the underrun
obsolete.
>> Unfortunately, there is currently no known way to determine whether
this
>> is the case or not. > > I think this helps for normal use-cases, so it'd be good to apply. > But, I prefer having a runtime option for such a behavior change > (although the default value can be the new behavior). > > Care to add it?
Sure, I can do that - it sounds like a good idea to have it
configurable.
Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of
alsa-plugins/pulse/pcm_pulse.c,
add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
Thanks! I've been on holiday this week but thought I would do it in the following week. Only nice to see it being done :-)
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
Before this patch , using pulse device also report underrun
When using hw device ,the sound is quite good , at least I cannot hear any distortion and the reported underrun only appear a few times on the terminal
underrun!!! (at least 0.210 ms long)
Did alsa-lib calculate 0.210 ms from hw_ptr , app_ptr and sample rate ?
aplay -v -Dhw:0,0 --buffer-size=128 t3.wav Playing WAVE 't3.wav' : Signed 16 bit Little Endian, Rate 44100 Hz, Stereo Hardware PCM card 0 'Aureal Vortex au8830' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 44100 exact rate : 44100 (44100/1) msbits : 16 buffer_size : 128 period_size : 32 period_time : 725 tstamp_mode : NONE period_step : 1 avail_min : 32 period_event : 0 start_threshold : 128 stop_threshold : 128 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 underrun!!! (at least 0.210 ms long) Status: state : XRUN trigger_time: 2794.391051578 tstamp : 2794.393149452 delay : 0 avail : 153 avail_max : 153 underrun!!! (at least 0.001 ms long) Status: state : XRUN trigger_time: 2801.590388423 tstamp : 2801.590394868 delay : 0 avail : 135 avail_max : 135 underrun!!! (at least 0.032 ms long) Status: state : XRUN trigger_time: 2801.718347873 tstamp : 2801.718661699 delay : 0 avail : 153 avail_max : 153 underrun!!! (at least 0.067 ms long) Status: state : XRUN trigger_time: 2805.978507564 tstamp : 2805.979173807 delay : 0 avail : 162 avail_max : 162 underrun!!! (at least 0.065 ms long) Status: state : XRUN trigger_time: 2807.118761300 tstamp : 2807.119411502 delay : 0 avail : 162 avail_max : 162 underrun!!! (at least 0.058 ms long) Status: state : XRUN trigger_time: 2808.685565670 tstamp : 2808.686139666 delay : 0 avail : 153 avail_max : 153 underrun!!! (at least 0.001 ms long) Status: state : XRUN trigger_time: 2808.699838404 tstamp : 2808.699844705 delay : 0 avail : 133 avail_max : 133
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
// David
You have to ask jackd user , they usually fine-tune(increase/decrease) the period-size of jackd manually based on the occurrence of underrun in order to achieve the lowest latency
This give a false impression to the user that PA can provide low latency without under-run and pulse device is even perform better than hw device.
Reporting underruns to ALSA seems to do more bad than good, for these
reasons:
* If pulseaudio gets an underrun, the normal way to end that underrun is to
feed it with more buffers. This is different from the ALSA way of dealing
with underruns, which requires hardware buffer pointers to be reset.
* In addition, underrun signals are delivered asynchronously from pulseaudio.
This means that there might be more buffers on the way to pulseaudio when
the underrun is reported, making the underrun obsolete. Unfortunately,
there is currently no known way to determine whether this is the case or
not.
Do you mean PA server get underrun from the sound driver when you mention that if pulseaudio get underrun, the normal way to end that underrun is to feed it with more buffers." ?
The normal way for ALSA application is to prevent underrun occur rather to let underrun occur be selecting a larger period size or wake up more frequently to feed data to the sound card
PA developer often complain the driver did not provide accurate playback position by the pcm_pointer callback
Did the pulse_pointer callback also provide accurate position of the pulse device ?
2010-07-10 00:58, Raymond Yau skrev:
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
2010-07-09 18:23, Takashi Iwai skrev:
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
>>> The second one (Do not report underruns to the ALSA layer) is more
a
>>> change of behavior, which could be questioned. The arguments for >>> removing that code are these: >>> >>> * If pulseaudio gets an underrun, the normal way to end that
underrun
>>> is to feed it with more buffers. This is different from the ALSA
way
of
>>> dealing with underruns, which requires hardware buffer pointers to
be reset.
>>> * In addition, underrun signals are delivered asynchronously from >>> pulseaudio. This means that there might be more buffers on the way
to
>>> pulseaudio when the underrun is reported, making the underrun
obsolete.
>>> Unfortunately, there is currently no known way to determine whether
this
>>> is the case or not. >> >> I think this helps for normal use-cases, so it'd be good to apply. >> But, I prefer having a runtime option for such a behavior change >> (although the default value can be the new behavior). >> >> Care to add it? > > Sure, I can do that - it sounds like a good idea to have it
configurable.
> Just so I understand you right, exactly how do you expect the user / > application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of
alsa-plugins/pulse/pcm_pulse.c,
add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
Thanks! I've been on holiday this week but thought I would do it in the following week. Only nice to see it being done :-)
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
Before this patch , using pulse device also report underrun
When using hw device ,the sound is quite good , at least I cannot hear any distortion and the reported underrun only appear a few times on the terminal
underrun!!! (at least 0.210 ms long)
Did alsa-lib calculate 0.210 ms from hw_ptr , app_ptr and sample rate ?
Good question, I'm sure you can read that in the source :-)
http://git.alsa-project.org/?p=alsa-utils.git;a=blob;f=aplay/aplay.c;h=e1d8e...
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
You have to ask jackd user , they usually fine-tune(increase/decrease) the period-size of jackd manually based on the occurrence of underrun in order to achieve the lowest latency
Correct me if I'm wrong, but I think jackd users are not likely to run jackd over the alsa-pulse bridge but rather run PA on top of jackd, so that is not an issue.
This give a false impression to the user that PA can provide low latency without under-run and pulse device is even perform better than hw device.
To complicate matters more, with PA in the stack you can have underruns both on the client app <=> PA side, and on the PA <=> alsa-driver side. So the false impression was there before, only it is slightly worse now.
Reporting underruns to ALSA seems to do more bad than good, for these
reasons:
- If pulseaudio gets an underrun, the normal way to end that underrun is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
Do you mean PA server get underrun from the sound driver when you mention that if pulseaudio get underrun, the normal way to end that underrun is to feed it with more buffers." ?
I'm talking about the client-app <=> PA underrun here. The PA <=> sound driver underrun is handled by PA internally, and, IIRC, is never reported to the client app (even before the patch).
The normal way for ALSA application is to prevent underrun occur rather to let underrun occur be selecting a larger period size or wake up more frequently to feed data to the sound card
Do you know an ALSA application which actually does this automatically, i e selects a larger period size when it detects an underrun?
PA developer often complain the driver did not provide accurate playback position by the pcm_pointer callback
Did the pulse_pointer callback also provide accurate position of the pulse device ?
Yes, but it provides the pointer to the fake ring buffer (the alsa-lib plugin system expects that all pcm plugins are ring buffers, which seems to me like a design flaw).
// David
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
PA developer often complain the driver did not provide accurate playback position by the pcm_pointer callback
Did the pulse_pointer callback also provide accurate position of the
pulse
device ?
Yes, but it provides the pointer to the fake ring buffer (the alsa-lib plugin system expects that all pcm plugins are ring buffers, which seems to me like a design flaw).
This mean that pulse_pointer jump suddenly when PA wake up from sleep and write audio to sound card until the ring buffer is empty, Those "0 bytes in queue " message is the result of PA server flush the ring buffer.
Did alsa-lib regard this condition as underrun, since the application pointer and hardware pointer of the pulse device seem at the same point ?
Is this the reason why alsa-time-test.c fail on the pulse device ?
2010-07-13 07:08, Raymond Yau skrev:
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
PA developer often complain the driver did not provide accurate playback position by the pcm_pointer callback
Did the pulse_pointer callback also provide accurate position of the
pulse
device ?
Yes, but it provides the pointer to the fake ring buffer (the alsa-lib plugin system expects that all pcm plugins are ring buffers, which seems to me like a design flaw).
This mean that pulse_pointer jump suddenly when PA wake up from sleep and write audio to sound card until the ring buffer is empty, Those "0 bytes in queue " message is the result of PA server flush the ring buffer.
Did alsa-lib regard this condition as underrun, since the application pointer and hardware pointer of the pulse device seem at the same point ?
No, I don't think so. An underrun message is a special message sent from the PA server and should not be directly related to the fake ring buffer, and it is that message now being ignored (configurable, thanks to Takashi).
My first patch (http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=1675414) fixes so that if pa_stream_writeable_size returns a value greater than the ring buffer's size, it only reports size-1 bytes available, which means that the alsa app won't see it as the application pointer and hw pointer being the same.
Is this the reason why alsa-time-test.c fail on the pulse device ?
No idea, haven't looked further into that application.
// David
2010/7/13 David Henningsson launchpad.web@epost.diwic.se
2010-07-13 07:08, Raymond Yau skrev:
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
PA developer often complain the driver did not provide accurate
playback
position by the pcm_pointer callback
Did the pulse_pointer callback also provide accurate position of the
pulse
device ?
Yes, but it provides the pointer to the fake ring buffer (the alsa-lib plugin system expects that all pcm plugins are ring buffers, which seems to me like a design flaw).
This mean that pulse_pointer jump suddenly when PA wake up from sleep and write audio to sound card until the ring buffer is empty, Those "0 bytes
in
queue " message is the result of PA server flush the ring buffer.
Did alsa-lib regard this condition as underrun, since the application pointer and hardware pointer of the pulse device seem at the same point
?
No, I don't think so. An underrun message is a special message sent from the PA server and should not be directly related to the fake ring buffer, and it is that message now being ignored (configurable, thanks to Takashi).
In fedora 13 , the two underrun messages appear almost immediately after starting mpg123 but mpg123 does not report any underrun on Fedora 10
So it look like regression in PA server side if you told me that "An underrun message is a special message sent from the PA server " instead of underrun detected by alsa-lib by checking the pulse_pointer and the application ptr of the pulse device
Under what condition did PA server send the underrun message ?
My first patch (http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=1675414) fixes so that if pa_stream_writeable_size returns a value greater than the ring buffer's size, it only reports size-1 bytes available, which means that the alsa app won't see it as the application pointer and hw pointer being the same.
Is this the reason why alsa-time-test.c fail on the pulse device ?
No idea, haven't looked further into that application.
// David
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
// David
Some application (e.g. xmms and audacious) allow user to configure the period/buffer time
I don't know why the alsa-lib/test/latency did not run when using pulse device
./latency -P pulse -C pulse -m 8192 -M 8192 -t 1 -p /home/raymond/alsa-lib/test/.libs/lt-latency: invalid option -- 't' !!!Scheduler set to Round Robin with priority 99 FAILED!!! Playback device is pulse Capture device is pulse Parameters are 22050Hz, S16_LE, 2 channels, non-blocking mode Poll mode: yes Loop limit is 661500 frames, minimum latency = 8192, maximum latency = 8192
alsa-lib/test/latency -P hw:1,0 -C hw:1,0 -m 8192 -M 8192 -t 1 -p /home/raymond/alsa-lib/test/.libs/lt-latency: invalid option -- 't' !!!Scheduler set to Round Robin with priority 99 FAILED!!! Playback device is hw:1,0 Capture device is hw:1,0 Parameters are 22050Hz, S16_LE, 2 channels, non-blocking mode Poll mode: yes Loop limit is 661500 frames, minimum latency = 8192, maximum latency = 8192 Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 22050 exact rate : 22050 (22050/1) msbits : 16 buffer_size : 8192 period_size : 4096 period_time : 185759 tstamp_mode : NONE period_step : 1 avail_min : 4096 period_event : 0 start_threshold : 2147483647 stop_threshold : 8192 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 22050 exact rate : 22050 (22050/1) msbits : 16 buffer_size : 8192 period_size : 4096 period_time : 185759 tstamp_mode : NONE period_step : 1 avail_min : 4096 period_event : 0 start_threshold : 2147483647 stop_threshold : 8192 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 Trying latency 8192 frames, 371519.274us, 371.519274ms (2.6917Hz) Success Playback: *** frames = 671744 *** state : RUNNING trigger_time: 14895.45132386 tstamp : 14925.138525741 delay : 8144 avail : 48 avail_max : 4152 Capture: *** frames = 663552 *** state : RUNNING trigger_time: 14895.45132386 tstamp : 14925.138585923 delay : 1 avail : 1 avail_max : 4097 Maximum read: 4096 frames Maximum read latency: 185759.637us, 185.759637ms (5.3833Hz) Hardware sync Playback time = 14895.45132, Record time = 14895.45132, diff = 0
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
2010-07-09 18:23, Takashi Iwai skrev:
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
>> The second one (Do not report underruns to the ALSA layer) is more
a
>> change of behavior, which could be questioned. The arguments for >> removing that code are these: >> >> * If pulseaudio gets an underrun, the normal way to end that
underrun
>> is to feed it with more buffers. This is different from the ALSA
way
of
>> dealing with underruns, which requires hardware buffer pointers to
be reset.
>> * In addition, underrun signals are delivered asynchronously from >> pulseaudio. This means that there might be more buffers on the way
to
>> pulseaudio when the underrun is reported, making the underrun
obsolete.
>> Unfortunately, there is currently no known way to determine whether
this
>> is the case or not. > > I think this helps for normal use-cases, so it'd be good to apply. > But, I prefer having a runtime option for such a behavior change > (although the default value can be the new behavior). > > Care to add it?
Sure, I can do that - it sounds like a good idea to have it
configurable.
Just so I understand you right, exactly how do you expect the user / application to configure it?
I suppose it can be simply added as an alsa-lib plugin config, i.e. in SND_PCM_PLUGIN_DEFINE_FUNC(pulse) of
alsa-plugins/pulse/pcm_pulse.c,
add the code like below:
int handle_underrun = 0; ... snd_config_for_each(i, next, conf) { ... if (strcmp(id, "handle_underrun") == 0) { handle_underrun = snd_config_get_bool(n); if (handle_underrun < 0) { SNDERR("Invalid value for %s", id); return handle_underrun; } continue; } }FYI, I modified the patch and applied to git tree right now.
Thanks! I've been on holiday this week but thought I would do it in the following week. Only nice to see it being done :-)
This behaviour is a little bit strange , using the same buffer size 128
aplay -Dhw:0,0 --buffer-size=128 test.wav
report underrun
aplay -D pulse --buffer-size=128 test.wav
did not report underrun but the sound is distorted
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
This patch by Lennart Poettering should answer your question
http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=660e0f16145167a96...
2010-07-29 09:11, Raymond Yau skrev:
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
This patch by Lennart Poettering should answer your question
http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=660e0f16145167a96...
Okay, so we've broken XMMS, at least the 2007 version of it?
The better solution to this problem would be to add a write_pointer to the xrun callback (and PA's native protocol), so we can detect whether the underrun is obsolete or not and act accordingly.
2010/7/29 David Henningsson david.henningsson@canonical.com
2010-07-29 09:11, Raymond Yau skrev:
2010/7/10 David Henningsson launchpad.web@epost.diwic.se
The biggest drawback by this change is that whether the sound goes well or not isn't reported any more to the application layer. It's an unfortunate design, but it's life with PA.
The question is - is there an application that detects the underrun condition and actually acts on that condition? If so, this could be a regression for that app. For other apps it's an improvement, for the reasons originally stated.
This patch by Lennart Poettering should answer your question
http://git.alsa-project.org/?p=alsa-plugins.git;a=commit;h=660e0f16145167a96...
Okay, so we've broken XMMS, at least the 2007 version of it?
The better solution to this problem would be to add a write_pointer to the xrun callback (and PA's native protocol), so we can detect whether the underrun is obsolete or not and act accordingly.
when using xmms libao plugin in mandriva 2010 , xmms freeze at the end of the playback
2010/7/10 Takashi Iwai tiwai@suse.de
At Fri, 9 Jul 2010 21:13:52 +0800, Raymond Yau wrote:
Other subsystem, other behavior. The detection of underrun is even unreliable on ALSA native, so user can't expect it'll get underrun.
_From my testing , only hda_intel can run the alsa-lib/test/latency with -m512 -M512 , au8830 always fail because of xrun and pulse cannot start the test
Did the latency test require the driver to support SNDRV_PCM_INFO_SYNC_START ?
latency.c seem using a very large value for the start_threshold
err = snd_pcm_sw_params_set_start_threshold(handle, swparams, 0x7fffffff);
./latency -r 44100 -Phw:1 -Chw:1 -m 512 -M 512 !!!Scheduler set to Round Robin with priority 99 FAILED!!! Playback device is hw:1 Capture device is hw:1 Parameters are 44100Hz, S16_LE, 2 channels, non-blocking mode Poll mode: no Loop limit is 1323000 frames, minimum latency = 512, maximum latency = 512 buffer_size :0 252 Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 44100 exact rate : 44100 (44100/1) msbits : 16 buffer_size : 512 period_size : 256 period_time : 5804 tstamp_mode : NONE period_step : 1 avail_min : 256 period_event : 0 start_threshold : 2147483647 stop_threshold : 512 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 44100 exact rate : 44100 (44100/1) msbits : 16 buffer_size : 512 period_size : 256 period_time : 5804 tstamp_mode : NONE period_step : 1 avail_min : 256 period_event : 0 start_threshold : 2147483647 stop_threshold : 512 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 Trying latency 512 frames, 11609.977us, 11.609977ms (86.1328Hz) Success Playback: *** frames = 1323512 *** state : RUNNING trigger_time: 18306.838476235 tstamp : 18336.838630238 delay : 464 avail : 48 avail_max : 80 Capture: *** frames = 1323000 *** state : RUNNING trigger_time: 18306.838476235 tstamp : 18336.838668663 delay : 0 avail : 0 avail_max : 32 Maximum read: 32 frames Maximum read latency: 725.624us, 0.725624ms (1378.1250Hz) Hardware sync Playback time = 18306.838476, Record time = 18306.838476, diff = 0
2010/6/24 David Henningsson launchpad.web@epost.diwic.se
These two patches are being used in Ubuntu Lucid (released this April) and are working well enough to have people asking us to backport them to Karmic. Today I got an email, asking for the upstream status of one of these patches, since he wanted them in Fedora. If posting them here isn't the correct way to upstream them, please tell me so.
The first one (Fix invalid buffer pointer return value) fixes broken logic:
This patch improves recovering from underruns, and prevents hangs inside snd_pcm_write* and snd_pcm_read* due to snd_pcm_avail* returning too low values. It especially helps low latency situations.
The second one (Do not report underruns to the ALSA layer) is more a change of behavior, which could be questioned. The arguments for removing that code are these:
- If pulseaudio gets an underrun, the normal way to end that underrun
is to feed it with more buffers. This is different from the ALSA way of dealing with underruns, which requires hardware buffer pointers to be reset.
- In addition, underrun signals are delivered asynchronously from
pulseaudio. This means that there might be more buffers on the way to pulseaudio when the underrun is reported, making the underrun obsolete. Unfortunately, there is currently no known way to determine whether this is the case or not.
// David
For your test case (mpg123 produces skippy output in 9.10
https://bugs.launchpad.net/ubuntu/+source/alsa-plugins/+bug/464008
play silence when there is underrun
http://www.mpg123.de/cgi-bin/viewvc.cgi/trunk/src/output/alsa.c?r1=377&r...
static void flush_alsa(audio_output_t *ao) { snd_pcm_t *pcm=(snd_pcm_t*)ao->userptr;
/* is this the optimal solution? - we should figure out what we really whant from this function */
debug("alsa drop"); snd_pcm_drop(pcm); debug("alsa prepare"); snd_pcm_prepare(pcm); debug("alsa flush done"); }
it implement pause by create an underrun
[libmpg123.c:1083] debug: tell: 2207/1 first 0 buffer 0; frame_outs=2542464 [libmpg123.c:623] debug: read of frame 2207 returned 1 (to_decode=1) at sample 2541935 [libmpg123.c:904] debug: got next frame, 1 [libmpg123.c:885] debug: decoding [term.c:158] debug: control for frame: 2207 [alsa.c:246] debug: alsa drop [alsa.c:248] debug: alsa prepare [alsa.c:250] debug: alsa flush done Stopped [alsa.c:246] debug: alsa drop [alsa.c:248] debug: alsa prepare [alsa.c:250] debug: alsa flush done [mpg123.c:632] debug: play_frame [libmpg123.c:620] debug: read frame [parse.c:456] debug: trying to get frame 2208 at 923915 [parse.c:772] debug: Frame 2208 fffb9260 414, next filepos=924333 [index.c:85] debug: wanting to add to fill 552, step 4, size 1000
participants (5)
-
Colin Guthrie -
David Henningsson -
David Henningsson -
Raymond Yau -
Takashi Iwai