From 2fc7aea2dc6e4b58730af310b2930e810b41620f Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Tue, 19 Jan 2021 23:05:22 +0100 Subject: [PATCH 1/4] bluetooth: Do not re-retrieve codecs in switch_codec The desired list of capabilities is provided by the caller; if an endpoint is successfully selected that endpoint resides in this hashmap. Perhaps choose_remote_endpoint should return the key-value pair instead? --- src/modules/bluetooth/bluez5-util.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c index 5f941f03c..b0a16182c 100644 --- a/src/modules/bluetooth/bluez5-util.c +++ b/src/modules/bluetooth/bluez5-util.c @@ -436,7 +436,6 @@ bool pa_bluetooth_device_switch_codec(pa_bluetooth_device *device, pa_bluetooth_ uint8_t config[MAX_A2DP_CAPS_SIZE]; uint8_t config_size; bool is_a2dp_sink; - pa_hashmap *all_endpoints; char *pa_endpoint; const char *endpoint; @@ -451,13 +450,8 @@ bool pa_bluetooth_device_switch_codec(pa_bluetooth_device *device, pa_bluetooth_ is_a2dp_sink = profile == PA_BLUETOOTH_PROFILE_A2DP_SINK; - all_endpoints = NULL; - all_endpoints = pa_hashmap_get(is_a2dp_sink ? device->a2dp_sink_endpoints : device->a2dp_source_endpoints, - &endpoint_conf->id); - pa_assert(all_endpoints); - pa_assert_se(endpoint = endpoint_conf->choose_remote_endpoint(capabilities_hashmap, &device->discovery->core->default_sample_spec, is_a2dp_sink)); - pa_assert_se(capabilities = pa_hashmap_get(all_endpoints, endpoint)); + pa_assert_se(capabilities = pa_hashmap_get(capabilities_hashmap, endpoint)); config_size = endpoint_conf->fill_preferred_configuration(&device->discovery->core->default_sample_spec, capabilities->buffer, capabilities->size, config); From a10f0f3640fb4a52fe0c730d5d82c6dd5b4f6ace Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Wed, 20 Jan 2021 01:06:54 +0100 Subject: [PATCH 2/4] bluetooth: Allow switching from sink to source profile and vice versa --- src/modules/bluetooth/bluez5-util.c | 6 ++- src/modules/bluetooth/module-bluez5-device.c | 43 ++++++++++++++++---- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c index b0a16182c..e9b39d0cf 100644 --- a/src/modules/bluetooth/bluez5-util.c +++ b/src/modules/bluetooth/bluez5-util.c @@ -432,7 +432,7 @@ bool pa_bluetooth_device_switch_codec(pa_bluetooth_device *device, pa_bluetooth_ DBusMessageIter iter, dict; DBusMessage *m; struct switch_codec_data *data; - pa_a2dp_codec_capabilities *capabilities; + const pa_a2dp_codec_capabilities *capabilities; uint8_t config[MAX_A2DP_CAPS_SIZE]; uint8_t config_size; bool is_a2dp_sink; @@ -453,6 +453,8 @@ bool pa_bluetooth_device_switch_codec(pa_bluetooth_device *device, pa_bluetooth_ pa_assert_se(endpoint = endpoint_conf->choose_remote_endpoint(capabilities_hashmap, &device->discovery->core->default_sample_spec, is_a2dp_sink)); pa_assert_se(capabilities = pa_hashmap_get(capabilities_hashmap, endpoint)); + pa_log_info("Switching to codec %s @ %s", endpoint_conf->bt_codec.name, endpoint); + config_size = endpoint_conf->fill_preferred_configuration(&device->discovery->core->default_sample_spec, capabilities->buffer, capabilities->size, config); if (config_size == 0) @@ -462,7 +464,7 @@ bool pa_bluetooth_device_switch_codec(pa_bluetooth_device *device, pa_bluetooth_ endpoint_conf->bt_codec.name); pa_assert_se(m = dbus_message_new_method_call(BLUEZ_SERVICE, endpoint, - BLUEZ_MEDIA_ENDPOINT_INTERFACE, "SetConfiguration")); + BLUEZ_MEDIA_ENDPOINT_INTERFACE, "SetConfiguration")); dbus_message_iter_init_append(m, &iter); pa_assert_se(dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &pa_endpoint)); diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index 0e49843c3..0fb3a8b3d 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -2082,29 +2082,56 @@ static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro return cp; } +static void switch_codec_cb_handler(bool success, pa_bluetooth_profile_t profile, void *userdata); + /* Run from main thread */ static int set_profile_cb(pa_card *c, pa_card_profile *new_profile) { struct userdata *u; - pa_bluetooth_profile_t *p; + pa_bluetooth_profile_t p; pa_assert(c); pa_assert(new_profile); pa_assert_se(u = c->userdata); - p = PA_CARD_PROFILE_DATA(new_profile); + p = *(pa_bluetooth_profile_t *)PA_CARD_PROFILE_DATA(new_profile); - if (*p != PA_BLUETOOTH_PROFILE_OFF) { + stop_thread(u); + + if (p != PA_BLUETOOTH_PROFILE_OFF) { const pa_bluetooth_device *d = u->device; - if (!d->transports[*p] || d->transports[*p]->state <= PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED) { - pa_log_warn("Refused to switch profile to %s: Not connected", new_profile->name); + if (!d->transports[p] || d->transports[p]->state <= PA_BLUETOOTH_TRANSPORT_STATE_DISCONNECTED) { + if (p != PA_BLUETOOTH_PROFILE_A2DP_SINK && p != PA_BLUETOOTH_PROFILE_A2DP_SOURCE) { + pa_log_warn("Refused to switch profile to %s: Not connected", new_profile->name); + return -PA_ERR_IO; + } + + bool is_a2dp_sink = p == PA_BLUETOOTH_PROFILE_A2DP_SINK; + + for (int i = 0; i < pa_bluetooth_a2dp_endpoint_conf_count(); ++i) { + pa_hashmap *capabilities_hashmap; + const pa_a2dp_endpoint_conf *endpoint_conf = pa_bluetooth_a2dp_endpoint_conf_iter(i); + + // Capabilities should already be filtered by this! + // if (!pa_bluetooth_a2dp_codec_is_codec_available(&endpoint_conf->id, is_a2dp_sink)) + // continue; + + capabilities_hashmap = pa_hashmap_get(is_a2dp_sink ? u->device->a2dp_sink_endpoints : u->device->a2dp_source_endpoints, &endpoint_conf->id); + if (!capabilities_hashmap) + continue; + + if (!pa_bluetooth_device_switch_codec(u->device, p, capabilities_hashmap, endpoint_conf, switch_codec_cb_handler, u)) + continue; + + return PA_OK; + } + + pa_log_warn("Refused to switch profile to %s: ", new_profile->name); return -PA_ERR_IO; } } - stop_thread(u); - - u->profile = *p; + u->profile = p; if (u->profile != PA_BLUETOOTH_PROFILE_OFF) if (init_profile(u) < 0) From 3f0e843bc862b5bce28b768da82d440c2f0c357a Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Thu, 28 Jan 2021 22:25:19 +0100 Subject: [PATCH 3/4] bluetooth: Mark profile available when the peer supports it --- src/modules/bluetooth/module-bluez5-device.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index 0fb3a8b3d..430574429 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -2074,10 +2074,7 @@ static pa_card_profile *create_card_profile(struct userdata *u, pa_bluetooth_pro *p = profile; - if (u->device->transports[*p]) - cp->available = transport_state_to_availability(u->device->transports[*p]->state); - else - cp->available = PA_AVAILABLE_NO; + cp->available = pa_bluetooth_device_supports_profile(u->device, profile) ? PA_AVAILABLE_YES : PA_AVAILABLE_NO; return cp; } @@ -2277,13 +2274,8 @@ static void handle_transport_state_change(struct userdata *u, struct pa_bluetoot pa_assert(t); pa_assert_se(cp = pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile))); + // TODO: Makes no sense to check this anymore, but maybe that breaks the boolean? oldavail = cp->available; - /* - * If codec switching is in progress, transport state change should not - * make profile unavailable. - */ - if (!t->device->codec_switching_in_progress) - pa_card_profile_set_available(cp, transport_state_to_availability(t->state)); /* Update port availability */ pa_assert_se(port = pa_hashmap_get(u->card->ports, u->output_port_name)); From ac36551888a327bda284264d28e9f5fc05b04e33 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Thu, 28 Jan 2021 23:14:27 +0100 Subject: [PATCH 4/4] BROKEN: bluetooth: Update card profile when active transport changes --- src/modules/bluetooth/module-bluez5-device.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/modules/bluetooth/module-bluez5-device.c b/src/modules/bluetooth/module-bluez5-device.c index 430574429..5fcba90c8 100644 --- a/src/modules/bluetooth/module-bluez5-device.c +++ b/src/modules/bluetooth/module-bluez5-device.c @@ -1341,6 +1341,8 @@ static int setup_transport(struct userdata *u) { return -1; /* We need to fail here until the interactions with module-suspend-on-idle and alike get improved */ } + pa_assert_se(pa_card_set_profile(u->card, pa_hashmap_get(u->card->profiles, pa_bluetooth_profile_to_string(t->profile)), false) >= 0); + return transport_config(u); } @@ -2092,6 +2094,11 @@ static int set_profile_cb(pa_card *c, pa_card_profile *new_profile) { p = *(pa_bluetooth_profile_t *)PA_CARD_PROFILE_DATA(new_profile); + if (p == u->profile) { + pa_log_debug("Profile %s already selected", pa_bluetooth_profile_to_string(p)); + return 0; + } + stop_thread(u); if (p != PA_BLUETOOTH_PROFILE_OFF) {