bluez5: cleanup hfp/hsp backend handling + config file option

It seems not uncommon that people have not properly configured ofono
running, which results to loss of HFP/HSP functionality. It's less
surprising if the backend selection is fixed in the configuration file,
and (by default) does not depend on running services.

Add a configuration file option for selecting HFP/HSP backend, and set
the default value to the native backend. Emit warnings if conflicting
backend services are detected to be running.

Also cleanup hfp/hsp backend handling a bit, now that it's mostly
abstracted behind an interface.

spa/plugins/bluez5/backend-hsphfpd.c

@@ -1414,7 +1414,7 @@ finish:
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
-static int backend_hsphfpd_add_filters(void *data)
+static int add_filters(void *data)
 {
 	struct impl *backend = data;
 	DBusError err;
@@ -1480,10 +1480,35 @@ static const struct spa_bt_backend_implementation backend_impl = {
 	SPA_VERSION_BT_BACKEND_IMPLEMENTATION,
 	.free = backend_hsphfpd_free,
 	.register_profiles = backend_hsphfpd_register,
-	.unregistered = backend_hsphfpd_unregistered,
+	.unregister_profiles = backend_hsphfpd_unregistered,
-	.add_filters = backend_hsphfpd_add_filters,
 };
 
+static bool is_available(struct impl *backend)
+{
+	DBusMessage *m, *r;
+	DBusError err;
+	bool success = false;
+
+	m = dbus_message_new_method_call(HSPHFPD_SERVICE, "/",
+			DBUS_INTERFACE_INTROSPECTABLE, "Introspect");
+	if (m == NULL)
+		return false;
+
+	dbus_error_init(&err);
+	r = dbus_connection_send_with_reply_and_block(backend->conn, m, -1, &err);
+	dbus_message_unref(m);
+
+	if (r && dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_METHOD_RETURN)
+		success = true;
+
+	if (r)
+		dbus_message_unref(r);
+	else
+		dbus_error_free(&err);
+
+	return success;
+}
+
 struct spa_bt_backend *backend_hsphfpd_new(struct spa_bt_monitor *monitor,
 		void *dbus_connection,
 		const struct spa_dict *info,
@@ -1506,6 +1531,8 @@ struct spa_bt_backend *backend_hsphfpd_new(struct spa_bt_monitor *monitor,
 
 	spa_bt_backend_set_implementation(&backend->this, &backend_impl, backend);
 
+	backend->this.name = "hsphfpd";
+	backend->this.exclusive = true;
 	backend->monitor = monitor;
 	backend->quirks = quirks;
 	backend->log = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log);
@@ -1543,5 +1570,15 @@ struct spa_bt_backend *backend_hsphfpd_new(struct spa_bt_monitor *monitor,
 		return NULL;
 	}
 
+	if (add_filters(backend) < 0) {
+		dbus_connection_unregister_object_path(backend->conn, HSPHFP_AUDIO_CLIENT_MSBC);
+		dbus_connection_unregister_object_path(backend->conn, HSPHFP_AUDIO_CLIENT_PCM_S16LE_8KHZ);
+		dbus_connection_unregister_object_path(backend->conn, APPLICATION_OBJECT_MANAGER_PATH);
+		free(backend);
+		return NULL;
+	}
+
+	backend->this.available = is_available(backend);
+
 	return &backend->this;
 }

spa/plugins/bluez5/backend-native.c

@@ -2126,6 +2126,38 @@ static const struct spa_bt_backend_implementation backend_impl = {
 	.supports_codec = backend_native_supports_codec,
 };
 
+static bool is_available(struct impl *backend)
+{
+	DBusMessage *m, *r;
+	DBusError err;
+	bool success = false;
+
+	m = dbus_message_new_method_call(BLUEZ_SERVICE, "/org/bluez",
+			DBUS_INTERFACE_INTROSPECTABLE, "Introspect");
+	if (m == NULL)
+		return false;
+
+	dbus_error_init(&err);
+	r = dbus_connection_send_with_reply_and_block(backend->conn, m, -1, &err);
+	dbus_message_unref(m);
+
+	if (r && dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_METHOD_RETURN) {
+		const char *str;
+		if (dbus_message_get_args(r, &err, DBUS_TYPE_STRING, &str, DBUS_TYPE_INVALID)) {
+			success = strstr(str, BLUEZ_PROFILE_MANAGER_INTERFACE) != NULL;
+		} else {
+			dbus_error_free(&err);
+		}
+	}
+
+	if (r)
+		dbus_message_unref(r);
+	else
+		dbus_error_free(&err);
+
+	return success;
+}
+
 struct spa_bt_backend *backend_native_new(struct spa_bt_monitor *monitor,
 		void *dbus_connection,
 		const struct spa_dict *info,
@@ -2145,6 +2177,7 @@ struct spa_bt_backend *backend_native_new(struct spa_bt_monitor *monitor,
 
 	spa_bt_backend_set_implementation(&backend->this, &backend_impl, backend);
 
+	backend->this.name = "native";
 	backend->monitor = monitor;
 	backend->quirks = quirks;
 	backend->log = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log);
@@ -2187,6 +2220,8 @@ struct spa_bt_backend *backend_native_new(struct spa_bt_monitor *monitor,
 	}
 #endif
 
+	backend->this.available = is_available(backend);
+
 	return &backend->this;
 
 #ifdef HAVE_BLUEZ_5_BACKEND_HFP_NATIVE

spa/plugins/bluez5/backend-ofono.c

@@ -713,10 +713,8 @@ fail:
 	return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
-static int backend_ofono_add_filters(void *data)
+static int add_filters(struct impl *backend)
 {
-	struct impl *backend = data;
-
 	DBusError err;
 
 	if (backend->filters_added)
@@ -765,9 +763,34 @@ static const struct spa_bt_backend_implementation backend_impl = {
 	SPA_VERSION_BT_BACKEND_IMPLEMENTATION,
 	.free = backend_ofono_free,
 	.register_profiles = backend_ofono_register,
-	.add_filters = backend_ofono_add_filters,
 };
 
+static bool is_available(struct impl *backend)
+{
+	DBusMessage *m, *r;
+	DBusError err;
+	bool success = false;
+
+	m = dbus_message_new_method_call(OFONO_SERVICE, "/",
+			DBUS_INTERFACE_INTROSPECTABLE, "Introspect");
+	if (m == NULL)
+		return false;
+
+	dbus_error_init(&err);
+	r = dbus_connection_send_with_reply_and_block(backend->conn, m, -1, &err);
+	dbus_message_unref(m);
+
+	if (r && dbus_message_get_type(r) == DBUS_MESSAGE_TYPE_METHOD_RETURN)
+		success = true;
+
+	if (r)
+		dbus_message_unref(r);
+	else
+		dbus_error_free(&err);
+
+	return success;
+}
+
 struct spa_bt_backend *backend_ofono_new(struct spa_bt_monitor *monitor,
 		void *dbus_connection,
 		const struct spa_dict *info,
@@ -787,6 +810,8 @@ struct spa_bt_backend *backend_ofono_new(struct spa_bt_monitor *monitor,
 
 	spa_bt_backend_set_implementation(&backend->this, &backend_impl, backend);
 
+	backend->this.name = "ofono";
+	backend->this.exclusive = true;
 	backend->monitor = monitor;
 	backend->quirks = quirks;
 	backend->log = spa_support_find(support, n_support, SPA_TYPE_INTERFACE_Log);
@@ -805,5 +830,13 @@ struct spa_bt_backend *backend_ofono_new(struct spa_bt_monitor *monitor,
 		return NULL;
 	}
 
+	if (add_filters(backend) < 0) {
+		dbus_connection_unregister_object_path(backend->conn, OFONO_AUDIO_CLIENT);
+		free(backend);
+		return NULL;
+	}
+
+	backend->this.available = is_available(backend);
+
 	return &backend->this;
 }

spa/plugins/bluez5/bluez5-dbus.c

@@ -55,6 +55,15 @@
 
 #define NAME "bluez5-monitor"
 
+enum backend_selection {
+	BACKEND_NONE = -2,
+	BACKEND_ANY = -1,
+	BACKEND_HSPHFPD = 0,
+	BACKEND_OFONO = 1,
+	BACKEND_NATIVE = 2,
+	BACKEND_NUM,
+};
+
 struct spa_bt_monitor {
 	struct spa_handle handle;
 	struct spa_device device;
@@ -85,9 +94,9 @@ struct spa_bt_monitor {
 	unsigned int filters_added:1;
 	unsigned int objects_listed:1;
 
-	struct spa_bt_backend *backend_native;
+	struct spa_bt_backend *backend;
-	struct spa_bt_backend *backend_ofono;
+	struct spa_bt_backend *backends[BACKEND_NUM];
-	struct spa_bt_backend *backend_hsphfpd;
+	enum backend_selection backend_selection;
 
 	struct spa_dict enabled_codecs;
 
@@ -95,10 +104,6 @@ struct spa_bt_monitor {
 
 	struct spa_bt_quirks *quirks;
 
-	unsigned int backend_native_registered:1;
-	unsigned int backend_ofono_registered:1;
-	unsigned int backend_hsphfpd_registered:1;
-
 	/* A reference audio info for A2DP codec configuration. */
 	struct a2dp_codec_audio_info default_audio_info;
 };
@@ -2668,21 +2673,13 @@ int spa_bt_device_ensure_a2dp_codec(struct spa_bt_device *device, const struct a
 int spa_bt_device_ensure_hfp_codec(struct spa_bt_device *device, unsigned int codec)
 {
 	struct spa_bt_monitor *monitor = device->monitor;
-	if (monitor->backend_hsphfpd_registered)
+	return spa_bt_backend_ensure_codec(monitor->backend, device, codec);
-		return spa_bt_backend_ensure_codec(monitor->backend_hsphfpd, device, codec);
-	if (monitor->backend_ofono_registered)
-		return spa_bt_backend_ensure_codec(monitor->backend_ofono, device, codec);
-	return spa_bt_backend_ensure_codec(monitor->backend_native, device, codec);
 }
 
 int spa_bt_device_supports_hfp_codec(struct spa_bt_device *device, unsigned int codec)
 {
 	struct spa_bt_monitor *monitor = device->monitor;
-	if (monitor->backend_hsphfpd_registered)
+	return spa_bt_backend_supports_codec(monitor->backend, device, codec);
-		return spa_bt_backend_supports_codec(monitor->backend_hsphfpd, device, codec);
-	if (monitor->backend_ofono_registered)
-		return spa_bt_backend_supports_codec(monitor->backend_ofono, device, codec);
-	return spa_bt_backend_supports_codec(monitor->backend_native, device, codec);
 }
 
 static DBusHandlerResult endpoint_set_configuration(DBusConnection *conn,
@@ -3314,6 +3311,68 @@ static int adapter_register_application(struct spa_bt_adapter *a) {
 	return 0;
 }
 
+static int switch_backend(struct spa_bt_monitor *monitor, struct spa_bt_backend *backend)
+{
+	int res;
+	size_t i;
+
+	spa_return_val_if_fail(backend != NULL, -EINVAL);
+
+	if (!backend->available)
+		return -ENODEV;
+
+	for (i = 0; i < SPA_N_ELEMENTS(monitor->backends); ++i) {
+		struct spa_bt_backend *b = monitor->backends[i];
+		if (backend != b && b && b->available && b->exclusive)
+			spa_log_warn(monitor->log,
+					"%s running, but not configured as HFP/HSP backend: "
+					"it may interfere with HFP/HSP functionality.",
+					b->name);
+	}
+
+	if (monitor->backend == backend)
+		return 0;
+
+	spa_log_info(monitor->log, "Switching to HFP/HSP backend %s", backend->name);
+
+	spa_bt_backend_unregister_profiles(monitor->backend);
+
+	if ((res = spa_bt_backend_register_profiles(backend)) < 0) {
+		monitor->backend = NULL;
+		return res;
+	}
+
+	monitor->backend = backend;
+	return 0;
+}
+
+static void reselect_backend(struct spa_bt_monitor *monitor)
+{
+	struct spa_bt_backend *backend;
+	size_t i;
+
+	spa_log_debug(monitor->log, NAME": re-selecting HFP/HSP backend");
+
+	if (monitor->backend_selection == BACKEND_NONE) {
+		spa_bt_backend_unregister_profiles(monitor->backend);
+		monitor->backend = NULL;
+		return;
+	} else if (monitor->backend_selection == BACKEND_ANY) {
+		for (i = 0; i < SPA_N_ELEMENTS(monitor->backends); ++i) {
+			backend = monitor->backends[i];
+			if (backend && switch_backend(monitor, backend) == 0)
+				return;
+		}
+	} else {
+		backend = monitor->backends[monitor->backend_selection];
+		if (backend && switch_backend(monitor, backend) == 0)
+			return;
+	}
+
+	spa_log_error(monitor->log, "Failed to start HFP/HSP backend %s",
+			backend ? backend->name : "none");
+}
+
 static void interface_added(struct spa_bt_monitor *monitor,
 			    DBusConnection *conn,
 			    const char *object_path,
@@ -3337,10 +3396,9 @@ static void interface_added(struct spa_bt_monitor *monitor,
 		adapter_register_application(a);
 	}
 	else if (spa_streq(interface_name, BLUEZ_PROFILE_MANAGER_INTERFACE)) {
-		if (!monitor->backend_ofono_registered && !monitor->backend_hsphfpd_registered) {
+		if (monitor->backends[BACKEND_NATIVE])
-			spa_bt_backend_register_profiles(monitor->backend_native);
+			monitor->backends[BACKEND_NATIVE]->available = true;
-			monitor->backend_native_registered = true;
+		reselect_backend(monitor);
-		}
 	}
 	else if (spa_streq(interface_name, BLUEZ_DEVICE_INTERFACE)) {
 		struct spa_bt_device *d;
@@ -3548,11 +3606,6 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 
 				monitor->objects_listed = false;
 
-				if (monitor->backend_native_registered) {
-					spa_bt_backend_unregister_profiles(monitor->backend_native);
-					monitor->backend_native_registered = false;
-				}
-
 				spa_list_consume(t, &monitor->transport_list, link)
 					spa_bt_transport_free(t);
 				spa_list_consume(ep, &monitor->remote_endpoint_list, link)
@@ -3561,55 +3614,24 @@ static DBusHandlerResult filter_cb(DBusConnection *bus, DBusMessage *m, void *us
 					device_free(d);
 				spa_list_consume(a, &monitor->adapter_list, link)
 					adapter_free(a);
+
+				if (monitor->backends[BACKEND_NATIVE])
+					monitor->backends[BACKEND_NATIVE]->available = false;
+				reselect_backend(monitor);
 			}
 
 			if (has_new_owner) {
 				spa_log_debug(monitor->log, "Bluetooth daemon appeared");
 				get_managed_objects(monitor);
 			}
-		} else if (spa_streq(name, OFONO_SERVICE) && monitor->backend_ofono) {
+		} else if (spa_streq(name, OFONO_SERVICE)) {
-			if (old_owner && *old_owner) {
+			if (monitor->backends[BACKEND_OFONO])
-				spa_log_debug(monitor->log, "oFono daemon disappeared");
+				monitor->backends[BACKEND_OFONO]->available = (new_owner && *new_owner);
-				monitor->backend_ofono_registered = false;
+			reselect_backend(monitor);
-				spa_bt_backend_register_profiles(monitor->backend_native);
+		} else if (spa_streq(name, HSPHFPD_SERVICE)) {
-				monitor->backend_native_registered = true;
+			if (monitor->backends[BACKEND_HSPHFPD])
-			}
+				monitor->backends[BACKEND_HSPHFPD]->available = (new_owner && *new_owner);
-
+			reselect_backend(monitor);
-			if (new_owner && *new_owner) {
-				spa_log_debug(monitor->log, "oFono daemon appeared");
-				if (monitor->backend_native_registered) {
-					spa_bt_backend_unregister_profiles(monitor->backend_native);
-					monitor->backend_native_registered = false;
-				}
-				if (spa_bt_backend_register_profiles(monitor->backend_ofono) == 0)
-					monitor->backend_ofono_registered = true;
-				else {
-					spa_bt_backend_register_profiles(monitor->backend_native);
-					monitor->backend_native_registered = true;
-				}
-			}
-		} else if (spa_streq(name, HSPHFPD_SERVICE) && monitor->backend_hsphfpd) {
-			if (old_owner && *old_owner) {
-				spa_log_debug(monitor->log, "hsphfpd daemon disappeared");
-				spa_bt_backend_unregistered(monitor->backend_hsphfpd);
-				monitor->backend_hsphfpd_registered = false;
-				spa_bt_backend_register_profiles(monitor->backend_native);
-				monitor->backend_native_registered = true;
-			}
-
-			if (new_owner && *new_owner) {
-				spa_log_debug(monitor->log, "hsphfpd daemon appeared");
-				if (monitor->backend_native_registered) {
-					spa_bt_backend_unregister_profiles(monitor->backend_native);
-					monitor->backend_native_registered = false;
-				}
-				if (spa_bt_backend_register_profiles(monitor->backend_hsphfpd) == 0)
-					monitor->backend_hsphfpd_registered = true;
-				else {
-					spa_bt_backend_register_profiles(monitor->backend_native);
-					monitor->backend_native_registered = true;
-				}
-			}
 		}
 	} else if (dbus_message_is_signal(m, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded")) {
 		DBusMessageIter it;
@@ -3800,12 +3822,6 @@ impl_device_add_listener(void *object, struct spa_hook *listener,
 	add_filters(this);
 	get_managed_objects(this);
 
-	if (this->backend_ofono)
-		spa_bt_backend_add_filters(this->backend_ofono);
-
-	if (this->backend_hsphfpd)
-		spa_bt_backend_add_filters(this->backend_hsphfpd);
-
         spa_hook_list_join(&this->hooks, &save);
 
 	return 0;
@@ -3840,6 +3856,7 @@ static int impl_clear(struct spa_handle *handle)
 	struct spa_bt_device *d;
 	struct spa_bt_remote_endpoint *ep;
 	struct spa_bt_transport *t;
+	size_t i;
 
 	monitor = (struct spa_bt_monitor *) handle;
 
@@ -3865,19 +3882,9 @@ static int impl_clear(struct spa_handle *handle)
 	spa_list_consume(a, &monitor->adapter_list, link)
 		adapter_free(a);
 
-	if (monitor->backend_native) {
+	for (i = 0; i < SPA_N_ELEMENTS(monitor->backends); ++i) {
-		spa_bt_backend_free(monitor->backend_native);
+		spa_bt_backend_free(monitor->backends[i]);
-		monitor->backend_native = NULL;
+		monitor->backends[i] = NULL;
-	}
-
-	if (monitor->backend_ofono) {
-		spa_bt_backend_free(monitor->backend_ofono);
-		monitor->backend_ofono = NULL;
-	}
-
-	if (monitor->backend_hsphfpd) {
-		spa_bt_backend_free(monitor->backend_hsphfpd);
-		monitor->backend_hsphfpd = NULL;
 	}
 
 	free((void*)monitor->enabled_codecs.items);
@@ -3891,9 +3898,9 @@ static int impl_clear(struct spa_handle *handle)
 	monitor->objects_listed = false;
 
 	monitor->connection_info_supported = false;
-	monitor->backend_native_registered = false;
+
-	monitor->backend_ofono_registered = false;
+	monitor->backend = NULL;
-	monitor->backend_hsphfpd_registered = false;
+	monitor->backend_selection = BACKEND_NATIVE;
 
 	spa_bt_quirks_destroy(monitor->quirks);
 
@@ -4105,6 +4112,8 @@ impl_init(const struct spa_handle_factory *factory,
 	this->default_audio_info.rate = A2DP_CODEC_DEFAULT_RATE;
 	this->default_audio_info.channels = A2DP_CODEC_DEFAULT_CHANNELS;
 
+	this->backend_selection = BACKEND_NATIVE;
+
 	if (info) {
 		const char *str;
 		uint32_t tmp;
@@ -4120,18 +4129,28 @@ impl_init(const struct spa_handle_factory *factory,
 		if ((str = spa_dict_lookup(info, "bluez5.default.channels")) != NULL &&
 		    ((tmp =  atoi(str)) > 0))
 			this->default_audio_info.channels = tmp;
+
+		if ((str = spa_dict_lookup(info, "bluez5.hfphsp-backend")) != NULL) {
+			if (spa_streq(str, "none"))
+				this->backend_selection = BACKEND_NONE;
+			else if (spa_streq(str, "any"))
+				this->backend_selection = BACKEND_ANY;
+			else if (spa_streq(str, "ofono"))
+				this->backend_selection = BACKEND_OFONO;
+			else if (spa_streq(str, "hsphfpd"))
+				this->backend_selection = BACKEND_HSPHFPD;
+			else if (spa_streq(str, "native"))
+				this->backend_selection = BACKEND_NATIVE;
+		}
 	}
 
 	register_media_application(this);
 
-	this->backend_native = backend_native_new(this, this->conn, info, this->quirks, support, n_support);
+	this->backends[BACKEND_NATIVE] = backend_native_new(this, this->conn, info, this->quirks, support, n_support);
-	this->backend_ofono = backend_ofono_new(this, this->conn, info, this->quirks, support, n_support);
+	this->backends[BACKEND_OFONO] = backend_ofono_new(this, this->conn, info, this->quirks, support, n_support);
-	this->backend_hsphfpd = backend_hsphfpd_new(this, this->conn, info, this->quirks, support, n_support);
+	this->backends[BACKEND_HSPHFPD] = backend_hsphfpd_new(this, this->conn, info, this->quirks, support, n_support);
 
-	if (this->backend_ofono && spa_bt_backend_register_profiles(this->backend_ofono) == 0)
+	reselect_backend(this);
-		this->backend_ofono_registered = true;
-	else if (this->backend_hsphfpd && spa_bt_backend_register_profiles(this->backend_hsphfpd) == 0)
-		this->backend_hsphfpd_registered = true;
 
 	return 0;
 

spa/plugins/bluez5/defs.h

@@ -696,14 +696,15 @@ struct spa_bt_backend_implementation {
 	int (*free) (void *data);
 	int (*register_profiles) (void *data);
 	int (*unregister_profiles) (void *data);
-	int (*unregistered) (void *data);
-	int (*add_filters) (void *data);
 	int (*ensure_codec) (void *data, struct spa_bt_device *device, unsigned int codec);
 	int (*supports_codec) (void *data, struct spa_bt_device *device, unsigned int codec);
 };
 
 struct spa_bt_backend {
 	struct spa_callbacks impl;
+	const char *name;
+	bool available;
+	bool exclusive;
 };
 
 #define spa_bt_backend_set_implementation(b,_impl,_data) \
@@ -722,8 +723,6 @@ struct spa_bt_backend {
 #define spa_bt_backend_free(b)			spa_bt_backend_impl(b, free, 0)
 #define spa_bt_backend_register_profiles(b)	spa_bt_backend_impl(b, register_profiles, 0)
 #define spa_bt_backend_unregister_profiles(b)	spa_bt_backend_impl(b, unregister_profiles, 0)
-#define spa_bt_backend_unregistered(b)		spa_bt_backend_impl(b, unregistered, 0)
-#define spa_bt_backend_add_filters(b)		spa_bt_backend_impl(b, add_filters, 0)
 #define spa_bt_backend_ensure_codec(b,...)	spa_bt_backend_impl(b, ensure_codec, 0, __VA_ARGS__)
 #define spa_bt_backend_supports_codec(b,...)	spa_bt_backend_impl(b, supports_codec, 0, __VA_ARGS__)
 

src/daemon/media-session.d/bluez-monitor.conf

@@ -29,6 +29,10 @@ properties = {
     # Enabled A2DP codecs (default: all).
     #bluez5.codecs = [ sbc sbc_xq aac ldac aptx aptx_hd aptx_ll aptx_ll_duplex faststream faststream_duplex ]
 
+    # HFP/HSP backend (default: native).
+    # Available values: any, none, hsphfpd, ofono, native
+    #bluez5.hfphsp-backend = native
+
     # Properties for the A2DP codec configuration
     #bluez5.default.rate     = 48000
     #bluez5.default.channels = 2