[pulseaudio-discuss] [PATCH] Don't use tsched on unsafe ALSA plugins

[David Henningsson]

On 2014-04-19 23:21, Alexander E. Patrakov wrote:
> Since the dca ALSA plugin is based on extplug, it has a slave and therefore
> a card. Thus, PulseAudio thinks that it looks like hardware and attempts
> rewinds, which this plugin cannot handle correctly, because ALSA never
> notifies extplug plugins about rewinds.
>
> The same mishandling of rewinds applies to some other plugins.

This sounds like an alsa-lib bug, and thus it would be wiser to correct 
it in ALSA rather than in PulseAudio?

>
> Work around this ALSA bug by switching to IRQ-based scheduling on strange
> plugin types.
> ---
> I have not deleted the "does a card exist" check, but for the AC3 encoder
> case (which is mentioned in the git log) it is now redundant. Feel free
> to delete if no other triggering use case is known.
>
> P.S. I see this line in alsa-lib/src/pcm/pcm_extplug.c:
>
>          params->info &= ~(SND_PCM_INFO_MMAP | SND_PCM_INFO_MMAP_VALID);
>
> but then also:
>
>          snd_pcm_access_mask_t saccess_mask = { SND_PCM_ACCBIT_MMAP };
>
> How should I understand this - is the intention here to disable mmap access
> or enable it?

Again, this sounds like an ALSA question?

In addition, there is a snd_pcm_rewindable function, but we don't use 
it, and I don't know how well the different ALSA pcm types work with it. 
But it seems to me like the best course of action could be to make sure 
it reports the correct value for all ALSA pcm types, and then for 
PulseAudio to start using it?

>
>   src/modules/alsa/alsa-util.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>
> diff --git a/src/modules/alsa/alsa-util.c b/src/modules/alsa/alsa-util.c
> index 1ce2e16..c6dd80c 100644
> --- a/src/modules/alsa/alsa-util.c
> +++ b/src/modules/alsa/alsa-util.c
> @@ -1384,12 +1384,60 @@ unsigned int *pa_alsa_get_supported_rates(snd_pcm_t *pcm, unsigned int fallback_
>       return rates;
>   }
>
> +static bool pa_alsa_type_is_safe_for_rewind(snd_pcm_type_t type)
> +{
> +    /* Some ALSA plugins have a non-trivial dependency between input
> +     * and output samples: one input sample can affect more than one
> +     * output sample, and vice versa. ALSA does not handle rewinds
> +     * on such plugins correctly. Notably, this affects extplug, which
> +     * is used by dcaenc.
> +     *
> +     * So here is a whitelist of stateless plugin types, based on what
> +     * ALSA uses as front and surround51 PCMs of the supported cards.
> +     *
> +     * Note: we should in fact check that every plugin in the slave
> +     * chain is safe, but this information is not available.
> +     */
> +
> +    if (type == SND_PCM_TYPE_HW) return true;        /* ovbiously */
> +    if (type == SND_PCM_TYPE_HOOKS) return true;     /* HDMI on Intel HDA, SPDIF */
> +    if (type == SND_PCM_TYPE_MULTI) return true;     /* EMU10K1, Audigy2, others */
> +    if (type == SND_PCM_TYPE_LINEAR) return true;    /* SPDIF on ICE1724 */
> +    if (type == SND_PCM_TYPE_ROUTE) return true;     /* ICE1712, VIA8237, others */
> +    if (type == SND_PCM_TYPE_LINEAR_FLOAT) return true;  /* just in case */
> +    if (type == SND_PCM_TYPE_SOFTVOL) return true;   /* many */
> +
> +    /* Specifically excluding these types as unsafe:
> +     *
> +     * SND_PCM_TYPE_ADPCM: uses history
> +     * SND_PCM_TYPE_RATE: uses history
> +     * SND_PCM_TYPE_LADSPA: may hide non-rewindable plugins
> +     * SND_PCM_TYPE_IEC958: rewinding does not update the preamble-related counter
> +     * SND_PCM_TYPE_IOPLUG: may hide non-rewindable encoders
> +     * SND_PCM_TYPE_EXTPLUG: may hide non-rewindable encoders
> +     *
> +     * In theory, SND_PCM_TYPE_PLUG can hide ADPCM, and thus is unsafe.
> +     * However, this doesn't happen with real-world sound cards. It can
> +     * also hide a resampler, but we specifically disarm this capability.
> +     * So it may be actually safe to whitelist, but, if we got to it, then
> +     * we are already running on some weird hardware. Better safe than
> +     * sorry for now.
> +     */
> +
> +    return false;
> +}
> +
>   bool pa_alsa_pcm_is_hw(snd_pcm_t *pcm) {
> +    snd_pcm_type_t type;
>       snd_pcm_info_t* info;
>       snd_pcm_info_alloca(&info);
>
>       pa_assert(pcm);
>
> +    type = snd_pcm_type(pcm);
> +    if (!pa_alsa_type_is_safe_for_rewind(type))
> +        return false;
> +
>       if (snd_pcm_info(pcm, info) < 0)
>           return false;
>
>