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
