From 2bf48487cb87e182b0fc8333509debe6350aac73 Mon Sep 17 00:00:00 2001 From: Pauli Virtanen Date: Thu, 5 Dec 2024 19:58:20 +0200 Subject: [PATCH] bluez5: handle A2DP inverted ClearConfiguration/SetConfiguration order When in A2DP sink role and remote end switches codec, BlueZ nowadays appears sometimes emit first SetConfiguration (creating new transport), and then ClearConfiguration (removing old transport). Handle this case: emit profiles_changed event always when transports come/go. Redefine profiles_changed() to take bitmask of profiles whose connection status changed, so we don't need to emit two remove+add events. --- spa/plugins/bluez5/bluez5-dbus.c | 18 +++++------ spa/plugins/bluez5/bluez5-device.c | 49 ++++++++++++++++++------------ spa/plugins/bluez5/defs.h | 2 +- 3 files changed, 37 insertions(+), 32 deletions(-) diff --git a/spa/plugins/bluez5/bluez5-dbus.c b/spa/plugins/bluez5/bluez5-dbus.c index c86a62521..d0e637758 100644 --- a/spa/plugins/bluez5/bluez5-dbus.c +++ b/spa/plugins/bluez5/bluez5-dbus.c @@ -2094,13 +2094,11 @@ static void device_update_set_status(struct spa_bt_device *device, bool force, c int spa_bt_device_connect_profile(struct spa_bt_device *device, enum spa_bt_profile profile) { - uint32_t prev_connected = device->connected_profiles; device->connected_profiles |= profile; if (profile & SPA_BT_PROFILE_BAP_DUPLEX) device_update_set_status(device, true, NULL); spa_bt_device_check_profiles(device, false); - if (device->connected_profiles != prev_connected) - spa_bt_device_emit_profiles_changed(device, device->profiles, prev_connected); + spa_bt_device_emit_profiles_changed(device, profile); return 0; } @@ -2458,8 +2456,7 @@ static int device_update_props(struct spa_bt_device *device, } if (device->profiles != prev_profiles) - spa_bt_device_emit_profiles_changed( - device, prev_profiles, device->connected_profiles); + spa_bt_device_emit_profiles_changed(device, 0); } else if (spa_streq(key, "Sets")) { device_update_device_sets_prop(device, &it[1]); @@ -2943,7 +2940,6 @@ void spa_bt_transport_free(struct spa_bt_transport *transport) if (device) { struct spa_bt_transport *t; uint32_t disconnected = transport->profile; - uint32_t prev_connected = device->connected_profiles; spa_list_remove(&transport->device_link); @@ -2953,8 +2949,8 @@ void spa_bt_transport_free(struct spa_bt_transport *transport) if (transport->profile & SPA_BT_PROFILE_BAP_DUPLEX) device_update_set_status(device, true, NULL); - if (device->connected_profiles != prev_connected) - spa_bt_device_emit_profiles_changed(device, device->profiles, prev_connected); + + spa_bt_device_emit_profiles_changed(device, transport->profile); } spa_list_remove(&transport->bap_transport_linked); @@ -5568,7 +5564,7 @@ static void interface_added(struct spa_bt_monitor *monitor, d = ep->device; if (d) - spa_bt_device_emit_profiles_changed(d, d->profiles, d->connected_profiles); + spa_bt_device_emit_profiles_changed(d, 0); if (spa_streq(ep->uuid, SPA_BT_UUID_BAP_BROADCAST_SINK)) { int ret, i; @@ -5662,7 +5658,7 @@ static void interfaces_removed(struct spa_bt_monitor *monitor, DBusMessageIter * struct spa_bt_device *d = ep->device; remote_endpoint_free(ep); if (d) - spa_bt_device_emit_profiles_changed(d, d->profiles, d->connected_profiles); + spa_bt_device_emit_profiles_changed(d, 0); } } else if (spa_streq(interface_name, BLUEZ_MEDIA_TRANSPORT_INTERFACE)) { struct spa_bt_transport *transport; @@ -5930,7 +5926,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us d = ep->device; if (d) - spa_bt_device_emit_profiles_changed(d, d->profiles, d->connected_profiles); + spa_bt_device_emit_profiles_changed(d, 0); } else if (spa_streq(iface, BLUEZ_MEDIA_TRANSPORT_INTERFACE)) { struct spa_bt_transport *transport; diff --git a/spa/plugins/bluez5/bluez5-device.c b/spa/plugins/bluez5/bluez5-device.c index 9f6872b51..3625bd7b5 100644 --- a/spa/plugins/bluez5/bluez5-device.c +++ b/spa/plugins/bluez5/bluez5-device.c @@ -1376,20 +1376,29 @@ static void codec_switched(void *userdata, int status) emit_info(this, false); } -static void profiles_changed(void *userdata, uint32_t prev_profiles, uint32_t prev_connected_profiles) +static bool device_set_needs_update(struct impl *this) +{ + struct device_set dset = { .impl = this }; + bool changed; + + if (this->profile != DEVICE_PROFILE_BAP) + return false; + + device_set_update(this, &dset); + changed = !device_set_equal(&dset, &this->device_set); + device_set_clear(this, &dset); + return changed; +} + +static void profiles_changed(void *userdata, uint32_t connected_change) { struct impl *this = userdata; - uint32_t connected_change; bool nodes_changed = false; - connected_change = (this->bt_dev->connected_profiles ^ prev_connected_profiles); - /* Profiles changed. We have to re-emit device information. */ - spa_log_info(this->log, "profiles changed to %08x %08x (prev %08x %08x, change %08x)" - " switching_codec:%d", - this->bt_dev->profiles, this->bt_dev->connected_profiles, - prev_profiles, prev_connected_profiles, connected_change, - this->switching_codec); + spa_log_info(this->log, "profiles changed to %08x %08x (change %08x) switching_codec:%d", + this->bt_dev->profiles, this->bt_dev->connected_profiles, + connected_change, this->switching_codec); if (this->switching_codec) return; @@ -1405,15 +1414,21 @@ static void profiles_changed(void *userdata, uint32_t prev_profiles, uint32_t pr break; case DEVICE_PROFILE_AG: nodes_changed = (connected_change & (SPA_BT_PROFILE_HEADSET_AUDIO_GATEWAY | - SPA_BT_PROFILE_MEDIA_SOURCE)); + SPA_BT_PROFILE_A2DP_SOURCE)); spa_log_debug(this->log, "profiles changed: AG nodes changed: %d", nodes_changed); break; case DEVICE_PROFILE_A2DP: + nodes_changed = (connected_change & SPA_BT_PROFILE_A2DP_DUPLEX); + spa_log_debug(this->log, "profiles changed: A2DP nodes changed: %d", + nodes_changed); + break; case DEVICE_PROFILE_BAP: - nodes_changed = (connected_change & (SPA_BT_PROFILE_MEDIA_SINK | - SPA_BT_PROFILE_MEDIA_SOURCE)); - spa_log_debug(this->log, "profiles changed: media nodes changed: %d", + nodes_changed = ((connected_change & SPA_BT_PROFILE_BAP_DUPLEX) + && device_set_needs_update(this)) + || (connected_change & (SPA_BT_PROFILE_BAP_BROADCAST_SINK | + SPA_BT_PROFILE_BAP_BROADCAST_SOURCE)); + spa_log_debug(this->log, "profiles changed: BAP nodes changed: %d", nodes_changed); break; case DEVICE_PROFILE_HSP_HFP: @@ -1441,17 +1456,11 @@ static void profiles_changed(void *userdata, uint32_t prev_profiles, uint32_t pr static void device_set_changed(void *userdata) { struct impl *this = userdata; - struct device_set dset = { .impl = this }; - bool changed; if (this->profile != DEVICE_PROFILE_BAP) return; - device_set_update(this, &dset); - changed = !device_set_equal(&dset, &this->device_set); - device_set_clear(this, &dset); - - if (!changed) { + if (!device_set_needs_update(this)) { spa_log_debug(this->log, "%p: device set not changed", this); return; } diff --git a/spa/plugins/bluez5/defs.h b/spa/plugins/bluez5/defs.h index 2069cb9ab..4249488c8 100644 --- a/spa/plugins/bluez5/defs.h +++ b/spa/plugins/bluez5/defs.h @@ -468,7 +468,7 @@ struct spa_bt_device_events { void (*codec_switched) (void *data, int status); /** Profile configuration changed */ - void (*profiles_changed) (void *data, uint32_t prev_profiles, uint32_t prev_connected); + void (*profiles_changed) (void *data, uint32_t connected_change); /** Device set configuration changed */ void (*device_set_changed) (void *data);