code.ossystems Code Review - openembedded-core.git/blob

   1 From 0b98309fdbcd36fa92ab53e33c51b485b905e294 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/5] 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 moving parts of pa_card_new()
  12 to pa_card_choose_initial_profile() and pa_card_put().
  13
  14 pa_card_choose_initial_profile() applies the profile selection policy,
  15 so module-alsa-card can first call pa_card_new(), then initialize the
  16 jack state, and then call pa_card_choose_initial_profile(). After that
  17 module-alsa-card can still override the profile selection policy, in
  18 case module-alsa-card was loaded with the "profile" argument. Finally,
  19 pa_card_put() finalizes the card creation.
  20
  21 An alternative solution would have been to move the jack
  22 initialization to happen before pa_card_new() and use pa_card_new_data
  23 instead of pa_card in the jack initialization code, but I disliked
  24 that idea (I want to get rid of the "new data" pattern eventually).
  25
  26 The order in which the initial profile policy is applied is reversed
  27 in this patch. Previously the first one to set it won, now the last
  28 one to set it wins. I think this is better, because if you have N
  29 parties that want to set the profile, we avoid checking N times
  30 whether someone else has already set the profile.
  31
  32 Upstream-Status: Accepted [expected in 10.0]
  33
  34 Signed-off-by: Tanu Kaskinen <tanuk@iki.fi>
  35 ---
  36  src/modules/alsa/module-alsa-card.c          | 32 +++++----
  37  src/modules/bluetooth/module-bluez4-device.c | 31 +++++----
  38  src/modules/bluetooth/module-bluez5-device.c |  2 +
  39  src/modules/macosx/module-coreaudio-device.c |  2 +
  40  src/modules/module-card-restore.c            | 36 +++++++---
  41  src/pulsecore/card.c                         | 99 +++++++++++++++++-----------
  42  src/pulsecore/card.h                         |  9 +++
  43  src/pulsecore/core.h                         |  1 +
  44  8 files changed, 143 insertions(+), 69 deletions(-)
  45
  46 diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c
  47 index d761683..1976230 100644
  48 --- a/src/modules/alsa/module-alsa-card.c
  49 +++ b/src/modules/alsa/module-alsa-card.c
  50 @@ -671,7 +671,7 @@ int pa__init(pa_module *m) {
  51      struct userdata *u;
  52      pa_reserve_wrapper *reserve = NULL;
  53      const char *description;
  54 -    const char *profile = NULL;
  55 +    const char *profile_str = NULL;
  56      char *fn = NULL;
  57      bool namereg_fail = false;
  58
  59 @@ -799,16 +799,6 @@ int pa__init(pa_module *m) {
  60          goto fail;
  61      }
  62
  63 -    if ((profile = pa_modargs_get_value(u->modargs, "profile", NULL))) {
  64 -        if (pa_hashmap_get(data.profiles, profile))
  65 -            pa_card_new_data_set_profile(&data, profile);
  66 -        else {
  67 -            pa_log("No such profile: %s", profile);
  68 -            pa_card_new_data_done(&data);
  69 -            goto fail;
  70 -        }
  71 -    }
  72 -
  73      u->card = pa_card_new(m->core, &data);
  74      pa_card_new_data_done(&data);
  75
  76 @@ -822,6 +812,26 @@ int pa__init(pa_module *m) {
  77              (pa_hook_cb_t) card_suspend_changed, u);
  78
  79      init_jacks(u);
  80 +
  81 +    pa_card_choose_initial_profile(u->card);
  82 +
  83 +    /* If the "profile" modarg is given, we have to override whatever the usual
  84 +     * policy chose in pa_card_choose_initial_profile(). */
  85 +    profile_str = pa_modargs_get_value(u->modargs, "profile", NULL);
  86 +    if (profile_str) {
  87 +        pa_card_profile *profile;
  88 +
  89 +        profile = pa_hashmap_get(u->card->profiles, profile_str);
  90 +        if (!profile) {
  91 +            pa_log("No such profile: %s", profile_str);
  92 +            goto fail;
  93 +        }
  94 +
  95 +        pa_card_set_profile(u->card, profile, false);
  96 +    }
  97 +
  98 +    pa_card_put(u->card);
  99 +
 100      init_profile(u);
 101      init_eld_ctls(u);
 102
 103 diff --git a/src/modules/bluetooth/module-bluez4-device.c b/src/modules/bluetooth/module-bluez4-device.c
 104 index a2de525..13fb7ab 100644
 105 --- a/src/modules/bluetooth/module-bluez4-device.c
 106 +++ b/src/modules/bluetooth/module-bluez4-device.c
 107 @@ -2238,7 +2238,7 @@ static int add_card(struct userdata *u) {
 108      pa_bluez4_profile_t *d;
 109      pa_bluez4_form_factor_t ff;
 110      char *n;
 111 -    const char *default_profile;
 112 +    const char *profile_str;
 113      const pa_bluez4_device *device;
 114      const pa_bluez4_uuid *uuid;
 115
 116 @@ -2298,16 +2298,6 @@ static int add_card(struct userdata *u) {
 117      *d = PA_BLUEZ4_PROFILE_OFF;
 118      pa_hashmap_put(data.profiles, p->name, p);
 119
 120 -    if ((default_profile = pa_modargs_get_value(u->modargs, "profile", NULL))) {
 121 -        if (pa_hashmap_get(data.profiles, default_profile))
 122 -            pa_card_new_data_set_profile(&data, default_profile);
 123 -        else {
 124 -            pa_log("Profile '%s' not valid or not supported by device.", default_profile);
 125 -            pa_card_new_data_done(&data);
 126 -            return -1;
 127 -        }
 128 -    }
 129 -
 130      u->card = pa_card_new(u->core, &data);
 131      pa_card_new_data_done(&data);
 132
 133 @@ -2319,6 +2309,25 @@ static int add_card(struct userdata *u) {
 134      u->card->userdata = u;
 135      u->card->set_profile = card_set_profile;
 136
 137 +    pa_card_choose_initial_profile(u->card);
 138 +
 139 +    /* If the "profile" modarg is given, we have to override whatever the usual
 140 +     * policy chose in pa_card_choose_initial_profile(). */
 141 +    profile_str = pa_modargs_get_value(u->modargs, "profile", NULL);
 142 +    if (profile_str) {
 143 +        pa_card_profile *profile;
 144 +
 145 +        profile = pa_hashmap_get(u->card->profiles, profile_str);
 146 +        if (!profile) {
 147 +            pa_log("No such profile: %s", profile_str);
 148 +            return -1;
 149 +        }
 150 +
 151 +        pa_card_set_profile(u->card, profile, false);
 152 +    }
 153 +
 154 +    pa_card_put(u->card);
 155 +
 156      d = PA_CARD_PROFILE_DATA(u->card->active_profile);
 157
 158      if (*d != PA_BLUEZ4_PROFILE_OFF && (!device->transports[*d] ||
 159 diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c
 160 index 84e6d55..498d0e1 100644
 161 --- a/src/modules/bluetooth/module-bluez5-device.c
 162 +++ b/src/modules/bluetooth/module-bluez5-device.c
 163 @@ -1953,6 +1953,8 @@ static int add_card(struct userdata *u) {
 164
 165      u->card->userdata = u;
 166      u->card->set_profile = set_profile_cb;
 167 +    pa_card_choose_initial_profile(u->card);
 168 +    pa_card_put(u->card);
 169
 170      p = PA_CARD_PROFILE_DATA(u->card->active_profile);
 171      u->profile = *p;
 172 diff --git a/src/modules/macosx/module-coreaudio-device.c b/src/modules/macosx/module-coreaudio-device.c
 173 index 6c9e55d..d91c656 100644
 174 --- a/src/modules/macosx/module-coreaudio-device.c
 175 +++ b/src/modules/macosx/module-coreaudio-device.c
 176 @@ -821,6 +821,8 @@ int pa__init(pa_module *m) {
 177      pa_card_new_data_done(&card_new_data);
 178      u->card->userdata = u;
 179      u->card->set_profile = card_set_profile;
 180 +    pa_card_choose_initial_profile(u->card);
 181 +    pa_card_put(u->card);
 182
 183      u->rtpoll = pa_rtpoll_new();
 184      pa_thread_mq_init(&u->thread_mq, m->core->mainloop, u->rtpoll);
 185 diff --git a/src/modules/module-card-restore.c b/src/modules/module-card-restore.c
 186 index 7545aa5..718a0dd 100644
 187 --- a/src/modules/module-card-restore.c
 188 +++ b/src/modules/module-card-restore.c
 189 @@ -551,16 +551,6 @@ static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new
 190      if (!(e = entry_read(u, new_data->name)))
 191          return PA_HOOK_OK;
 192
 193 -    if (e->profile[0]) {
 194 -        if (!new_data->active_profile) {
 195 -            pa_card_new_data_set_profile(new_data, e->profile);
 196 -            pa_log_info("Restored profile '%s' for card %s.", new_data->active_profile, new_data->name);
 197 -            new_data->save_profile = true;
 198 -
 199 -        } else
 200 -            pa_log_debug("Not restoring profile for card %s, because already set.", new_data->name);
 201 -    }
 202 -
 203      /* Always restore the latency offsets because their
 204       * initial value is always 0 */
 205
 206 @@ -590,6 +580,30 @@ static pa_hook_result_t card_new_hook_callback(pa_core *c, pa_card_new_data *new
 207      return PA_HOOK_OK;
 208  }
 209
 210 +static pa_hook_result_t card_choose_initial_profile_callback(pa_core *core, pa_card *card, struct userdata *u) {
 211 +    struct entry *e;
 212 +
 213 +    if (!(e = entry_read(u, card->name)))
 214 +        return PA_HOOK_OK;
 215 +
 216 +    if (e->profile[0]) {
 217 +        pa_card_profile *profile;
 218 +
 219 +        profile = pa_hashmap_get(card->profiles, e->profile);
 220 +        if (profile) {
 221 +            pa_log_info("Restoring profile '%s' for card %s.", card->active_profile->name, card->name);
 222 +            pa_card_set_profile(card, profile, true);
 223 +        } else {
 224 +            pa_log_debug("Tried to restore profile %s for card %s, but the card doesn't have such profile.",
 225 +                         e->profile, card->name);
 226 +        }
 227 +    }
 228 +
 229 +    entry_free(e);
 230 +
 231 +    return PA_HOOK_OK;
 232 +}
 233 +
 234  static pa_hook_result_t card_preferred_port_changed_callback(pa_core *core, pa_card_preferred_port_changed_hook_data *data,
 235                                                               struct userdata *u) {
 236      struct entry *e;
 237 @@ -634,6 +648,8 @@ int pa__init(pa_module*m) {
 238      u->module = m;
 239
 240      pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_CARD_NEW], PA_HOOK_EARLY, (pa_hook_cb_t) card_new_hook_callback, u);
 241 +    pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_CARD_CHOOSE_INITIAL_PROFILE], PA_HOOK_NORMAL,
 242 +                           (pa_hook_cb_t) card_choose_initial_profile_callback, u);
 243      pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_CARD_PUT], PA_HOOK_NORMAL, (pa_hook_cb_t) card_put_hook_callback, u);
 244      pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_CARD_PREFERRED_PORT_CHANGED], PA_HOOK_NORMAL, (pa_hook_cb_t) card_preferred_port_changed_callback, u);
 245      pa_module_hook_connect(m, &m->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], PA_HOOK_NORMAL, (pa_hook_cb_t) card_profile_changed_callback, u);
 246 diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
 247 index 0ac70b9..a0c3d93 100644
 248 --- a/src/pulsecore/card.c
 249 +++ b/src/pulsecore/card.c
 250 @@ -176,38 +176,56 @@ pa_card *pa_card_new(pa_core *core, pa_card_new_data *data) {
 251      c->preferred_input_port = data->preferred_input_port;
 252      c->preferred_output_port = data->preferred_output_port;
 253
 254 -    if (data->active_profile)
 255 -        if ((c->active_profile = pa_hashmap_get(c->profiles, data->active_profile)))
 256 -            c->save_profile = data->save_profile;
 257 +    pa_device_init_description(c->proplist, c);
 258 +    pa_device_init_icon(c->proplist, true);
 259 +    pa_device_init_intended_roles(c->proplist);
 260
 261 -    if (!c->active_profile) {
 262 -        PA_HASHMAP_FOREACH(profile, c->profiles, state) {
 263 -            if (profile->available == PA_AVAILABLE_NO)
 264 -                continue;
 265 +    return c;
 266 +}
 267
 268 -            if (!c->active_profile || profile->priority > c->active_profile->priority)
 269 -                c->active_profile = profile;
 270 -        }
 271 -        /* If all profiles are not available, then we still need to pick one */
 272 -        if (!c->active_profile) {
 273 -            PA_HASHMAP_FOREACH(profile, c->profiles, state)
 274 -                if (!c->active_profile || profile->priority > c->active_profile->priority)
 275 -                    c->active_profile = profile;
 276 +void pa_card_choose_initial_profile(pa_card *card) {
 277 +    pa_card_profile *profile;
 278 +    void *state;
 279 +    pa_card_profile *best = NULL;
 280 +
 281 +    pa_assert(card);
 282 +
 283 +    /* By default, pick the highest priority profile that is not unavailable,
 284 +     * or if all profiles are unavailable, pick the profile with the highest
 285 +     * priority regardless of its availability. */
 286 +
 287 +    PA_HASHMAP_FOREACH(profile, card->profiles, state) {
 288 +        if (profile->available == PA_AVAILABLE_NO)
 289 +            continue;
 290 +
 291 +        if (!best || profile->priority > best->priority)
 292 +            best = profile;
 293 +    }
 294 +
 295 +    if (!best) {
 296 +        PA_HASHMAP_FOREACH(profile, card->profiles, state) {
 297 +            if (!best || profile->priority > best->priority)
 298 +                best = profile;
 299          }
 300 -        pa_assert(c->active_profile);
 301      }
 302 +    pa_assert(best);
 303
 304 -    pa_device_init_description(c->proplist, c);
 305 -    pa_device_init_icon(c->proplist, true);
 306 -    pa_device_init_intended_roles(c->proplist);
 307 +    card->active_profile = best;
 308 +    card->save_profile = false;
 309
 310 -    pa_assert_se(pa_idxset_put(core->cards, c, &c->index) >= 0);
 311 +    /* Let policy modules override the default. */
 312 +    pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_CHOOSE_INITIAL_PROFILE], card);
 313 +}
 314
 315 -    pa_log_info("Created %u \"%s\"", c->index, c->name);
 316 -    pa_subscription_post(core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, c->index);
 317 +void pa_card_put(pa_card *card) {
 318 +    pa_assert(card);
 319
 320 -    pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_PUT], c);
 321 -    return c;
 322 +    pa_assert_se(pa_idxset_put(card->core->cards, card, &card->index) >= 0);
 323 +    card->linked = true;
 324 +
 325 +    pa_log_info("Created %u \"%s\"", card->index, card->name);
 326 +    pa_hook_fire(&card->core->hooks[PA_CORE_HOOK_CARD_PUT], card);
 327 +    pa_subscription_post(card->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_NEW, card->index);
 328  }
 329
 330  void pa_card_free(pa_card *c) {
 331 @@ -218,15 +236,15 @@ void pa_card_free(pa_card *c) {
 332
 333      core = c->core;
 334
 335 -    pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_UNLINK], c);
 336 -
 337 -    pa_namereg_unregister(core, c->name);
 338 -
 339 -    pa_idxset_remove_by_data(c->core->cards, c, NULL);
 340 +    if (c->linked) {
 341 +        pa_hook_fire(&core->hooks[PA_CORE_HOOK_CARD_UNLINK], c);
 342
 343 -    pa_log_info("Freed %u \"%s\"", c->index, c->name);
 344 +        pa_idxset_remove_by_data(c->core->cards, c, NULL);
 345 +        pa_log_info("Freed %u \"%s\"", c->index, c->name);
 346 +        pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_REMOVE, c->index);
 347 +    }
 348
 349 -    pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_REMOVE, c->index);
 350 +    pa_namereg_unregister(core, c->name);
 351
 352      pa_assert(pa_idxset_isempty(c->sinks));
 353      pa_idxset_free(c->sinks, NULL);
 354 @@ -298,20 +316,27 @@ int pa_card_set_profile(pa_card *c, pa_card_profile *profile, bool save) {
 355          return 0;
 356      }
 357
 358 -    if ((r = c->set_profile(c, profile)) < 0)
 359 +    /* If we're setting the initial profile, we shouldn't call set_profile(),
 360 +     * because the implementations don't expect that (for historical reasons).
 361 +     * We should just set c->active_profile, and the implementations will
 362 +     * properly set up that profile after pa_card_put() has returned. It would
 363 +     * be probably good to change this so that also the initial profile can be
 364 +     * set up in set_profile(), but if set_profile() fails, that would need
 365 +     * some better handling than what we do here currently. */
 366 +    if (c->linked && (r = c->set_profile(c, profile)) < 0)
 367          return r;
 368
 369 -    pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index);
 370 -
 371 -    pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name);
 372 -
 373      c->active_profile = profile;
 374      c->save_profile = save;
 375
 376      if (save)
 377          update_port_preferred_profile(c);
 378
 379 -    pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c);
 380 +    if (c->linked) {
 381 +        pa_log_info("Changed profile of card %u \"%s\" to %s", c->index, c->name, profile->name);
 382 +        pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_CARD_PROFILE_CHANGED], c);
 383 +        pa_subscription_post(c->core, PA_SUBSCRIPTION_EVENT_CARD|PA_SUBSCRIPTION_EVENT_CHANGE, c->index);
 384 +    }
 385
 386      return 0;
 387  }
 388 diff --git a/src/pulsecore/card.h b/src/pulsecore/card.h
 389 index d4970e3..fd1fe0a 100644
 390 --- a/src/pulsecore/card.h
 391 +++ b/src/pulsecore/card.h
 392 @@ -86,6 +86,8 @@ struct pa_card {
 393
 394      pa_suspend_cause_t suspend_cause;
 395
 396 +    bool linked;
 397 +
 398      void *userdata;
 399
 400      int (*set_profile)(pa_card *c, pa_card_profile *profile);
 401 @@ -128,6 +130,13 @@ void pa_card_new_data_set_preferred_port(pa_card_new_data *data, pa_direction_t
 402  void pa_card_new_data_done(pa_card_new_data *data);
 403
 404  pa_card *pa_card_new(pa_core *c, pa_card_new_data *data);
 405 +
 406 +/* Select the initial card profile according to the configured policies. This
 407 + * must be called between pa_card_new() and pa_card_put(), after the port and
 408 + * profile availablities have been initialized. */
 409 +void pa_card_choose_initial_profile(pa_card *card);
 410 +
 411 +void pa_card_put(pa_card *c);
 412  void pa_card_free(pa_card *c);
 413
 414  void pa_card_add_profile(pa_card *c, pa_card_profile *profile);
 415 diff --git a/src/pulsecore/core.h b/src/pulsecore/core.h
 416 index 00d7f2f..802111b 100644
 417 --- a/src/pulsecore/core.h
 418 +++ b/src/pulsecore/core.h
 419 @@ -118,6 +118,7 @@ typedef enum pa_core_hook {
 420      PA_CORE_HOOK_CLIENT_PROPLIST_CHANGED,
 421      PA_CORE_HOOK_CLIENT_SEND_EVENT,
 422      PA_CORE_HOOK_CARD_NEW,
 423 +    PA_CORE_HOOK_CARD_CHOOSE_INITIAL_PROFILE,
 424      PA_CORE_HOOK_CARD_PUT,
 425      PA_CORE_HOOK_CARD_UNLINK,
 426      PA_CORE_HOOK_CARD_PREFERRED_PORT_CHANGED,
 427 --
 428 2.8.1
 429