PulseAudio and softvol

PulseAudio and softvol

[Arun Raghavan]

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.

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.

Cheers,
Arun

[1] https://bugzilla.redhat.com/show_bug.cgi?id=953352
[2] https://bugs.freedesktop.org/show_bug.cgi?id=64299

Re: PulseAudio and softvol

[Jaroslav Kysela]

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.

> 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.

					Jaroslav

> 
> Cheers,
> Arun
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=953352
> [2] https://bugs.freedesktop.org/show_bug.cgi?id=64299

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

Re: PulseAudio and softvol

[Takashi Iwai]

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).


thanks,

Takashi

Re: PulseAudio and softvol

[Jaroslav Kysela]

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..

					Jaroslav

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

Re: PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 12:53:30 +0200,
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..

Oh, yeah, I completely forgot it!


Takashi

Re: PulseAudio and softvol

[David Henningsson]

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?

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: PulseAudio and softvol

[Jaroslav Kysela]

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

					Jaroslav

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

Re: PulseAudio and softvol

[David Henningsson]

On 05/15/2013 01:22 PM, 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

Or perhaps add a SND_CTL_NO_SOFTVOL flag that can be used in the call to 
snd_mixer_open / snd_ctl_open? That would make it somewhat consistent 
with the approach recommended for snd_pcm_open.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: PulseAudio and softvol

[Takashi Iwai]

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.


Takashi

Re: PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 13:33:01 +0200,
David Henningsson wrote:
> 
> On 05/15/2013 01:22 PM, 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
> 
> Or perhaps add a SND_CTL_NO_SOFTVOL flag that can be used in the call to 
> snd_mixer_open / snd_ctl_open? That would make it somewhat consistent 
> with the approach recommended for snd_pcm_open.

It'd be rather a flag to exclude the user controls, not specific to
softvol.  But, the problem is that snd_mixer_attach() takes no extra
flag argument.  So, we may need to add a new function, either defining
only the attach mode, or an equivalent function with
snd_mixer_attach() but with an extra flag argument.


Takashi

Re: PulseAudio and softvol

[David Henningsson]

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)
  {
         snd_mixer_t *mixer;
         assert(mixerp);
         mixer = calloc(1, sizeof(*mixer));
         if (mixer == NULL)
                 return -ENOMEM;
+       mixer->attach_mode = mode;
         INIT_LIST_HEAD(&mixer->slaves);
         INIT_LIST_HEAD(&mixer->classes);
         INIT_LIST_HEAD(&mixer->elems);
@@ -200,7 +201,7 @@ int snd_mixer_attach(snd_mixer_t *mixer, const char 
*name)
         snd_hctl_t *hctl;
         int err;

-       err = snd_hctl_open(&hctl, name, 0);
+       err = snd_hctl_open(&hctl, name, mixer->attach_mode);
         if (err < 0)
                 return err;
         err = snd_mixer_attach_hctl(mixer, hctl);
diff --git a/src/mixer/mixer_local.h b/src/mixer/mixer_local.h
index 27b4a3b..2d1866e 100644
--- a/src/mixer/mixer_local.h
+++ b/src/mixer/mixer_local.h
@@ -71,6 +71,7 @@ struct _snd_mixer {
         unsigned int count;
         unsigned int alloc;
         unsigned int events;
+       int attach_mode;
         snd_mixer_callback_t callback;
         void *callback_private;
         snd_mixer_compare_t compare;


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 14:47:15 +0200,
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)

Let's hope that no one sets the mode value ever... :)
But yes, other than that, it looks feasible indeed.


Takashi

>   {
>          snd_mixer_t *mixer;
>          assert(mixerp);
>          mixer = calloc(1, sizeof(*mixer));
>          if (mixer == NULL)
>                  return -ENOMEM;
> +       mixer->attach_mode = mode;
>          INIT_LIST_HEAD(&mixer->slaves);
>          INIT_LIST_HEAD(&mixer->classes);
>          INIT_LIST_HEAD(&mixer->elems);
> @@ -200,7 +201,7 @@ int snd_mixer_attach(snd_mixer_t *mixer, const char 
> *name)
>          snd_hctl_t *hctl;
>          int err;
> 
> -       err = snd_hctl_open(&hctl, name, 0);
> +       err = snd_hctl_open(&hctl, name, mixer->attach_mode);
>          if (err < 0)
>                  return err;
>          err = snd_mixer_attach_hctl(mixer, hctl);
> diff --git a/src/mixer/mixer_local.h b/src/mixer/mixer_local.h
> index 27b4a3b..2d1866e 100644
> --- a/src/mixer/mixer_local.h
> +++ b/src/mixer/mixer_local.h
> @@ -71,6 +71,7 @@ struct _snd_mixer {
>          unsigned int count;
>          unsigned int alloc;
>          unsigned int events;
> +       int attach_mode;
>          snd_mixer_callback_t callback;
>          void *callback_private;
>          snd_mixer_compare_t compare;
> 
> 
> -- 
> David Henningsson, Canonical Ltd.
> https://launchpad.net/~diwic
> 

Re: PulseAudio and softvol

[Jaroslav Kysela]

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.

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).

					Jaroslav

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

Re: PulseAudio and softvol

[Arun Raghavan]

On Wed, 2013-05-15 at 12:26 +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.

Unfortunately, after the time I got the backtrace, I haven't been able
to reproduce the problem.

-- Arun

Re: 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

Re: PulseAudio and softvol

[Jaroslav Kysela]

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.

					Jaroslav

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

Re: PulseAudio and softvol

[Takashi Iwai]

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.

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.

Certainly this is a corner case, but the requirement is incompatible
with old data.  If it's only about the change of the library code, it
would work by a simple update.  But if an additional metadata has to
be embedded, it's a different question...


Takashi

Re: PulseAudio and softvol

[Jaroslav Kysela]

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.

Re: PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 16:55:05 +0200,
Jaroslav Kysela wrote:
> 
> 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.

Enough for...?  It restores the value without the new TLV, thus it
doesn't show it's a softvol element.

> > 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.

But how would you know exactly?  Parsing the PCM definition at each
time if a user ctl element is found and check whether it *might*
belong to softvol plugin defined in some of card's default config?
What if a user takes own definition temporarily?

There can be endless corner cases.

BTW, does the alsaloop device just work as is, i.e. without specifying
anything in PA's configuration?
I'm asking it because what we're dealing with is the case where PA
probes as default via "front", "spdif" or such pre-definitions bound
with a real sound card instance.  The special filter could be used
only for these cases.  For the devices specified by user, it doesn't
need such filters.


Takashi

Re: PulseAudio and softvol

[Jaroslav Kysela]

Date 15.5.2013 17:06, Takashi Iwai wrote:
> At Wed, 15 May 2013 16:55:05 +0200,
> Jaroslav Kysela wrote:
>>
>> 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.
> 
> Enough for...?  It restores the value without the new TLV, thus it
> doesn't show it's a softvol element.

Yes, for the first call, but then PA will open the pcm device and this
call will add the softvol information to TLV which inactivates the
softvol control in the mixer.

>>> 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.
> 
> But how would you know exactly?  Parsing the PCM definition at each
> time if a user ctl element is found and check whether it *might*
> belong to softvol plugin defined in some of card's default config?

No. The information in TLV would be enough to determine the softvol
functionality.

> What if a user takes own definition temporarily?

Then user is responsible to remove this control later.

> There can be endless corner cases.

I'm not sure what you talk about. Yes, everything can be misused, but
we're talking about the standard usage. You can do weird things with the
softvol user controls anyway (remove them during runtime, because
they're not locked).

> BTW, does the alsaloop device just work as is, i.e. without specifying
> anything in PA's configuration?

The alsaloop can run on top of the snd-aloop, so it behaves like a
standard hardware, only the mixer controls can be rerouted using the
user controls to the real card.

					Jaroslav

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

Re: PulseAudio and softvol

[Takashi Iwai]

At Wed, 15 May 2013 17:25:08 +0200,
Jaroslav Kysela wrote:
> 
> Date 15.5.2013 17:06, Takashi Iwai wrote:
> > At Wed, 15 May 2013 16:55:05 +0200,
> > Jaroslav Kysela wrote:
> >>
> >> 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.
> > 
> > Enough for...?  It restores the value without the new TLV, thus it
> > doesn't show it's a softvol element.
> 
> Yes, for the first call, but then PA will open the pcm device and this
> call will add the softvol information to TLV which inactivates the
> softvol control in the mixer.

Yes, but the invocation of PCM softvol isn't guaranteed to be first
before the reference to the already existing user ctl element.
snd_mixer_open() can be called before that.

> >>> 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.
> > 
> > But how would you know exactly?  Parsing the PCM definition at each
> > time if a user ctl element is found and check whether it *might*
> > belong to softvol plugin defined in some of card's default config?
> 
> No. The information in TLV would be enough to determine the softvol
> functionality.
> 
> > What if a user takes own definition temporarily?
> 
> Then user is responsible to remove this control later.

That's messy.  We provide no such tool.

> > There can be endless corner cases.
> 
> I'm not sure what you talk about. Yes, everything can be misused, but
> we're talking about the standard usage. You can do weird things with the
> softvol user controls anyway (remove them during runtime, because
> they're not locked).

I know I'm picky, but adding a new metadata *onto* the existing data
structure has to be always done carefully.  Otherwise it'll hit back
us later.

For example, another corner case I can think of easily is that user
downgrades alsa-lib.  Then softvol overrides the TLV again without the
new tag (the current softvol code assumes the single TLV and overrides
if it doesn't fit), and it can be saved so...

> > BTW, does the alsaloop device just work as is, i.e. without specifying
> > anything in PA's configuration?
> 
> The alsaloop can run on top of the snd-aloop, so it behaves like a
> standard hardware, only the mixer controls can be rerouted using the
> user controls to the real card.

OK.  But, looking at the current implementation, you are allowed to
map freely the source ctl element.  It can lead to the same problem in
PA.

The very reason we'd like to filter out the mixer control created by
softvol is that this mixer element confuses PA as if it actually
changes the volume (e.g. "PCM") although PA ignores the softvol.  If
user creates PCM volume in alsaloop in a different fashion as PA
expected, the similar problem may happen.  How can we detect this
logically...?  In other words, how can PA adjust the mixer elements
for alsaloop properly?


Takashi

Re: [pulseaudio-discuss] PulseAudio and softvol

[Raymond Yau]

>
> 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.
>
> 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?

Because  alc660 codec did not has any hardware volume control on the output

http://www.alsa-project.org/db/?f=a327b0925a1697d60ea427fe3f16821d7ef7030b

Re: PulseAudio and softvol

[David Henningsson]

On 05/15/2013 06:28 PM, Takashi Iwai wrote:
> At Wed, 15 May 2013 17:25:08 +0200,
> Jaroslav Kysela wrote:
>>
>> Date 15.5.2013 17:06, Takashi Iwai wrote:
>>> Enough for...?  It restores the value without the new TLV, thus it
>>> doesn't show it's a softvol element.
>>
>> Yes, for the first call, but then PA will open the pcm device and this
>> call will add the softvol information to TLV which inactivates the
>> softvol control in the mixer.
>
> Yes, but the invocation of PCM softvol isn't guaranteed to be first
> before the reference to the already existing user ctl element.
> snd_mixer_open() can be called before that.

So this is a transient problem, right? As soon as the first PCM is 
opened, the TLV would be corrected, and then stay corrected for all 
times to come.

And looking at the current PulseAudio code, it does open the pcm device 
before it opens the mixer/ctl device.
So, if this isn't possible to solve in a better way, maybe we need to be 
pragmatic about it - PulseAudio is the only application we know that 
would care, and it opens the pcm device first. So in practice, it looks 
like the TLV approach would work.


>>>>> 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.
>>>
>>> But how would you know exactly?  Parsing the PCM definition at each
>>> time if a user ctl element is found and check whether it *might*
>>> belong to softvol plugin defined in some of card's default config?
>>
>> No. The information in TLV would be enough to determine the softvol
>> functionality.
>>
>>> What if a user takes own definition temporarily?
>>
>> Then user is responsible to remove this control later.
>
> That's messy.  We provide no such tool.
>
>>> There can be endless corner cases.
>>
>> I'm not sure what you talk about. Yes, everything can be misused, but
>> we're talking about the standard usage. You can do weird things with the
>> softvol user controls anyway (remove them during runtime, because
>> they're not locked).
>
> I know I'm picky, but adding a new metadata *onto* the existing data
> structure has to be always done carefully.  Otherwise it'll hit back
> us later.

I think it's good that you are picky. Missing a use case can cause 
problems indeed. But is there another solution to this problem which is 
better?

> For example, another corner case I can think of easily is that user
> downgrades alsa-lib.  Then softvol overrides the TLV again without the
> new tag (the current softvol code assumes the single TLV and overrides
> if it doesn't fit), and it can be saved so...
>
>>> BTW, does the alsaloop device just work as is, i.e. without specifying
>>> anything in PA's configuration?
>>
>> The alsaloop can run on top of the snd-aloop, so it behaves like a
>> standard hardware, only the mixer controls can be rerouted using the
>> user controls to the real card.
>
> OK.  But, looking at the current implementation, you are allowed to
> map freely the source ctl element.  It can lead to the same problem in
> PA.
>
> The very reason we'd like to filter out the mixer control created by
> softvol is that this mixer element confuses PA as if it actually
> changes the volume (e.g. "PCM") although PA ignores the softvol.  If
> user creates PCM volume in alsaloop in a different fashion as PA
> expected, the similar problem may happen.  How can we detect this
> logically...?  In other words, how can PA adjust the mixer elements
> for alsaloop properly?

So if alsaloop is run, only once, that could cause a control to be added 
for all future, due to alsactl saving and restoring it?

If so, that looks like a problem with alsaloop. If it adds controls, it 
should also remove them.

If no, I don't think we need to worry. Alsaloop is probably mostly used 
on non-PA systems (as PA has module-loopback which does the same thing).


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: PulseAudio and softvol

[Jaroslav Kysela]

Date 16.5.2013 08:31, David Henningsson wrote:
> On 05/15/2013 06:28 PM, Takashi Iwai wrote:
>> At Wed, 15 May 2013 17:25:08 +0200,
>> Jaroslav Kysela wrote:
>>>
>>> Date 15.5.2013 17:06, Takashi Iwai wrote:
>>>> Enough for...?  It restores the value without the new TLV, thus it
>>>> doesn't show it's a softvol element.
>>>
>>> Yes, for the first call, but then PA will open the pcm device and this
>>> call will add the softvol information to TLV which inactivates the
>>> softvol control in the mixer.
>>
>> Yes, but the invocation of PCM softvol isn't guaranteed to be first
>> before the reference to the already existing user ctl element.
>> snd_mixer_open() can be called before that.
> 
> So this is a transient problem, right? As soon as the first PCM is 
> opened, the TLV would be corrected, and then stay corrected for all 
> times to come.
> 
> And looking at the current PulseAudio code, it does open the pcm device 
> before it opens the mixer/ctl device.
> So, if this isn't possible to solve in a better way, maybe we need to be 
> pragmatic about it - PulseAudio is the only application we know that 
> would care, and it opens the pcm device first. So in practice, it looks 
> like the TLV approach would work.

It would work even if the mixer is opened before PCM, because the mixer
will get notified about the TLV change and can make the PCM element
inactive after. PA should only handle this situation correctly.

>>>>>> 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.
>>>>
>>>> But how would you know exactly?  Parsing the PCM definition at each
>>>> time if a user ctl element is found and check whether it *might*
>>>> belong to softvol plugin defined in some of card's default config?
>>>
>>> No. The information in TLV would be enough to determine the softvol
>>> functionality.
>>>
>>>> What if a user takes own definition temporarily?
>>>
>>> Then user is responsible to remove this control later.
>>
>> That's messy.  We provide no such tool.

Perhaps amixer should be extended.

>>>> There can be endless corner cases.
>>>
>>> I'm not sure what you talk about. Yes, everything can be misused, but
>>> we're talking about the standard usage. You can do weird things with the
>>> softvol user controls anyway (remove them during runtime, because
>>> they're not locked).
>>
>> I know I'm picky, but adding a new metadata *onto* the existing data
>> structure has to be always done carefully.  Otherwise it'll hit back
>> us later.
> 
> I think it's good that you are picky. Missing a use case can cause 
> problems indeed. But is there another solution to this problem which is 
> better?
> 
>> For example, another corner case I can think of easily is that user
>> downgrades alsa-lib.  Then softvol overrides the TLV again without the
>> new tag (the current softvol code assumes the single TLV and overrides
>> if it doesn't fit), and it can be saved so...
>>
>>>> BTW, does the alsaloop device just work as is, i.e. without specifying
>>>> anything in PA's configuration?
>>>
>>> The alsaloop can run on top of the snd-aloop, so it behaves like a
>>> standard hardware, only the mixer controls can be rerouted using the
>>> user controls to the real card.
>>
>> OK.  But, looking at the current implementation, you are allowed to
>> map freely the source ctl element.  It can lead to the same problem in
>> PA.
>>
>> The very reason we'd like to filter out the mixer control created by
>> softvol is that this mixer element confuses PA as if it actually

Why it can confuse PA? It behaves like standard soundcard and user can
select the forwarded controls (for example Master and PCM or Front
only). PA don't have any hint that the audio is rerouted and it's correct.

>> changes the volume (e.g. "PCM") although PA ignores the softvol.  If
>> user creates PCM volume in alsaloop in a different fashion as PA
>> expected, the similar problem may happen.  How can we detect this
>> logically...?  In other words, how can PA adjust the mixer elements
>> for alsaloop properly?
> 
> So if alsaloop is run, only once, that could cause a control to be added 
> for all future, due to alsactl saving and restoring it?
> 
> If so, that looks like a problem with alsaloop. If it adds controls, it 
> should also remove them.

These controls are removed when alsaloop exits.

					Jaroslav

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

Re: [alsa-devel] PulseAudio and softvol

[Arun Raghavan]

On Thu, 2013-05-16 at 08:31 +0200, David Henningsson wrote:
> On 05/15/2013 06:28 PM, Takashi Iwai wrote:
[...]
> > Yes, but the invocation of PCM softvol isn't guaranteed to be first
> > before the reference to the already existing user ctl element.
> > snd_mixer_open() can be called before that.
> 
> So this is a transient problem, right? As soon as the first PCM is 
> opened, the TLV would be corrected, and then stay corrected for all 
> times to come.
> 
> And looking at the current PulseAudio code, it does open the pcm device 
> before it opens the mixer/ctl device.
> So, if this isn't possible to solve in a better way, maybe we need to be 
> pragmatic about it - PulseAudio is the only application we know that 
> would care, and it opens the pcm device first. So in practice, it looks 
> like the TLV approach would work.
> 
[...]
> > The very reason we'd like to filter out the mixer control created by
> > softvol is that this mixer element confuses PA as if it actually
> > changes the volume (e.g. "PCM") although PA ignores the softvol.  If

Actually, we don't currently ignore softvol. I guess we could add the
no-softvol flag once we're able to make sure we don't have any softvol
controls.

> > user creates PCM volume in alsaloop in a different fashion as PA
> > expected, the similar problem may happen.  How can we detect this
> > logically...?  In other words, how can PA adjust the mixer elements
> > for alsaloop properly?
> 
> So if alsaloop is run, only once, that could cause a control to be added 
> for all future, due to alsactl saving and restoring it?
> 
> If so, that looks like a problem with alsaloop. If it adds controls, it 
> should also remove them.
> 
> If no, I don't think we need to worry. Alsaloop is probably mostly used 
> on non-PA systems (as PA has module-loopback which does the same thing).

Seems this thread died out. I didn't quite understand the
alsaloop/alsactl-specific concerns, tbh. What do we need to do to take
this forwards?

Cheers,
Arun

Re: [alsa-devel] PulseAudio and softvol

[David Henningsson]

On 06/26/2013 08:59 AM, Arun Raghavan wrote:
> On Thu, 2013-05-16 at 08:31 +0200, David Henningsson wrote:
>> On 05/15/2013 06:28 PM, Takashi Iwai wrote:
> [...]
>>> Yes, but the invocation of PCM softvol isn't guaranteed to be first
>>> before the reference to the already existing user ctl element.
>>> snd_mixer_open() can be called before that.
>>
>> So this is a transient problem, right? As soon as the first PCM is
>> opened, the TLV would be corrected, and then stay corrected for all
>> times to come.
>>
>> And looking at the current PulseAudio code, it does open the pcm device
>> before it opens the mixer/ctl device.
>> So, if this isn't possible to solve in a better way, maybe we need to be
>> pragmatic about it - PulseAudio is the only application we know that
>> would care, and it opens the pcm device first. So in practice, it looks
>> like the TLV approach would work.
>>
> [...]
>>> The very reason we'd like to filter out the mixer control created by
>>> softvol is that this mixer element confuses PA as if it actually
>>> changes the volume (e.g. "PCM") although PA ignores the softvol.  If
>
> Actually, we don't currently ignore softvol. I guess we could add the
> no-softvol flag once we're able to make sure we don't have any softvol
> controls.

Correct.

>>> user creates PCM volume in alsaloop in a different fashion as PA
>>> expected, the similar problem may happen.  How can we detect this
>>> logically...?  In other words, how can PA adjust the mixer elements
>>> for alsaloop properly?
>>
>> So if alsaloop is run, only once, that could cause a control to be added
>> for all future, due to alsactl saving and restoring it?
>>
>> If so, that looks like a problem with alsaloop. If it adds controls, it
>> should also remove them.
>>
>> If no, I don't think we need to worry. Alsaloop is probably mostly used
>> on non-PA systems (as PA has module-loopback which does the same thing).
>
> Seems this thread died out. I didn't quite understand the
> alsaloop/alsactl-specific concerns, tbh. What do we need to do to take
> this forwards?

It was suggested that softvol controls would store this information in 
TLV, which leads to a transient problem on upgrading, but nobody 
suggested anything better.

The transient problem is caused by alsactl loading the control from 
asound.state, which contains the old TLV information. But as soon as a 
PCM stream is opened, this will be corrected, AFAIU, and then the 
correct TLV information will be stored in asound.state at next reboot.

So I'm guessing that patches are welcome for that solution (in 
combination with a CTL_OPEN_NO_SOFTVOL flag), and nobody volunteered yet 
to implement it.


-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic