[PATCH BlueZ v0 0/7] Multiple Bluetooth SCO connections (userspace)

[PATCH BlueZ v0 0/7] Multiple Bluetooth SCO connections (userspace)

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

This patch series includes patches that have been useful to connect two HCI-based SCO links simultaneously. This can be used for example to connect to HSP headsets at the same time.

The patch series is divided in three groups: kernel patches, BlueZ userspace patches and PulseAudio patches for module-bluetooth-device.

The kernel patches include some code cleanup and more importantly a dynamically changing alternate setting in btusb driver. These ideas have been taken from the patches I found in [1]. The last patch, “Bluetooth: Remove outgoing MTU check” should be considered with care, since there probably are better approaches to solve this (WIP).

The BlueZ userspace patches add some necessary infrastructure to support such use-cases.

The PulseAudio patches provide some changes to be able to test the rest of the code. The first three patches have been reused from a previously submitted patch series, and only the last two patches are relevant for this purpose. They provide some simple workarounds and should not be considered a proper solution.

As I said, the easiest may to test these patches is by using two Bluetooth headsets. You should use the Media API (Enable=Media in audio.conf), connect both headsets, and use pacmd to set their profile to hsp.

[1] http://bluetooth-alsa.sourceforge.net/future.html

Mikel Astiz (7):
  btio: Fix bind SCO socket only if server
  audio: Fix gateway state check
  audio: Add multiple device search to manager
  media: Support multiple transports per endpoint
  media: Create multiple transports if needed
  media: Enable parallel requests to endpoint
  media: Increase hardcoded incoming MTU

 audio/device.c    |    2 +-
 audio/manager.c   |   33 +++++++++-
 audio/manager.h   |    7 ++
 audio/media.c     |  183 +++++++++++++++++++++++++++++++++++-----------------
 audio/transport.c |    4 +-
 btio/btio.c       |    5 +-
 6 files changed, 165 insertions(+), 69 deletions(-)

-- 
1.7.7.6

[PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

When the socket is open is client mode, the bind() system call should
not be used.
---
 btio/btio.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index 9781ec4..7564d85 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -1317,8 +1317,9 @@ static GIOChannel *create_io(BtIOType type, gboolean server,
 			ERROR_FAILED(err, "socket(SEQPACKET, SCO)", errno);
 			return NULL;
 		}
-		if (sco_bind(sock, &opts->src, err) < 0)
-			goto failed;
+		if (server)
+			if (sco_bind(sock, &opts->src, err) < 0)
+				goto failed;
 		if (!sco_set(sock, opts->mtu, err))
 			goto failed;
 		break;
-- 
1.7.7.6

[PATCH BlueZ v0 2/7] audio: Fix gateway state check

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Gateway should be considered active also if connecting or playing.

This could for example lead to manager_find_device() not returning a
device that is connecting, and thus the corresponding endpoint would
never be created in the Media API.
---
 audio/device.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/device.c b/audio/device.c
index a9d35f9..ee1ade1 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -701,7 +701,7 @@ gboolean audio_device_is_active(struct audio_device *dev,
 				control_is_active(dev))
 		return TRUE;
 	else if (!strcmp(interface, AUDIO_GATEWAY_INTERFACE) && dev->gateway &&
-				gateway_is_connected(dev))
+				gateway_is_active(dev))
 		return TRUE;
 
 	return FALSE;
-- 
1.7.7.6

[PATCH BlueZ v0 3/7] audio: Add multiple device search to manager

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

This method is useful to search or than one device fulfulling certain
criteria.
---
 audio/manager.c |   33 +++++++++++++++++++++++++++++----
 audio/manager.h |    7 +++++++
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 170ed23..b17e349 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -1300,17 +1300,23 @@ void audio_manager_exit(void)
 	btd_unregister_device_driver(&audio_driver);
 }
 
-struct audio_device *manager_find_device(const char *path,
+GSList *manager_find_devices(const char *path,
 					const bdaddr_t *src,
 					const bdaddr_t *dst,
 					const char *interface,
-					gboolean connected)
+					gboolean connected,
+					size_t max_count)
 {
+	GSList *result = NULL;
+	size_t result_size = 0;
 	GSList *l;
 
 	for (l = devices; l != NULL; l = l->next) {
 		struct audio_device *dev = l->data;
 
+		if ((max_count > 0) && (result_size == max_count))
+			break;
+
 		if ((path && (strcmp(path, "")) && strcmp(dev->path, path)))
 			continue;
 
@@ -1343,10 +1349,29 @@ struct audio_device *manager_find_device(const char *path,
 		if (connected && !audio_device_is_active(dev, interface))
 			continue;
 
-		return dev;
+		result = g_slist_append(result, dev);
+		result_size++;
 	}
 
-	return NULL;
+	return result;
+}
+
+struct audio_device *manager_find_device(const char *path,
+					const bdaddr_t *src,
+					const bdaddr_t *dst,
+					const char *interface,
+					gboolean connected)
+{
+	struct audio_device *result;
+	GSList *l;
+
+	l = manager_find_devices(path, src, dst, interface, connected, 1);
+	if (l == NULL)
+		return NULL;
+
+	result = l->data;
+	g_slist_free(l);
+	return result;
 }
 
 struct audio_device *manager_get_device(const bdaddr_t *src,
diff --git a/audio/manager.h b/audio/manager.h
index cfc646c..57bcc18 100644
--- a/audio/manager.h
+++ b/audio/manager.h
@@ -46,6 +46,13 @@ struct audio_device *manager_find_device(const char *path,
 					const char *interface,
 					gboolean connected);
 
+GSList *manager_find_devices(const char *path,
+					const bdaddr_t *src,
+					const bdaddr_t *dst,
+					const char *interface,
+					gboolean connected,
+					size_t max_count);
+
 struct audio_device *manager_get_device(const bdaddr_t *src,
 					const bdaddr_t *dst,
 					gboolean create);
-- 
1.7.7.6

[PATCH BlueZ v0 4/7] media: Support multiple transports per endpoint

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

Several transports may exist for each endpoint, for example if several
HFGW are connected. This should be exposed to the endpoint as one
transport each.
---
 audio/media.c |  111 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 78 insertions(+), 33 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index c0fd0c3..23c3217 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -86,8 +86,8 @@ struct media_endpoint {
 	guint			ag_watch;
 	guint			watch;
 	struct endpoint_request *request;
-	struct media_transport	*transport;
 	struct media_adapter	*adapter;
+	GSList			*transports;
 };
 
 struct media_player {
@@ -153,8 +153,8 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
 	if (endpoint->request)
 		media_endpoint_cancel(endpoint);
 
-	if (endpoint->transport)
-		media_transport_destroy(endpoint->transport);
+	g_slist_free_full(endpoint->transports,
+				(GDestroyNotify) media_transport_destroy);
 
 	g_dbus_remove_watch(adapter->conn, endpoint->watch);
 	g_free(endpoint->capabilities);
@@ -200,18 +200,12 @@ static void headset_setconf_cb(struct media_endpoint *endpoint, void *ret,
 	headset_shutdown(dev);
 }
 
-static void clear_configuration(struct media_endpoint *endpoint)
+static void clear_configuration(struct media_endpoint *endpoint,
+					struct media_transport *transport)
 {
 	DBusConnection *conn;
 	DBusMessage *msg;
 	const char *path;
-	struct media_transport *transport = endpoint->transport;
-
-	if (endpoint->transport == NULL)
-		return;
-
-	if (endpoint->request)
-		media_endpoint_cancel(endpoint);
 
 	conn = endpoint->adapter->conn;
 
@@ -223,15 +217,24 @@ static void clear_configuration(struct media_endpoint *endpoint)
 		goto done;
 	}
 
-	path = media_transport_get_path(endpoint->transport);
+	path = media_transport_get_path(transport);
 	dbus_message_append_args(msg, DBUS_TYPE_OBJECT_PATH, &path,
 							DBUS_TYPE_INVALID);
 	g_dbus_send_message(conn, msg);
 done:
-	endpoint->transport = NULL;
+	endpoint->transports = g_slist_remove(endpoint->transports, transport);
 	media_transport_destroy(transport);
 }
 
+static void clear_endpoint(struct media_endpoint *endpoint)
+{
+	if (endpoint->request)
+		media_endpoint_cancel(endpoint);
+
+	while (endpoint->transports != NULL)
+		clear_configuration(endpoint, endpoint->transports->data);
+}
+
 static void endpoint_reply(DBusPendingCall *call, void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
@@ -256,7 +259,7 @@ static void endpoint_reply(DBusPendingCall *call, void *user_data)
 			if (request->cb)
 				request->cb(endpoint, NULL, size,
 							request->user_data);
-			clear_configuration(endpoint);
+			clear_endpoint(endpoint);
 			dbus_message_unref(reply);
 			dbus_error_free(&err);
 			return;
@@ -369,6 +372,31 @@ static gboolean select_configuration(struct media_endpoint *endpoint,
 								destroy);
 }
 
+static gint transport_device_cmp(gconstpointer data, gconstpointer user_data)
+{
+	struct media_transport *transport = (struct media_transport *) data;
+	const struct audio_device *device = user_data;
+
+	if (device == media_transport_get_dev(transport))
+		return 0;
+
+	return -1;
+}
+
+static struct media_transport *find_device_transport(
+					struct media_endpoint *endpoint,
+					struct audio_device *device)
+{
+	GSList *match;
+
+	match = g_slist_find_custom(endpoint->transports, device,
+							transport_device_cmp);
+	if (match == NULL)
+		return NULL;
+
+	return match->data;
+}
+
 static gboolean set_configuration(struct media_endpoint *endpoint,
 					struct audio_device *device,
 					uint8_t *configuration, size_t size,
@@ -380,15 +408,18 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	DBusMessage *msg;
 	const char *path;
 	DBusMessageIter iter;
+	struct media_transport *transport;
+
+	transport = find_device_transport(endpoint, device);
 
-	if (endpoint->transport != NULL || endpoint->request != NULL)
+	if (transport != NULL || endpoint->request != NULL)
 		return FALSE;
 
 	conn = endpoint->adapter->conn;
 
-	endpoint->transport = media_transport_create(conn, endpoint, device,
+	transport = media_transport_create(conn, endpoint, device,
 						configuration, size);
-	if (endpoint->transport == NULL)
+	if (transport == NULL)
 		return FALSE;
 
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
@@ -396,15 +427,18 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 						"SetConfiguration");
 	if (msg == NULL) {
 		error("Couldn't allocate D-Bus message");
+		media_transport_destroy(transport);
 		return FALSE;
 	}
 
+	endpoint->transports = g_slist_append(endpoint->transports, transport);
+
 	dbus_message_iter_init_append(msg, &iter);
 
-	path = media_transport_get_path(endpoint->transport);
+	path = media_transport_get_path(transport);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, &path);
 
-	transport_get_properties(endpoint->transport, &iter);
+	transport_get_properties(transport, &iter);
 
 	return media_endpoint_async_call(conn, msg, endpoint, cb, user_data,
 								destroy);
@@ -440,16 +474,17 @@ static void headset_state_changed(struct audio_device *dev,
 					void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
+	struct media_transport *transport;
 
 	DBG("");
 
 	switch (new_state) {
 	case HEADSET_STATE_DISCONNECTED:
-		if (endpoint->transport &&
-			media_transport_get_dev(endpoint->transport) == dev) {
+		transport = find_device_transport(endpoint, dev);
 
+		if (transport != NULL) {
 			DBG("Clear endpoint %p", endpoint);
-			clear_configuration(endpoint);
+			clear_configuration(endpoint, transport);
 		}
 		break;
 	case HEADSET_STATE_CONNECTING:
@@ -551,17 +586,30 @@ static void clear_config(struct a2dp_sep *sep, void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
 
-	clear_configuration(endpoint);
+	clear_endpoint(endpoint);
+}
+
+static struct media_transport *get_unique_transport(
+					struct media_endpoint *endpoint)
+{
+	if (endpoint->transports == NULL)
+		return NULL;
+
+	if (endpoint->transports->next != NULL)
+		return NULL;
+
+	return endpoint->transports->data;
 }
 
 static void set_delay(struct a2dp_sep *sep, uint16_t delay, void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
+	struct media_transport *transport = get_unique_transport(endpoint);
 
-	if (endpoint->transport == NULL)
+	if (transport == NULL)
 		return;
 
-	media_transport_update_delay(endpoint->transport, delay);
+	media_transport_update_delay(transport, delay);
 }
 
 static struct a2dp_endpoint a2dp_endpoint = {
@@ -577,10 +625,7 @@ static void a2dp_destroy_endpoint(void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
 
-	if (endpoint->transport) {
-		media_transport_destroy(endpoint->transport);
-		endpoint->transport = NULL;
-	}
+	clear_endpoint(endpoint);
 
 	endpoint->sep = NULL;
 	release_endpoint(endpoint);
@@ -603,16 +648,16 @@ static void gateway_state_changed(struct audio_device *dev,
 					void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
+	struct media_transport *transport;
 
 	DBG("");
 
 	switch (new_state) {
 	case GATEWAY_STATE_DISCONNECTED:
-		if (endpoint->transport &&
-			media_transport_get_dev(endpoint->transport) == dev) {
-
+		transport = find_device_transport(endpoint, dev);
+		if (transport != NULL) {
 			DBG("Clear endpoint %p", endpoint);
-			clear_configuration(endpoint);
+			clear_configuration(endpoint, transport);
 		}
 		break;
 	case GATEWAY_STATE_CONNECTING:
-- 
1.7.7.6

[PATCH BlueZ v0 5/7] media: Create multiple transports if needed

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

During endpoint registration one than one device might be connected.
Thus all matching devices should have one transport each.
---
 audio/media.c |   32 ++++++++++++++++++++++++--------
 1 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index 23c3217..3324103 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -713,26 +713,42 @@ static struct media_endpoint *media_endpoint_create(struct media_adapter *adapte
 			goto failed;
 	} else if (strcasecmp(uuid, HFP_AG_UUID) == 0 ||
 					strcasecmp(uuid, HSP_AG_UUID) == 0) {
-		struct audio_device *dev;
+		GSList *list;
+		GSList *l;
 
 		endpoint->hs_watch = headset_add_state_cb(headset_state_changed,
 								endpoint);
-		dev = manager_find_device(NULL, &adapter->src, BDADDR_ANY,
-						AUDIO_HEADSET_INTERFACE, TRUE);
-		if (dev)
+		list = manager_find_devices(NULL, &adapter->src, BDADDR_ANY,
+						AUDIO_HEADSET_INTERFACE, TRUE,
+						0);
+
+		for (l = list; l != NULL; l = l->next) {
+			struct audio_device *dev = l->data;
+
 			set_configuration(endpoint, dev, NULL, 0,
 						headset_setconf_cb, dev, NULL);
+		}
+
+		g_slist_free(list);
 	} else if (strcasecmp(uuid, HFP_HS_UUID) == 0 ||
 					strcasecmp(uuid, HSP_HS_UUID) == 0) {
-		struct audio_device *dev;
+		GSList *list;
+		GSList *l;
 
 		endpoint->ag_watch = gateway_add_state_cb(gateway_state_changed,
 								endpoint);
-		dev = manager_find_device(NULL, &adapter->src, BDADDR_ANY,
-						AUDIO_GATEWAY_INTERFACE, TRUE);
-		if (dev)
+		list = manager_find_devices(NULL, &adapter->src, BDADDR_ANY,
+						AUDIO_GATEWAY_INTERFACE, TRUE,
+						0);
+
+		for (l = list; l != NULL; l = l->next) {
+			struct audio_device *dev = l->data;
+
 			set_configuration(endpoint, dev, NULL, 0,
 						gateway_setconf_cb, dev, NULL);
+		}
+
+		g_slist_free(list);
 	} else {
 		if (err)
 			*err = -EINVAL;
-- 
1.7.7.6

[PATCH BlueZ v0 6/7] media: Enable parallel requests to endpoint

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

If endpoint is handling several devices, it should be able to handle
multiple requests, one per each.
---
 audio/media.c |   46 ++++++++++++++++++++++++----------------------
 1 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/audio/media.c b/audio/media.c
index 3324103..4384758 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -67,6 +67,7 @@ struct media_adapter {
 };
 
 struct endpoint_request {
+	struct media_endpoint	*endpoint;
 	DBusMessage		*msg;
 	DBusPendingCall		*call;
 	media_endpoint_cb_t	cb;
@@ -85,7 +86,7 @@ struct media_endpoint {
 	guint			hs_watch;
 	guint			ag_watch;
 	guint			watch;
-	struct endpoint_request *request;
+	GSList			*requests;
 	struct media_adapter	*adapter;
 	GSList			*transports;
 };
@@ -127,15 +128,22 @@ static void endpoint_request_free(struct endpoint_request *request)
 	g_free(request);
 }
 
-static void media_endpoint_cancel(struct media_endpoint *endpoint)
+static void media_endpoint_cancel(struct endpoint_request *request)
 {
-	struct endpoint_request *request = endpoint->request;
+	struct media_endpoint *endpoint = request->endpoint;
 
 	if (request->call)
 		dbus_pending_call_cancel(request->call);
 
+	endpoint->requests = g_slist_remove(endpoint->requests, request);
+
 	endpoint_request_free(request);
-	endpoint->request = NULL;
+}
+
+static void media_endpoint_cancel_all(struct media_endpoint *endpoint)
+{
+	while (endpoint->requests != NULL)
+		media_endpoint_cancel(endpoint->requests->data);
 }
 
 static void media_endpoint_destroy(struct media_endpoint *endpoint)
@@ -150,8 +158,7 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
 	if (endpoint->ag_watch)
 		gateway_remove_state_cb(endpoint->ag_watch);
 
-	if (endpoint->request)
-		media_endpoint_cancel(endpoint);
+	media_endpoint_cancel_all(endpoint);
 
 	g_slist_free_full(endpoint->transports,
 				(GDestroyNotify) media_transport_destroy);
@@ -228,8 +235,7 @@ done:
 
 static void clear_endpoint(struct media_endpoint *endpoint)
 {
-	if (endpoint->request)
-		media_endpoint_cancel(endpoint);
+	media_endpoint_cancel_all(endpoint);
 
 	while (endpoint->transports != NULL)
 		clear_configuration(endpoint, endpoint->transports->data);
@@ -237,8 +243,8 @@ static void clear_endpoint(struct media_endpoint *endpoint)
 
 static void endpoint_reply(DBusPendingCall *call, void *user_data)
 {
-	struct media_endpoint *endpoint = user_data;
-	struct endpoint_request *request = endpoint->request;
+	struct endpoint_request *request = user_data;
+	struct media_endpoint *endpoint = request->endpoint;
 	DBusMessage *reply;
 	DBusError err;
 	gboolean value;
@@ -299,9 +305,8 @@ done:
 	if (request->cb)
 		request->cb(endpoint, ret, size, request->user_data);
 
-	if (endpoint->request)
-		endpoint_request_free(endpoint->request);
-	endpoint->request = NULL;
+	endpoint->requests = g_slist_remove(endpoint->requests, request);
+	endpoint_request_free(request);
 }
 
 static gboolean media_endpoint_async_call(DBusConnection *conn,
@@ -313,9 +318,6 @@ static gboolean media_endpoint_async_call(DBusConnection *conn,
 {
 	struct endpoint_request *request;
 
-	if (endpoint->request)
-		return FALSE;
-
 	request = g_new0(struct endpoint_request, 1);
 
 	/* Timeout should be less than avdtp request timeout (4 seconds) */
@@ -326,13 +328,16 @@ static gboolean media_endpoint_async_call(DBusConnection *conn,
 		return FALSE;
 	}
 
-	dbus_pending_call_set_notify(request->call, endpoint_reply, endpoint, NULL);
+	dbus_pending_call_set_notify(request->call, endpoint_reply, request,
+									NULL);
 
+	request->endpoint = endpoint;
 	request->msg = msg;
 	request->cb = cb;
 	request->destroy = destroy;
 	request->user_data = user_data;
-	endpoint->request = request;
+
+	endpoint->requests = g_slist_append(endpoint->requests, request);
 
 	DBG("Calling %s: name = %s path = %s", dbus_message_get_member(msg),
 			dbus_message_get_destination(msg),
@@ -351,9 +356,6 @@ static gboolean select_configuration(struct media_endpoint *endpoint,
 	DBusConnection *conn;
 	DBusMessage *msg;
 
-	if (endpoint->request != NULL)
-		return FALSE;
-
 	conn = endpoint->adapter->conn;
 
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
@@ -412,7 +414,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 
 	transport = find_device_transport(endpoint, device);
 
-	if (transport != NULL || endpoint->request != NULL)
+	if (transport != NULL)
 		return FALSE;
 
 	conn = endpoint->adapter->conn;
-- 
1.7.7.6

[PATCH BlueZ v0 7/7] media: Increase hardcoded incoming MTU

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

When multiple SCO links are active, the packet size typically doubles.
---
 audio/transport.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/transport.c b/audio/transport.c
index 7bf7309..672fa52 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -356,7 +356,7 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
 	if (fd < 0)
 		goto fail;
 
-	imtu = 48;
+	imtu = 96;
 	omtu = 48;
 
 	media_transport_set_fd(transport, fd, imtu, omtu);
@@ -461,7 +461,7 @@ static void gateway_resume_complete(struct audio_device *dev, GError *err,
 	if (fd < 0)
 		goto fail;
 
-	imtu = 48;
+	imtu = 96;
 	omtu = 48;
 
 	media_transport_set_fd(transport, fd, imtu, omtu);
-- 
1.7.7.6

Re: [PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> When the socket is open is client mode, the bind() system call should
> not be used.
> ---
>  btio/btio.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index 9781ec4..7564d85 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -1317,8 +1317,9 @@ static GIOChannel *create_io(BtIOType type, gboolean server,
>                        ERROR_FAILED(err, "socket(SEQPACKET, SCO)", errno);
>                        return NULL;
>                }
> -               if (sco_bind(sock, &opts->src, err) < 0)
> -                       goto failed;
> +               if (server)
> +                       if (sco_bind(sock, &opts->src, err) < 0)
> +                               goto failed;
>                if (!sco_set(sock, opts->mtu, err))
>                        goto failed;
>                break;
> --
> 1.7.7.6

Ack, in future send this kind of trivial/obvious fix in separated so
it can easily be picked.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 2/7] audio: Fix gateway state check

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> Gateway should be considered active also if connecting or playing.
>
> This could for example lead to manager_find_device() not returning a
> device that is connecting, and thus the corresponding endpoint would
> never be created in the Media API.
> ---
>  audio/device.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/device.c b/audio/device.c
> index a9d35f9..ee1ade1 100644
> --- a/audio/device.c
> +++ b/audio/device.c
> @@ -701,7 +701,7 @@ gboolean audio_device_is_active(struct audio_device *dev,
>                                control_is_active(dev))
>                return TRUE;
>        else if (!strcmp(interface, AUDIO_GATEWAY_INTERFACE) && dev->gateway &&
> -                               gateway_is_connected(dev))
> +                               gateway_is_active(dev))
>                return TRUE;
>
>        return FALSE;
> --
> 1.7.7.6
>
> --

Looks good, ack.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 2:20 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Mikel,
>
> On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
>> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>>
>> When the socket is open is client mode, the bind() system call should
>> not be used.
>> ---
>>  btio/btio.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/btio/btio.c b/btio/btio.c
>> index 9781ec4..7564d85 100644
>> --- a/btio/btio.c
>> +++ b/btio/btio.c
>> @@ -1317,8 +1317,9 @@ static GIOChannel *create_io(BtIOType type, gboolean server,
>>                        ERROR_FAILED(err, "socket(SEQPACKET, SCO)", errno);
>>                        return NULL;
>>                }
>> -               if (sco_bind(sock, &opts->src, err) < 0)
>> -                       goto failed;
>> +               if (server)
>> +                       if (sco_bind(sock, &opts->src, err) < 0)
>> +                               goto failed;
>>                if (!sco_set(sock, opts->mtu, err))
>>                        goto failed;
>>                break;
>> --
>> 1.7.7.6
>
> Ack, in future send this kind of trivial/obvious fix in separated so
> it can easily be picked.

Actually I take it back, the bind is necessary to set the adapter
address otherwise you can't choose with which adapter you want to
connect so it always use adapter any.

-- 
Luiz Augusto von Dentz

RE: [PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Mikel Astiz]

Hi Luiz,

> 
> Hi Mikel,
> 
> On Wed, Apr 11, 2012 at 2:20 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Hi Mikel,
> >
> > On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz
> <mikel.astiz.oss@gmail.com> wrote:
> >> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
> >>
> >> When the socket is open is client mode, the bind() system call
> should
> >> not be used.
> >> ---
> >>  btio/btio.c |    5 +++--
> >>  1 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/btio/btio.c b/btio/btio.c index 9781ec4..7564d85 100644
> >> --- a/btio/btio.c
> >> +++ b/btio/btio.c
> >> @@ -1317,8 +1317,9 @@ static GIOChannel *create_io(BtIOType type,
> >> gboolean server,
> >>                        ERROR_FAILED(err, "socket(SEQPACKET, SCO)",
> >> errno);
> >>                        return NULL;
> >>                }
> >> -               if (sco_bind(sock, &opts->src, err) < 0)
> >> -                       goto failed;
> >> +               if (server)
> >> +                       if (sco_bind(sock, &opts->src, err) < 0)
> >> +                               goto failed;
> >>                if (!sco_set(sock, opts->mtu, err))
> >>                        goto failed;
> >>                break;
> >> --
> >> 1.7.7.6
> >
> > Ack, in future send this kind of trivial/obvious fix in separated so
> > it can easily be picked.
> 
> Actually I take it back, the bind is necessary to set the adapter
> address otherwise you can't choose with which adapter you want to
> connect so it always use adapter any.
> 

Nice catch, it wasn't that simple after all.

If I understand this correctly, this means we need some kernel patches. Either (a) we add the source address in sockaddr_sco so we can drop the bind() call (because sco_sock_connect would already receive both addresses), or (b) we modify sco_sock_bind such that it doesn't complain with EADDRINUSE.

I don't have much experience with sockets but the second approach seems a workaround to me.

On the other hand, the first approach would need special care not to break existing userland code. However this doesn't seem very difficult.

Any other proposals?

Cheers,
Mikel

Re: [PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Johan Hedberg]

Hi Mikel,

On Thu, Apr 12, 2012, Mikel Astiz wrote:
> If I understand this correctly, this means we need some kernel
> patches. Either (a) we add the source address in sockaddr_sco so we
> can drop the bind() call (because sco_sock_connect would already
> receive both addresses), or (b) we modify sco_sock_bind such that it
> doesn't complain with EADDRINUSE.
> 
> I don't have much experience with sockets but the second approach
> seems a workaround to me.

Actually b) sounds more like a bugfix than a workaround (and a doesn't
sound like a good idea at all). Returning that error only makes sense
for server sockets but we can't know if this is a server socket or not
at the bind stage. What should probably be done is that the EADDRINUSE
isn't returned for bind() but for listen().

Johan

Re: [PATCH BlueZ v0 1/7] btio: Fix bind SCO socket only if server

[Mikel Astiz]

Hi Johan,

On Thu, Apr 12, 2012 at 11:18 AM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>
> Hi Mikel,
>
> On Thu, Apr 12, 2012, Mikel Astiz wrote:
> > If I understand this correctly, this means we need some kernel
> > patches. Either (a) we add the source address in sockaddr_sco so we
> > can drop the bind() call (because sco_sock_connect would already
> > receive both addresses), or (b) we modify sco_sock_bind such that it
> > doesn't complain with EADDRINUSE.
> >
> > I don't have much experience with sockets but the second approach
> > seems a workaround to me.
>
> Actually b) sounds more like a bugfix than a workaround (and a doesn't
> sound like a good idea at all). Returning that error only makes sense
> for server sockets but we can't know if this is a server socket or not
> at the bind stage. What should probably be done is that the EADDRINUSE
> isn't returned for bind() but for listen().
>
> Johan

I will soon send a kernel patch that follows your suggestion.

Cheers,
Mikel

Re: [PATCH BlueZ v0 3/7] audio: Add multiple device search to manager

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
> -struct audio_device *manager_find_device(const char *path,
> +GSList *manager_find_devices(const char *path,
>                                        const bdaddr_t *src,
>                                        const bdaddr_t *dst,
>                                        const char *interface,
> -                                       gboolean connected)
> +                                       gboolean connected,
> +                                       size_t max_count)
>  {

It doesn't look like this max_count parameter is going to be useful,
iirc you only use 0 in the patches so I would recommending leaving it
out for now.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 4/7] media: Support multiple transports per endpoint

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> +static struct media_transport *get_unique_transport(
> +                                       struct media_endpoint *endpoint)
> +{
> +       if (endpoint->transports == NULL)
> +               return NULL;
> +
> +       if (endpoint->transports->next != NULL)
> +               return NULL;
> +
> +       return endpoint->transports->data;
>  }

This doesn't look necessary in case of A2DP the endpoint cannot be
reused because of in_use flag, but overall the rest looks good.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 6/7] media: Enable parallel requests to endpoint

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> If endpoint is handling several devices, it should be able to handle
> multiple requests, one per each.
> ---
>  audio/media.c |   46 ++++++++++++++++++++++++----------------------
>  1 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/audio/media.c b/audio/media.c
> index 3324103..4384758 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -67,6 +67,7 @@ struct media_adapter {
>  };
>
>  struct endpoint_request {
> +       struct media_endpoint   *endpoint;
>        DBusMessage             *msg;
>        DBusPendingCall         *call;
>        media_endpoint_cb_t     cb;
> @@ -85,7 +86,7 @@ struct media_endpoint {
>        guint                   hs_watch;
>        guint                   ag_watch;
>        guint                   watch;
> -       struct endpoint_request *request;
> +       GSList                  *requests;
>        struct media_adapter    *adapter;
>        GSList                  *transports;
>  };
> @@ -127,15 +128,22 @@ static void endpoint_request_free(struct endpoint_request *request)
>        g_free(request);
>  }
>
> -static void media_endpoint_cancel(struct media_endpoint *endpoint)
> +static void media_endpoint_cancel(struct endpoint_request *request)
>  {
> -       struct endpoint_request *request = endpoint->request;
> +       struct media_endpoint *endpoint = request->endpoint;
>
>        if (request->call)
>                dbus_pending_call_cancel(request->call);
>
> +       endpoint->requests = g_slist_remove(endpoint->requests, request);
> +
>        endpoint_request_free(request);
> -       endpoint->request = NULL;
> +}
> +
> +static void media_endpoint_cancel_all(struct media_endpoint *endpoint)
> +{
> +       while (endpoint->requests != NULL)
> +               media_endpoint_cancel(endpoint->requests->data);
>  }
>
>  static void media_endpoint_destroy(struct media_endpoint *endpoint)
> @@ -150,8 +158,7 @@ static void media_endpoint_destroy(struct media_endpoint *endpoint)
>        if (endpoint->ag_watch)
>                gateway_remove_state_cb(endpoint->ag_watch);
>
> -       if (endpoint->request)
> -               media_endpoint_cancel(endpoint);
> +       media_endpoint_cancel_all(endpoint);
>
>        g_slist_free_full(endpoint->transports,
>                                (GDestroyNotify) media_transport_destroy);
> @@ -228,8 +235,7 @@ done:
>
>  static void clear_endpoint(struct media_endpoint *endpoint)
>  {
> -       if (endpoint->request)
> -               media_endpoint_cancel(endpoint);
> +       media_endpoint_cancel_all(endpoint);
>
>        while (endpoint->transports != NULL)
>                clear_configuration(endpoint, endpoint->transports->data);
> @@ -237,8 +243,8 @@ static void clear_endpoint(struct media_endpoint *endpoint)
>
>  static void endpoint_reply(DBusPendingCall *call, void *user_data)
>  {
> -       struct media_endpoint *endpoint = user_data;
> -       struct endpoint_request *request = endpoint->request;
> +       struct endpoint_request *request = user_data;
> +       struct media_endpoint *endpoint = request->endpoint;
>        DBusMessage *reply;
>        DBusError err;
>        gboolean value;
> @@ -299,9 +305,8 @@ done:
>        if (request->cb)
>                request->cb(endpoint, ret, size, request->user_data);
>
> -       if (endpoint->request)
> -               endpoint_request_free(endpoint->request);
> -       endpoint->request = NULL;
> +       endpoint->requests = g_slist_remove(endpoint->requests, request);
> +       endpoint_request_free(request);
>  }
>
>  static gboolean media_endpoint_async_call(DBusConnection *conn,
> @@ -313,9 +318,6 @@ static gboolean media_endpoint_async_call(DBusConnection *conn,
>  {
>        struct endpoint_request *request;
>
> -       if (endpoint->request)
> -               return FALSE;
> -
>        request = g_new0(struct endpoint_request, 1);
>
>        /* Timeout should be less than avdtp request timeout (4 seconds) */
> @@ -326,13 +328,16 @@ static gboolean media_endpoint_async_call(DBusConnection *conn,
>                return FALSE;
>        }
>
> -       dbus_pending_call_set_notify(request->call, endpoint_reply, endpoint, NULL);
> +       dbus_pending_call_set_notify(request->call, endpoint_reply, request,
> +                                                                       NULL);
>
> +       request->endpoint = endpoint;
>        request->msg = msg;
>        request->cb = cb;
>        request->destroy = destroy;
>        request->user_data = user_data;
> -       endpoint->request = request;
> +
> +       endpoint->requests = g_slist_append(endpoint->requests, request);
>
>        DBG("Calling %s: name = %s path = %s", dbus_message_get_member(msg),
>                        dbus_message_get_destination(msg),
> @@ -351,9 +356,6 @@ static gboolean select_configuration(struct media_endpoint *endpoint,
>        DBusConnection *conn;
>        DBusMessage *msg;
>
> -       if (endpoint->request != NULL)
> -               return FALSE;
> -
>        conn = endpoint->adapter->conn;
>
>        msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
> @@ -412,7 +414,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
>
>        transport = find_device_transport(endpoint, device);
>
> -       if (transport != NULL || endpoint->request != NULL)
> +       if (transport != NULL)
>                return FALSE;
>
>        conn = endpoint->adapter->conn;
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ack

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 7/7] media: Increase hardcoded incoming MTU

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> When multiple SCO links are active, the packet size typically doubles.
> ---
>  audio/transport.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/audio/transport.c b/audio/transport.c
> index 7bf7309..672fa52 100644
> --- a/audio/transport.c
> +++ b/audio/transport.c
> @@ -356,7 +356,7 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
>        if (fd < 0)
>                goto fail;
>
> -       imtu = 48;
> +       imtu = 96;
>        omtu = 48;
>
>        media_transport_set_fd(transport, fd, imtu, omtu);
> @@ -461,7 +461,7 @@ static void gateway_resume_complete(struct audio_device *dev, GError *err,
>        if (fd < 0)
>                goto fail;
>
> -       imtu = 48;
> +       imtu = 96;
>        omtu = 48;
>
>        media_transport_set_fd(transport, fd, imtu, omtu);
> --
> 1.7.7.6

Nak, this probably needs more investigation why doubling the MTU works.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v0 3/7] audio: Add multiple device search to manager

[Mikel Astiz]

Hi Luiz,

On Tue, Apr 17, 2012 at 2:20 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Mikel,
>
> On Wed, Apr 11, 2012 at 9:57 AM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
>> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>> -struct audio_device *manager_find_device(const char *path,
>> +GSList *manager_find_devices(const char *path,
>>                                        const bdaddr_t *src,
>>                                        const bdaddr_t *dst,
>>                                        const char *interface,
>> -                                       gboolean connected)
>> +                                       gboolean connected,
>> +                                       size_t max_count)
>>  {
>
> It doesn't look like this max_count parameter is going to be useful,
> iirc you only use 0 in the patches so I would recommending leaving it
> out for now.

It's actually being used in manager_find_device() with max_count==1,
after the lines from the old function have been moved to
manager_find_devices() to avoid duplicated code.

Cheers,
Mikel