card: move profile selection after pa_card_new()

I want module-alsa-card to set the availability of unavailable
profiles before the initial card profile gets selected, so that the
selection logic can use correct availability information.
module-alsa-card initializes the jack state after calling
pa_card_new(), however, and the profile selection happens in
pa_card_new(). This patch solves that by moving parts of pa_card_new()
to pa_card_choose_initial_profile() and pa_card_put().

pa_card_choose_initial_profile() applies the profile selection policy,
so module-alsa-card can first call pa_card_new(), then initialize the
jack state, and then call pa_card_choose_initial_profile(). After that
module-alsa-card can still override the profile selection policy, in
case module-alsa-card was loaded with the "profile" argument. Finally,
pa_card_put() finalizes the card creation.

An alternative solution would have been to move the jack
initialization to happen before pa_card_new() and use pa_card_new_data
instead of pa_card in the jack initialization code, but I disliked
that idea (I want to get rid of the "new data" pattern eventually).

The order in which the initial profile policy is applied is reversed
in this patch. Previously the first one to set it won, now the last
one to set it wins. I think this is better, because if you have N
parties that want to set the profile, we avoid checking N times
whether someone else has already set the profile.

3 files changed, 72 insertions, 37 deletions

diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
index 0ac70b941..a0c3d93d4 100644
--- a/src/pulsecore/card.c
+++ b/src/pulsecore/card.c
@@ -176,38 +176,56 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
     c->preferred_input_port = data->preferred_input_port;
     c->preferred_output_port = data->preferred_output_port;
 
-    if (data->active_profile)
-        if ((c->active_profile = pa_hashmap_get(c->profiles, data->active_profile)))
-            c->save_profile = data->save_profile;
+    pa_device_init_description(c->proplist, c);
+    pa_device_init_icon(c->proplist, true);
+    pa_device_init_intended_roles(c->proplist);
 
-    if (!c->active_profile) {
-        PA_HASHMAP_FOREACH(profile, c->profiles, state) {
-            if (profile->available == PA_AVAILABLE_NO)
-                continue;
+    return c;
+}
 
-            if (!c->active_profile || profile->priority > c->active_profile->priority)
-                c->active_profile = profile;
-        }
-        /* If all profiles are not available, then we still need to pick one */
-        if (!c->active_profile) {
-            PA_HASHMAP_FOREACH(profile, c->profiles, state)
-                if (!c->active_profile || profile->priority > c->active_profile->priority)
-                    c->active_profile = profile;
+void pa_card_choose_initial_profile(pa_card *card) {
+    pa_card_profile *profile;
+    void *state;
+    pa_card_profile *best = NULL;
+
+    pa_assert(card);
+
+    /* By default, pick the highest priority profile that is not unavailable,
+     * or if all profiles are unavailable, pick the profile with the highest
+     * priority regardless of its availability. */
+
+    PA_HASHMAP_FOREACH(profile, card->profiles, state) {
+        if (profile->available == PA_AVAILABLE_NO)
+            continue;
+
+        if (!best || profile->priority > best->priority)
+            best = profile;
+    }
+
+    if (!best) {
+        PA_HASHMAP_FOREACH(profile, card->profiles, state) {
+            if (!best || profile->priority > best->priority)
+                best = profile;
         }
-        pa_assert(c->active_profile);
     }
+    pa_assert(best);
 
-    pa_device_init_description(c->proplist, c);
-    pa_device_init_icon(c->proplist, true);
-    pa_device_init_intended_roles(c->proplist);
+    card->active_profile = best;
+    card->save_profile = false;
 
-    pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0);
+    /* Let policy modules override the default. */
+    pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_CHOOSE_INITIAL_PROFILE], card);
+}
 
-    pa_log_info("Created %u \"%s\"", c->index, c->name);
-    pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, c->index);
+void pa_card_put(pa_card *card) {
+    pa_assert(card);
 
-    pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_PUT], c);
-    return c;
+    pa_assert_se(pa_idxset_put(card->core->cards, card, &card->index) >= 0);
+    card->linked = true;
+
+    pa_log_info("Created %u \"%s\"", card->index, card->name);
+    pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_PUT], card);
+    pa_subscription_post(card->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, card->index);
 }
 
 void pa_card_free(pa_card *c) {
@@ -218,15 +236,15 @@ void pa_card_free(pa_card *c) {
 
     core = c->core;
 
-    pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_UNLINK], c);
-
-    pa_namereg_unregister(core, c->name);
-
-    pa_idxset_remove_by_data(c->core->cards, c, NULL);
+    if (c->linked) {
+        pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_UNLINK], c);
 
-    pa_log_info("Freed %u \"%s\"", c->index, c->name);
+        pa_idxset_remove_by_data(c->core->cards, c, NULL);
+        pa_log_info("Freed %u \"%s\"", c->index, c->name);
+        pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_REMOVE, c->index);
+    }
 
-    pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_REMOVE, c->index);
+    pa_namereg_unregister(core, c->name);
 
     pa_assert(pa_idxset_isempty(c->sinks));
     pa_idxset_free(c->sinks, NULL);
@@ -298,20 +316,27 @@ int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save) {
         return 0;
     }
 
-    if ((r = c->set_profile(c, profile)) < 0)
+    /* If we're setting the initial profile, we shouldn't call set_profile(),
+     * because the implementations don't expect that (for historical reasons).
+     * We should just set c->active_profile, and the implementations will
+     * properly set up that profile after pa_card_put() has returned. It would
+     * be probably good to change this so that also the initial profile can be
+     * set up in set_profile(), but if set_profile() fails, that would need
+     * some better handling than what we do here currently. */
+    if (c->linked && (r = c->set_profile(c, profile)) < 0)
         return r;
 
-    pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index);
-
-    pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name);
-
     c->active_profile = profile;
     c->save_profile = save;
 
     if (save)
         update_port_preferred_profile(c);
 
-    pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c);
+    if (c->linked) {
+        pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name);
+        pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c);
+        pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index);
+    }
 
     return 0;
 }
diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h
index d4970e342..fd1fe0af6 100644
--- a/src/pulsecore/card.h
+++ b/src/pulsecore/card.h
@@ -86,6 +86,8 @@ struct pa_card {
 
     pa_suspend_cause_t suspend_cause;
 
+    bool linked;
+
     void *userdata;
 
     int (*set_profile)(pa_card *c, pa_card_profile *profile);
@@ -128,6 +130,13 @@ void pa_card_new_data_set_preferred_port(pa_card_new_data *data, pa_direction_t
 void pa_card_new_data_done(pa_card_new_data *data);
 
 pa_card *pa_card_new(pa_core *c, pa_card_new_data *data);
+
+/* Select the initial card profile according to the configured policies. This
+ * must be called between pa_card_new() and pa_card_put(), after the port and
+ * profile availablities have been initialized. */
+void pa_card_choose_initial_profile(pa_card *card);
+
+void pa_card_put(pa_card *c);
 void pa_card_free(pa_card *c);
 
 void pa_card_add_profile(pa_card *c, pa_card_profile *profile);
diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
index 00d7f2f2d..802111ba1 100644
--- a/src/pulsecore/core.h
+++ b/src/pulsecore/core.h
@@ -118,6 +118,7 @@ typedef enum pa_core_hook {
     PA_CORE_HOOK_CLIENT_PROPLIST_CHANGED,
     PA_CORE_HOOK_CLIENT_SEND_EVENT,
     PA_CORE_HOOK_CARD_NEW,
+    PA_CORE_HOOK_CARD_CHOOSE_INITIAL_PROFILE,
     PA_CORE_HOOK_CARD_PUT,
     PA_CORE_HOOK_CARD_UNLINK,
     PA_CORE_HOOK_CARD_PREFERRED_PORT_CHANGED,