[alsa-devel] PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 14:52:53 +0200,
Jaroslav Kysela wrote:
> 
> Date 15.5.2013 14:47, David Henningsson wrote:
> > On 05/15/2013 02:42 PM, Takashi Iwai wrote:
> >> At Wed, 15 May 2013 13:22:03 +0200,
> >> Jaroslav Kysela wrote:
> >>>
> >>> Date 15.5.2013 13:03, David Henningsson wrote:
> >>>> On 05/15/2013 12:53 PM, Jaroslav Kysela wrote:
> >>>>> Date 15.5.2013 12:48, Takashi Iwai wrote:
> >>>>>> At Wed, 15 May 2013 12:26:51 +0200,
> >>>>>> Jaroslav Kysela wrote:
> >>>>>>>
> >>>>>>> Date 15.5.2013 11:55, Arun Raghavan wrote:
> >>>>>>>> Hello,
> >>>>>>>> A number of users have intermittently(?) been hitting a crash in
> >>>>>>>> alsa-lib 1.0.27 [1, 2] related to the softvol plugin. I'm not able to
> >>>>>>>> reproduce this reliably, so can't find an easy way to debug/fix.
> >>>>>>>
> >>>>>>> The problem is that the offsets are not in sync in this case [1]:
> >>>>>>>
> >>>>>>> src_offset = 38560
> >>>>>>> dst_offset = 38568
> >>>>>>> frames = 16374
> >>>>>>>
> >>>>>>> Could you reproduce this bug in any way? At least snd_pcm_dump() before
> >>>>>>> the failing snd_pcm_mmap_commit() call might help to determine what was
> >>>>>>> the status before the assert() was entered.
> >>>>>>
> >>>>>> Yep.  And this path is actually with volume 0dB, that is, a simply
> >>>>>> passthrough in softvol.  Thus the bug may hit essentially any
> >>>>>> plugins, not specifically softvol.
> >>>>>>
> >>>>>>
> >>>>>>>> However, this raises a tangential question - why do we need softvol to
> >>>>>>>> be plugged for 'front' at all? David explained to me that this is to
> >>>>>>>> guarantee the existence of a PCM control. Perhaps I don't fully
> >>>>>>>> understand this, because I'm unconvinced by the reason. Could someone
> >>>>>>>> explain/refute?
> >>>>>>>>
> >>>>>>>> This is especially bad for us, from PulseAudio's perspective, because we
> >>>>>>>> aren't getting a zero-copy path.
> >>>>>>>
> >>>>>>> If the softvol is set to 0dB (no attenuation or gain), then the ring
> >>>>>>> buffer pointers are moved without any sample processing, so the
> >>>>>>> zero-copy functionality is kept.
> >>>>>>
> >>>>>> Yeah, a sort of.  The mmap is cleared in the slave PCM, so actually
> >>>>>> there will be copy operations in underlying layers even though softvol
> >>>>>> itself does zero copy.
> >>>>>>
> >>>>>> Actually it makes no sense to keep softvol for PA, but the problem is
> >>>>>> always the regression.  There are certainly users without PA, which
> >>>>>> might still rely on the softvol for such hardware without the amp
> >>>>>> control.
> >>>>>>
> >>>>>> Maybe We can add some flag to indicate whether to handle softvol or
> >>>>>> not, e.g. defaults.pcm.skip_softvol, and let PA set this in its config
> >>>>>> space.  Setting a config item itself would break anything, so it'll
> >>>>>> still work with old alsa-lib (but with softvol).
> >>>>>
> >>>>> We have already SND_PCM_NO_SOFTVOL open mode for this purpose, so I
> >>>>> wonder, why PA does not use it..
> >>>>
> >>>> The problem is knowing whether PCM is a softvol or not. In some cases,
> >>>> we need to set PCM to control hardware volume.
> >>>>
> >>>> Maybe, if we could figure this out somehow, we could ignore the PCM
> >>>> mixer control (or possibly set it to zero) in case PCM is a softvol,
> >>>> and actually use it if PCM is not a softvol.
> >>>>
> >>>> It does not look like this is currently possible from the simple mixer
> >>>> interface, but I might be missing something?
> >>>
> >>> It is not possible. Perhaps, we may create a new dummy mixer control (in
> >>> an inactive state) which will identify the presence of the softvol
> >>> plugin, like:
> >>>
> >>> "Softvol PCM Playback Volume" - full name for the raw control API
> >>> "Softvol PCM" - simple mixer name
> >>
> >> Well, if changing in such a way, I'd rather drop softvol from
> >> HDA-Intel.conf.
> >>
> >> If we could give some flag in mixer API, we could add a code to filter
> >> out the user controls from the mixer's hctl.  But snd_mixer_attach()
> >> takes only the string, and the string modifier may lead to the
> >> incompatibility when used with an older version.  Hmm.
> > 
> > That seems solvable to me, something like this:
> > 
> > diff --git a/src/mixer/mixer.c b/src/mixer/mixer.c
> > index 56e023d..4afa979 100644
> > --- a/src/mixer/mixer.c
> > +++ b/src/mixer/mixer.c
> > @@ -65,13 +65,14 @@ static int snd_mixer_compare_default(const 
> > snd_mixer_elem_t *c1,
> >    * \param mode Open mode
> >    * \return 0 on success otherwise a negative error code
> >    */
> > -int snd_mixer_open(snd_mixer_t **mixerp, int mode ATTRIBUTE_UNUSED)
> > +int snd_mixer_open(snd_mixer_t **mixerp, int mode)
> 
> Yes, it could be implemented in this way. A special TLV entry may be
> introduced to detect, if the control is created by softvol.

The additional TLV won't work if a control is restored by alsactl, for
example, unfortunately.

> I wouldn't ignore all user created controls, because they can be used to
> reroute the controls to the real hardware (the alsaloop daemon does it
> in this way and PA can run on top).

Yeah, it's a difficult point.  Even a PCM control created by softvol
might be used by other plugins.  We can't exclude such a possibility.

In other words, if user wants to run PA in special environment with
virtual devices, it needs a special setup that allows indirect
accesses, basically without any limitation.


Takashi