[pulseaudio-discuss] [RFC PATCH] alsa-mixer: Cache failure to open inputs

[Tanu Kaskinen]

On Fri, 2012-09-21 at 10:37 +0200, David Henningsson wrote:
> We spend most of our startup time probing soundcards, so I was hoping this
> would improve bootup speed, but it doesn't seem to do so here.
> Do we want this anyway? At least it reduces the logs a little.
> 
> Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> ---
>  src/modules/alsa/alsa-mixer.c |   25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
> index 8072fbb..1cf502d 100644
> --- a/src/modules/alsa/alsa-mixer.c
> +++ b/src/modules/alsa/alsa-mixer.c
> @@ -3920,6 +3920,10 @@ static void profile_set_add_auto(pa_alsa_profile_set *ps) {
>  
>      pa_assert(ps);
>  
> +    /* Try input only first, for caching */

This comment could be more explicit in explaining that for the caching
to be useful, we depend on the fact that pa_hashmap iteration happens in
the order in which the profiles were created. This reordering seemed
nonsensical before I checked how the iteration is implemented.

I think it would make sense to also create all the output-only profiles
before the combinations.

> +    PA_HASHMAP_FOREACH(n, ps->mappings, n_state)
> +        profile_set_add_auto_pair(ps, NULL, n);
> +
>      PA_HASHMAP_FOREACH(m, ps->mappings, m_state) {
>          profile_set_add_auto_pair(ps, m, NULL);
>  
> @@ -3927,8 +3931,6 @@ static void profile_set_add_auto(pa_alsa_profile_set *ps) {
>              profile_set_add_auto_pair(ps, m, n);
>      }
>  
> -    PA_HASHMAP_FOREACH(n, ps->mappings, n_state)
> -        profile_set_add_auto_pair(ps, NULL, n);
>  }
>  
>  static int profile_verify(pa_alsa_profile *p) {
> @@ -4293,6 +4295,7 @@ void pa_alsa_profile_set_probe(
>      void *state;
>      pa_alsa_profile *p, *last = NULL;
>      pa_alsa_mapping *m;
> +    pa_hashmap *broken_inputs;
>  
>      pa_assert(ps);
>      pa_assert(dev_id);
> @@ -4301,6 +4304,8 @@ void pa_alsa_profile_set_probe(
>      if (ps->probed)
>          return;
>  
> +    broken_inputs = pa_hashmap_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func);

Wouldn't it make sense to have broken_outputs too?

> +
>      PA_HASHMAP_FOREACH(p, ps->profiles, state) {
>          uint32_t idx;
>  
> @@ -4311,8 +4316,16 @@ void pa_alsa_profile_set_probe(
>              profile_finalize_probing(last, p);
>              p->supported = TRUE;
>  
> +            if (p->input_mappings)
> +                PA_IDXSET_FOREACH(m, p->input_mappings, idx)

I see that it's still not codified in the coding style document, but
didn't we agree a while ago in IRC that for multiline block contents
braces should be always used, even if they are not strictly necessary?

> +                    if (pa_hashmap_get(broken_inputs, m) == m) {
> +                        pa_log_debug("Skipping - will not be able to open input:%s", m->name);

A space would be nice before %s.

> +                        p->supported = FALSE;
> +                        break;
> +                    }
> +
>              /* Check if we can open all new ones */
> -            if (p->output_mappings)
> +            if (p->supported && p->output_mappings)
>                  PA_IDXSET_FOREACH(m, p->output_mappings, idx) {
>  
>                      if (m->output_pcm)
> @@ -4340,6 +4353,11 @@ void pa_alsa_profile_set_probe(
>                                                            default_n_fragments,
>                                                            default_fragment_size_msec))) {
>                          p->supported = FALSE;
> +                        if (pa_idxset_size(p->input_mappings) == 1 &&
> +                            ((!p->output_mappings) || pa_idxset_size(p->output_mappings) == 0)) {

As far as I can see, p->output_mappings can never have zero size.

-- 
Tanu