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
