[alsa-devel] [PATCH] Two patches for Alsa-plugins (pulse)

[Takashi Iwai]

At Fri, 9 Jul 2010 21:13:52 +0800,
Raymond Yau wrote:
> 
> 2010/7/9 Takashi Iwai <tiwai at 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