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
