From 2dea448709a6b840c1758302a56bdf5e31b78f07 Mon Sep 17 00:00:00 2001 From: buque Date: Mon, 19 Jun 2023 15:33:41 +0800 Subject: [PATCH] alsa-ucm: Make modifiers track conflicting/supported devices as idxsets.Add enable, disable, status helpers for devices.Set profiles by their struct instance, not their name Modifiers currently keep their conflicting and supported devices's names, and these names are resolved to devices every time we need to use them. Instead, resolve these device names while creating the modifier struct and keep track of the resulting device structs in idxsets, same as how device structs keep track of their support relations. Right now manipulating device status is done inline once while setting a port. However, we will need to reuse this code to disable conflicting devices of a device we want to enable. Split it into enable and disable helper functions. There is another issue with the device enable logic, where trying to disabling an already disabled device sometimes fails. To avoid that, implement a status helper and check if the device we want to enable is already enabled/disabled before trying to do so. While switching profiles, it's possible that we will want to do more work besides switching UCM verbs. The alsa-card module already has our profiles as structs, but passes in only the names instead of the entire struct. Make things work with the struct instead, so we can add other things (like a UCM context) to it and use those here. --- ...ble-disable-status-helpers-for-devic.patch | 139 +++++++++++++ ...difiers-track-conflicting-supported-.patch | 184 ++++++++++++++++++ ...files-by-their-struct-instance-not-t.patch | 102 ++++++++++ pulseaudio.spec | 15 +- 4 files changed, 439 insertions(+), 1 deletion(-) create mode 100644 0001-alsa-ucm-Add-enable-disable-status-helpers-for-devic.patch create mode 100644 0001-alsa-ucm-Make-modifiers-track-conflicting-supported-.patch create mode 100644 0001-alsa-ucm-Set-profiles-by-their-struct-instance-not-t.patch diff --git a/0001-alsa-ucm-Add-enable-disable-status-helpers-for-devic.patch b/0001-alsa-ucm-Add-enable-disable-status-helpers-for-devic.patch new file mode 100644 index 0000000..7175910 --- /dev/null +++ b/0001-alsa-ucm-Add-enable-disable-status-helpers-for-devic.patch @@ -0,0 +1,139 @@ +From c83b34516929214876bd2c46b5f346e53a3adcf7 Mon Sep 17 00:00:00 2001 +From: Alper Nebi Yasak +Date: Thu, 24 Jun 2021 09:11:59 +0300 +Subject: [PATCH] alsa-ucm: Add enable, disable, status helpers for devices + +Right now manipulating device status is done inline once while setting a +port. However, we will need to reuse this code to disable conflicting +devices of a device we want to enable. Split it into enable and disable +helper functions. + +There is another issue with the device enable logic, where trying to +disabling an already disabled device sometimes fails. To avoid that, +implement a status helper and check if the device we want to enable is +already enabled/disabled before trying to do so. + +Signed-off-by: Alper Nebi Yasak +Part-of: +Signed-off-by: buque +--- + src/modules/alsa/alsa-ucm.c | 81 ++++++++++++++++++++++++++++--------- + 1 file changed, 63 insertions(+), 18 deletions(-) + +diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c +index 858aa011a..7b857785d 100644 +--- a/src/modules/alsa/alsa-ucm.c ++++ b/src/modules/alsa/alsa-ucm.c +@@ -631,6 +631,59 @@ static int ucm_get_devices(pa_alsa_ucm_verb *verb, snd_use_case_mgr_t *uc_mgr) { + return 0; + }; + ++static long ucm_device_status(pa_alsa_ucm_config *ucm, pa_alsa_ucm_device *dev) { ++ const char *dev_name = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_NAME); ++ char *devstatus; ++ long status = 0; ++ ++ devstatus = pa_sprintf_malloc("_devstatus/%s", dev_name); ++ if (snd_use_case_geti(ucm->ucm_mgr, devstatus, &status) < 0) { ++ pa_log_debug("Failed to get status for UCM device %s", dev_name); ++ status = -1; ++ } ++ pa_xfree(devstatus); ++ ++ return status; ++} ++ ++static int ucm_device_disable(pa_alsa_ucm_config *ucm, pa_alsa_ucm_device *dev) { ++ const char *dev_name = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_NAME); ++ ++ /* If any of dev's conflicting devices is enabled, trying to disable ++ * dev gives an error despite the fact that it's already disabled. ++ * Check that dev is enabled to avoid this error. */ ++ if (ucm_device_status(ucm, dev) == 0) { ++ pa_log_debug("UCM device %s is already disabled", dev_name); ++ return 0; ++ } ++ ++ pa_log_debug("Disabling UCM device %s", dev_name); ++ if (snd_use_case_set(ucm->ucm_mgr, "_disdev", dev_name) < 0) { ++ pa_log("Failed to disable UCM device %s", dev_name); ++ return -1; ++ } ++ ++ return 0; ++} ++ ++static int ucm_device_enable(pa_alsa_ucm_config *ucm, pa_alsa_ucm_device *dev) { ++ const char *dev_name = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_NAME); ++ ++ /* We don't need to enable devices that are already enabled */ ++ if (ucm_device_status(ucm, dev) > 0) { ++ pa_log_debug("UCM device %s is already enabled", dev_name); ++ return 0; ++ } ++ ++ pa_log_debug("Enabling UCM device %s", dev_name); ++ if (snd_use_case_set(ucm->ucm_mgr, "_enadev", dev_name) < 0) { ++ pa_log("Failed to enable UCM device %s", dev_name); ++ return -1; ++ } ++ ++ return 0; ++} ++ + static int ucm_get_modifiers(pa_alsa_ucm_verb *verb, snd_use_case_mgr_t *uc_mgr) { + const char **mod_list; + int num_mod, i; +@@ -1417,7 +1470,7 @@ int pa_alsa_ucm_set_port(pa_alsa_ucm_mapping_context *context, pa_device_port *p + int i; + int ret = 0; + pa_alsa_ucm_config *ucm; +- const char **enable_devs; ++ pa_alsa_ucm_device **enable_devs; + int enable_num = 0; + uint32_t idx; + pa_alsa_ucm_device *dev; +@@ -1427,31 +1480,23 @@ int pa_alsa_ucm_set_port(pa_alsa_ucm_mapping_context *context, pa_device_port *p + ucm = context->ucm; + pa_assert(ucm->ucm_mgr); + +- enable_devs = pa_xnew(const char *, pa_idxset_size(context->ucm_devices)); ++ enable_devs = pa_xnew(pa_alsa_ucm_device *, pa_idxset_size(context->ucm_devices)); + + /* first disable then enable */ + PA_IDXSET_FOREACH(dev, context->ucm_devices, idx) { + const char *dev_name = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_NAME); + + if (ucm_port_contains(port->name, dev_name, is_sink)) +- enable_devs[enable_num++] = dev_name; +- else { +- pa_log_debug("Disable ucm device %s", dev_name); +- if (snd_use_case_set(ucm->ucm_mgr, "_disdev", dev_name) > 0) { +- pa_log("Failed to disable ucm device %s", dev_name); +- ret = -1; +- break; +- } +- } +- } ++ enable_devs[enable_num++] = dev; ++ else ++ ret = ucm_device_disable(ucm, dev); + +- for (i = 0; i < enable_num; i++) { +- pa_log_debug("Enable ucm device %s", enable_devs[i]); +- if (snd_use_case_set(ucm->ucm_mgr, "_enadev", enable_devs[i]) < 0) { +- pa_log("Failed to enable ucm device %s", enable_devs[i]); +- ret = -1; ++ if (ret < 0) + break; +- } ++ } ++ ++ for (i = 0; i < enable_num && ret == 0; i++) { ++ ret = ucm_device_enable(ucm, enable_devs[i]); + } + + pa_xfree(enable_devs); +-- +2.33.0 + diff --git a/0001-alsa-ucm-Make-modifiers-track-conflicting-supported-.patch b/0001-alsa-ucm-Make-modifiers-track-conflicting-supported-.patch new file mode 100644 index 0000000..49d518b --- /dev/null +++ b/0001-alsa-ucm-Make-modifiers-track-conflicting-supported-.patch @@ -0,0 +1,184 @@ +From 9b06e8fef43b161b85bf860e39a73492cd02bfb5 Mon Sep 17 00:00:00 2001 +From: Alper Nebi Yasak +Date: Thu, 24 Jun 2021 08:40:03 +0300 +Subject: [PATCH] alsa-ucm: Make modifiers track conflicting/supported devices + as idxsets + +Modifiers currently keep their conflicting and supported devices's +names, and these names are resolved to devices every time we need to use +them. Instead, resolve these device names while creating the modifier +struct and keep track of the resulting device structs in idxsets, same +as how device structs keep track of their support relations. + +Signed-off-by: Alper Nebi Yasak +Part-of: +Signed-off-by: buque +--- + src/modules/alsa/alsa-ucm.c | 71 +++++++++++++++++++++---------------- + src/modules/alsa/alsa-ucm.h | 7 ++-- + 2 files changed, 42 insertions(+), 36 deletions(-) + +diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c +index 3e23bc416..858aa011a 100644 +--- a/src/modules/alsa/alsa-ucm.c ++++ b/src/modules/alsa/alsa-ucm.c +@@ -547,10 +547,16 @@ static int ucm_get_device_property( + }; + + /* Create a property list for this ucm modifier */ +-static int ucm_get_modifier_property(pa_alsa_ucm_modifier *modifier, snd_use_case_mgr_t *uc_mgr, const char *modifier_name) { ++static int ucm_get_modifier_property( ++ pa_alsa_ucm_modifier *modifier, ++ snd_use_case_mgr_t *uc_mgr, ++ pa_alsa_ucm_verb *verb, ++ const char *modifier_name) { + const char *value; + char *id; + int i; ++ const char **devices; ++ int n_confdev, n_suppdev; + + for (i = 0; item[i].id; i++) { + int err; +@@ -567,16 +573,28 @@ static int ucm_get_modifier_property(pa_alsa_ucm_modifier *modifier, snd_use_cas + } + + id = pa_sprintf_malloc("%s/%s", "_conflictingdevs", modifier_name); +- modifier->n_confdev = snd_use_case_get_list(uc_mgr, id, &modifier->conflicting_devices); ++ n_confdev = snd_use_case_get_list(uc_mgr, id, &devices); + pa_xfree(id); +- if (modifier->n_confdev < 0) ++ ++ modifier->conflicting_devices = pa_idxset_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); ++ if (n_confdev <= 0) + pa_log_debug("No %s for modifier %s", "_conflictingdevs", modifier_name); ++ else { ++ ucm_add_devices_to_idxset(modifier->conflicting_devices, NULL, verb->devices, devices, n_confdev); ++ snd_use_case_free_list(devices, n_confdev); ++ } + + id = pa_sprintf_malloc("%s/%s", "_supporteddevs", modifier_name); +- modifier->n_suppdev = snd_use_case_get_list(uc_mgr, id, &modifier->supported_devices); ++ n_suppdev = snd_use_case_get_list(uc_mgr, id, &devices); + pa_xfree(id); +- if (modifier->n_suppdev < 0) ++ ++ modifier->supported_devices = pa_idxset_new(pa_idxset_trivial_hash_func, pa_idxset_trivial_compare_func); ++ if (n_suppdev <= 0) + pa_log_debug("No %s for modifier %s", "_supporteddevs", modifier_name); ++ else { ++ ucm_add_devices_to_idxset(modifier->supported_devices, NULL, verb->devices, devices, n_suppdev); ++ snd_use_case_free_list(devices, n_suppdev); ++ } + + return 0; + }; +@@ -659,23 +677,15 @@ static void add_role_to_device(pa_alsa_ucm_device *dev, const char *dev_name, co + role_name)); + } + +-static void add_media_role(const char *name, pa_alsa_ucm_device *list, const char *role_name, const char *role, bool is_sink) { +- pa_alsa_ucm_device *d; +- +- PA_LLIST_FOREACH(d, list) { +- const char *dev_name = pa_proplist_gets(d->proplist, PA_ALSA_PROP_UCM_NAME); +- +- if (pa_streq(dev_name, name)) { +- const char *sink = pa_proplist_gets(d->proplist, PA_ALSA_PROP_UCM_SINK); +- const char *source = pa_proplist_gets(d->proplist, PA_ALSA_PROP_UCM_SOURCE); ++static void add_media_role(pa_alsa_ucm_device *dev, const char *role_name, const char *role, bool is_sink) { ++ const char *dev_name = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_NAME); ++ const char *sink = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_SINK); ++ const char *source = pa_proplist_gets(dev->proplist, PA_ALSA_PROP_UCM_SOURCE); + +- if (is_sink && sink) +- add_role_to_device(d, dev_name, role_name, role); +- else if (!is_sink && source) +- add_role_to_device(d, dev_name, role_name, role); +- break; +- } +- } ++ if (is_sink && sink) ++ add_role_to_device(dev, dev_name, role_name, role); ++ else if (!is_sink && source) ++ add_role_to_device(dev, dev_name, role_name, role); + } + + static char *modifier_name_to_role(const char *mod_name, bool *is_sink) { +@@ -704,11 +714,12 @@ static char *modifier_name_to_role(const char *mod_name, bool *is_sink) { + return sub; + } + +-static void ucm_set_media_roles(pa_alsa_ucm_modifier *modifier, pa_alsa_ucm_device *list, const char *mod_name) { +- int i; ++static void ucm_set_media_roles(pa_alsa_ucm_modifier *modifier, const char *mod_name) { ++ pa_alsa_ucm_device *dev; + bool is_sink = false; + char *sub = NULL; + const char *role_name; ++ uint32_t idx; + + sub = modifier_name_to_role(mod_name, &is_sink); + if (!sub) +@@ -718,11 +729,11 @@ static void ucm_set_media_roles(pa_alsa_ucm_modifier *modifier, pa_alsa_ucm_devi + modifier->media_role = sub; + + role_name = is_sink ? PA_ALSA_PROP_UCM_PLAYBACK_ROLES : PA_ALSA_PROP_UCM_CAPTURE_ROLES; +- for (i = 0; i < modifier->n_suppdev; i++) { ++ PA_IDXSET_FOREACH(dev, modifier->supported_devices, idx) { + /* if modifier has no specific pcm, we add role intent to its supported devices */ + if (!pa_proplist_gets(modifier->proplist, PA_ALSA_PROP_UCM_SINK) && + !pa_proplist_gets(modifier->proplist, PA_ALSA_PROP_UCM_SOURCE)) +- add_media_role(modifier->supported_devices[i], list, role_name, sub, is_sink); ++ add_media_role(dev, role_name, sub, is_sink); + } + } + +@@ -879,11 +890,11 @@ int pa_alsa_ucm_get_verb(snd_use_case_mgr_t *uc_mgr, const char *verb_name, cons + const char *mod_name = pa_proplist_gets(mod->proplist, PA_ALSA_PROP_UCM_NAME); + + /* Modifier properties */ +- ucm_get_modifier_property(mod, uc_mgr, mod_name); ++ ucm_get_modifier_property(mod, uc_mgr, verb, mod_name); + + /* Set PA_PROP_DEVICE_INTENDED_ROLES property to devices */ + pa_log_debug("Set media roles for verb %s, modifier %s", verb_name, mod_name); +- ucm_set_media_roles(mod, verb->devices, mod_name); ++ ucm_set_media_roles(mod, mod_name); + } + + *p_verb = verb; +@@ -2116,10 +2127,8 @@ static void free_verb(pa_alsa_ucm_verb *verb) { + PA_LLIST_FOREACH_SAFE(mi, mn, verb->modifiers) { + PA_LLIST_REMOVE(pa_alsa_ucm_modifier, verb->modifiers, mi); + pa_proplist_free(mi->proplist); +- if (mi->n_suppdev > 0) +- snd_use_case_free_list(mi->supported_devices, mi->n_suppdev); +- if (mi->n_confdev > 0) +- snd_use_case_free_list(mi->conflicting_devices, mi->n_confdev); ++ pa_idxset_free(mi->conflicting_devices, NULL); ++ pa_idxset_free(mi->supported_devices, NULL); + pa_xfree(mi->media_role); + pa_xfree(mi); + } +diff --git a/src/modules/alsa/alsa-ucm.h b/src/modules/alsa/alsa-ucm.h +index 255bc0d5a..cb72837de 100644 +--- a/src/modules/alsa/alsa-ucm.h ++++ b/src/modules/alsa/alsa-ucm.h +@@ -221,11 +221,8 @@ struct pa_alsa_ucm_modifier { + + pa_proplist *proplist; + +- int n_confdev; +- int n_suppdev; +- +- const char **conflicting_devices; +- const char **supported_devices; ++ pa_idxset *conflicting_devices; ++ pa_idxset *supported_devices; + + pa_direction_t action_direction; + +-- +2.33.0 + diff --git a/0001-alsa-ucm-Set-profiles-by-their-struct-instance-not-t.patch b/0001-alsa-ucm-Set-profiles-by-their-struct-instance-not-t.patch new file mode 100644 index 0000000..3c0a929 --- /dev/null +++ b/0001-alsa-ucm-Set-profiles-by-their-struct-instance-not-t.patch @@ -0,0 +1,102 @@ +From 880ff393f13ddc497dc9f9edff16050a02656fbd Mon Sep 17 00:00:00 2001 +From: Jaroslav Kysela +Date: Sun, 6 Jun 2021 22:06:46 +0300 +Subject: [PATCH] alsa-ucm: Set profiles by their struct instance, not their + name + +While switching profiles, it's possible that we will want to do more +work besides switching UCM verbs. The alsa-card module already has our +profiles as structs, but passes in only the names instead of the entire +struct. Make things work with the struct instead, so we can add other +things (like a UCM context) to it and use those here. + +Co-authored-by: Tanu Kaskinen +[Alper: Split into its own commit and integrated Tanu's snippet.] + +Signed-off-by: Alper Nebi Yasak +Part-of: +Signed-off-by: buque +--- + src/modules/alsa/alsa-ucm.c | 15 +++++++-------- + src/modules/alsa/alsa-ucm.h | 2 +- + src/modules/alsa/module-alsa-card.c | 5 ++--- + 3 files changed, 10 insertions(+), 12 deletions(-) + +diff --git a/src/modules/alsa/alsa-ucm.c b/src/modules/alsa/alsa-ucm.c +index 7b857785d..a40c3d764 100644 +--- a/src/modules/alsa/alsa-ucm.c ++++ b/src/modules/alsa/alsa-ucm.c +@@ -1430,19 +1430,18 @@ void pa_alsa_ucm_add_ports( + } + + /* Change UCM verb and device to match selected card profile */ +-int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, const char *new_profile, const char *old_profile) { ++int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, pa_alsa_profile *new_profile, pa_alsa_profile *old_profile) { + int ret = 0; + const char *profile; + pa_alsa_ucm_verb *verb; + + if (new_profile == old_profile) +- return ret; +- else if (new_profile == NULL || old_profile == NULL) +- profile = new_profile ? new_profile : SND_USE_CASE_VERB_INACTIVE; +- else if (!pa_streq(new_profile, old_profile)) +- profile = new_profile; ++ return 0; ++ ++ if (new_profile == NULL) ++ profile = SND_USE_CASE_VERB_INACTIVE; + else +- return ret; ++ profile = new_profile->name; + + /* change verb */ + pa_log_info("Set UCM verb to %s", profile); +@@ -2430,7 +2429,7 @@ pa_alsa_profile_set* pa_alsa_ucm_add_profile_set(pa_alsa_ucm_config *ucm, pa_cha + return NULL; + } + +-int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, const char *new_profile, const char *old_profile) { ++int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, pa_alsa_profile *new_profile, pa_alsa_profile *old_profile) { + return -1; + } + +diff --git a/src/modules/alsa/alsa-ucm.h b/src/modules/alsa/alsa-ucm.h +index cb72837de..e411a9262 100644 +--- a/src/modules/alsa/alsa-ucm.h ++++ b/src/modules/alsa/alsa-ucm.h +@@ -145,7 +145,7 @@ typedef struct pa_alsa_ucm_volume pa_alsa_ucm_volume; + + int pa_alsa_ucm_query_profiles(pa_alsa_ucm_config *ucm, int card_index); + pa_alsa_profile_set* pa_alsa_ucm_add_profile_set(pa_alsa_ucm_config *ucm, pa_channel_map *default_channel_map); +-int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, const char *new_profile, const char *old_profile); ++int pa_alsa_ucm_set_profile(pa_alsa_ucm_config *ucm, pa_card *card, pa_alsa_profile *new_profile, pa_alsa_profile *old_profile); + + int pa_alsa_ucm_get_verb(snd_use_case_mgr_t *uc_mgr, const char *verb_name, const char *verb_desc, pa_alsa_ucm_verb **p_verb); + +diff --git a/src/modules/alsa/module-alsa-card.c b/src/modules/alsa/module-alsa-card.c +index 0fe8893f4..2efa96a0f 100644 +--- a/src/modules/alsa/module-alsa-card.c ++++ b/src/modules/alsa/module-alsa-card.c +@@ -249,8 +249,7 @@ static int card_set_profile(pa_card *c, pa_card_profile *new_profile) { + + /* if UCM is available for this card then update the verb */ + if (u->use_ucm) { +- if (pa_alsa_ucm_set_profile(&u->ucm, c, nd->profile ? nd->profile->name : NULL, +- od->profile ? od->profile->name : NULL) < 0) { ++ if (pa_alsa_ucm_set_profile(&u->ucm, c, nd->profile, od->profile) < 0) { + ret = -1; + goto finish; + } +@@ -302,7 +301,7 @@ static void init_profile(struct userdata *u) { + + if (d->profile && u->use_ucm) { + /* Set initial verb */ +- if (pa_alsa_ucm_set_profile(ucm, u->card, d->profile->name, NULL) < 0) { ++ if (pa_alsa_ucm_set_profile(ucm, u->card, d->profile, NULL) < 0) { + pa_log("Failed to set ucm profile %s", d->profile->name); + return; + } +-- +2.33.0 + diff --git a/pulseaudio.spec b/pulseaudio.spec index 87ff959..30fb59c 100644 --- a/pulseaudio.spec +++ b/pulseaudio.spec @@ -21,6 +21,10 @@ Patch1004: 0001-idxset-Add-set-contains-function.patch Patch1005: 0002-idxset-Add-set-comparison-operations.patch Patch1006: 0003-idxset-Add-reverse-iteration-functions.patch Patch1007: 0001-alsa-ucm-Always-create-device-conflicting-supported-.patch +Patch1008: 0001-alsa-ucm-Make-modifiers-track-conflicting-supported-.patch +Patch1009: 0001-alsa-ucm-Add-enable-disable-status-helpers-for-devic.patch +Patch1010: 0001-alsa-ucm-Set-profiles-by-their-struct-instance-not-t.patch + BuildRequires: meson BuildRequires: automake libtool gcc-c++ bash-completion @@ -248,7 +252,16 @@ exit 0 %{_mandir}/man*/* %changelog -* Thu April 27 2023 wuxu - 16.1-9 +* Mon Jun 19 2023 wuxu - 16.1-12 +- alsa-ucm: Set profiles by their struct instance, not their name + +* Mon Jun 19 2023 wuxu - 16.1-11 +- alsa-ucm: Add enable, disable, status helpers for devices + +* Mon Jun 19 2023 wuxu - 16.1-10 +- alsa-ucm: Make modifiers track conflicting/supported devices as idxsets + +* Thu Apr 27 2023 wuxu - 16.1-9 - alsa-ucm: Always create device conflicting/supported device idxsets * Tue Mar 14 2023 peijiankang - 16.1-8 -- Gitee