From cc41c8a3149ef04d4aa2db3d15032605a5504658 Mon Sep 17 00:00:00 2001 From: Tanu Kaskinen Date: Fri, 23 Oct 2015 12:59:53 +0300 Subject: [PATCH 3/4] 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 introducing pa_card_put() and moving the profile selection code there. 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 CARD_NEW hook is used when applying the initial profile policy, so that was moved to pa_card_put(). That required changing the hook data from pa_card_new_data to pa_card. module-card-restore now uses pa_card_set_profile() instead of pa_card_new_data_set_profile(). That required adding a state variable to pa_card, because pa_card_set_profile() needs to distinguish between setting the initial profile and setting the profile in other situations. 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. http://bugzilla.yoctoproject.org/show_bug.cgi?id=8448 Upstream-Status: Submitted [http://lists.freedesktop.org/archives/pulseaudio-discuss/2015-October/024614.html] Signed-off-by: Jussi Kukkonen --- src/modules/alsa/module-alsa-card.c | 19 +++--- src/modules/bluetooth/module-bluez4-device.c | 18 +++--- src/modules/bluetooth/module-bluez5-device.c | 1 + src/modules/macosx/module-coreaudio-device.c | 1 + src/modules/module-card-restore.c | 24 ++++---- src/pulsecore/card.c | 86 +++++++++++++++------------- src/pulsecore/card.h | 7 +++ 7 files changed, 87 insertions(+), 69 deletions(-) diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c index 32f517e..5b39654 100644 --- a/src/modules/alsa/module-alsa-card.c +++ b/src/modules/alsa/module-alsa-card.c @@ -754,15 +754,6 @@ int pa__init(pa_module *m) { goto fail; } - if ((profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { - if (pa_hashmap_get(data.profiles, profile)) - pa_card_new_data_set_profile(&data, profile); - else { - pa_log("No such profile: %s", profile); - goto fail; - } - } - u->card = pa_card_new(m->core, &data); pa_card_new_data_done(&data); @@ -773,6 +764,16 @@ int pa__init(pa_module *m) { u->card->set_profile = card_set_profile; init_jacks(u); + pa_card_put(u->card); + + if ((profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { + u->card->active_profile = pa_hashmap_get(u->card->profiles, profile); + if (!u->card->active_profile) { + pa_log("No such profile: %s", profile); + goto fail; + } + } + init_profile(u); init_eld_ctls(u); diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c index 94e6988..5efc5dc 100644 --- a/src/modules/bluetooth/module-bluez4-device.c +++ b/src/modules/bluetooth/module-bluez4-device.c @@ -2307,15 +2307,6 @@ static int add_card(struct userdata *u) { *d = PA_BLUEZ4_PROFILE_OFF; pa_hashmap_put(data.profiles, p->name, p); - if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { - if (pa_hashmap_get(data.profiles, default_profile)) - pa_card_new_data_set_profile(&data, default_profile); - else { - pa_log("Profile '%s' not valid or not supported by device.", default_profile); - return -1; - } - } - u->card = pa_card_new(u->core, &data); pa_card_new_data_done(&data); @@ -2326,6 +2317,15 @@ static int add_card(struct userdata *u) { u->card->userdata = u; u->card->set_profile = card_set_profile; + pa_card_put(u->card); + + if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { + u->card->active_profile = pa_hashmap_get(u->card->profiles, default_profile); + if (!u->card->active_profile) { + pa_log("Profile '%s' not valid or not supported by device.", default_profile); + return -1; + } + } d = PA_CARD_PROFILE_DATA(u->card->active_profile); diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index 3321785..0081a21 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -1959,6 +1959,7 @@ static int add_card(struct userdata *u) { u->card->userdata = u; u->card->set_profile = set_profile_cb; + pa_card_put(u->card); p = PA_CARD_PROFILE_DATA(u->card->active_profile); u->profile = *p; diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c index 4bbb5d5..41f151f 100644 --- a/src/modules/macosx/module-coreaudio-device.c +++ b/src/modules/macosx/module-coreaudio-device.c @@ -764,6 +764,7 @@ int pa__init(pa_module *m) { pa_card_new_data_done(&card_new_data); u->card->userdata = u; u->card->set_profile = card_set_profile; + pa_card_put(u->card); u->rtpoll = pa_rtpoll_new(); pa_thread_mq_init(&u->thread_mq, m->core->mainloop, u->rtpoll); diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c index baa2f4f..0501ac8 100644 --- a/src/modules/module-card-restore.c +++ b/src/modules/module-card-restore.c @@ -485,34 +485,38 @@ static pa_hook_result_t port_offset_change_callback(pa_core *c, pa_device_port * return PA_HOOK_OK; } -static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new_data, struct userdata *u) { +static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card *card, struct userdata *u) { struct entry *e; void *state; pa_device_port *p; struct port_info *p_info; - pa_assert(new_data); + pa_assert(c); + pa_assert(card); + pa_assert(u); - if (!(e = entry_read(u, new_data->name))) + if (!(e = entry_read(u, card->name))) return PA_HOOK_OK; if (e->profile[0]) { - if (!new_data->active_profile) { - pa_card_new_data_set_profile(new_data, e->profile); - pa_log_info("Restored profile '%s' for card %s.", new_data->active_profile, new_data->name); - new_data->save_profile = true; + pa_card_profile *profile; + profile = pa_hashmap_get(card->profiles, e->profile); + if (profile) { + pa_card_set_profile(card, profile, true); + pa_log_info("Restored profile '%s' for card %s.", card->active_profile->name, card->name); } else - pa_log_debug("Not restoring profile for card %s, because already set.", new_data->name); + pa_log_debug("Tried to restore profile %s for card %s, but the card doesn't have such profile.", + e->profile, card->name); } /* Always restore the latency offsets because their * initial value is always 0 */ - pa_log_info("Restoring port latency offsets for card %s.", new_data->name); + pa_log_info("Restoring port latency offsets for card %s.", card->name); PA_HASHMAP_FOREACH(p_info, e->ports, state) - if ((p = pa_hashmap_get(new_data->ports, p_info->name))) + if ((p = pa_hashmap_get(card->ports, p_info->name))) p->latency_offset = p_info->offset; entry_free(e); diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c index cc4c784..1b7f71b 100644 --- a/src/pulsecore/card.c +++ b/src/pulsecore/card.c @@ -151,6 +151,7 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { pa_assert(!pa_hashmap_isempty(data->profiles)); c = pa_xnew(pa_card, 1); + c->state = PA_CARD_STATE_INIT; if (!(name = pa_namereg_register(core, data->name, PA_NAMEREG_CARD, c, data->namereg_fail))) { pa_xfree(c); @@ -159,12 +160,6 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { pa_card_new_data_set_name(data, name); - if (pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_NEW], data) < 0) { - pa_xfree(c); - pa_namereg_unregister(core, name); - return NULL; - } - c->core = core; c->name = pa_xstrdup(data->name); c->proplist = pa_proplist_copy(data->proplist); @@ -187,30 +182,6 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { PA_HASHMAP_FOREACH(port, c->ports, state) port->card = c; - c->active_profile = NULL; - c->save_profile = false; - - if (data->active_profile) - if ((c->active_profile = pa_hashmap_get(c->profiles, data->active_profile))) - c->save_profile = data->save_profile; - - if (!c->active_profile) { - PA_HASHMAP_FOREACH(profile, c->profiles, state) { - if (profile->available == PA_AVAILABLE_NO) - continue; - - 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; - } - pa_assert(c->active_profile); - } - c->userdata = NULL; c->set_profile = NULL; c->active_profile = NULL; @@ -219,13 +190,39 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { pa_device_init_icon(c->proplist, true); pa_device_init_intended_roles(c->proplist); - pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0); + return c; +} - 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_card_profile *profile; + void *state; - pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_PUT], c); - return c; + pa_assert(card); + + PA_HASHMAP_FOREACH(profile, card->profiles, state) { + if (profile->available == PA_AVAILABLE_NO) + continue; + + if (!card->active_profile || profile->priority > card->active_profile->priority) + card->active_profile = profile; + } + + /* If all profiles are unavailable, then we still need to pick one */ + if (!card->active_profile) { + PA_HASHMAP_FOREACH(profile, card->profiles, state) + if (!card->active_profile || profile->priority > card->active_profile->priority) + card->active_profile = profile; + } + pa_assert(card->active_profile); + + pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_NEW], card); + + pa_assert_se(pa_idxset_put(card->core->cards, card, &card->index) >= 0); + card->state = PA_CARD_STATE_LINKED; + + 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) { @@ -292,17 +289,24 @@ 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->state != PA_CARD_STATE_INIT && (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; - pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c); + if (c->state != PA_CARD_STATE_INIT) { + 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 1c33958..dbbc1c2 100644 --- a/src/pulsecore/card.h +++ b/src/pulsecore/card.h @@ -37,6 +37,11 @@ typedef enum pa_available { #include #include +typedef enum pa_card_state { + PA_CARD_STATE_INIT, + PA_CARD_STATE_LINKED, +} pa_card_state_t; + typedef struct pa_card_profile { pa_card *card; char *name; @@ -61,6 +66,7 @@ typedef struct pa_card_profile { struct pa_card { uint32_t index; + pa_card_state_t state; pa_core *core; char *name; @@ -115,6 +121,7 @@ void pa_card_new_data_set_profile(pa_card_new_data *data, const char *profile); void pa_card_new_data_done(pa_card_new_data *data); pa_card *pa_card_new(pa_core *c, pa_card_new_data *data); +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); -- 2.1.4