From 5f561334fb98bd3eb3d6e491bb315bde8e3dcb58 Mon Sep 17 00:00:00 2001 From: Huang-Huang Bao Date: Tue, 29 Dec 2020 10:21:03 +0800 Subject: [PATCH] bluez5: properly handle dbus signals Fixes pipewire/pipewire#502. Also move media application creating from filter callback to initialization. Application object path and endpoint paths only need to be registered once. Signed-off-by: Huang-Huang Bao --- spa/plugins/bluez5/backend-hsp-native.c | 28 ++- spa/plugins/bluez5/bluez5-dbus.c | 307 +++++++++++++++++------- 2 files changed, 245 insertions(+), 90 deletions(-) diff --git a/spa/plugins/bluez5/backend-hsp-native.c b/spa/plugins/bluez5/backend-hsp-native.c index 4709d00d7..b4995dfc2 100644 --- a/spa/plugins/bluez5/backend-hsp-native.c +++ b/spa/plugins/bluez5/backend-hsp-native.c @@ -574,9 +574,6 @@ static void register_profile_reply(DBusPendingCall *pending, void *user_data) static int register_profile(struct spa_bt_backend *backend, const char *profile, const char *uuid) { - static const DBusObjectPathVTable vtable_profile = { - .message_function = profile_handler, - }; DBusMessage *m; DBusMessageIter it[4]; dbus_bool_t autoconnect; @@ -586,11 +583,6 @@ static int register_profile(struct spa_bt_backend *backend, const char *profile, spa_log_debug(backend->log, NAME": Registering Profile %s %s", profile, uuid); - if (!dbus_connection_register_object_path(backend->conn, - profile, - &vtable_profile, backend)) - return -EIO; - m = dbus_message_new_method_call(BLUEZ_SERVICE, "/org/bluez", BLUEZ_PROFILE_MANAGER_INTERFACE, "RegisterProfile"); if (m == NULL) @@ -649,6 +641,8 @@ void backend_hsp_native_register_profiles(struct spa_bt_backend *backend) void backend_hsp_native_free(struct spa_bt_backend *backend) { + dbus_connection_unregister_object_path(backend->conn, PROFILE_HSP_AG); + dbus_connection_unregister_object_path(backend->conn, PROFILE_HSP_HS); free(backend); } @@ -658,6 +652,9 @@ struct spa_bt_backend *backend_hsp_native_new(struct spa_bt_monitor *monitor, uint32_t n_support) { struct spa_bt_backend *backend; + static const DBusObjectPathVTable vtable_profile = { + .message_function = profile_handler, + }; backend = calloc(1, sizeof(struct spa_bt_backend)); if (backend == NULL) @@ -669,5 +666,20 @@ struct spa_bt_backend *backend_hsp_native_new(struct spa_bt_monitor *monitor, backend->main_loop = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Loop); backend->conn = dbus_connection; + if (!dbus_connection_register_object_path(backend->conn, + PROFILE_HSP_AG, + &vtable_profile, backend)) { + goto fail; + } + + if (!dbus_connection_register_object_path(backend->conn, + PROFILE_HSP_HS, + &vtable_profile, backend)) { + goto fail; + } + return backend; +fail: + free(backend); + return NULL; } diff --git a/spa/plugins/bluez5/bluez5-dbus.c b/spa/plugins/bluez5/bluez5-dbus.c index 51dd4174b..f6ab64b4e 100644 --- a/spa/plugins/bluez5/bluez5-dbus.c +++ b/spa/plugins/bluez5/bluez5-dbus.c @@ -73,6 +73,7 @@ struct spa_bt_monitor { struct spa_list transport_list; unsigned int filters_added:1; + unsigned int objects_listed:1; struct spa_bt_backend *backend_hsp_native; struct spa_bt_backend *backend_ofono; @@ -85,6 +86,16 @@ static inline void add_dict(struct spa_pod_builder *builder, const char *key, co spa_pod_builder_string(builder, val); } +static int a2dp_codec_to_endpoint(const struct a2dp_codec *codec, + const char * endpoint, + char** object_path) +{ + *object_path = spa_aprintf("%s/%s", endpoint, codec->name); + if (*object_path == NULL) + return -errno; + return 0; +} + static const struct a2dp_codec *a2dp_endpoint_to_codec(const char *endpoint) { const char *codec_name; @@ -1194,16 +1205,20 @@ static void append_basic_array_variant_dict_entry(DBusMessageIter *dict, int key } static int bluez_register_endpoint(struct spa_bt_monitor *monitor, - const char *path, const char *uuid, - const struct a2dp_codec *codec, const char *object_path) { - char* str; + const char *path, const char *endpoint, + const char *uuid, const struct a2dp_codec *codec) { + char *str, *object_path = NULL; DBusMessage *m; DBusMessageIter object_it, dict_it; DBusPendingCall *call; uint8_t caps[A2DP_MAX_CAPS_SIZE]; - int caps_size; + int ret, caps_size; uint16_t codec_id = codec->codec_id; + ret = a2dp_codec_to_endpoint(codec, endpoint, &object_path); + if (ret < 0) + return ret; + caps_size = codec->fill_caps(codec, 0, caps); if (caps_size < 0) return caps_size; @@ -1233,34 +1248,41 @@ static int bluez_register_endpoint(struct spa_bt_monitor *monitor, dbus_pending_call_set_notify(call, bluez_register_endpoint_reply, monitor, NULL); dbus_message_unref(m); + free(object_path); + return 0; } static int register_a2dp_endpoint(struct spa_bt_monitor *monitor, - const struct a2dp_codec *codec, const char *endpoint, char** object_path) + const struct a2dp_codec *codec, const char *endpoint) { + int ret; + char* object_path = NULL; const DBusObjectPathVTable vtable_endpoint = { .message_function = endpoint_handler, }; - *object_path = spa_aprintf("%s/%s", endpoint, codec->name); - if (*object_path == NULL) - return -errno; + ret = a2dp_codec_to_endpoint(codec, endpoint, &object_path); + if (ret < 0) + return ret; - spa_log_info(monitor->log, "Registering endpoint: %s", *object_path); + spa_log_info(monitor->log, "Registering endpoint: %s", object_path); if (!dbus_connection_register_object_path(monitor->conn, - *object_path, - &vtable_endpoint, monitor)) + object_path, + &vtable_endpoint, monitor)) { + free(object_path); return -EIO; + } + free(object_path); return 0; + } static int adapter_register_endpoints(struct spa_bt_adapter *a) { struct spa_bt_monitor *monitor = a->monitor; - char *endpoint_path = NULL; int i; int err = 0; @@ -1282,22 +1304,16 @@ static int adapter_register_endpoints(struct spa_bt_adapter *a) if (codec->codec_id != A2DP_CODEC_SBC) continue; - if ((err = register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT, &endpoint_path))) - goto out; if ((err = bluez_register_endpoint(monitor, a->path, + A2DP_SOURCE_ENDPOINT, SPA_BT_UUID_A2DP_SOURCE, - codec, - endpoint_path))) + codec))) goto out; - free(endpoint_path); - endpoint_path = NULL; - if ((err = register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT, &endpoint_path))) - goto out; if ((err = bluez_register_endpoint(monitor, a->path, + A2DP_SINK_ENDPOINT, SPA_BT_UUID_A2DP_SINK, - codec, - endpoint_path))) + codec))) goto out; a->endpoints_registered = true; @@ -1314,8 +1330,6 @@ static int adapter_register_endpoints(struct spa_bt_adapter *a) if (err) { spa_log_error(monitor->log, "Failed to register bluez5 endpoints"); } - if (endpoint_path) - free(endpoint_path); return err; } @@ -1388,7 +1402,7 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage * for (i = 0; a2dp_codecs[i]; i++) { const struct a2dp_codec *codec = a2dp_codecs[i]; uint8_t caps[A2DP_MAX_CAPS_SIZE]; - int caps_size; + int caps_size, ret; uint16_t codec_id = codec->codec_id; caps_size = codec->fill_caps(codec, 0, caps); @@ -1396,18 +1410,23 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage * continue; if (codec->decode != NULL) { - spa_log_info(monitor->log, "register A2DP codec %s", a2dp_codecs[i]->name); - endpoint = spa_aprintf("%s/%s", A2DP_SINK_ENDPOINT, codec->name); - append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SINK, - codec_id, caps, caps_size); - free(endpoint); + ret = a2dp_codec_to_endpoint(codec, A2DP_SINK_ENDPOINT, &endpoint); + if (ret == 0) { + spa_log_info(monitor->log, "register A2DP sink codec %s: %s", a2dp_codecs[i]->name, endpoint); + append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SINK, + codec_id, caps, caps_size); + free(endpoint); + } } if (codec->encode != NULL) { - endpoint = spa_aprintf("%s/%s", A2DP_SOURCE_ENDPOINT, codec->name); - append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SOURCE, - codec_id, caps, caps_size); - free(endpoint); + ret = a2dp_codec_to_endpoint(codec, A2DP_SOURCE_ENDPOINT, &endpoint); + if (ret == 0) { + spa_log_info(monitor->log, "register A2DP source codec %s: %s", a2dp_codecs[i]->name, endpoint); + append_a2dp_object(&array, endpoint, SPA_BT_UUID_A2DP_SOURCE, + codec_id, caps, caps_size); + free(endpoint); + } } } @@ -1424,12 +1443,10 @@ static DBusHandlerResult object_manager_handler(DBusConnection *c, DBusMessage * static void bluez_register_application_reply(DBusPendingCall *pending, void *user_data) { - char *endpoint_path; struct spa_bt_adapter *adapter = user_data; struct spa_bt_monitor *monitor = adapter->monitor; DBusMessage *r; bool fallback = true; - int i; r = dbus_pending_call_steal_reply(pending); if (r == NULL) @@ -1448,20 +1465,8 @@ static void bluez_register_application_reply(DBusPendingCall *pending, void *use fallback = false; adapter->application_registered = true; - /* The application was registered in bluez5 - time to register the endpoints */ - for (i = 0; a2dp_codecs[i]; i++) { - const struct a2dp_codec *codec = a2dp_codecs[i]; - register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT, &endpoint_path); - if (endpoint_path) - free(endpoint_path); - - register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT, &endpoint_path); - if (endpoint_path) - free(endpoint_path); - } - - finish: +finish: dbus_message_unref(r); dbus_pending_call_unref(pending); @@ -1469,12 +1474,54 @@ static void bluez_register_application_reply(DBusPendingCall *pending, void *use adapter_register_endpoints(adapter); } -static int adapter_register_application(struct spa_bt_adapter *a) { - const char *object_manager_path = A2DP_OBJECT_MANAGER_PATH; - struct spa_bt_monitor *monitor = a->monitor; +static int register_media_application(struct spa_bt_monitor * monitor) +{ const DBusObjectPathVTable vtable_object_manager = { .message_function = object_manager_handler, }; + + spa_log_info(monitor->log, "Registering media application object: " A2DP_OBJECT_MANAGER_PATH); + + if (!dbus_connection_register_object_path(monitor->conn, + A2DP_OBJECT_MANAGER_PATH, + &vtable_object_manager, monitor)) + return -EIO; + + for (int i = 0; a2dp_codecs[i]; i++) { + const struct a2dp_codec *codec = a2dp_codecs[i]; + register_a2dp_endpoint(monitor, codec, A2DP_SOURCE_ENDPOINT); + register_a2dp_endpoint(monitor, codec, A2DP_SINK_ENDPOINT); + } + + return 0; +} + +static void unregister_media_application(struct spa_bt_monitor * monitor) +{ + int ret; + char *object_path = NULL; + dbus_connection_unregister_object_path(monitor->conn, A2DP_OBJECT_MANAGER_PATH); + + for (int i = 0; a2dp_codecs[i]; i++) { + const struct a2dp_codec *codec = a2dp_codecs[i]; + + ret = a2dp_codec_to_endpoint(codec, A2DP_SOURCE_ENDPOINT, &object_path); + if (ret == 0) { + dbus_connection_unregister_object_path(monitor->conn, object_path); + free(object_path); + } + + ret = a2dp_codec_to_endpoint(codec, A2DP_SINK_ENDPOINT, &object_path); + if (ret == 0) { + dbus_connection_unregister_object_path(monitor->conn, object_path); + free(object_path); + } + } +} + +static int adapter_register_application(struct spa_bt_adapter *a) { + const char *object_manager_path = A2DP_OBJECT_MANAGER_PATH; + struct spa_bt_monitor *monitor = a->monitor; DBusMessage *m; DBusMessageIter i, d; DBusPendingCall *call; @@ -1484,19 +1531,12 @@ static int adapter_register_application(struct spa_bt_adapter *a) { spa_log_debug(monitor->log, "Registering bluez5 media application on adapter %s", a->path); - if (!dbus_connection_register_object_path(monitor->conn, - object_manager_path, - &vtable_object_manager, monitor)) - return -EIO; - m = dbus_message_new_method_call(BLUEZ_SERVICE, a->path, BLUEZ_MEDIA_INTERFACE, "RegisterApplication"); - if (m == NULL) { - dbus_connection_unregister_object_path(monitor->conn, object_manager_path); + if (m == NULL) return -EIO; - } dbus_message_iter_init_append(m, &i); dbus_message_iter_append_basic(&i, DBUS_TYPE_OBJECT_PATH, &object_manager_path); @@ -1551,6 +1591,60 @@ static void interface_added(struct spa_bt_monitor *monitor, } } +static void interfaces_added(struct spa_bt_monitor *monitor, DBusMessageIter *arg_iter) +{ + DBusMessageIter it[3]; + const char *object_path; + + dbus_message_iter_get_basic(arg_iter, &object_path); + dbus_message_iter_next(arg_iter); + dbus_message_iter_recurse(arg_iter, &it[0]); + + while (dbus_message_iter_get_arg_type(&it[0]) != DBUS_TYPE_INVALID) { + const char *interface_name; + + dbus_message_iter_recurse(&it[0], &it[1]); + dbus_message_iter_get_basic(&it[1], &interface_name); + dbus_message_iter_next(&it[1]); + dbus_message_iter_recurse(&it[1], &it[2]); + + interface_added(monitor, monitor->conn, + object_path, interface_name, + &it[2]); + + dbus_message_iter_next(&it[0]); + } +} + +static void interfaces_removed(struct spa_bt_monitor *monitor, DBusMessageIter *arg_iter) +{ + const char *object_path; + DBusMessageIter it; + + dbus_message_iter_get_basic(arg_iter, &object_path); + dbus_message_iter_next(arg_iter); + dbus_message_iter_recurse(arg_iter, &it); + + while (dbus_message_iter_get_arg_type(&it) != DBUS_TYPE_INVALID) { + const char *interface_name; + + dbus_message_iter_get_basic(&it, &interface_name); + + if (strcmp(interface_name, BLUEZ_DEVICE_INTERFACE) == 0) { + struct spa_bt_device *d; + spa_list_consume(d, &monitor->device_list, link) + device_free(d); + } else if (strcmp(interface_name, BLUEZ_ADAPTER_INTERFACE) == 0) { + struct spa_bt_adapter *a; + spa_list_consume(a, &monitor->adapter_list, link) { + adapter_free(a); + } + } + + dbus_message_iter_next(&it); + } +} + static void get_managed_objects_reply(DBusPendingCall *pending, void *user_data) { struct spa_bt_monitor *monitor = user_data; @@ -1581,27 +1675,10 @@ static void get_managed_objects_reply(DBusPendingCall *pending, void *user_data) dbus_message_iter_recurse(&it[0], &it[1]); while (dbus_message_iter_get_arg_type(&it[1]) != DBUS_TYPE_INVALID) { - const char *object_path; - dbus_message_iter_recurse(&it[1], &it[2]); - dbus_message_iter_get_basic(&it[2], &object_path); - dbus_message_iter_next(&it[2]); - dbus_message_iter_recurse(&it[2], &it[3]); - while (dbus_message_iter_get_arg_type(&it[3]) != DBUS_TYPE_INVALID) { - const char *interface_name; + interfaces_added(monitor, &it[2]); - dbus_message_iter_recurse(&it[3], &it[4]); - dbus_message_iter_get_basic(&it[4], &interface_name); - dbus_message_iter_next(&it[4]); - dbus_message_iter_recurse(&it[4], &it[5]); - - interface_added(monitor, monitor->conn, - object_path, interface_name, - &it[5]); - - dbus_message_iter_next(&it[3]); - } dbus_message_iter_next(&it[1]); } @@ -1634,11 +1711,69 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us dbus_error_init(&err); if (dbus_message_is_signal(m, "org.freedesktop.DBus", "NameOwnerChanged")) { + const char *name, *old_owner, *new_owner; + spa_log_debug(monitor->log, "Name owner changed %s", dbus_message_get_path(m)); + + if (!dbus_message_get_args(m, &err, + DBUS_TYPE_STRING, &name, + DBUS_TYPE_STRING, &old_owner, + DBUS_TYPE_STRING, &new_owner, + DBUS_TYPE_INVALID)) { + spa_log_error(monitor->log, NAME": Failed to parse org.freedesktop.DBus.NameOwnerChanged: %s", err.message); + goto fail; + } + + if (strcmp(name, BLUEZ_SERVICE) == 0) { + if (old_owner && *old_owner) { + struct spa_bt_adapter *a; + struct spa_bt_device *d; + struct spa_bt_transport *t; + + spa_log_debug(monitor->log, "Bluetooth daemon disappeared"); + monitor->objects_listed = false; + + spa_list_consume(t, &monitor->transport_list, link) + spa_bt_transport_free(t); + spa_list_consume(d, &monitor->device_list, link) + device_free(d); + spa_list_consume(a, &monitor->adapter_list, link) + adapter_free(a); + } + + if (new_owner && *new_owner) { + spa_log_debug(monitor->log, "Bluetooth daemon appeared"); + get_managed_objects(monitor); + } + } } else if (dbus_message_is_signal(m, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded")) { + DBusMessageIter it; + spa_log_debug(monitor->log, "interfaces added %s", dbus_message_get_path(m)); + + if (!monitor->objects_listed) + goto finish; + + if (!dbus_message_iter_init(m, &it) || strcmp(dbus_message_get_signature(m), "oa{sa{sv}}") != 0) { + spa_log_error(monitor->log, NAME": Invalid signature found in InterfacesAdded"); + goto finish; + } + + interfaces_added(monitor, &it); } else if (dbus_message_is_signal(m, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved")) { + DBusMessageIter it; + spa_log_debug(monitor->log, "interfaces removed %s", dbus_message_get_path(m)); + + if (!monitor->objects_listed) + goto finish; + + if (!dbus_message_iter_init(m, &it) || strcmp(dbus_message_get_signature(m), "oas") != 0) { + spa_log_error(monitor->log, NAME": Invalid signature found in InterfacesRemoved"); + goto finish; + } + + interfaces_removed(monitor, &it); } else if (dbus_message_is_signal(m, "org.freedesktop.DBus.Properties", "PropertiesChanged")) { DBusMessageIter it[2]; const char *iface, *path; @@ -1646,7 +1781,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us if (!dbus_message_iter_init(m, &it[0]) || strcmp(dbus_message_get_signature(m), "sa{sv}as") != 0) { spa_log_error(monitor->log, "Invalid signature found in PropertiesChanged"); - goto fail; + goto finish; } path = dbus_message_get_path(m); @@ -1661,7 +1796,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us if (a == NULL) { spa_log_warn(monitor->log, "Properties changed in unknown adapter %s", path); - goto fail; + goto finish; } spa_log_debug(monitor->log, "Properties changed in adapter %s", path); @@ -1674,7 +1809,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us if (d == NULL) { spa_log_debug(monitor->log, "Properties changed in unknown device %s", path); - goto fail; + goto finish; } spa_log_debug(monitor->log, "Properties changed in device %s", path); @@ -1687,7 +1822,7 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us if (transport == NULL) { spa_log_warn(monitor->log, "Properties changed in unknown transport %s", path); - goto fail; + goto finish; } spa_log_debug(monitor->log, "Properties changed in transport %s", path); @@ -1697,6 +1832,8 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us } fail: + dbus_error_free(&err); +finish: return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } @@ -1760,6 +1897,8 @@ impl_device_add_listener(void *object, struct spa_hook *listener, add_filters(this); get_managed_objects(this); + this->objects_listed = true; + if (this->backend_ofono) backend_ofono_add_filters(this->backend_ofono); @@ -1802,6 +1941,8 @@ static int impl_clear(struct spa_handle *handle) monitor = (struct spa_bt_monitor *) handle; + unregister_media_application(monitor); + spa_list_consume(t, &monitor->transport_list, link) spa_bt_transport_free(t); spa_list_consume(d, &monitor->device_list, link) @@ -1879,6 +2020,8 @@ impl_init(const struct spa_handle_factory *factory, spa_list_init(&this->device_list); spa_list_init(&this->transport_list); + register_media_application(this); + this->backend_hsp_native = backend_hsp_native_new(this, this->conn, support, n_support); this->backend_ofono = backend_ofono_new(this, this->conn, info, support, n_support); this->backend_hsphfpd = backend_hsphfpd_new(this, this->conn, info, support, n_support);