Patrick Williams | f1e5d69 | 2016-03-30 15:21:19 -0500 | [diff] [blame^] | 1 | From cc41c8a3149ef04d4aa2db3d15032605a5504658 Mon Sep 17 00:00:00 2001 |
| 2 | From: Tanu Kaskinen <tanuk@iki.fi> |
| 3 | Date: Fri, 23 Oct 2015 12:59:53 +0300 |
| 4 | Subject: [PATCH 3/4] card: move profile selection after pa_card_new() |
| 5 | |
| 6 | I want module-alsa-card to set the availability of unavailable |
| 7 | profiles before the initial card profile gets selected, so that the |
| 8 | selection logic can use correct availability information. |
| 9 | module-alsa-card initializes the jack state after calling |
| 10 | pa_card_new(), however, and the profile selection happens in |
| 11 | pa_card_new(). This patch solves that by introducing pa_card_put() and |
| 12 | moving the profile selection code there. |
| 13 | |
| 14 | An alternative solution would have been to move the jack |
| 15 | initialization to happen before pa_card_new() and use pa_card_new_data |
| 16 | instead of pa_card in the jack initialization code, but I disliked |
| 17 | that idea (I want to get rid of the "new data" pattern eventually). |
| 18 | |
| 19 | The CARD_NEW hook is used when applying the initial profile policy, so |
| 20 | that was moved to pa_card_put(). That required changing the hook data |
| 21 | from pa_card_new_data to pa_card. module-card-restore now uses |
| 22 | pa_card_set_profile() instead of pa_card_new_data_set_profile(). That |
| 23 | required adding a state variable to pa_card, because |
| 24 | pa_card_set_profile() needs to distinguish between setting the initial |
| 25 | profile and setting the profile in other situations. |
| 26 | |
| 27 | The order in which the initial profile policy is applied is reversed |
| 28 | in this patch. Previously the first one to set it won, now the last |
| 29 | one to set it wins. I think this is better, because if you have N |
| 30 | parties that want to set the profile, we avoid checking N times |
| 31 | whether someone else has already set the profile. |
| 32 | |
| 33 | http://bugzilla.yoctoproject.org/show_bug.cgi?id=8448 |
| 34 | |
| 35 | Upstream-Status: Submitted [http://lists.freedesktop.org/archives/pulseaudio-discuss/2015-October/024614.html] |
| 36 | Signed-off-by: Jussi Kukkonen <jussi.kukkonen@intel.com> |
| 37 | --- |
| 38 | src/modules/alsa/module-alsa-card.c | 19 +++--- |
| 39 | src/modules/bluetooth/module-bluez4-device.c | 18 +++--- |
| 40 | src/modules/bluetooth/module-bluez5-device.c | 1 + |
| 41 | src/modules/macosx/module-coreaudio-device.c | 1 + |
| 42 | src/modules/module-card-restore.c | 24 ++++---- |
| 43 | src/pulsecore/card.c | 86 +++++++++++++++------------- |
| 44 | src/pulsecore/card.h | 7 +++ |
| 45 | 7 files changed, 87 insertions(+), 69 deletions(-) |
| 46 | |
| 47 | diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c |
| 48 | index 32f517e..5b39654 100644 |
| 49 | --- a/src/modules/alsa/module-alsa-card.c |
| 50 | +++ b/src/modules/alsa/module-alsa-card.c |
| 51 | @@ -754,15 +754,6 @@ int pa__init(pa_module *m) { |
| 52 | goto fail; |
| 53 | } |
| 54 | |
| 55 | - if ((profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { |
| 56 | - if (pa_hashmap_get(data.profiles, profile)) |
| 57 | - pa_card_new_data_set_profile(&data, profile); |
| 58 | - else { |
| 59 | - pa_log("No such profile: %s", profile); |
| 60 | - goto fail; |
| 61 | - } |
| 62 | - } |
| 63 | - |
| 64 | u->card = pa_card_new(m->core, &data); |
| 65 | pa_card_new_data_done(&data); |
| 66 | |
| 67 | @@ -773,6 +764,16 @@ int pa__init(pa_module *m) { |
| 68 | u->card->set_profile = card_set_profile; |
| 69 | |
| 70 | init_jacks(u); |
| 71 | + pa_card_put(u->card); |
| 72 | + |
| 73 | + if ((profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { |
| 74 | + u->card->active_profile = pa_hashmap_get(u->card->profiles, profile); |
| 75 | + if (!u->card->active_profile) { |
| 76 | + pa_log("No such profile: %s", profile); |
| 77 | + goto fail; |
| 78 | + } |
| 79 | + } |
| 80 | + |
| 81 | init_profile(u); |
| 82 | init_eld_ctls(u); |
| 83 | |
| 84 | diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c |
| 85 | index 94e6988..5efc5dc 100644 |
| 86 | --- a/src/modules/bluetooth/module-bluez4-device.c |
| 87 | +++ b/src/modules/bluetooth/module-bluez4-device.c |
| 88 | @@ -2307,15 +2307,6 @@ static int add_card(struct userdata *u) { |
| 89 | *d = PA_BLUEZ4_PROFILE_OFF; |
| 90 | pa_hashmap_put(data.profiles, p->name, p); |
| 91 | |
| 92 | - if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { |
| 93 | - if (pa_hashmap_get(data.profiles, default_profile)) |
| 94 | - pa_card_new_data_set_profile(&data, default_profile); |
| 95 | - else { |
| 96 | - pa_log("Profile '%s' not valid or not supported by device.", default_profile); |
| 97 | - return -1; |
| 98 | - } |
| 99 | - } |
| 100 | - |
| 101 | u->card = pa_card_new(u->core, &data); |
| 102 | pa_card_new_data_done(&data); |
| 103 | |
| 104 | @@ -2326,6 +2317,15 @@ static int add_card(struct userdata *u) { |
| 105 | |
| 106 | u->card->userdata = u; |
| 107 | u->card->set_profile = card_set_profile; |
| 108 | + pa_card_put(u->card); |
| 109 | + |
| 110 | + if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) { |
| 111 | + u->card->active_profile = pa_hashmap_get(u->card->profiles, default_profile); |
| 112 | + if (!u->card->active_profile) { |
| 113 | + pa_log("Profile '%s' not valid or not supported by device.", default_profile); |
| 114 | + return -1; |
| 115 | + } |
| 116 | + } |
| 117 | |
| 118 | d = PA_CARD_PROFILE_DATA(u->card->active_profile); |
| 119 | |
| 120 | diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c |
| 121 | index 3321785..0081a21 100644 |
| 122 | --- a/src/modules/bluetooth/module-bluez5-device.c |
| 123 | +++ b/src/modules/bluetooth/module-bluez5-device.c |
| 124 | @@ -1959,6 +1959,7 @@ static int add_card(struct userdata *u) { |
| 125 | |
| 126 | u->card->userdata = u; |
| 127 | u->card->set_profile = set_profile_cb; |
| 128 | + pa_card_put(u->card); |
| 129 | |
| 130 | p = PA_CARD_PROFILE_DATA(u->card->active_profile); |
| 131 | u->profile = *p; |
| 132 | diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c |
| 133 | index 4bbb5d5..41f151f 100644 |
| 134 | --- a/src/modules/macosx/module-coreaudio-device.c |
| 135 | +++ b/src/modules/macosx/module-coreaudio-device.c |
| 136 | @@ -764,6 +764,7 @@ int pa__init(pa_module *m) { |
| 137 | pa_card_new_data_done(&card_new_data); |
| 138 | u->card->userdata = u; |
| 139 | u->card->set_profile = card_set_profile; |
| 140 | + pa_card_put(u->card); |
| 141 | |
| 142 | u->rtpoll = pa_rtpoll_new(); |
| 143 | pa_thread_mq_init(&u->thread_mq, m->core->mainloop, u->rtpoll); |
| 144 | diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c |
| 145 | index baa2f4f..0501ac8 100644 |
| 146 | --- a/src/modules/module-card-restore.c |
| 147 | +++ b/src/modules/module-card-restore.c |
| 148 | @@ -485,34 +485,38 @@ static pa_hook_result_t port_offset_change_callback(pa_core *c, pa_device_port * |
| 149 | return PA_HOOK_OK; |
| 150 | } |
| 151 | |
| 152 | -static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new_data, struct userdata *u) { |
| 153 | +static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card *card, struct userdata *u) { |
| 154 | struct entry *e; |
| 155 | void *state; |
| 156 | pa_device_port *p; |
| 157 | struct port_info *p_info; |
| 158 | |
| 159 | - pa_assert(new_data); |
| 160 | + pa_assert(c); |
| 161 | + pa_assert(card); |
| 162 | + pa_assert(u); |
| 163 | |
| 164 | - if (!(e = entry_read(u, new_data->name))) |
| 165 | + if (!(e = entry_read(u, card->name))) |
| 166 | return PA_HOOK_OK; |
| 167 | |
| 168 | if (e->profile[0]) { |
| 169 | - if (!new_data->active_profile) { |
| 170 | - pa_card_new_data_set_profile(new_data, e->profile); |
| 171 | - pa_log_info("Restored profile '%s' for card %s.", new_data->active_profile, new_data->name); |
| 172 | - new_data->save_profile = true; |
| 173 | + pa_card_profile *profile; |
| 174 | |
| 175 | + profile = pa_hashmap_get(card->profiles, e->profile); |
| 176 | + if (profile) { |
| 177 | + pa_card_set_profile(card, profile, true); |
| 178 | + pa_log_info("Restored profile '%s' for card %s.", card->active_profile->name, card->name); |
| 179 | } else |
| 180 | - pa_log_debug("Not restoring profile for card %s, because already set.", new_data->name); |
| 181 | + pa_log_debug("Tried to restore profile %s for card %s, but the card doesn't have such profile.", |
| 182 | + e->profile, card->name); |
| 183 | } |
| 184 | |
| 185 | /* Always restore the latency offsets because their |
| 186 | * initial value is always 0 */ |
| 187 | |
| 188 | - pa_log_info("Restoring port latency offsets for card %s.", new_data->name); |
| 189 | + pa_log_info("Restoring port latency offsets for card %s.", card->name); |
| 190 | |
| 191 | PA_HASHMAP_FOREACH(p_info, e->ports, state) |
| 192 | - if ((p = pa_hashmap_get(new_data->ports, p_info->name))) |
| 193 | + if ((p = pa_hashmap_get(card->ports, p_info->name))) |
| 194 | p->latency_offset = p_info->offset; |
| 195 | |
| 196 | entry_free(e); |
| 197 | diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c |
| 198 | index cc4c784..1b7f71b 100644 |
| 199 | --- a/src/pulsecore/card.c |
| 200 | +++ b/src/pulsecore/card.c |
| 201 | @@ -151,6 +151,7 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { |
| 202 | pa_assert(!pa_hashmap_isempty(data->profiles)); |
| 203 | |
| 204 | c = pa_xnew(pa_card, 1); |
| 205 | + c->state = PA_CARD_STATE_INIT; |
| 206 | |
| 207 | if (!(name = pa_namereg_register(core, data->name, PA_NAMEREG_CARD, c, data->namereg_fail))) { |
| 208 | pa_xfree(c); |
| 209 | @@ -159,12 +160,6 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { |
| 210 | |
| 211 | pa_card_new_data_set_name(data, name); |
| 212 | |
| 213 | - if (pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_NEW], data) < 0) { |
| 214 | - pa_xfree(c); |
| 215 | - pa_namereg_unregister(core, name); |
| 216 | - return NULL; |
| 217 | - } |
| 218 | - |
| 219 | c->core = core; |
| 220 | c->name = pa_xstrdup(data->name); |
| 221 | c->proplist = pa_proplist_copy(data->proplist); |
| 222 | @@ -187,30 +182,6 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { |
| 223 | PA_HASHMAP_FOREACH(port, c->ports, state) |
| 224 | port->card = c; |
| 225 | |
| 226 | - c->active_profile = NULL; |
| 227 | - c->save_profile = false; |
| 228 | - |
| 229 | - if (data->active_profile) |
| 230 | - if ((c->active_profile = pa_hashmap_get(c->profiles, data->active_profile))) |
| 231 | - c->save_profile = data->save_profile; |
| 232 | - |
| 233 | - if (!c->active_profile) { |
| 234 | - PA_HASHMAP_FOREACH(profile, c->profiles, state) { |
| 235 | - if (profile->available == PA_AVAILABLE_NO) |
| 236 | - continue; |
| 237 | - |
| 238 | - if (!c->active_profile || profile->priority > c->active_profile->priority) |
| 239 | - c->active_profile = profile; |
| 240 | - } |
| 241 | - /* If all profiles are not available, then we still need to pick one */ |
| 242 | - if (!c->active_profile) { |
| 243 | - PA_HASHMAP_FOREACH(profile, c->profiles, state) |
| 244 | - if (!c->active_profile || profile->priority > c->active_profile->priority) |
| 245 | - c->active_profile = profile; |
| 246 | - } |
| 247 | - pa_assert(c->active_profile); |
| 248 | - } |
| 249 | - |
| 250 | c->userdata = NULL; |
| 251 | c->set_profile = NULL; |
| 252 | c->active_profile = NULL; |
| 253 | @@ -219,13 +190,39 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) { |
| 254 | pa_device_init_icon(c->proplist, true); |
| 255 | pa_device_init_intended_roles(c->proplist); |
| 256 | |
| 257 | - pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0); |
| 258 | + return c; |
| 259 | +} |
| 260 | |
| 261 | - pa_log_info("Created %u \"%s\"", c->index, c->name); |
| 262 | - pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, c->index); |
| 263 | +void pa_card_put(pa_card *card) { |
| 264 | + pa_card_profile *profile; |
| 265 | + void *state; |
| 266 | |
| 267 | - pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_PUT], c); |
| 268 | - return c; |
| 269 | + pa_assert(card); |
| 270 | + |
| 271 | + PA_HASHMAP_FOREACH(profile, card->profiles, state) { |
| 272 | + if (profile->available == PA_AVAILABLE_NO) |
| 273 | + continue; |
| 274 | + |
| 275 | + if (!card->active_profile || profile->priority > card->active_profile->priority) |
| 276 | + card->active_profile = profile; |
| 277 | + } |
| 278 | + |
| 279 | + /* If all profiles are unavailable, then we still need to pick one */ |
| 280 | + if (!card->active_profile) { |
| 281 | + PA_HASHMAP_FOREACH(profile, card->profiles, state) |
| 282 | + if (!card->active_profile || profile->priority > card->active_profile->priority) |
| 283 | + card->active_profile = profile; |
| 284 | + } |
| 285 | + pa_assert(card->active_profile); |
| 286 | + |
| 287 | + pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_NEW], card); |
| 288 | + |
| 289 | + pa_assert_se(pa_idxset_put(card->core->cards, card, &card->index) >= 0); |
| 290 | + card->state = PA_CARD_STATE_LINKED; |
| 291 | + |
| 292 | + pa_log_info("Created %u \"%s\"", card->index, card->name); |
| 293 | + pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_PUT], card); |
| 294 | + pa_subscription_post(card->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, card->index); |
| 295 | } |
| 296 | |
| 297 | void pa_card_free(pa_card *c) { |
| 298 | @@ -292,17 +289,24 @@ int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save) { |
| 299 | return 0; |
| 300 | } |
| 301 | |
| 302 | - if ((r = c->set_profile(c, profile)) < 0) |
| 303 | + /* If we're setting the initial profile, we shouldn't call set_profile(), |
| 304 | + * because the implementations don't expect that (for historical reasons). |
| 305 | + * We should just set c->active_profile, and the implementations will |
| 306 | + * properly set up that profile after pa_card_put() has returned. It would |
| 307 | + * be probably good to change this so that also the initial profile can be |
| 308 | + * set up in set_profile(), but if set_profile() fails, that would need |
| 309 | + * some better handling than what we do here currently. */ |
| 310 | + if (c->state != PA_CARD_STATE_INIT && (r = c->set_profile(c, profile)) < 0) |
| 311 | return r; |
| 312 | |
| 313 | - pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index); |
| 314 | - |
| 315 | - pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name); |
| 316 | - |
| 317 | c->active_profile = profile; |
| 318 | c->save_profile = save; |
| 319 | |
| 320 | - pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c); |
| 321 | + if (c->state != PA_CARD_STATE_INIT) { |
| 322 | + pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name); |
| 323 | + pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c); |
| 324 | + pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index); |
| 325 | + } |
| 326 | |
| 327 | return 0; |
| 328 | } |
| 329 | diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h |
| 330 | index 1c33958..dbbc1c2 100644 |
| 331 | --- a/src/pulsecore/card.h |
| 332 | +++ b/src/pulsecore/card.h |
| 333 | @@ -37,6 +37,11 @@ typedef enum pa_available { |
| 334 | #include <pulsecore/module.h> |
| 335 | #include <pulsecore/idxset.h> |
| 336 | |
| 337 | +typedef enum pa_card_state { |
| 338 | + PA_CARD_STATE_INIT, |
| 339 | + PA_CARD_STATE_LINKED, |
| 340 | +} pa_card_state_t; |
| 341 | + |
| 342 | typedef struct pa_card_profile { |
| 343 | pa_card *card; |
| 344 | char *name; |
| 345 | @@ -61,6 +66,7 @@ typedef struct pa_card_profile { |
| 346 | |
| 347 | struct pa_card { |
| 348 | uint32_t index; |
| 349 | + pa_card_state_t state; |
| 350 | pa_core *core; |
| 351 | |
| 352 | char *name; |
| 353 | @@ -115,6 +121,7 @@ void pa_card_new_data_set_profile(pa_card_new_data *data, const char *profile); |
| 354 | void pa_card_new_data_done(pa_card_new_data *data); |
| 355 | |
| 356 | pa_card *pa_card_new(pa_core *c, pa_card_new_data *data); |
| 357 | +void pa_card_put(pa_card *c); |
| 358 | void pa_card_free(pa_card *c); |
| 359 | |
| 360 | void pa_card_add_profile(pa_card *c, pa_card_profile *profile); |
| 361 | -- |
| 362 | 2.1.4 |
| 363 | |