Re: PulseAudio and softvol

[Jaroslav Kysela <perex@perex.cz>]

Date 15.5.2013 15:26, Takashi Iwai wrote:
> At Wed, 15 May 2013 15:12:17 +0200,
> Jaroslav Kysela wrote:
>>
>> Date 15.5.2013 15:05, Takashi Iwai wrote:
>>> 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.
>>
>> This looks like a bug, doesn't?
>> Anyway, I see some TLV restore code in
>> alsactl, but the support for all control types should be added not only
>> for SND_CTL_ELEM_TYPE_INTEGER.
> 
> Well, alsactl would just restore what's saved.  So, if the saved data
> already contains the softvol ctl element with the old TLV, it's simply
> restored as is.

It's enough.

> You may think of adding the code to softvol plugin to automatically
> rewrite TLV of the existing ctl element if it contains no new TLV
> type.  But, PA shall skip softvol.  Thus, it won't be touched.  And
> yet, PA would like to skip the control elements that have been created
> beforehand.

The alsa-lib code can be modified to create or modify the user space
control also in the SND_PCM_NO_SOFTVOL case, so the mixer API will be
informed that the PCM controls belongs to softvol.

I don't see any other problems.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project; Red Hat, Inc.