linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
@ 2019-01-22 13:45 Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 2/9] a2dp: Expose " Luiz Augusto von Dentz
                   ` (8 more replies)
  0 siblings, 9 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the possibility to expose remote SEP using MediaEndpoint
interface to allow setting a configuration.
---
 doc/media-api.txt | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index b5ad2db12..af9485342 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -500,14 +500,23 @@ Properties	object Player [readonly]
 MediaEndpoint1 hierarchy
 ========================
 
-Service		unique name
+Service		unique name (Server role)
+		org.bluez (Client role)
 Interface	org.bluez.MediaEndpoint1
-Object path	freely definable
+Object path	freely definable (Server role)
+		[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
+		(Client role)
 
 Methods		void SetConfiguration(object transport, dict properties)
 
 			Set configuration for the transport.
 
+			For client role transport must be set with a server
+			endpoint oject which will be configured and the
+			properties must contain the following properties:
+
+				array{byte} Capabilities
+
 		array{byte} SelectConfiguration(array{byte} capabilities)
 
 			Select preferable configuration from the supported
@@ -532,6 +541,20 @@ Methods		void SetConfiguration(object transport, dict properties)
 			endpoint, because when this method gets called it has
 			already been unregistered.
 
+Properties	string UUID [readonly, optional]:
+
+			UUID of the profile which the endpoint is for.
+
+		byte Codec [readonly, optional]:
+
+			Assigned number of codec that the endpoint implements.
+			The values should match the profile specification which
+			is indicated by the UUID.
+
+		array{byte} Capabilities [readonly, optional]:
+
+			Capabilities blob, it is used as it is so the size and
+			byte order must match.
 
 MediaTransport1 hierarchy
 =========================
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 2/9] a2dp: Expose remote SEP
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 3/9] doc/media-api: Add Endpoint property to MediaTransport Luiz Augusto von Dentz
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This implements MediaEndpoint for remote SEP which can be used by
clients to switch configuration on demand.
---
 profiles/audio/a2dp.c  | 351 ++++++++++++++++++++++++++++++++++++++++-
 profiles/audio/a2dp.h  |   1 +
 profiles/audio/avdtp.c |  10 ++
 profiles/audio/avdtp.h |   4 +
 profiles/audio/media.c |   8 +
 5 files changed, 367 insertions(+), 7 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 344459332..4025776aa 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -27,7 +27,10 @@
 #include <config.h>
 #endif
 
+#define _GNU_SOURCE
+
 #include <stdlib.h>
+#include <stdio.h>
 #include <errno.h>
 
 #include <dbus/dbus.h>
@@ -38,14 +41,19 @@
 #include "lib/sdp_lib.h"
 #include "lib/uuid.h"
 
+#include "gdbus/gdbus.h"
+
 #include "src/plugin.h"
 #include "src/adapter.h"
 #include "src/device.h"
+#include "src/dbus-common.h"
+#include "src/error.h"
 #include "src/profile.h"
 #include "src/service.h"
 #include "src/log.h"
 #include "src/sdpd.h"
 #include "src/shared/queue.h"
+#include "src/shared/util.h"
 
 #include "btio/btio.h"
 
@@ -63,6 +71,8 @@
 
 #define AVDTP_PSM 25
 
+#define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
+
 struct a2dp_sep {
 	struct a2dp_server *server;
 	struct a2dp_endpoint *endpoint;
@@ -93,6 +103,7 @@ struct a2dp_setup_cb {
 };
 
 struct a2dp_setup {
+	struct a2dp_channel *chan;
 	struct avdtp *session;
 	struct a2dp_sep *sep;
 	struct avdtp_remote_sep *rsep;
@@ -121,6 +132,12 @@ struct a2dp_server {
 	struct queue *channels;
 };
 
+struct a2dp_remote_sep {
+	struct a2dp_channel *chan;
+	char *path;
+	struct avdtp_remote_sep *sep;
+};
+
 struct a2dp_channel {
 	struct a2dp_server *server;
 	struct btd_device *device;
@@ -129,6 +146,7 @@ struct a2dp_channel {
 	unsigned int state_id;
 	unsigned int auth_id;
 	struct avdtp *session;
+	struct queue *seps;
 };
 
 static GSList *servers = NULL;
@@ -144,12 +162,42 @@ static struct a2dp_setup *setup_ref(struct a2dp_setup *setup)
 	return setup;
 }
 
+static bool match_by_session(const void *data, const void *user_data)
+{
+	const struct a2dp_channel *chan = data;
+	const struct avdtp *session = user_data;
+
+	return chan->session == session;
+}
+
+static struct a2dp_channel *find_channel(struct avdtp *session)
+{
+	GSList *l;
+
+	for (l = servers; l; l = g_slist_next(l)) {
+		struct a2dp_server *server = l->data;
+		struct a2dp_channel *chan;
+
+		chan = queue_find(server->channels, match_by_session, session);
+		if (chan)
+			return chan;
+	}
+
+	return NULL;
+}
+
 static struct a2dp_setup *setup_new(struct avdtp *session)
 {
 	struct a2dp_setup *setup;
+	struct a2dp_channel *chan;
+
+	chan = find_channel(session);
+	if (!chan)
+		return NULL;
 
 	setup = g_new0(struct a2dp_setup, 1);
 	setup->session = avdtp_ref(session);
+	setup->chan = find_channel(session);
 	setups = g_slist_append(setups, setup);
 
 	return setup;
@@ -1299,6 +1347,14 @@ static struct a2dp_server *find_server(GSList *list, struct btd_adapter *a)
 	return NULL;
 }
 
+static void remove_remote_sep(void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+
+	g_dbus_unregister_interface(btd_get_dbus_connection(), sep->path,
+						MEDIA_ENDPOINT_INTERFACE);
+}
+
 static void channel_free(void *data)
 {
 	struct a2dp_channel *chan = data;
@@ -1316,6 +1372,7 @@ static void channel_free(void *data)
 
 	avdtp_remove_state_cb(chan->state_id);
 
+	queue_destroy(chan->seps, remove_remote_sep);
 	g_free(chan);
 }
 
@@ -1371,6 +1428,7 @@ static struct a2dp_channel *channel_new(struct a2dp_server *server,
 	chan = g_new0(struct a2dp_channel, 1);
 	chan->server = server;
 	chan->device = device;
+	chan->seps = queue_new();
 	chan->state_id = avdtp_add_state_cb(device, avdtp_state_cb, chan);
 
 	if (!queue_push_tail(server->channels, chan)) {
@@ -1805,16 +1863,11 @@ void a2dp_remove_sep(struct a2dp_sep *sep)
 	a2dp_unregister_sep(sep);
 }
 
-static void select_cb(struct a2dp_setup *setup, void *ret, int size)
+static void setup_add_caps(struct a2dp_setup *setup, uint8_t *caps, size_t size)
 {
 	struct avdtp_service_capability *media_transport, *media_codec;
 	struct avdtp_media_codec_capability *cap;
 
-	if (size < 0) {
-		DBG("Endpoint replied an invalid configuration");
-		goto done;
-	}
-
 	media_transport = avdtp_service_cap_new(AVDTP_MEDIA_TRANSPORT,
 						NULL, 0);
 
@@ -1823,13 +1876,23 @@ static void select_cb(struct a2dp_setup *setup, void *ret, int size)
 	cap = g_malloc0(sizeof(*cap) + size);
 	cap->media_type = AVDTP_MEDIA_TYPE_AUDIO;
 	cap->media_codec_type = setup->sep->codec;
-	memcpy(cap->data, ret, size);
+	memcpy(cap->data, caps, size);
 
 	media_codec = avdtp_service_cap_new(AVDTP_MEDIA_CODEC, cap,
 						sizeof(*cap) + size);
 
 	setup->caps = g_slist_append(setup->caps, media_codec);
 	g_free(cap);
+}
+
+static void select_cb(struct a2dp_setup *setup, void *ret, int size)
+{
+	if (size < 0) {
+		DBG("Endpoint replied an invalid configuration");
+		goto done;
+	}
+
+	setup_add_caps(setup, ret, size);
 
 done:
 	finalize_select(setup);
@@ -1885,6 +1948,277 @@ static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
 	return a2dp_find_sep(session, l, NULL);
 }
 
+struct client {
+	const char *sender;
+	const char *path;
+};
+
+static int match_client(const void *data, const void *user_data)
+{
+	struct a2dp_sep *sep = (void *) data;
+	const struct a2dp_endpoint *endpoint = sep->endpoint;
+	const struct client *client = user_data;
+
+	if (strcmp(client->sender, endpoint->get_name(sep, sep->user_data)))
+		return -1;
+
+	return strcmp(client->path, endpoint->get_path(sep, sep->user_data));
+}
+
+static struct a2dp_sep *find_sep(struct a2dp_server *server, const char *sender,
+							const char *path)
+{
+	GSList *l;
+	struct client client = { sender, path };
+
+	l = g_slist_find_custom(server->sources, &client, match_client);
+	if (l)
+		return l->data;
+
+	l = g_slist_find_custom(server->sinks, &client, match_client);
+	if (l)
+		return l->data;
+
+	return NULL;
+}
+
+static int parse_properties(DBusMessageIter *props, uint8_t **caps, int *size)
+{
+	while (dbus_message_iter_get_arg_type(props) == DBUS_TYPE_DICT_ENTRY) {
+		const char *key;
+		DBusMessageIter value, entry;
+		int var;
+
+		dbus_message_iter_recurse(props, &entry);
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+		dbus_message_iter_recurse(&entry, &value);
+
+		var = dbus_message_iter_get_arg_type(&value);
+		if (strcasecmp(key, "Capabilities") == 0) {
+			DBusMessageIter array;
+
+			if (var != DBUS_TYPE_ARRAY)
+				return -EINVAL;
+
+			dbus_message_iter_recurse(&value, &array);
+			dbus_message_iter_get_fixed_array(&array, caps, size);
+			return 0;
+		}
+
+		dbus_message_iter_next(props);
+	}
+
+	return -EINVAL;
+}
+
+static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
+			struct a2dp_sep *lsep, struct a2dp_remote_sep *rsep,
+			uint8_t *caps, int size)
+{
+	struct a2dp_setup *setup;
+	const struct queue_entry *entry;
+	int err;
+
+	setup = a2dp_setup_get(chan->session);
+	if (!setup)
+		return -ENOMEM;
+
+	setup->sep = lsep;
+	setup->rsep = rsep->sep;
+
+	setup_add_caps(setup, caps, size);
+
+	/* Check for existing stream and close it */
+	for (entry = queue_get_entries(chan->server->seps); entry;
+						entry = entry->next) {
+		struct a2dp_sep *tmp = entry->data;
+
+		/* Attempt to reconfigure if a stream already exists */
+		if (tmp->stream) {
+			/* Only allow switching sep from the same sender */
+			if (strcmp(sender, tmp->endpoint->get_name(tmp,
+							tmp->user_data)))
+				return -EPERM;
+
+			err = avdtp_close(chan->session, tmp->stream, FALSE);
+			if (err < 0) {
+				error("avdtp_close: %s", strerror(-err));
+				return err;
+			}
+
+			setup->reconfigure = TRUE;
+
+			return 0;
+		}
+	}
+
+	err = avdtp_set_configuration(setup->session, setup->rsep,
+						lsep->lsep,
+						setup->caps,
+						&setup->stream);
+	if (err < 0) {
+		error("avdtp_set_configuration: %s", strerror(-err));
+		return err;
+	}
+
+	return 0;
+}
+
+static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct a2dp_remote_sep *rsep = data;
+	struct a2dp_channel *chan = rsep->chan;
+	struct a2dp_sep *lsep;
+	struct avdtp_service_capability *service;
+	struct avdtp_media_codec_capability *codec;
+	DBusMessageIter args, props;
+	const char *sender, *path;
+	uint8_t *caps;
+	int err, size = 0;
+
+	sender = dbus_message_get_sender(msg);
+
+	dbus_message_iter_init(msg, &args);
+
+	dbus_message_iter_get_basic(&args, &path);
+	dbus_message_iter_next(&args);
+
+	lsep = find_sep(chan->server, sender, path);
+	if (!lsep)
+		return btd_error_invalid_args(msg);
+
+	/* Check if SEPs are no the same role */
+	if (avdtp_get_type(rsep->sep) == lsep->type)
+		return btd_error_invalid_args(msg);
+
+	service = avdtp_get_codec(rsep->sep);
+	codec = (struct avdtp_media_codec_capability *) service->data;
+
+	/* Check if codec match */
+	if (!endpoint_match_codec_ind(chan->session, codec, lsep))
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&args, &props);
+	if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
+		return btd_error_invalid_args(msg);
+
+	if (parse_properties(&props, &caps, &size) < 0)
+		return btd_error_invalid_args(msg);
+
+	err = a2dp_reconfig(chan, sender, lsep, rsep, caps, size);
+	if (err < 0)
+		return btd_error_failed(msg, strerror(-err));
+
+	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+}
+
+static const GDBusMethodTable sep_methods[] = {
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("SetConfiguration",
+					GDBUS_ARGS({ "endpoint", "o" },
+						{ "properties", "a{sv}" } ),
+					NULL, set_configuration) },
+	{ },
+};
+
+static gboolean get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	const char *uuid;
+
+	switch (avdtp_get_type(sep->sep)) {
+	case AVDTP_SEP_TYPE_SOURCE:
+		uuid = A2DP_SOURCE_UUID;
+		break;
+	case AVDTP_SEP_TYPE_SINK:
+		uuid = A2DP_SOURCE_UUID;
+		break;
+	default:
+		uuid = "";
+	}
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+
+	return TRUE;
+}
+
+static gboolean get_codec(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep);
+	struct avdtp_media_codec_capability *codec = (void *) cap->data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
+						&codec->media_codec_type);
+
+	return TRUE;
+}
+
+static gboolean get_capabilities(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	struct avdtp_service_capability *service = avdtp_get_codec(sep->sep);
+	struct avdtp_media_codec_capability *codec = (void *) service->data;
+	uint8_t *caps = codec->data;
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_BYTE_AS_STRING, &array);
+
+	dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE, &caps,
+					service->length - sizeof(*codec));
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable sep_properties[] = {
+	{ "UUID", "s", get_uuid, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Codec", "y", get_codec, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Capabilities", "ay", get_capabilities, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ }
+};
+
+static void remote_sep_free(void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+
+	free(sep->path);
+	free(sep);
+}
+
+static void register_remote_sep(void *data, void *user_data)
+{
+	struct avdtp_remote_sep *rsep = data;
+	struct a2dp_setup *setup = user_data;
+	struct a2dp_remote_sep *sep;
+
+	sep = new0(struct a2dp_remote_sep, 1);
+	sep->chan = setup->chan;
+	sep->sep = rsep;
+	asprintf(&sep->path, "%s/sep%d", device_get_path(setup->chan->device),
+							avdtp_get_seid(rsep));
+
+	if (g_dbus_register_interface(btd_get_dbus_connection(),
+				sep->path, MEDIA_ENDPOINT_INTERFACE,
+				sep_methods, NULL, sep_properties,
+				sep, remote_sep_free) == FALSE) {
+		error("Could not register remote sep %s", sep->path);
+		remote_sep_free(sep);
+	}
+
+	queue_push_tail(setup->chan->seps, sep);
+}
+
 static void discover_cb(struct avdtp *session, GSList *seps,
 				struct avdtp_error *err, void *user_data)
 {
@@ -1895,6 +2229,9 @@ static void discover_cb(struct avdtp *session, GSList *seps,
 	setup->seps = seps;
 	setup->err = err;
 
+	if (!err && queue_isempty(setup->chan->seps))
+		g_slist_foreach(seps, register_remote_sep, setup);
+
 	finalize_discover(setup);
 }
 
diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
index 2c388bb68..7f38c75f3 100644
--- a/profiles/audio/a2dp.h
+++ b/profiles/audio/a2dp.h
@@ -32,6 +32,7 @@ typedef void (*a2dp_endpoint_config_t) (struct a2dp_setup *setup, gboolean ret);
 
 struct a2dp_endpoint {
 	const char *(*get_name) (struct a2dp_sep *sep, void *user_data);
+	const char *(*get_path) (struct a2dp_sep *sep, void *user_data);
 	size_t (*get_capabilities) (struct a2dp_sep *sep,
 						uint8_t **capabilities,
 						void *user_data);
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index c20cec386..49551a921 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -3170,6 +3170,16 @@ static int process_queue(struct avdtp *session)
 	return send_req(session, FALSE, req);
 }
 
+uint8_t avdtp_get_seid(struct avdtp_remote_sep *sep)
+{
+	return sep->seid;
+}
+
+uint8_t avdtp_get_type(struct avdtp_remote_sep *sep)
+{
+	return sep->type;
+}
+
 struct avdtp_service_capability *avdtp_get_codec(struct avdtp_remote_sep *sep)
 {
 	return sep->codec;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index 621a6e3cf..e5fc40c89 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -223,6 +223,10 @@ struct avdtp *avdtp_ref(struct avdtp *session);
 struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
 							void *data, int size);
 
+uint8_t avdtp_get_seid(struct avdtp_remote_sep *sep);
+
+uint8_t avdtp_get_type(struct avdtp_remote_sep *sep);
+
 struct avdtp_service_capability *avdtp_get_codec(struct avdtp_remote_sep *sep);
 
 int avdtp_discover(struct avdtp *session, avdtp_discover_cb_t cb,
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index e2a447e56..9d7564cf0 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -489,6 +489,13 @@ static const char *get_name(struct a2dp_sep *sep, void *user_data)
 	return endpoint->sender;
 }
 
+static const char *get_path(struct a2dp_sep *sep, void *user_data)
+{
+	struct media_endpoint *endpoint = user_data;
+
+	return endpoint->path;
+}
+
 static size_t get_capabilities(struct a2dp_sep *sep, uint8_t **capabilities,
 							void *user_data)
 {
@@ -579,6 +586,7 @@ static void set_delay(struct a2dp_sep *sep, uint16_t delay, void *user_data)
 
 static struct a2dp_endpoint a2dp_endpoint = {
 	.get_name = get_name,
+	.get_path = get_path,
 	.get_capabilities = get_capabilities,
 	.select_configuration = select_config,
 	.set_configuration = set_config,
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 3/9] doc/media-api: Add Endpoint property to MediaTransport
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 2/9] a2dp: Expose " Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 4/9] a2dp: Implement MediaTransport.Endpoint Luiz Augusto von Dentz
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Adds endpoint object to MediaTransport so application can resolve which
MediaEndpoint is in use.
---
 doc/media-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index af9485342..292e9034c 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -627,3 +627,8 @@ Properties	object Device [readonly]
 			acquired by the sender.
 
 			Possible Values: 0-127
+
+		object Endpoint [readonly, optional, experimental]
+
+			Endpoint object which the transport is associated
+			with.
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 4/9] a2dp: Implement MediaTransport.Endpoint
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 2/9] a2dp: Expose " Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 3/9] doc/media-api: Add Endpoint property to MediaTransport Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 5/9] doc/settings-storage: Add Endpoint group to cache Luiz Augusto von Dentz
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This implements MediaTransport.Endpoint property which exposes what
endpoint is being used by the transport.
---
 profiles/audio/a2dp.c      | 91 +++++++++++++++++++++++++++++---------
 profiles/audio/a2dp.h      |  1 +
 profiles/audio/media.c     |  5 ++-
 profiles/audio/transport.c | 28 +++++++++++-
 profiles/audio/transport.h |  1 +
 5 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 4025776aa..4fa01894a 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -106,7 +106,7 @@ struct a2dp_setup {
 	struct a2dp_channel *chan;
 	struct avdtp *session;
 	struct a2dp_sep *sep;
-	struct avdtp_remote_sep *rsep;
+	struct a2dp_remote_sep *rsep;
 	struct avdtp_stream *stream;
 	struct avdtp_error *err;
 	avdtp_set_configuration_cb setconf_cb;
@@ -1065,6 +1065,24 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 	return TRUE;
 }
 
+static bool match_remote_sep(const void *data, const void *user_data)
+{
+	const struct a2dp_remote_sep *sep = data;
+	const struct avdtp_remote_sep *rsep = user_data;
+
+	return sep->sep == rsep;
+}
+
+static struct a2dp_remote_sep *find_remote_sep(struct a2dp_channel *chan,
+						struct a2dp_sep *sep)
+{
+	struct avdtp_remote_sep *rsep;
+
+	rsep = avdtp_find_remote_sep(chan->session, sep->lsep);
+
+	return queue_find(chan->seps, match_remote_sep, rsep);
+}
+
 static gboolean a2dp_reconfigure(gpointer data)
 {
 	struct a2dp_setup *setup = data;
@@ -1074,14 +1092,14 @@ static gboolean a2dp_reconfigure(gpointer data)
 	struct avdtp_service_capability *cap;
 
 	if (setup->rsep) {
-		cap = avdtp_get_codec(setup->rsep);
+		cap = avdtp_get_codec(setup->rsep->sep);
 		rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
 	}
 
 	if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
-		setup->rsep = avdtp_find_remote_sep(setup->session, sep->lsep);
+		setup->rsep = find_remote_sep(setup->chan, sep);
 
-	posix_err = avdtp_set_configuration(setup->session, setup->rsep,
+	posix_err = avdtp_set_configuration(setup->session, setup->rsep->sep,
 						sep->lsep,
 						setup->caps,
 						&setup->stream);
@@ -1097,6 +1115,16 @@ failed:
 	return FALSE;
 }
 
+static struct a2dp_remote_sep *get_remote_sep(struct a2dp_channel *chan,
+						struct avdtp_stream *stream)
+{
+	struct avdtp_remote_sep *rsep;
+
+	rsep = avdtp_stream_get_remote_sep(stream);
+
+	return queue_find(chan->seps, match_remote_sep, rsep);
+}
+
 static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 			struct avdtp_stream *stream, struct avdtp_error *err,
 			void *user_data)
@@ -1121,7 +1149,7 @@ static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	}
 
 	if (!setup->rsep)
-		setup->rsep = avdtp_stream_get_remote_sep(stream);
+		setup->rsep = get_remote_sep(setup->chan, stream);
 
 	if (setup->reconfigure)
 		g_timeout_add(RECONFIGURE_TIMEOUT, a2dp_reconfigure, setup);
@@ -1347,10 +1375,23 @@ static struct a2dp_server *find_server(GSList *list, struct btd_adapter *a)
 	return NULL;
 }
 
+static void remote_sep_free(void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+
+	free(sep->path);
+	free(sep);
+}
+
 static void remove_remote_sep(void *data)
 {
 	struct a2dp_remote_sep *sep = data;
 
+	if (!sep->path) {
+		remote_sep_free(sep);
+		return;
+	}
+
 	g_dbus_unregister_interface(btd_get_dbus_connection(), sep->path,
 						MEDIA_ENDPOINT_INTERFACE);
 }
@@ -2026,7 +2067,7 @@ static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
 		return -ENOMEM;
 
 	setup->sep = lsep;
-	setup->rsep = rsep->sep;
+	setup->rsep = rsep;
 
 	setup_add_caps(setup, caps, size);
 
@@ -2054,7 +2095,7 @@ static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
 		}
 	}
 
-	err = avdtp_set_configuration(setup->session, setup->rsep,
+	err = avdtp_set_configuration(setup->session, setup->rsep->sep,
 						lsep->lsep,
 						setup->caps,
 						&setup->stream);
@@ -2188,14 +2229,6 @@ static const GDBusPropertyTable sep_properties[] = {
 	{ }
 };
 
-static void remote_sep_free(void *data)
-{
-	struct a2dp_remote_sep *sep = data;
-
-	free(sep->path);
-	free(sep);
-}
-
 static void register_remote_sep(void *data, void *user_data)
 {
 	struct avdtp_remote_sep *rsep = data;
@@ -2205,6 +2238,10 @@ static void register_remote_sep(void *data, void *user_data)
 	sep = new0(struct a2dp_remote_sep, 1);
 	sep->chan = setup->chan;
 	sep->sep = rsep;
+
+	if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL))
+		goto done;
+
 	asprintf(&sep->path, "%s/sep%d", device_get_path(setup->chan->device),
 							avdtp_get_seid(rsep));
 
@@ -2213,9 +2250,13 @@ static void register_remote_sep(void *data, void *user_data)
 				sep_methods, NULL, sep_properties,
 				sep, remote_sep_free) == FALSE) {
 		error("Could not register remote sep %s", sep->path);
-		remote_sep_free(sep);
+		free(sep->path);
+		sep->path = NULL;
 	}
 
+	DBG("Found remote SEP: %s", sep->path);
+
+done:
 	queue_push_tail(setup->chan->seps, sep);
 }
 
@@ -2283,14 +2324,14 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
 	cb_data->user_data = user_data;
 
 	setup->sep = sep;
-	setup->rsep = avdtp_find_remote_sep(session, sep->lsep);
+	setup->rsep = find_remote_sep(setup->chan, sep);
 
 	if (setup->rsep == NULL) {
 		error("Could not find remote sep");
 		goto fail;
 	}
 
-	service = avdtp_get_codec(setup->rsep);
+	service = avdtp_get_codec(setup->rsep->sep);
 	codec = (struct avdtp_media_codec_capability *) service->data;
 
 	err = sep->endpoint->select_configuration(sep, codec->data,
@@ -2384,13 +2425,13 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
 			break;
 		}
 
-		setup->rsep = avdtp_find_remote_sep(session, sep->lsep);
+		setup->rsep = find_remote_sep(setup->chan, sep);
 		if (setup->rsep == NULL) {
 			error("No matching ACP and INT SEPs found");
 			goto failed;
 		}
 
-		posix_err = avdtp_set_configuration(session, setup->rsep,
+		posix_err = avdtp_set_configuration(session, setup->rsep->sep,
 							sep->lsep, caps,
 							&setup->stream);
 		if (posix_err < 0) {
@@ -2632,6 +2673,16 @@ struct btd_device *a2dp_setup_get_device(struct a2dp_setup *setup)
 	return avdtp_get_device(setup->session);
 }
 
+const char *a2dp_setup_remote_path(struct a2dp_setup *setup)
+{
+	if (setup->rsep) {
+		if (setup->rsep->path)
+			return setup->rsep->path;
+	}
+
+	return NULL;
+}
+
 static int a2dp_source_probe(struct btd_service *service)
 {
 	struct btd_device *dev = btd_service_get_device(service);
diff --git a/profiles/audio/a2dp.h b/profiles/audio/a2dp.h
index 7f38c75f3..19466a428 100644
--- a/profiles/audio/a2dp.h
+++ b/profiles/audio/a2dp.h
@@ -91,4 +91,5 @@ gboolean a2dp_sep_lock(struct a2dp_sep *sep, struct avdtp *session);
 gboolean a2dp_sep_unlock(struct a2dp_sep *sep, struct avdtp *session);
 struct avdtp_stream *a2dp_sep_get_stream(struct a2dp_sep *sep);
 struct btd_device *a2dp_setup_get_device(struct a2dp_setup *setup);
+const char *a2dp_setup_remote_path(struct a2dp_setup *setup);
 struct avdtp *a2dp_avdtp_get(struct btd_device *device);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 9d7564cf0..28fa70668 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -430,8 +430,9 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 	if (transport != NULL)
 		return FALSE;
 
-	transport = media_transport_create(device, configuration, size,
-								endpoint);
+	transport = media_transport_create(device,
+					a2dp_setup_remote_path(data->setup),
+					configuration, size, endpoint);
 	if (transport == NULL)
 		return FALSE;
 
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 98f4e1ffd..48fabba9b 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -92,6 +92,7 @@ struct a2dp_transport {
 struct media_transport {
 	char			*path;		/* Transport object path */
 	struct btd_device	*device;	/* Transport device */
+	const char		*remote_endpoint; /* Transport remote SEP */
 	struct media_endpoint	*endpoint;	/* Transport endpoint */
 	struct media_owner	*owner;		/* Transport owner */
 	uint8_t			*configuration; /* Transport configuration */
@@ -689,6 +690,24 @@ static void set_volume(const GDBusPropertyTable *property,
 	avrcp_set_volume(transport->device, volume, notify);
 }
 
+static gboolean endpoint_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_transport *transport = data;
+
+	return transport->remote_endpoint != NULL;
+}
+
+static gboolean get_endpoint(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_transport *transport = data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH,
+					&transport->remote_endpoint);
+
+	return TRUE;
+}
+
 static const GDBusMethodTable transport_methods[] = {
 	{ GDBUS_ASYNC_METHOD("Acquire",
 			NULL,
@@ -712,6 +731,8 @@ static const GDBusPropertyTable transport_properties[] = {
 	{ "State", "s", get_state },
 	{ "Delay", "q", get_delay, NULL, delay_exists },
 	{ "Volume", "q", get_volume, set_volume, volume_exists },
+	{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists,
+				G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
 };
 
@@ -836,6 +857,7 @@ static int media_transport_init_sink(struct media_transport *transport)
 }
 
 struct media_transport *media_transport_create(struct btd_device *device,
+						const char *remote_endpoint,
 						uint8_t *configuration,
 						size_t size, void *data)
 {
@@ -850,8 +872,10 @@ struct media_transport *media_transport_create(struct btd_device *device,
 	transport->configuration = g_new(uint8_t, size);
 	memcpy(transport->configuration, configuration, size);
 	transport->size = size;
-	transport->path = g_strdup_printf("%s/fd%d", device_get_path(device),
-									fd++);
+	transport->remote_endpoint = remote_endpoint;
+	transport->path = g_strdup_printf("%s/fd%d",
+				remote_endpoint ? remote_endpoint :
+				device_get_path(device), fd++);
 	transport->fd = -1;
 
 	uuid = media_endpoint_get_uuid(endpoint);
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 505ad5c54..ac542bf6c 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -25,6 +25,7 @@
 struct media_transport;
 
 struct media_transport *media_transport_create(struct btd_device *device,
+						const char *remote_endpoint,
 						uint8_t *configuration,
 						size_t size, void *data);
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 5/9] doc/settings-storage: Add Endpoint group to cache
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 4/9] a2dp: Implement MediaTransport.Endpoint Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 6/9] a2dp: Cache remote endpoints Luiz Augusto von Dentz
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This documents how A2DP endpoints should be stored in the cache.
---
 doc/settings-storage.txt | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/doc/settings-storage.txt b/doc/settings-storage.txt
index 90357f735..44b0b4961 100644
--- a/doc/settings-storage.txt
+++ b/doc/settings-storage.txt
@@ -159,7 +159,7 @@ Cache directory file format
 ============================
 
 Each file, named by remote device address, may includes multiple groups
-(General, ServiceRecords, Attributes).
+(General, ServiceRecords, Attributes, Endpoints).
 
 In ServiceRecords, SDP records are stored using their handle as key
 (hexadecimal format).
@@ -168,6 +168,9 @@ In "Attributes" group GATT database is stored using attribute handle as key
 (hexadecimal format). Value associated with this handle is serialized form of
 all data required to re-create given attribute. ":" is used to separate fields.
 
+In "Endpoints" group A2DP remote endpoints are stored using the seid as key
+(hexadecimal format) and ":" is used to separate fields.
+
 [General] group contains:
 
   Name		String		Remote device friendly name
@@ -211,6 +214,12 @@ Sample Attributes section:
   002b=2803:002c:02:00002a38-0000-1000-8000-00805f9b34fb
   002d=2803:002e:08:00002a39-0000-1000-8000-00805f9b34fb
 
+[Endpoints] group contains:
+
+	<xx>:<xx>:<xxxxxxxx...>	String	First field is the endpoint type,
+					followed by codec type and its
+					capabilies as hexadecimal encoded
+					string.
 
 Info file format
 ================
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 6/9] a2dp: Cache remote endpoints
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (3 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 5/9] doc/settings-storage: Add Endpoint group to cache Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 7/9] doc/media-api: Add Device property to MediaEndpoint Luiz Augusto von Dentz
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

In order to always have the Endpoint interface available the remote
endpoints needs to be cached since the remote stack may config a stream
on its own there may not be a chance to discover the endpoits available
which would make it impossible to switch endpoints.
---
 profiles/audio/a2dp.c  | 724 +++++++++++++++++++++++++----------------
 profiles/audio/avdtp.c |  46 ++-
 profiles/audio/avdtp.h |   5 +
 3 files changed, 492 insertions(+), 283 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 4fa01894a..6975682c9 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1441,6 +1441,410 @@ static gboolean disconnect_cb(GIOChannel *io, GIOCondition cond, gpointer data)
 	return FALSE;
 }
 
+static void caps_add_codec(GSList **l, uint8_t codec, uint8_t *caps,
+							size_t size)
+{
+	struct avdtp_service_capability *media_transport, *media_codec;
+	struct avdtp_media_codec_capability *cap;
+
+	media_transport = avdtp_service_cap_new(AVDTP_MEDIA_TRANSPORT,
+						NULL, 0);
+
+	*l = g_slist_append(*l, media_transport);
+
+	cap = g_malloc0(sizeof(*cap) + size);
+	cap->media_type = AVDTP_MEDIA_TYPE_AUDIO;
+	cap->media_codec_type = codec;
+	memcpy(cap->data, caps, size);
+
+	media_codec = avdtp_service_cap_new(AVDTP_MEDIA_CODEC, cap,
+						sizeof(*cap) + size);
+
+	*l = g_slist_append(*l, media_codec);
+	g_free(cap);
+}
+
+struct client {
+	const char *sender;
+	const char *path;
+};
+
+static int match_client(const void *data, const void *user_data)
+{
+	struct a2dp_sep *sep = (void *) data;
+	const struct a2dp_endpoint *endpoint = sep->endpoint;
+	const struct client *client = user_data;
+
+	if (strcmp(client->sender, endpoint->get_name(sep, sep->user_data)))
+		return -1;
+
+	return strcmp(client->path, endpoint->get_path(sep, sep->user_data));
+}
+
+static struct a2dp_sep *find_sep(struct a2dp_server *server, uint8_t type,
+					const char *sender, const char *path)
+{
+	GSList *l;
+	struct client client = { sender, path };
+
+	l = type == AVDTP_SEP_TYPE_SINK ? server->sources : server->sinks;
+
+	l = g_slist_find_custom(l, &client, match_client);
+	if (l)
+		return l->data;
+
+	return NULL;
+}
+
+static int parse_properties(DBusMessageIter *props, uint8_t **caps, int *size)
+{
+	while (dbus_message_iter_get_arg_type(props) == DBUS_TYPE_DICT_ENTRY) {
+		const char *key;
+		DBusMessageIter value, entry;
+		int var;
+
+		dbus_message_iter_recurse(props, &entry);
+		dbus_message_iter_get_basic(&entry, &key);
+
+		dbus_message_iter_next(&entry);
+		dbus_message_iter_recurse(&entry, &value);
+
+		var = dbus_message_iter_get_arg_type(&value);
+		if (strcasecmp(key, "Capabilities") == 0) {
+			DBusMessageIter array;
+
+			if (var != DBUS_TYPE_ARRAY)
+				return -EINVAL;
+
+			dbus_message_iter_recurse(&value, &array);
+			dbus_message_iter_get_fixed_array(&array, caps, size);
+			return 0;
+		}
+
+		dbus_message_iter_next(props);
+	}
+
+	return -EINVAL;
+}
+
+static void reconfig_cb(struct avdtp *session, struct a2dp_sep *sep,
+			struct avdtp_stream *stream, int err, void *user_data)
+{
+	DBusMessage *msg = user_data;
+
+	if (err)
+		g_dbus_send_message(btd_get_dbus_connection(),
+					btd_error_failed(msg, strerror(-err)));
+	else
+		g_dbus_send_reply(btd_get_dbus_connection(), msg,
+					DBUS_TYPE_INVALID);
+
+	dbus_message_unref(msg);
+}
+
+static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
+			struct a2dp_sep *lsep, struct a2dp_remote_sep *rsep,
+			uint8_t *caps, int size, void *user_data)
+{
+	struct a2dp_setup *setup;
+	struct a2dp_setup_cb *cb_data;
+	GSList *l;
+	int err;
+
+	setup = a2dp_setup_get(chan->session);
+	if (!setup)
+		return -ENOMEM;
+
+	cb_data = setup_cb_new(setup);
+	cb_data->config_cb = reconfig_cb;
+	cb_data->user_data = user_data;
+
+	setup->sep = lsep;
+	setup->rsep = rsep;
+
+	caps_add_codec(&setup->caps, setup->sep->codec, caps, size);
+
+	l = avdtp_get_type(rsep->sep) == AVDTP_SEP_TYPE_SINK ?
+					chan->server->sources :
+					chan->server->sinks;
+
+	/* Check for existing stream and close it */
+	for (; l; l = g_slist_next(l)) {
+		struct a2dp_sep *tmp = l->data;
+
+		/* Attempt to reconfigure if a stream already exists */
+		if (tmp->stream) {
+			/* Only allow switching sep from the same sender */
+			if (strcmp(sender, tmp->endpoint->get_name(tmp,
+							tmp->user_data)))
+				return -EPERM;
+
+			err = avdtp_close(chan->session, tmp->stream, FALSE);
+			if (err < 0) {
+				error("avdtp_close: %s", strerror(-err));
+				goto fail;
+			}
+
+			setup->reconfigure = TRUE;
+
+			return 0;
+		}
+	}
+
+	err = avdtp_set_configuration(setup->session, setup->rsep->sep,
+						lsep->lsep,
+						setup->caps,
+						&setup->stream);
+	if (err < 0) {
+		error("avdtp_set_configuration: %s", strerror(-err));
+		goto fail;
+	}
+
+	return 0;
+
+fail:
+	setup_unref(setup);
+	return err;
+}
+
+static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
+								void *data)
+{
+	struct a2dp_remote_sep *rsep = data;
+	struct a2dp_channel *chan = rsep->chan;
+	struct a2dp_sep *lsep = NULL;
+	struct avdtp_service_capability *service;
+	struct avdtp_media_codec_capability *codec;
+	DBusMessageIter args, props;
+	const char *sender, *path;
+	uint8_t *caps;
+	int err, size = 0;
+
+	sender = dbus_message_get_sender(msg);
+
+	dbus_message_iter_init(msg, &args);
+
+	dbus_message_iter_get_basic(&args, &path);
+	dbus_message_iter_next(&args);
+
+	lsep = find_sep(chan->server, avdtp_get_type(rsep->sep), sender, path);
+	if (!lsep)
+		return btd_error_invalid_args(msg);
+
+	service = avdtp_get_codec(rsep->sep);
+	codec = (struct avdtp_media_codec_capability *) service->data;
+
+	/* Check if codec really matches */
+	if (!endpoint_match_codec_ind(chan->session, codec, lsep))
+		return btd_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&args, &props);
+	if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
+		return btd_error_invalid_args(msg);
+
+	if (parse_properties(&props, &caps, &size) < 0)
+		return btd_error_invalid_args(msg);
+
+	err = a2dp_reconfig(chan, sender, lsep, rsep, caps, size,
+					dbus_message_ref(msg));
+	if (err < 0) {
+		dbus_message_unref(msg);
+		return btd_error_failed(msg, strerror(-err));
+	}
+
+	return NULL;
+}
+
+static const GDBusMethodTable sep_methods[] = {
+	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("SetConfiguration",
+					GDBUS_ARGS({ "endpoint", "o" },
+						{ "properties", "a{sv}" } ),
+					NULL, set_configuration) },
+	{ },
+};
+
+static gboolean get_uuid(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	const char *uuid;
+
+	switch (avdtp_get_type(sep->sep)) {
+	case AVDTP_SEP_TYPE_SOURCE:
+		uuid = A2DP_SOURCE_UUID;
+		break;
+	case AVDTP_SEP_TYPE_SINK:
+		uuid = A2DP_SOURCE_UUID;
+		break;
+	default:
+		uuid = "";
+	}
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+
+	return TRUE;
+}
+
+static gboolean get_codec(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep);
+	struct avdtp_media_codec_capability *codec = (void *) cap->data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
+						&codec->media_codec_type);
+
+	return TRUE;
+}
+
+static gboolean get_capabilities(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	struct avdtp_service_capability *service = avdtp_get_codec(sep->sep);
+	struct avdtp_media_codec_capability *codec = (void *) service->data;
+	uint8_t *caps = codec->data;
+	DBusMessageIter array;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_BYTE_AS_STRING, &array);
+
+	dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE, &caps,
+					service->length - sizeof(*codec));
+
+	dbus_message_iter_close_container(iter, &array);
+
+	return TRUE;
+}
+
+static const GDBusPropertyTable sep_properties[] = {
+	{ "UUID", "s", get_uuid, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Codec", "y", get_codec, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Capabilities", "ay", get_capabilities, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ }
+};
+
+static void register_remote_sep(void *data, void *user_data)
+{
+	struct avdtp_remote_sep *rsep = data;
+	struct a2dp_channel *chan = user_data;
+	struct a2dp_remote_sep *sep;
+
+	sep = queue_find(chan->seps, match_remote_sep, rsep);
+	if (sep)
+		return;
+
+	sep = new0(struct a2dp_remote_sep, 1);
+	sep->chan = chan;
+	sep->sep = rsep;
+
+	if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL))
+		goto done;
+
+	asprintf(&sep->path, "%s/sep%d", device_get_path(chan->device),
+							avdtp_get_seid(rsep));
+
+	if (g_dbus_register_interface(btd_get_dbus_connection(),
+				sep->path, MEDIA_ENDPOINT_INTERFACE,
+				sep_methods, NULL, sep_properties,
+				sep, remote_sep_free) == FALSE) {
+		error("Could not register remote sep %s", sep->path);
+		free(sep->path);
+		sep->path = NULL;
+		goto done;
+	}
+
+	DBG("Found remote SEP: %s", sep->path);
+
+done:
+	queue_push_tail(chan->seps, sep);
+}
+
+static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
+								char **seids)
+{
+	struct avdtp_remote_sep *sep;
+
+	if (!seids)
+		return;
+
+	for (; *seids; seids++) {
+		uint8_t seid;
+		uint8_t type;
+		uint8_t codec;
+		char *value, caps[256];
+		uint8_t data[128];
+		int i, size;
+		GSList *l = NULL;
+
+		if (sscanf(*seids, "%02hhx", &seid) != 1)
+			continue;
+
+		value = g_key_file_get_string(key_file, "Endpoints", *seids,
+								NULL);
+		if (!value)
+			continue;
+
+		if (sscanf(value, "%02hhx:%02hhx:%s", &type, &codec,
+								caps) != 3) {
+			warn("Unable to load Endpoint: seid %u", seid);
+			g_free(value);
+			continue;
+		}
+
+		for (i = 0, size = strlen(caps); i < size; i += 2) {
+			uint8_t *tmp = data + i / 2;
+
+			if (sscanf(caps + i, "%02hhx", tmp) != 1) {
+				warn("Unable to load Endpoint: seid %u", seid);
+				break;
+			}
+		}
+
+		g_free(value);
+
+		if (i != size)
+			continue;
+
+		caps_add_codec(&l, codec, data, size / 2);
+
+		sep = avdtp_register_remote_sep(chan->session, seid, type, l);
+		if (!sep) {
+			warn("Unable to register Endpoint: seid %u", seid);
+			continue;
+		}
+
+		register_remote_sep(sep, chan);
+	}
+}
+
+static void load_remote_seps(struct a2dp_channel *chan)
+{
+	struct btd_device *device = chan->device;
+	char filename[PATH_MAX];
+	char dst_addr[18];
+	char **keys;
+	GKeyFile *key_file;
+
+	ba2str(device_get_address(device), dst_addr);
+
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
+			btd_adapter_get_storage_dir(device_get_adapter(device)),
+			dst_addr);
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	keys = g_key_file_get_keys(key_file, "Endpoints", NULL, NULL);
+
+	load_remote_sep(chan, key_file, keys);
+
+	g_strfreev(keys);
+	g_key_file_free(key_file);
+}
+
 static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session,
 					avdtp_session_state_t old_state,
 					avdtp_session_state_t new_state,
@@ -1456,6 +1860,9 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session,
 	case AVDTP_SESSION_STATE_CONNECTING:
 		break;
 	case AVDTP_SESSION_STATE_CONNECTED:
+		if (!chan->session)
+			chan->session = session;
+		load_remote_seps(chan);
 		break;
 	}
 }
@@ -1904,28 +2311,6 @@ void a2dp_remove_sep(struct a2dp_sep *sep)
 	a2dp_unregister_sep(sep);
 }
 
-static void setup_add_caps(struct a2dp_setup *setup, uint8_t *caps, size_t size)
-{
-	struct avdtp_service_capability *media_transport, *media_codec;
-	struct avdtp_media_codec_capability *cap;
-
-	media_transport = avdtp_service_cap_new(AVDTP_MEDIA_TRANSPORT,
-						NULL, 0);
-
-	setup->caps = g_slist_append(setup->caps, media_transport);
-
-	cap = g_malloc0(sizeof(*cap) + size);
-	cap->media_type = AVDTP_MEDIA_TYPE_AUDIO;
-	cap->media_codec_type = setup->sep->codec;
-	memcpy(cap->data, caps, size);
-
-	media_codec = avdtp_service_cap_new(AVDTP_MEDIA_CODEC, cap,
-						sizeof(*cap) + size);
-
-	setup->caps = g_slist_append(setup->caps, media_codec);
-	g_free(cap);
-}
-
 static void select_cb(struct a2dp_setup *setup, void *ret, int size)
 {
 	if (size < 0) {
@@ -1933,7 +2318,7 @@ static void select_cb(struct a2dp_setup *setup, void *ret, int size)
 		goto done;
 	}
 
-	setup_add_caps(setup, ret, size);
+	caps_add_codec(&setup->caps, setup->sep->codec, ret, size);
 
 done:
 	finalize_select(setup);
@@ -1989,275 +2374,58 @@ static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
 	return a2dp_find_sep(session, l, NULL);
 }
 
-struct client {
-	const char *sender;
-	const char *path;
-};
-
-static int match_client(const void *data, const void *user_data)
-{
-	struct a2dp_sep *sep = (void *) data;
-	const struct a2dp_endpoint *endpoint = sep->endpoint;
-	const struct client *client = user_data;
-
-	if (strcmp(client->sender, endpoint->get_name(sep, sep->user_data)))
-		return -1;
-
-	return strcmp(client->path, endpoint->get_path(sep, sep->user_data));
-}
-
-static struct a2dp_sep *find_sep(struct a2dp_server *server, const char *sender,
-							const char *path)
-{
-	GSList *l;
-	struct client client = { sender, path };
-
-	l = g_slist_find_custom(server->sources, &client, match_client);
-	if (l)
-		return l->data;
-
-	l = g_slist_find_custom(server->sinks, &client, match_client);
-	if (l)
-		return l->data;
-
-	return NULL;
-}
-
-static int parse_properties(DBusMessageIter *props, uint8_t **caps, int *size)
-{
-	while (dbus_message_iter_get_arg_type(props) == DBUS_TYPE_DICT_ENTRY) {
-		const char *key;
-		DBusMessageIter value, entry;
-		int var;
-
-		dbus_message_iter_recurse(props, &entry);
-		dbus_message_iter_get_basic(&entry, &key);
-
-		dbus_message_iter_next(&entry);
-		dbus_message_iter_recurse(&entry, &value);
-
-		var = dbus_message_iter_get_arg_type(&value);
-		if (strcasecmp(key, "Capabilities") == 0) {
-			DBusMessageIter array;
-
-			if (var != DBUS_TYPE_ARRAY)
-				return -EINVAL;
-
-			dbus_message_iter_recurse(&value, &array);
-			dbus_message_iter_get_fixed_array(&array, caps, size);
-			return 0;
-		}
-
-		dbus_message_iter_next(props);
-	}
-
-	return -EINVAL;
-}
-
-static int a2dp_reconfig(struct a2dp_channel *chan, const char *sender,
-			struct a2dp_sep *lsep, struct a2dp_remote_sep *rsep,
-			uint8_t *caps, int size)
-{
-	struct a2dp_setup *setup;
-	const struct queue_entry *entry;
-	int err;
-
-	setup = a2dp_setup_get(chan->session);
-	if (!setup)
-		return -ENOMEM;
-
-	setup->sep = lsep;
-	setup->rsep = rsep;
-
-	setup_add_caps(setup, caps, size);
-
-	/* Check for existing stream and close it */
-	for (entry = queue_get_entries(chan->server->seps); entry;
-						entry = entry->next) {
-		struct a2dp_sep *tmp = entry->data;
-
-		/* Attempt to reconfigure if a stream already exists */
-		if (tmp->stream) {
-			/* Only allow switching sep from the same sender */
-			if (strcmp(sender, tmp->endpoint->get_name(tmp,
-							tmp->user_data)))
-				return -EPERM;
-
-			err = avdtp_close(chan->session, tmp->stream, FALSE);
-			if (err < 0) {
-				error("avdtp_close: %s", strerror(-err));
-				return err;
-			}
-
-			setup->reconfigure = TRUE;
-
-			return 0;
-		}
-	}
-
-	err = avdtp_set_configuration(setup->session, setup->rsep->sep,
-						lsep->lsep,
-						setup->caps,
-						&setup->stream);
-	if (err < 0) {
-		error("avdtp_set_configuration: %s", strerror(-err));
-		return err;
-	}
-
-	return 0;
-}
-
-static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
-								void *data)
-{
-	struct a2dp_remote_sep *rsep = data;
-	struct a2dp_channel *chan = rsep->chan;
-	struct a2dp_sep *lsep;
-	struct avdtp_service_capability *service;
-	struct avdtp_media_codec_capability *codec;
-	DBusMessageIter args, props;
-	const char *sender, *path;
-	uint8_t *caps;
-	int err, size = 0;
-
-	sender = dbus_message_get_sender(msg);
-
-	dbus_message_iter_init(msg, &args);
-
-	dbus_message_iter_get_basic(&args, &path);
-	dbus_message_iter_next(&args);
-
-	lsep = find_sep(chan->server, sender, path);
-	if (!lsep)
-		return btd_error_invalid_args(msg);
-
-	/* Check if SEPs are no the same role */
-	if (avdtp_get_type(rsep->sep) == lsep->type)
-		return btd_error_invalid_args(msg);
-
-	service = avdtp_get_codec(rsep->sep);
-	codec = (struct avdtp_media_codec_capability *) service->data;
-
-	/* Check if codec match */
-	if (!endpoint_match_codec_ind(chan->session, codec, lsep))
-		return btd_error_invalid_args(msg);
-
-	dbus_message_iter_recurse(&args, &props);
-	if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
-		return btd_error_invalid_args(msg);
-
-	if (parse_properties(&props, &caps, &size) < 0)
-		return btd_error_invalid_args(msg);
-
-	err = a2dp_reconfig(chan, sender, lsep, rsep, caps, size);
-	if (err < 0)
-		return btd_error_failed(msg, strerror(-err));
-
-	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
-}
-
-static const GDBusMethodTable sep_methods[] = {
-	{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("SetConfiguration",
-					GDBUS_ARGS({ "endpoint", "o" },
-						{ "properties", "a{sv}" } ),
-					NULL, set_configuration) },
-	{ },
-};
-
-static gboolean get_uuid(const GDBusPropertyTable *property,
-					DBusMessageIter *iter, void *data)
-{
-	struct a2dp_remote_sep *sep = data;
-	const char *uuid;
-
-	switch (avdtp_get_type(sep->sep)) {
-	case AVDTP_SEP_TYPE_SOURCE:
-		uuid = A2DP_SOURCE_UUID;
-		break;
-	case AVDTP_SEP_TYPE_SINK:
-		uuid = A2DP_SOURCE_UUID;
-		break;
-	default:
-		uuid = "";
-	}
-
-	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
-
-	return TRUE;
-}
-
-static gboolean get_codec(const GDBusPropertyTable *property,
-					DBusMessageIter *iter, void *data)
-{
-	struct a2dp_remote_sep *sep = data;
-	struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep);
-	struct avdtp_media_codec_capability *codec = (void *) cap->data;
-
-	dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
-						&codec->media_codec_type);
-
-	return TRUE;
-}
-
-static gboolean get_capabilities(const GDBusPropertyTable *property,
-					DBusMessageIter *iter, void *data)
+static void store_remote_sep(void *data, void *user_data)
 {
 	struct a2dp_remote_sep *sep = data;
+	GKeyFile *key_file = (void *) user_data;
+	char seid[4], value[256];
 	struct avdtp_service_capability *service = avdtp_get_codec(sep->sep);
 	struct avdtp_media_codec_capability *codec = (void *) service->data;
-	uint8_t *caps = codec->data;
-	DBusMessageIter array;
+	unsigned int i;
+	ssize_t offset;
 
-	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
-					DBUS_TYPE_BYTE_AS_STRING, &array);
+	sprintf(seid, "%02hhx", avdtp_get_seid(sep->sep));
 
-	dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE, &caps,
-					service->length - sizeof(*codec));
+	offset = sprintf(value, "%02hhx:%02hhx:", avdtp_get_type(sep->sep),
+						codec->media_codec_type);
 
-	dbus_message_iter_close_container(iter, &array);
+	for (i = 0; i < service->length - sizeof(*codec); i++)
+		offset += sprintf(value + offset, "%02hhx", codec->data[i]);
 
-	return TRUE;
-}
 
-static const GDBusPropertyTable sep_properties[] = {
-	{ "UUID", "s", get_uuid, NULL, NULL,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Codec", "y", get_codec, NULL, NULL,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ "Capabilities", "ay", get_capabilities, NULL, NULL,
-					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
-	{ }
-};
+	g_key_file_set_string(key_file, "Endpoints", seid, value);
+}
 
-static void register_remote_sep(void *data, void *user_data)
+static void store_remote_seps(struct a2dp_channel *chan)
 {
-	struct avdtp_remote_sep *rsep = data;
-	struct a2dp_setup *setup = user_data;
-	struct a2dp_remote_sep *sep;
+	struct btd_device *device = chan->device;
+	char filename[PATH_MAX];
+	char dst_addr[18];
+	GKeyFile *key_file;
+	char *data;
+	gsize length = 0;
 
-	sep = new0(struct a2dp_remote_sep, 1);
-	sep->chan = setup->chan;
-	sep->sep = rsep;
+	if (queue_isempty(chan->seps))
+		return;
 
-	if (!(g_dbus_get_flags() & G_DBUS_FLAG_ENABLE_EXPERIMENTAL))
-		goto done;
+	ba2str(device_get_address(device), dst_addr);
 
-	asprintf(&sep->path, "%s/sep%d", device_get_path(setup->chan->device),
-							avdtp_get_seid(rsep));
+	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s",
+			btd_adapter_get_storage_dir(device_get_adapter(device)),
+			dst_addr);
+	key_file = g_key_file_new();
+	g_key_file_load_from_file(key_file, filename, 0, NULL);
 
-	if (g_dbus_register_interface(btd_get_dbus_connection(),
-				sep->path, MEDIA_ENDPOINT_INTERFACE,
-				sep_methods, NULL, sep_properties,
-				sep, remote_sep_free) == FALSE) {
-		error("Could not register remote sep %s", sep->path);
-		free(sep->path);
-		sep->path = NULL;
-	}
+	/* Remove current endpoints since it might have changed */
+	g_key_file_remove_group(key_file, "Endpoints", NULL);
 
-	DBG("Found remote SEP: %s", sep->path);
+	queue_foreach(chan->seps, store_remote_sep, key_file);
 
-done:
-	queue_push_tail(setup->chan->seps, sep);
+	data = g_key_file_to_data(key_file, &length, NULL);
+	g_file_set_contents(filename, data, length, NULL);
+
+	g_free(data);
+	g_key_file_free(key_file);
 }
 
 static void discover_cb(struct avdtp *session, GSList *seps,
@@ -2270,8 +2438,10 @@ static void discover_cb(struct avdtp *session, GSList *seps,
 	setup->seps = seps;
 	setup->err = err;
 
-	if (!err && queue_isempty(setup->chan->seps))
-		g_slist_foreach(seps, register_remote_sep, setup);
+	if (!err) {
+		g_slist_foreach(seps, register_remote_sep, setup->chan);
+		store_remote_seps(setup->chan);
+	}
 
 	finalize_discover(setup);
 }
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 49551a921..f4c9a2ed8 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2647,12 +2647,15 @@ static gboolean avdtp_discover_resp(struct avdtp *session,
 		stream = find_stream_by_rseid(session, resp->seps[i].seid);
 
 		sep = find_remote_sep(session->seps, resp->seps[i].seid);
-		if (!sep) {
-			if (resp->seps[i].inuse && !stream)
-				continue;
-			sep = g_new0(struct avdtp_remote_sep, 1);
-			session->seps = g_slist_append(session->seps, sep);
-		}
+		if (sep && sep->type == resp->seps[i].type &&
+				sep->media_type == resp->seps[i].media_type)
+			continue;
+
+		if (resp->seps[i].inuse && !stream)
+			continue;
+
+		sep = g_new0(struct avdtp_remote_sep, 1);
+		session->seps = g_slist_append(session->seps, sep);
 
 		sep->stream = stream;
 		sep->seid = resp->seps[i].seid;
@@ -3201,6 +3204,37 @@ struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
 	return cap;
 }
 
+struct avdtp_remote_sep *avdtp_register_remote_sep(struct avdtp *session,
+							uint8_t seid,
+							uint8_t type,
+							GSList *caps)
+{
+	struct avdtp_remote_sep *sep;
+	GSList *l;
+
+	sep = find_remote_sep(session->seps, seid);
+	if (sep)
+		return sep;
+
+	sep = g_new0(struct avdtp_remote_sep, 1);
+	session->seps = g_slist_append(session->seps, sep);
+	sep->seid = seid;
+	sep->type = type;
+	sep->media_type = AVDTP_MEDIA_TYPE_AUDIO;
+	sep->caps = caps;
+
+	for (l = caps; l; l = g_slist_next(l)) {
+		struct avdtp_service_capability *cap = l->data;
+
+		if (cap->category == AVDTP_MEDIA_CODEC)
+			sep->codec = cap;
+	}
+
+	DBG("seid %d type %d media %d", sep->seid, sep->type, sep->media_type);
+
+	return sep;
+}
+
 static gboolean process_discover(gpointer data)
 {
 	struct avdtp *session = data;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index e5fc40c89..b03ca9030 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -223,6 +223,11 @@ struct avdtp *avdtp_ref(struct avdtp *session);
 struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
 							void *data, int size);
 
+struct avdtp_remote_sep *avdtp_register_remote_sep(struct avdtp *session,
+							uint8_t seid,
+							uint8_t type,
+							GSList *caps);
+
 uint8_t avdtp_get_seid(struct avdtp_remote_sep *sep);
 
 uint8_t avdtp_get_type(struct avdtp_remote_sep *sep);
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 7/9] doc/media-api: Add Device property to MediaEndpoint
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (4 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 6/9] a2dp: Cache remote endpoints Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 8/9] a2dp: Add implementation of MediaEndpoint.Device Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds Device property which indicates which device the endpoint
belongs to.
---
 doc/media-api.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 292e9034c..bca8c9563 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -556,6 +556,10 @@ Properties	string UUID [readonly, optional]:
 			Capabilities blob, it is used as it is so the size and
 			byte order must match.
 
+		object Device [readonly, optional]:
+
+			Device object which the endpoint is belongs to.
+
 MediaTransport1 hierarchy
 =========================
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 8/9] a2dp: Add implementation of MediaEndpoint.Device
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (5 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 7/9] doc/media-api: Add Device property to MediaEndpoint Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 13:45 ` [PATCH v3 9/9] a2dp: Add reverse discovery Luiz Augusto von Dentz
  2019-01-22 14:20 ` [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds the implementation of MediaEndpoint.Device property so the
clints don't need to guess what device the endpoint belongs.
---
 profiles/audio/a2dp.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 6975682c9..ff384cd23 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1718,6 +1718,19 @@ static gboolean get_capabilities(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean get_device(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct a2dp_remote_sep *sep = data;
+	const char *path;
+
+	path = device_get_path(sep->chan->device);
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_OBJECT_PATH, &path);
+
+	return TRUE;
+}
+
 static const GDBusPropertyTable sep_properties[] = {
 	{ "UUID", "s", get_uuid, NULL, NULL,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
@@ -1725,6 +1738,8 @@ static const GDBusPropertyTable sep_properties[] = {
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ "Capabilities", "ay", get_capabilities, NULL, NULL,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Device", "o", get_device, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
 };
 
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v3 9/9] a2dp: Add reverse discovery
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (6 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 8/9] a2dp: Add implementation of MediaEndpoint.Device Luiz Augusto von Dentz
@ 2019-01-22 13:45 ` Luiz Augusto von Dentz
  2019-01-22 14:20 ` [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
  8 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 13:45 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Now that remote endpoints are exposed there is a chance that a
configured device will reconnect and initiate SetConfiguration skipping
the discovery phase which is now required in order to be able to switch
endpoints, so this introduces the reverse discovery logic in order to
find out about remote endpoints capabilities if they have not been
found yet.
---
 profiles/audio/a2dp.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index ff384cd23..4001ea0ea 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -583,6 +583,12 @@ static gboolean endpoint_match_codec_ind(struct avdtp *session,
 	return TRUE;
 }
 
+static void reverse_discover(struct avdtp *session, GSList *seps, int err,
+							void *user_data)
+{
+	DBG("err %d", err);
+}
+
 static gboolean endpoint_setconf_ind(struct avdtp *session,
 						struct avdtp_local_sep *sep,
 						struct avdtp_stream *stream,
@@ -638,8 +644,14 @@ static gboolean endpoint_setconf_ind(struct avdtp *session,
 						setup_ref(setup),
 						endpoint_setconf_cb,
 						a2dp_sep->user_data);
-		if (ret == 0)
+		if (ret == 0) {
+			/* Attempt to reverve discover if there are no remote
+			 * SEPs.
+			 */
+			if (queue_isempty(setup->chan->seps))
+				a2dp_discover(session, reverse_discover, NULL);
 			return TRUE;
+		}
 
 		setup_unref(setup);
 		setup->err = g_new(struct avdtp_error, 1);
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
                   ` (7 preceding siblings ...)
  2019-01-22 13:45 ` [PATCH v3 9/9] a2dp: Add reverse discovery Luiz Augusto von Dentz
@ 2019-01-22 14:20 ` Luiz Augusto von Dentz
  2019-01-22 17:56   ` Pali Rohár
  8 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-22 14:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Pali Rohár

Hi Pali,
On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This adds the possibility to expose remote SEP using MediaEndpoint
> interface to allow setting a configuration.
> ---
>  doc/media-api.txt | 27 +++++++++++++++++++++++++--
>  1 file changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index b5ad2db12..af9485342 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -500,14 +500,23 @@ Properties        object Player [readonly]
>  MediaEndpoint1 hierarchy
>  ========================
>
> -Service                unique name
> +Service                unique name (Server role)
> +               org.bluez (Client role)
>  Interface      org.bluez.MediaEndpoint1
> -Object path    freely definable
> +Object path    freely definable (Server role)
> +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> +               (Client role)
>
>  Methods                void SetConfiguration(object transport, dict properties)
>
>                         Set configuration for the transport.
>
> +                       For client role transport must be set with a server
> +                       endpoint oject which will be configured and the
> +                       properties must contain the following properties:
> +
> +                               array{byte} Capabilities
> +
>                 array{byte} SelectConfiguration(array{byte} capabilities)
>
>                         Select preferable configuration from the supported
> @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
>                         endpoint, because when this method gets called it has
>                         already been unregistered.
>
> +Properties     string UUID [readonly, optional]:
> +
> +                       UUID of the profile which the endpoint is for.
> +
> +               byte Codec [readonly, optional]:
> +
> +                       Assigned number of codec that the endpoint implements.
> +                       The values should match the profile specification which
> +                       is indicated by the UUID.
> +
> +               array{byte} Capabilities [readonly, optional]:
> +
> +                       Capabilities blob, it is used as it is so the size and
> +                       byte order must match.
>
>  MediaTransport1 hierarchy
>  =========================
> --
> 2.17.2

Can you try this set?

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-22 14:20 ` [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
@ 2019-01-22 17:56   ` Pali Rohár
  2019-01-23 11:24     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-01-22 17:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 3041 bytes --]

On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> Hi Pali,
> On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This adds the possibility to expose remote SEP using MediaEndpoint
> > interface to allow setting a configuration.
> > ---
> >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > index b5ad2db12..af9485342 100644
> > --- a/doc/media-api.txt
> > +++ b/doc/media-api.txt
> > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> >  MediaEndpoint1 hierarchy
> >  ========================
> >
> > -Service                unique name
> > +Service                unique name (Server role)
> > +               org.bluez (Client role)
> >  Interface      org.bluez.MediaEndpoint1
> > -Object path    freely definable
> > +Object path    freely definable (Server role)
> > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > +               (Client role)
> >
> >  Methods                void SetConfiguration(object transport, dict properties)
> >
> >                         Set configuration for the transport.
> >
> > +                       For client role transport must be set with a server
> > +                       endpoint oject which will be configured and the
> > +                       properties must contain the following properties:
> > +
> > +                               array{byte} Capabilities
> > +
> >                 array{byte} SelectConfiguration(array{byte} capabilities)
> >
> >                         Select preferable configuration from the supported
> > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> >                         endpoint, because when this method gets called it has
> >                         already been unregistered.
> >
> > +Properties     string UUID [readonly, optional]:
> > +
> > +                       UUID of the profile which the endpoint is for.
> > +
> > +               byte Codec [readonly, optional]:
> > +
> > +                       Assigned number of codec that the endpoint implements.
> > +                       The values should match the profile specification which
> > +                       is indicated by the UUID.
> > +
> > +               array{byte} Capabilities [readonly, optional]:
> > +
> > +                       Capabilities blob, it is used as it is so the size and
> > +                       byte order must match.
> >
> >  MediaTransport1 hierarchy
> >  =========================
> > --
> > 2.17.2
> 
> Can you try this set?

Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
I tested my setup also with this patch and there is no difference,
pulseaudio is working fine :-)

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-22 17:56   ` Pali Rohár
@ 2019-01-23 11:24     ` Luiz Augusto von Dentz
  2019-01-27  2:00       ` Pali Rohár
                         ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-01-23 11:24 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,
On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > interface to allow setting a configuration.
> > > ---
> > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > index b5ad2db12..af9485342 100644
> > > --- a/doc/media-api.txt
> > > +++ b/doc/media-api.txt
> > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > >  MediaEndpoint1 hierarchy
> > >  ========================
> > >
> > > -Service                unique name
> > > +Service                unique name (Server role)
> > > +               org.bluez (Client role)
> > >  Interface      org.bluez.MediaEndpoint1
> > > -Object path    freely definable
> > > +Object path    freely definable (Server role)
> > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > +               (Client role)
> > >
> > >  Methods                void SetConfiguration(object transport, dict properties)
> > >
> > >                         Set configuration for the transport.
> > >
> > > +                       For client role transport must be set with a server
> > > +                       endpoint oject which will be configured and the
> > > +                       properties must contain the following properties:
> > > +
> > > +                               array{byte} Capabilities
> > > +
> > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > >
> > >                         Select preferable configuration from the supported
> > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > >                         endpoint, because when this method gets called it has
> > >                         already been unregistered.
> > >
> > > +Properties     string UUID [readonly, optional]:
> > > +
> > > +                       UUID of the profile which the endpoint is for.
> > > +
> > > +               byte Codec [readonly, optional]:
> > > +
> > > +                       Assigned number of codec that the endpoint implements.
> > > +                       The values should match the profile specification which
> > > +                       is indicated by the UUID.
> > > +
> > > +               array{byte} Capabilities [readonly, optional]:
> > > +
> > > +                       Capabilities blob, it is used as it is so the size and
> > > +                       byte order must match.
> > >
> > >  MediaTransport1 hierarchy
> > >  =========================
> > > --
> > > 2.17.2
> >
> > Can you try this set?
>
> Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> I tested my setup also with this patch and there is no difference,
> pulseaudio is working fine :-)
>

Applied.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-23 11:24     ` Luiz Augusto von Dentz
@ 2019-01-27  2:00       ` Pali Rohár
  2019-03-27 11:14         ` Pali Rohár
  2019-04-05 16:47       ` Pali Rohár
  2019-05-03  6:17       ` Pali Rohár
  2 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-01-27  2:00 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 4059 bytes --]

On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> Hi Pali,
> On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > interface to allow setting a configuration.
> > > > ---
> > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > index b5ad2db12..af9485342 100644
> > > > --- a/doc/media-api.txt
> > > > +++ b/doc/media-api.txt
> > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > >  MediaEndpoint1 hierarchy
> > > >  ========================
> > > >
> > > > -Service                unique name
> > > > +Service                unique name (Server role)
> > > > +               org.bluez (Client role)
> > > >  Interface      org.bluez.MediaEndpoint1
> > > > -Object path    freely definable
> > > > +Object path    freely definable (Server role)
> > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > +               (Client role)
> > > >
> > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > >
> > > >                         Set configuration for the transport.
> > > >
> > > > +                       For client role transport must be set with a server
> > > > +                       endpoint oject which will be configured and the
> > > > +                       properties must contain the following properties:
> > > > +
> > > > +                               array{byte} Capabilities
> > > > +
> > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > >
> > > >                         Select preferable configuration from the supported
> > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > >                         endpoint, because when this method gets called it has
> > > >                         already been unregistered.
> > > >
> > > > +Properties     string UUID [readonly, optional]:
> > > > +
> > > > +                       UUID of the profile which the endpoint is for.
> > > > +
> > > > +               byte Codec [readonly, optional]:
> > > > +
> > > > +                       Assigned number of codec that the endpoint implements.
> > > > +                       The values should match the profile specification which
> > > > +                       is indicated by the UUID.
> > > > +
> > > > +               array{byte} Capabilities [readonly, optional]:
> > > > +
> > > > +                       Capabilities blob, it is used as it is so the size and
> > > > +                       byte order must match.
> > > >
> > > >  MediaTransport1 hierarchy
> > > >  =========================
> > > > --
> > > > 2.17.2
> > >
> > > Can you try this set?
> >
> > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > I tested my setup also with this patch and there is no difference,
> > pulseaudio is working fine :-)
> >
> 
> Applied.

Hi! I have one bug report for these patches.

When I manually disconnect A2DP profile, but let HFP active then all SEP
paths on D-Bus disappear.

  qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb

So pulseaudio would still see bluetooth device as active (because HFP is
in use), but does not see any A2DP codec as all remote SEPs from DBus
were removed.

It is possible to not remove remote SEPs when A2DP sink profile is
manually disconnected?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-27  2:00       ` Pali Rohár
@ 2019-03-27 11:14         ` Pali Rohár
  2019-04-19  8:03           ` Pali Rohár
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-03-27 11:14 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > >
> > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > interface to allow setting a configuration.
> > > > > ---
> > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > index b5ad2db12..af9485342 100644
> > > > > --- a/doc/media-api.txt
> > > > > +++ b/doc/media-api.txt
> > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > >  MediaEndpoint1 hierarchy
> > > > >  ========================
> > > > >
> > > > > -Service                unique name
> > > > > +Service                unique name (Server role)
> > > > > +               org.bluez (Client role)
> > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > -Object path    freely definable
> > > > > +Object path    freely definable (Server role)
> > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > +               (Client role)
> > > > >
> > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > >
> > > > >                         Set configuration for the transport.
> > > > >
> > > > > +                       For client role transport must be set with a server
> > > > > +                       endpoint oject which will be configured and the
> > > > > +                       properties must contain the following properties:
> > > > > +
> > > > > +                               array{byte} Capabilities
> > > > > +
> > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > >
> > > > >                         Select preferable configuration from the supported
> > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > >                         endpoint, because when this method gets called it has
> > > > >                         already been unregistered.
> > > > >
> > > > > +Properties     string UUID [readonly, optional]:
> > > > > +
> > > > > +                       UUID of the profile which the endpoint is for.
> > > > > +
> > > > > +               byte Codec [readonly, optional]:
> > > > > +
> > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > +                       The values should match the profile specification which
> > > > > +                       is indicated by the UUID.
> > > > > +
> > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > +
> > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > +                       byte order must match.
> > > > >
> > > > >  MediaTransport1 hierarchy
> > > > >  =========================
> > > > > --
> > > > > 2.17.2
> > > >
> > > > Can you try this set?
> > >
> > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > I tested my setup also with this patch and there is no difference,
> > > pulseaudio is working fine :-)
> > >
> > 
> > Applied.
> 
> Hi! I have one bug report for these patches.
> 
> When I manually disconnect A2DP profile, but let HFP active then all SEP
> paths on D-Bus disappear.
> 
>   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> 
> So pulseaudio would still see bluetooth device as active (because HFP is
> in use), but does not see any A2DP codec as all remote SEPs from DBus
> were removed.
> 
> It is possible to not remove remote SEPs when A2DP sink profile is
> manually disconnected?

Hi Luiz! Have you looked at above problem?

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-23 11:24     ` Luiz Augusto von Dentz
  2019-01-27  2:00       ` Pali Rohár
@ 2019-04-05 16:47       ` Pali Rohár
  2019-04-08  9:41         ` Luiz Augusto von Dentz
  2019-05-03  6:17       ` Pali Rohár
  2 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-04-05 16:47 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 5160 bytes --]

On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> Hi Pali,
> On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > interface to allow setting a configuration.
> > > > ---
> > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > index b5ad2db12..af9485342 100644
> > > > --- a/doc/media-api.txt
> > > > +++ b/doc/media-api.txt
> > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > >  MediaEndpoint1 hierarchy
> > > >  ========================
> > > >
> > > > -Service                unique name
> > > > +Service                unique name (Server role)
> > > > +               org.bluez (Client role)
> > > >  Interface      org.bluez.MediaEndpoint1
> > > > -Object path    freely definable
> > > > +Object path    freely definable (Server role)
> > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > +               (Client role)
> > > >
> > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > >
> > > >                         Set configuration for the transport.
> > > >
> > > > +                       For client role transport must be set with a server
> > > > +                       endpoint oject which will be configured and the
> > > > +                       properties must contain the following properties:
> > > > +
> > > > +                               array{byte} Capabilities
> > > > +
> > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > >
> > > >                         Select preferable configuration from the supported
> > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > >                         endpoint, because when this method gets called it has
> > > >                         already been unregistered.
> > > >
> > > > +Properties     string UUID [readonly, optional]:
> > > > +
> > > > +                       UUID of the profile which the endpoint is for.
> > > > +
> > > > +               byte Codec [readonly, optional]:
> > > > +
> > > > +                       Assigned number of codec that the endpoint implements.
> > > > +                       The values should match the profile specification which
> > > > +                       is indicated by the UUID.
> > > > +
> > > > +               array{byte} Capabilities [readonly, optional]:
> > > > +
> > > > +                       Capabilities blob, it is used as it is so the size and
> > > > +                       byte order must match.
> > > >
> > > >  MediaTransport1 hierarchy
> > > >  =========================
> > > > --
> > > > 2.17.2
> > >
> > > Can you try this set?
> >
> > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > I tested my setup also with this patch and there is no difference,
> > pulseaudio is working fine :-)
> >
> 
> Applied.
> 

Hello, I have found another problem with this patch series.

I have there Nokia N900 device with Community SSU updates which acts as
both A2DP sink and A2DP source.

When I scan A2DP capabilities of Nokia N900 I get following output:

$ ./tools/avinfo XX:XX:XX:XX:XX:XX
Connecting ... 
Stream End-Point #1: Audio Source 
        Media Codec: SBC
                Channel Modes: Mono DualChannel Stereo JointStereo
                Frequencies: 16Khz 32Khz 44.1Khz 48Khz 
                Subbands: 4 8
                Blocks: 4 8 12 16 
                Bitpool Range: 2-64
Stream End-Point #2: Audio Sink 
        Media Codec: SBC
                Channel Modes: Mono DualChannel Stereo JointStereo
                Frequencies: 16Khz 32Khz 44.1Khz 48Khz 
                Subbands: 4 8
                Blocks: 4 8 12 16 
                Bitpool Range: 2-64 

$ qdbus --system org.bluez
/
/org
/org/bluez
/org/bluez/hci0
/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX
/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/fd2
/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/player0
/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1
/org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2
/org/bluez/test

So there is one SEP as Audio Source and one SEP as Audio Sink.

But when I ask sep1 and sep2 for UUID via dbus, I get that both are just
Audio Sinks:

$ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1 org.bluez.MediaEndpoint1.UUID
0000110a-0000-1000-8000-00805f9b34fb

$ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2 org.bluez.MediaEndpoint1.UUID
0000110a-0000-1000-8000-00805f9b34fb

So this is a clear bug.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-05 16:47       ` Pali Rohár
@ 2019-04-08  9:41         ` Luiz Augusto von Dentz
  2019-04-08  9:47           ` Luiz Augusto von Dentz
  2019-04-19  8:03           ` Pali Rohár
  0 siblings, 2 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-08  9:41 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Fri, Apr 5, 2019 at 7:47 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > <luiz.dentz@gmail.com> wrote:
> > > > >
> > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > >
> > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > interface to allow setting a configuration.
> > > > > ---
> > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > index b5ad2db12..af9485342 100644
> > > > > --- a/doc/media-api.txt
> > > > > +++ b/doc/media-api.txt
> > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > >  MediaEndpoint1 hierarchy
> > > > >  ========================
> > > > >
> > > > > -Service                unique name
> > > > > +Service                unique name (Server role)
> > > > > +               org.bluez (Client role)
> > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > -Object path    freely definable
> > > > > +Object path    freely definable (Server role)
> > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > +               (Client role)
> > > > >
> > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > >
> > > > >                         Set configuration for the transport.
> > > > >
> > > > > +                       For client role transport must be set with a server
> > > > > +                       endpoint oject which will be configured and the
> > > > > +                       properties must contain the following properties:
> > > > > +
> > > > > +                               array{byte} Capabilities
> > > > > +
> > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > >
> > > > >                         Select preferable configuration from the supported
> > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > >                         endpoint, because when this method gets called it has
> > > > >                         already been unregistered.
> > > > >
> > > > > +Properties     string UUID [readonly, optional]:
> > > > > +
> > > > > +                       UUID of the profile which the endpoint is for.
> > > > > +
> > > > > +               byte Codec [readonly, optional]:
> > > > > +
> > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > +                       The values should match the profile specification which
> > > > > +                       is indicated by the UUID.
> > > > > +
> > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > +
> > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > +                       byte order must match.
> > > > >
> > > > >  MediaTransport1 hierarchy
> > > > >  =========================
> > > > > --
> > > > > 2.17.2
> > > >
> > > > Can you try this set?
> > >
> > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > I tested my setup also with this patch and there is no difference,
> > > pulseaudio is working fine :-)
> > >
> >
> > Applied.
> >
>
> Hello, I have found another problem with this patch series.
>
> I have there Nokia N900 device with Community SSU updates which acts as
> both A2DP sink and A2DP source.
>
> When I scan A2DP capabilities of Nokia N900 I get following output:
>
> $ ./tools/avinfo XX:XX:XX:XX:XX:XX
> Connecting ...
> Stream End-Point #1: Audio Source
>         Media Codec: SBC
>                 Channel Modes: Mono DualChannel Stereo JointStereo
>                 Frequencies: 16Khz 32Khz 44.1Khz 48Khz
>                 Subbands: 4 8
>                 Blocks: 4 8 12 16
>                 Bitpool Range: 2-64
> Stream End-Point #2: Audio Sink
>         Media Codec: SBC
>                 Channel Modes: Mono DualChannel Stereo JointStereo
>                 Frequencies: 16Khz 32Khz 44.1Khz 48Khz
>                 Subbands: 4 8
>                 Blocks: 4 8 12 16
>                 Bitpool Range: 2-64
>
> $ qdbus --system org.bluez
> /
> /org
> /org/bluez
> /org/bluez/hci0
> /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX
> /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/fd2
> /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/player0
> /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1
> /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2
> /org/bluez/test
>
> So there is one SEP as Audio Source and one SEP as Audio Sink.
>
> But when I ask sep1 and sep2 for UUID via dbus, I get that both are just
> Audio Sinks:
>
> $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1 org.bluez.MediaEndpoint1.UUID
> 0000110a-0000-1000-8000-00805f9b34fb
>
> $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2 org.bluez.MediaEndpoint1.UUID
> 0000110a-0000-1000-8000-00805f9b34fb
>
> So this is a clear bug.

I will send a fix shortly.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-08  9:41         ` Luiz Augusto von Dentz
@ 2019-04-08  9:47           ` Luiz Augusto von Dentz
  2019-04-08  9:53             ` Pali Rohár
  2019-04-19  8:03           ` Pali Rohár
  1 sibling, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-08  9:47 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Mon, Apr 8, 2019 at 12:41 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1 org.bluez.MediaEndpoint1.UUID
> > 0000110a-0000-1000-8000-00805f9b34fb
> >
> > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2 org.bluez.MediaEndpoint1.UUID
> > 0000110a-0000-1000-8000-00805f9b34fb

Btw, 110a is a remote source I wonder if why you were saying they were
sinks? Or you were referring to the local role?

> > So this is a clear bug.
>
> I will send a fix shortly.
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-08  9:47           ` Luiz Augusto von Dentz
@ 2019-04-08  9:53             ` Pali Rohár
  0 siblings, 0 replies; 28+ messages in thread
From: Pali Rohár @ 2019-04-08  9:53 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

On Monday 08 April 2019 12:47:40 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Mon, Apr 8, 2019 at 12:41 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1 org.bluez.MediaEndpoint1.UUID
> > > 0000110a-0000-1000-8000-00805f9b34fb
> > >
> > > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2 org.bluez.MediaEndpoint1.UUID
> > > 0000110a-0000-1000-8000-00805f9b34fb
> 
> Btw, 110a is a remote source I wonder if why you were saying they were
> sinks? Or you were referring to the local role?

Main problem is that these two UUIDs are same. There should be one for
remote source and one remote sink. Remote source is local sink role.

> > > So this is a clear bug.
> >
> > I will send a fix shortly.
> >
> > --
> > Luiz Augusto von Dentz

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-08  9:41         ` Luiz Augusto von Dentz
  2019-04-08  9:47           ` Luiz Augusto von Dentz
@ 2019-04-19  8:03           ` Pali Rohár
  1 sibling, 0 replies; 28+ messages in thread
From: Pali Rohár @ 2019-04-19  8:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

On Monday 08 April 2019 12:41:06 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Fri, Apr 5, 2019 at 7:47 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > Hello, I have found another problem with this patch series.
> >
> > I have there Nokia N900 device with Community SSU updates which acts as
> > both A2DP sink and A2DP source.
> >
> > When I scan A2DP capabilities of Nokia N900 I get following output:
> >
> > $ ./tools/avinfo XX:XX:XX:XX:XX:XX
> > Connecting ...
> > Stream End-Point #1: Audio Source
> >         Media Codec: SBC
> >                 Channel Modes: Mono DualChannel Stereo JointStereo
> >                 Frequencies: 16Khz 32Khz 44.1Khz 48Khz
> >                 Subbands: 4 8
> >                 Blocks: 4 8 12 16
> >                 Bitpool Range: 2-64
> > Stream End-Point #2: Audio Sink
> >         Media Codec: SBC
> >                 Channel Modes: Mono DualChannel Stereo JointStereo
> >                 Frequencies: 16Khz 32Khz 44.1Khz 48Khz
> >                 Subbands: 4 8
> >                 Blocks: 4 8 12 16
> >                 Bitpool Range: 2-64
> >
> > $ qdbus --system org.bluez
> > /
> > /org
> > /org/bluez
> > /org/bluez/hci0
> > /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX
> > /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/fd2
> > /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/player0
> > /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1
> > /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2
> > /org/bluez/test
> >
> > So there is one SEP as Audio Source and one SEP as Audio Sink.
> >
> > But when I ask sep1 and sep2 for UUID via dbus, I get that both are just
> > Audio Sinks:
> >
> > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep1 org.bluez.MediaEndpoint1.UUID
> > 0000110a-0000-1000-8000-00805f9b34fb
> >
> > $ qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX/sep2 org.bluez.MediaEndpoint1.UUID
> > 0000110a-0000-1000-8000-00805f9b34fb
> >
> > So this is a clear bug.
> 
> I will send a fix shortly.

I tested patch which is in bluez master git repository and it is working
fine. I needed to fix my pulseaudio code as I have there switched SINK
and SOURCE macros.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-03-27 11:14         ` Pali Rohár
@ 2019-04-19  8:03           ` Pali Rohár
  2019-04-19 10:04             ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-04-19  8:03 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 4634 bytes --]

On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > >
> > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > Hi Pali,
> > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > <luiz.dentz@gmail.com> wrote:
> > > > > >
> > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > >
> > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > interface to allow setting a configuration.
> > > > > > ---
> > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > index b5ad2db12..af9485342 100644
> > > > > > --- a/doc/media-api.txt
> > > > > > +++ b/doc/media-api.txt
> > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > >  MediaEndpoint1 hierarchy
> > > > > >  ========================
> > > > > >
> > > > > > -Service                unique name
> > > > > > +Service                unique name (Server role)
> > > > > > +               org.bluez (Client role)
> > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > -Object path    freely definable
> > > > > > +Object path    freely definable (Server role)
> > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > +               (Client role)
> > > > > >
> > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > >
> > > > > >                         Set configuration for the transport.
> > > > > >
> > > > > > +                       For client role transport must be set with a server
> > > > > > +                       endpoint oject which will be configured and the
> > > > > > +                       properties must contain the following properties:
> > > > > > +
> > > > > > +                               array{byte} Capabilities
> > > > > > +
> > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > >
> > > > > >                         Select preferable configuration from the supported
> > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > >                         endpoint, because when this method gets called it has
> > > > > >                         already been unregistered.
> > > > > >
> > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > +
> > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > +
> > > > > > +               byte Codec [readonly, optional]:
> > > > > > +
> > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > +                       The values should match the profile specification which
> > > > > > +                       is indicated by the UUID.
> > > > > > +
> > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > +
> > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > +                       byte order must match.
> > > > > >
> > > > > >  MediaTransport1 hierarchy
> > > > > >  =========================
> > > > > > --
> > > > > > 2.17.2
> > > > >
> > > > > Can you try this set?
> > > >
> > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > I tested my setup also with this patch and there is no difference,
> > > > pulseaudio is working fine :-)
> > > >
> > > 
> > > Applied.
> > 
> > Hi! I have one bug report for these patches.
> > 
> > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > paths on D-Bus disappear.
> > 
> >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > 
> > So pulseaudio would still see bluetooth device as active (because HFP is
> > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > were removed.
> > 
> > It is possible to not remove remote SEPs when A2DP sink profile is
> > manually disconnected?
> 
> Hi Luiz! Have you looked at above problem?

Hi! This one problem is still there.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-19  8:03           ` Pali Rohár
@ 2019-04-19 10:04             ` Luiz Augusto von Dentz
  2019-04-19 11:01               ` Pali Rohár
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-19 10:04 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > >
> > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > Hi Pali,
> > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > >
> > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > >
> > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > interface to allow setting a configuration.
> > > > > > > ---
> > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > --- a/doc/media-api.txt
> > > > > > > +++ b/doc/media-api.txt
> > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > >  MediaEndpoint1 hierarchy
> > > > > > >  ========================
> > > > > > >
> > > > > > > -Service                unique name
> > > > > > > +Service                unique name (Server role)
> > > > > > > +               org.bluez (Client role)
> > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > -Object path    freely definable
> > > > > > > +Object path    freely definable (Server role)
> > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > +               (Client role)
> > > > > > >
> > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > >
> > > > > > >                         Set configuration for the transport.
> > > > > > >
> > > > > > > +                       For client role transport must be set with a server
> > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > +                       properties must contain the following properties:
> > > > > > > +
> > > > > > > +                               array{byte} Capabilities
> > > > > > > +
> > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > >
> > > > > > >                         Select preferable configuration from the supported
> > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > >                         endpoint, because when this method gets called it has
> > > > > > >                         already been unregistered.
> > > > > > >
> > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > +
> > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > +
> > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > +
> > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > +                       The values should match the profile specification which
> > > > > > > +                       is indicated by the UUID.
> > > > > > > +
> > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > +
> > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > +                       byte order must match.
> > > > > > >
> > > > > > >  MediaTransport1 hierarchy
> > > > > > >  =========================
> > > > > > > --
> > > > > > > 2.17.2
> > > > > >
> > > > > > Can you try this set?
> > > > >
> > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > I tested my setup also with this patch and there is no difference,
> > > > > pulseaudio is working fine :-)
> > > > >
> > > >
> > > > Applied.
> > >
> > > Hi! I have one bug report for these patches.
> > >
> > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > paths on D-Bus disappear.
> > >
> > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > >
> > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > were removed.
> > >
> > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > manually disconnected?
> >
> > Hi Luiz! Have you looked at above problem?
>
> Hi! This one problem is still there.

This should have been fixed:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-19 10:04             ` Luiz Augusto von Dentz
@ 2019-04-19 11:01               ` Pali Rohár
  2019-04-19 11:48                 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-04-19 11:01 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 5607 bytes --]

On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > Hi Pali,
> > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > >
> > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > Hi Pali,
> > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > >
> > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > >
> > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > interface to allow setting a configuration.
> > > > > > > > ---
> > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > --- a/doc/media-api.txt
> > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > >  ========================
> > > > > > > >
> > > > > > > > -Service                unique name
> > > > > > > > +Service                unique name (Server role)
> > > > > > > > +               org.bluez (Client role)
> > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > -Object path    freely definable
> > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > +               (Client role)
> > > > > > > >
> > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > >
> > > > > > > >                         Set configuration for the transport.
> > > > > > > >
> > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > +                       properties must contain the following properties:
> > > > > > > > +
> > > > > > > > +                               array{byte} Capabilities
> > > > > > > > +
> > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > >
> > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > >                         already been unregistered.
> > > > > > > >
> > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > +
> > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > +
> > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > +
> > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > +                       The values should match the profile specification which
> > > > > > > > +                       is indicated by the UUID.
> > > > > > > > +
> > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > +
> > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > +                       byte order must match.
> > > > > > > >
> > > > > > > >  MediaTransport1 hierarchy
> > > > > > > >  =========================
> > > > > > > > --
> > > > > > > > 2.17.2
> > > > > > >
> > > > > > > Can you try this set?
> > > > > >
> > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > pulseaudio is working fine :-)
> > > > > >
> > > > >
> > > > > Applied.
> > > >
> > > > Hi! I have one bug report for these patches.
> > > >
> > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > paths on D-Bus disappear.
> > > >
> > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > >
> > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > were removed.
> > > >
> > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > manually disconnected?
> > >
> > > Hi Luiz! Have you looked at above problem?
> >
> > Hi! This one problem is still there.
> 
> This should have been fixed:
> 
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe

Hi! It is still not fixed. I have that commit applied (it is in master
branch), but SEPs still disappears from dbus after calling
DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
above; even HFP/HSP is still active.

Can you reproduce that problem?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-19 11:01               ` Pali Rohár
@ 2019-04-19 11:48                 ` Luiz Augusto von Dentz
  2019-04-19 12:04                   ` Pali Rohár
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-19 11:48 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Fri, Apr 19, 2019 at 2:02 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > > Hi Pali,
> > > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > >
> > > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > > Hi Pali,
> > > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > >
> > > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > > interface to allow setting a configuration.
> > > > > > > > > ---
> > > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > > --- a/doc/media-api.txt
> > > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > > >  ========================
> > > > > > > > >
> > > > > > > > > -Service                unique name
> > > > > > > > > +Service                unique name (Server role)
> > > > > > > > > +               org.bluez (Client role)
> > > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > > -Object path    freely definable
> > > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > > +               (Client role)
> > > > > > > > >
> > > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > > >
> > > > > > > > >                         Set configuration for the transport.
> > > > > > > > >
> > > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > > +                       properties must contain the following properties:
> > > > > > > > > +
> > > > > > > > > +                               array{byte} Capabilities
> > > > > > > > > +
> > > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > > >
> > > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > > >                         already been unregistered.
> > > > > > > > >
> > > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > > +
> > > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > > +
> > > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > > +
> > > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > > +                       The values should match the profile specification which
> > > > > > > > > +                       is indicated by the UUID.
> > > > > > > > > +
> > > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > > +
> > > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > > +                       byte order must match.
> > > > > > > > >
> > > > > > > > >  MediaTransport1 hierarchy
> > > > > > > > >  =========================
> > > > > > > > > --
> > > > > > > > > 2.17.2
> > > > > > > >
> > > > > > > > Can you try this set?
> > > > > > >
> > > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > > pulseaudio is working fine :-)
> > > > > > >
> > > > > >
> > > > > > Applied.
> > > > >
> > > > > Hi! I have one bug report for these patches.
> > > > >
> > > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > > paths on D-Bus disappear.
> > > > >
> > > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > > >
> > > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > > were removed.
> > > > >
> > > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > > manually disconnected?
> > > >
> > > > Hi Luiz! Have you looked at above problem?
> > >
> > > Hi! This one problem is still there.
> >
> > This should have been fixed:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe
>
> Hi! It is still not fixed. I have that commit applied (it is in master
> branch), but SEPs still disappears from dbus after calling
> DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
> above; even HFP/HSP is still active.
>
> Can you reproduce that problem?

We remove the objects when we disconnect AVDTP, this is working as
intended as we don't want to initiate the connection directly from the
endpoint as those may have disappeared or changed to some other codec
while disconnected.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-19 11:48                 ` Luiz Augusto von Dentz
@ 2019-04-19 12:04                   ` Pali Rohár
  2019-04-20  6:43                     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-04-19 12:04 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 7042 bytes --]

On Friday 19 April 2019 14:48:38 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Fri, Apr 19, 2019 at 2:02 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > >
> > > > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > > > Hi Pali,
> > > > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > > > Hi Pali,
> > > > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > >
> > > > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > > > interface to allow setting a configuration.
> > > > > > > > > > ---
> > > > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > > > --- a/doc/media-api.txt
> > > > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > > > >  ========================
> > > > > > > > > >
> > > > > > > > > > -Service                unique name
> > > > > > > > > > +Service                unique name (Server role)
> > > > > > > > > > +               org.bluez (Client role)
> > > > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > > > -Object path    freely definable
> > > > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > > > +               (Client role)
> > > > > > > > > >
> > > > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > > > >
> > > > > > > > > >                         Set configuration for the transport.
> > > > > > > > > >
> > > > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > > > +                       properties must contain the following properties:
> > > > > > > > > > +
> > > > > > > > > > +                               array{byte} Capabilities
> > > > > > > > > > +
> > > > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > > > >
> > > > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > > > >                         already been unregistered.
> > > > > > > > > >
> > > > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > > > +
> > > > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > > > +
> > > > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > > > +
> > > > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > > > +                       The values should match the profile specification which
> > > > > > > > > > +                       is indicated by the UUID.
> > > > > > > > > > +
> > > > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > > > +
> > > > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > > > +                       byte order must match.
> > > > > > > > > >
> > > > > > > > > >  MediaTransport1 hierarchy
> > > > > > > > > >  =========================
> > > > > > > > > > --
> > > > > > > > > > 2.17.2
> > > > > > > > >
> > > > > > > > > Can you try this set?
> > > > > > > >
> > > > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > > > pulseaudio is working fine :-)
> > > > > > > >
> > > > > > >
> > > > > > > Applied.
> > > > > >
> > > > > > Hi! I have one bug report for these patches.
> > > > > >
> > > > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > > > paths on D-Bus disappear.
> > > > > >
> > > > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > > > >
> > > > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > > > were removed.
> > > > > >
> > > > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > > > manually disconnected?
> > > > >
> > > > > Hi Luiz! Have you looked at above problem?
> > > >
> > > > Hi! This one problem is still there.
> > >
> > > This should have been fixed:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe
> >
> > Hi! It is still not fixed. I have that commit applied (it is in master
> > branch), but SEPs still disappears from dbus after calling
> > DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
> > above; even HFP/HSP is still active.
> >
> > Can you reproduce that problem?
> 
> We remove the objects when we disconnect AVDTP, this is working as
> intended as we don't want to initiate the connection directly from the
> endpoint as those may have disappeared or changed to some other codec
> while disconnected.

That is a bit problematic. Pulseaudio see device as active when either
A2DP or HFP/HSP profile is connected. It makes sense. But when A2DP is
disconnected then bluez remove SEP from dbus which says to pulseaudio
that all codecs (as they belongs to SEPs) are unsupported. Which leads
to situation that pulseaudio has no idea which codecs are supported nor
if A2DP is supported at all.

So if bluez remove all SEPs from dbus, how can pulseaudio again activate
A2DP connection with codec X? It is impossible as SEP endpoint exported
by dbus does not exist anymore.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-19 12:04                   ` Pali Rohár
@ 2019-04-20  6:43                     ` Luiz Augusto von Dentz
  2019-04-20  7:00                       ` Pali Rohár
  0 siblings, 1 reply; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-20  6:43 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Fri, Apr 19, 2019 at 3:04 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Friday 19 April 2019 14:48:38 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Fri, Apr 19, 2019 at 2:02 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > >
> > > > > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > > > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > > > > Hi Pali,
> > > > > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > > > > Hi Pali,
> > > > > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > >
> > > > > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > > > > interface to allow setting a configuration.
> > > > > > > > > > > ---
> > > > > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > > > > --- a/doc/media-api.txt
> > > > > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > > > > >  ========================
> > > > > > > > > > >
> > > > > > > > > > > -Service                unique name
> > > > > > > > > > > +Service                unique name (Server role)
> > > > > > > > > > > +               org.bluez (Client role)
> > > > > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > > > > -Object path    freely definable
> > > > > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > > > > +               (Client role)
> > > > > > > > > > >
> > > > > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > > > > >
> > > > > > > > > > >                         Set configuration for the transport.
> > > > > > > > > > >
> > > > > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > > > > +                       properties must contain the following properties:
> > > > > > > > > > > +
> > > > > > > > > > > +                               array{byte} Capabilities
> > > > > > > > > > > +
> > > > > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > > > > >
> > > > > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > > > > >                         already been unregistered.
> > > > > > > > > > >
> > > > > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > > > > +
> > > > > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > > > > +
> > > > > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > > > > +
> > > > > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > > > > +                       The values should match the profile specification which
> > > > > > > > > > > +                       is indicated by the UUID.
> > > > > > > > > > > +
> > > > > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > > > > +
> > > > > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > > > > +                       byte order must match.
> > > > > > > > > > >
> > > > > > > > > > >  MediaTransport1 hierarchy
> > > > > > > > > > >  =========================
> > > > > > > > > > > --
> > > > > > > > > > > 2.17.2
> > > > > > > > > >
> > > > > > > > > > Can you try this set?
> > > > > > > > >
> > > > > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > > > > pulseaudio is working fine :-)
> > > > > > > > >
> > > > > > > >
> > > > > > > > Applied.
> > > > > > >
> > > > > > > Hi! I have one bug report for these patches.
> > > > > > >
> > > > > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > > > > paths on D-Bus disappear.
> > > > > > >
> > > > > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > > > > >
> > > > > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > > > > were removed.
> > > > > > >
> > > > > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > > > > manually disconnected?
> > > > > >
> > > > > > Hi Luiz! Have you looked at above problem?
> > > > >
> > > > > Hi! This one problem is still there.
> > > >
> > > > This should have been fixed:
> > > >
> > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe
> > >
> > > Hi! It is still not fixed. I have that commit applied (it is in master
> > > branch), but SEPs still disappears from dbus after calling
> > > DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
> > > above; even HFP/HSP is still active.
> > >
> > > Can you reproduce that problem?
> >
> > We remove the objects when we disconnect AVDTP, this is working as
> > intended as we don't want to initiate the connection directly from the
> > endpoint as those may have disappeared or changed to some other codec
> > while disconnected.
>
> That is a bit problematic. Pulseaudio see device as active when either
> A2DP or HFP/HSP profile is connected. It makes sense. But when A2DP is
> disconnected then bluez remove SEP from dbus which says to pulseaudio
> that all codecs (as they belongs to SEPs) are unsupported. Which leads
> to situation that pulseaudio has no idea which codecs are supported nor
> if A2DP is supported at all.

Usually, all audio profiles are connected simultaneously, we even have
a policy plugin to enforce this. Also keep in mind that we never
intend to PulseAudio to trigger connections, for that you use the
Bluetooth system settings.

> So if bluez remove all SEPs from dbus, how can pulseaudio again activate
> A2DP connection with codec X? It is impossible as SEP endpoint exported
> by dbus does not exist anymore.

It doesn't, if you want to reconnect that should be triggered via
Device.Connect, the daemon will remember the last connected SEP and do
the right thing, if the user wants to later overwrite the default it
is free to do so by selecting a different SEP. So, in short, it is a
design choice not to support connecting directly from SEP, anyway in
most cases that would not even be possible since the device should not
be connected PA would not list any audio device for it.


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-20  6:43                     ` Luiz Augusto von Dentz
@ 2019-04-20  7:00                       ` Pali Rohár
  2019-04-20  7:31                         ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 28+ messages in thread
From: Pali Rohár @ 2019-04-20  7:00 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 9348 bytes --]

On Saturday 20 April 2019 09:43:17 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Fri, Apr 19, 2019 at 3:04 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Friday 19 April 2019 14:48:38 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > >
> > > On Fri, Apr 19, 2019 at 2:02 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > >
> > > > On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > >
> > > > > > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > > > > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > > > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > > > > > Hi Pali,
> > > > > > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > > > > >
> > > > > > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > > > > > Hi Pali,
> > > > > > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > > >
> > > > > > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > > > > > interface to allow setting a configuration.
> > > > > > > > > > > > ---
> > > > > > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > > > > > --- a/doc/media-api.txt
> > > > > > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > > > > > >  ========================
> > > > > > > > > > > >
> > > > > > > > > > > > -Service                unique name
> > > > > > > > > > > > +Service                unique name (Server role)
> > > > > > > > > > > > +               org.bluez (Client role)
> > > > > > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > > > > > -Object path    freely definable
> > > > > > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > > > > > +               (Client role)
> > > > > > > > > > > >
> > > > > > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > > > > > >
> > > > > > > > > > > >                         Set configuration for the transport.
> > > > > > > > > > > >
> > > > > > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > > > > > +                       properties must contain the following properties:
> > > > > > > > > > > > +
> > > > > > > > > > > > +                               array{byte} Capabilities
> > > > > > > > > > > > +
> > > > > > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > > > > > >
> > > > > > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > > > > > >                         already been unregistered.
> > > > > > > > > > > >
> > > > > > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > > > > > +
> > > > > > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > > > > > +
> > > > > > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > > > > > +
> > > > > > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > > > > > +                       The values should match the profile specification which
> > > > > > > > > > > > +                       is indicated by the UUID.
> > > > > > > > > > > > +
> > > > > > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > > > > > +
> > > > > > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > > > > > +                       byte order must match.
> > > > > > > > > > > >
> > > > > > > > > > > >  MediaTransport1 hierarchy
> > > > > > > > > > > >  =========================
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.17.2
> > > > > > > > > > >
> > > > > > > > > > > Can you try this set?
> > > > > > > > > >
> > > > > > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > > > > > pulseaudio is working fine :-)
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Applied.
> > > > > > > >
> > > > > > > > Hi! I have one bug report for these patches.
> > > > > > > >
> > > > > > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > > > > > paths on D-Bus disappear.
> > > > > > > >
> > > > > > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > > > > > >
> > > > > > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > > > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > > > > > were removed.
> > > > > > > >
> > > > > > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > > > > > manually disconnected?
> > > > > > >
> > > > > > > Hi Luiz! Have you looked at above problem?
> > > > > >
> > > > > > Hi! This one problem is still there.
> > > > >
> > > > > This should have been fixed:
> > > > >
> > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe
> > > >
> > > > Hi! It is still not fixed. I have that commit applied (it is in master
> > > > branch), but SEPs still disappears from dbus after calling
> > > > DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
> > > > above; even HFP/HSP is still active.
> > > >
> > > > Can you reproduce that problem?
> > >
> > > We remove the objects when we disconnect AVDTP, this is working as
> > > intended as we don't want to initiate the connection directly from the
> > > endpoint as those may have disappeared or changed to some other codec
> > > while disconnected.
> >
> > That is a bit problematic. Pulseaudio see device as active when either
> > A2DP or HFP/HSP profile is connected. It makes sense. But when A2DP is
> > disconnected then bluez remove SEP from dbus which says to pulseaudio
> > that all codecs (as they belongs to SEPs) are unsupported. Which leads
> > to situation that pulseaudio has no idea which codecs are supported nor
> > if A2DP is supported at all.
> 
> Usually, all audio profiles are connected simultaneously, we even have
> a policy plugin to enforce this. Also keep in mind that we never
> intend to PulseAudio to trigger connections, for that you use the
> Bluetooth system settings.

Look, I'm able to achieve state when A2DP is not connected and HFP/HSP
is. Only via public dbus API. So such thing is possible and it could be
available in the future in some program (command line or bluetooth gui).

And as this state is not only possible, but also valid then pulseaudio
needs to deal with it.

And as you wrote that pulseaudio should not trigger connection, how it
should know list of available codecs?

> > So if bluez remove all SEPs from dbus, how can pulseaudio again activate
> > A2DP connection with codec X? It is impossible as SEP endpoint exported
> > by dbus does not exist anymore.
> 
> It doesn't, if you want to reconnect that should be triggered via
> Device.Connect, the daemon will remember the last connected SEP and do
> the right thing, if the user wants to later overwrite the default it
> is free to do so by selecting a different SEP.

A2DP is connected, HSP is. Currently HSP audio output is used in
pulseaudio and user want to switch it to codec X. bluez has remembered
codec Y (but this information is not available to pulseaudio).

How should now pulseaudio activate A2DP with codec X?

> So, in short, it is a
> design choice not to support connecting directly from SEP, anyway in
> most cases that would not even be possible since the device should not
> be connected PA would not list any audio device for it.

If device is not connected then pulseaudio does not list audio device.

But if HSP is connected and A2DP not, then pulseaudio show that
bluetooth audio device.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-04-20  7:00                       ` Pali Rohár
@ 2019-04-20  7:31                         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 28+ messages in thread
From: Luiz Augusto von Dentz @ 2019-04-20  7:31 UTC (permalink / raw)
  To: Pali Rohár; +Cc: linux-bluetooth

Hi Pali,

On Sat, Apr 20, 2019 at 10:01 AM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Saturday 20 April 2019 09:43:17 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Fri, Apr 19, 2019 at 3:04 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Friday 19 April 2019 14:48:38 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Fri, Apr 19, 2019 at 2:02 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > >
> > > > > On Friday 19 April 2019 13:04:01 Luiz Augusto von Dentz wrote:
> > > > > > Hi Pali,
> > > > > >
> > > > > > On Fri, Apr 19, 2019 at 11:03 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > >
> > > > > > > On Wednesday 27 March 2019 12:14:30 Pali Rohár wrote:
> > > > > > > > On Sunday 27 January 2019 03:00:38 Pali Rohár wrote:
> > > > > > > > > On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> > > > > > > > > > Hi Pali,
> > > > > > > > > > On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > > > > > > > > > > Hi Pali,
> > > > > > > > > > > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > > > > > > > > > > <luiz.dentz@gmail.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > > > > > > > > > > >
> > > > > > > > > > > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > > > > > > > > > > interface to allow setting a configuration.
> > > > > > > > > > > > > ---
> > > > > > > > > > > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > > > > > > > > > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > > > > > > > > > > index b5ad2db12..af9485342 100644
> > > > > > > > > > > > > --- a/doc/media-api.txt
> > > > > > > > > > > > > +++ b/doc/media-api.txt
> > > > > > > > > > > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > > > > > > > > > > >  MediaEndpoint1 hierarchy
> > > > > > > > > > > > >  ========================
> > > > > > > > > > > > >
> > > > > > > > > > > > > -Service                unique name
> > > > > > > > > > > > > +Service                unique name (Server role)
> > > > > > > > > > > > > +               org.bluez (Client role)
> > > > > > > > > > > > >  Interface      org.bluez.MediaEndpoint1
> > > > > > > > > > > > > -Object path    freely definable
> > > > > > > > > > > > > +Object path    freely definable (Server role)
> > > > > > > > > > > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > > > > > > > > > > +               (Client role)
> > > > > > > > > > > > >
> > > > > > > > > > > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > > > > > > > > > > >
> > > > > > > > > > > > >                         Set configuration for the transport.
> > > > > > > > > > > > >
> > > > > > > > > > > > > +                       For client role transport must be set with a server
> > > > > > > > > > > > > +                       endpoint oject which will be configured and the
> > > > > > > > > > > > > +                       properties must contain the following properties:
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +                               array{byte} Capabilities
> > > > > > > > > > > > > +
> > > > > > > > > > > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > > > > > > > > > > >
> > > > > > > > > > > > >                         Select preferable configuration from the supported
> > > > > > > > > > > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > > > > > > > > > > >                         endpoint, because when this method gets called it has
> > > > > > > > > > > > >                         already been unregistered.
> > > > > > > > > > > > >
> > > > > > > > > > > > > +Properties     string UUID [readonly, optional]:
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +                       UUID of the profile which the endpoint is for.
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +               byte Codec [readonly, optional]:
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +                       Assigned number of codec that the endpoint implements.
> > > > > > > > > > > > > +                       The values should match the profile specification which
> > > > > > > > > > > > > +                       is indicated by the UUID.
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +               array{byte} Capabilities [readonly, optional]:
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +                       Capabilities blob, it is used as it is so the size and
> > > > > > > > > > > > > +                       byte order must match.
> > > > > > > > > > > > >
> > > > > > > > > > > > >  MediaTransport1 hierarchy
> > > > > > > > > > > > >  =========================
> > > > > > > > > > > > > --
> > > > > > > > > > > > > 2.17.2
> > > > > > > > > > > >
> > > > > > > > > > > > Can you try this set?
> > > > > > > > > > >
> > > > > > > > > > > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > > > > > > > > > > I tested my setup also with this patch and there is no difference,
> > > > > > > > > > > pulseaudio is working fine :-)
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Applied.
> > > > > > > > >
> > > > > > > > > Hi! I have one bug report for these patches.
> > > > > > > > >
> > > > > > > > > When I manually disconnect A2DP profile, but let HFP active then all SEP
> > > > > > > > > paths on D-Bus disappear.
> > > > > > > > >
> > > > > > > > >   qdbus --system org.bluez /org/bluez/hci0/dev_XX_XX_XX_XX_XX_XX org.bluez.Device1.DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb
> > > > > > > > >
> > > > > > > > > So pulseaudio would still see bluetooth device as active (because HFP is
> > > > > > > > > in use), but does not see any A2DP codec as all remote SEPs from DBus
> > > > > > > > > were removed.
> > > > > > > > >
> > > > > > > > > It is possible to not remove remote SEPs when A2DP sink profile is
> > > > > > > > > manually disconnected?
> > > > > > > >
> > > > > > > > Hi Luiz! Have you looked at above problem?
> > > > > > >
> > > > > > > Hi! This one problem is still there.
> > > > > >
> > > > > > This should have been fixed:
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=9f7659a44601e043bdb6724b0ab4f3f59c46e9fe
> > > > >
> > > > > Hi! It is still not fixed. I have that commit applied (it is in master
> > > > > branch), but SEPs still disappears from dbus after calling
> > > > > DisconnectProfile 0000110b-0000-1000-8000-00805f9b34fb as described
> > > > > above; even HFP/HSP is still active.
> > > > >
> > > > > Can you reproduce that problem?
> > > >
> > > > We remove the objects when we disconnect AVDTP, this is working as
> > > > intended as we don't want to initiate the connection directly from the
> > > > endpoint as those may have disappeared or changed to some other codec
> > > > while disconnected.
> > >
> > > That is a bit problematic. Pulseaudio see device as active when either
> > > A2DP or HFP/HSP profile is connected. It makes sense. But when A2DP is
> > > disconnected then bluez remove SEP from dbus which says to pulseaudio
> > > that all codecs (as they belongs to SEPs) are unsupported. Which leads
> > > to situation that pulseaudio has no idea which codecs are supported nor
> > > if A2DP is supported at all.
> >
> > Usually, all audio profiles are connected simultaneously, we even have
> > a policy plugin to enforce this. Also keep in mind that we never
> > intend to PulseAudio to trigger connections, for that you use the
> > Bluetooth system settings.
>
> Look, I'm able to achieve state when A2DP is not connected and HFP/HSP
> is. Only via public dbus API. So such thing is possible and it could be
> available in the future in some program (command line or bluetooth gui).
>
> And as this state is not only possible, but also valid then pulseaudio
> needs to deal with it.
>
> And as you wrote that pulseaudio should not trigger connection, how it
> should know list of available codecs?

User just have to connect A2DP, Bluetooth settings should know it is
not connected, it is a user action after all.

> > > So if bluez remove all SEPs from dbus, how can pulseaudio again activate
> > > A2DP connection with codec X? It is impossible as SEP endpoint exported
> > > by dbus does not exist anymore.
> >
> > It doesn't, if you want to reconnect that should be triggered via
> > Device.Connect, the daemon will remember the last connected SEP and do
> > the right thing, if the user wants to later overwrite the default it
> > is free to do so by selecting a different SEP.
>
> A2DP is connected, HSP is. Currently HSP audio output is used in
> pulseaudio and user want to switch it to codec X. bluez has remembered
> codec Y (but this information is not available to pulseaudio).
>
> How should now pulseaudio activate A2DP with codec X?
>
> > So, in short, it is a
> > design choice not to support connecting directly from SEP, anyway in
> > most cases that would not even be possible since the device should not
> > be connected PA would not list any audio device for it.
>
> If device is not connected then pulseaudio does not list audio device.
>
> But if HSP is connected and A2DP not, then pulseaudio show that
> bluetooth audio device.

User has to connect A2DP and then he has to option to switch to
whatever codec that has been discovered, as I said this is a design
choice, we don't want to duplicate controls.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP
  2019-01-23 11:24     ` Luiz Augusto von Dentz
  2019-01-27  2:00       ` Pali Rohár
  2019-04-05 16:47       ` Pali Rohár
@ 2019-05-03  6:17       ` Pali Rohár
  2 siblings, 0 replies; 28+ messages in thread
From: Pali Rohár @ 2019-05-03  6:17 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

[-- Attachment #1: Type: text/plain, Size: 4805 bytes --]

On Wednesday 23 January 2019 13:24:22 Luiz Augusto von Dentz wrote:
> Hi Pali,
> On Tue, Jan 22, 2019 at 7:56 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Tuesday 22 January 2019 16:20:12 Luiz Augusto von Dentz wrote:
> > > Hi Pali,
> > > On Tue, Jan 22, 2019 at 3:45 PM Luiz Augusto von Dentz
> > > <luiz.dentz@gmail.com> wrote:
> > > >
> > > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > > >
> > > > This adds the possibility to expose remote SEP using MediaEndpoint
> > > > interface to allow setting a configuration.
> > > > ---
> > > >  doc/media-api.txt | 27 +++++++++++++++++++++++++--
> > > >  1 file changed, 25 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > > index b5ad2db12..af9485342 100644
> > > > --- a/doc/media-api.txt
> > > > +++ b/doc/media-api.txt
> > > > @@ -500,14 +500,23 @@ Properties        object Player [readonly]
> > > >  MediaEndpoint1 hierarchy
> > > >  ========================
> > > >
> > > > -Service                unique name
> > > > +Service                unique name (Server role)
> > > > +               org.bluez (Client role)
> > > >  Interface      org.bluez.MediaEndpoint1
> > > > -Object path    freely definable
> > > > +Object path    freely definable (Server role)
> > > > +               [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/sepX
> > > > +               (Client role)
> > > >
> > > >  Methods                void SetConfiguration(object transport, dict properties)
> > > >
> > > >                         Set configuration for the transport.
> > > >
> > > > +                       For client role transport must be set with a server
> > > > +                       endpoint oject which will be configured and the
> > > > +                       properties must contain the following properties:
> > > > +
> > > > +                               array{byte} Capabilities
> > > > +
> > > >                 array{byte} SelectConfiguration(array{byte} capabilities)
> > > >
> > > >                         Select preferable configuration from the supported
> > > > @@ -532,6 +541,20 @@ Methods            void SetConfiguration(object transport, dict properties)
> > > >                         endpoint, because when this method gets called it has
> > > >                         already been unregistered.
> > > >
> > > > +Properties     string UUID [readonly, optional]:
> > > > +
> > > > +                       UUID of the profile which the endpoint is for.
> > > > +
> > > > +               byte Codec [readonly, optional]:
> > > > +
> > > > +                       Assigned number of codec that the endpoint implements.
> > > > +                       The values should match the profile specification which
> > > > +                       is indicated by the UUID.
> > > > +
> > > > +               array{byte} Capabilities [readonly, optional]:
> > > > +
> > > > +                       Capabilities blob, it is used as it is so the size and
> > > > +                       byte order must match.
> > > >
> > > >  MediaTransport1 hierarchy
> > > >  =========================
> > > > --
> > > > 2.17.2
> > >
> > > Can you try this set?
> >
> > Hi! In V3 you added only "a2dp: Add reverse discovery" patch right?
> > I tested my setup also with this patch and there is no difference,
> > pulseaudio is working fine :-)
> >
> 
> Applied.
> 

Now I found another bug. The whole codec switching does not work with
some Ausdom headset. When pulseaudio try to switch codec profile it just
get from bluez org.bluez.Error.Failed: Invalid argument DBus error
message. And in bluez log is just: avdtp_close: Invalid argument

In btmon I'm seeing following output:

< ACL Data TX: Handle 35 flags 0x00 dlen 7              #4093 [hci0] 112.879357
      Channel: 2242 len 3 [PSM 0 mode 0] {chan 0}
        60 09 04                                         `..             
> HCI Event: Number of Completed Packets (0x13) plen 5  #4094 [hci0] 112.879474
        Num handles: 1
        Handle: 35
        Count: 1
> HCI Event: Number of Completed Packets (0x13) plen 5  #4095 [hci0] 112.883600
        Num handles: 1
        Handle: 35
        Count: 2
> HCI Event: Number of Completed Packets (0x13) plen 5  #4096 [hci0] 112.884598
        Num handles: 1
        Handle: 35
        Count: 1
> ACL Data RX: Handle 35 flags 0x02 dlen 6              #4097 [hci0] 112.906614
      Channel: 66 len 2 [PSM 0 mode 0] {chan 0}
        62 09                                            b.              
= bluetoothd: avdtp_close: Invalid argument                          112.908009

Any idea why that avdp_close is failing?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-05-03  6:17 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 13:45 [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 2/9] a2dp: Expose " Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 3/9] doc/media-api: Add Endpoint property to MediaTransport Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 4/9] a2dp: Implement MediaTransport.Endpoint Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 5/9] doc/settings-storage: Add Endpoint group to cache Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 6/9] a2dp: Cache remote endpoints Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 7/9] doc/media-api: Add Device property to MediaEndpoint Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 8/9] a2dp: Add implementation of MediaEndpoint.Device Luiz Augusto von Dentz
2019-01-22 13:45 ` [PATCH v3 9/9] a2dp: Add reverse discovery Luiz Augusto von Dentz
2019-01-22 14:20 ` [PATCH v3 1/9] doc/media-api: Enable MediaEndpoint to expose remote SEP Luiz Augusto von Dentz
2019-01-22 17:56   ` Pali Rohár
2019-01-23 11:24     ` Luiz Augusto von Dentz
2019-01-27  2:00       ` Pali Rohár
2019-03-27 11:14         ` Pali Rohár
2019-04-19  8:03           ` Pali Rohár
2019-04-19 10:04             ` Luiz Augusto von Dentz
2019-04-19 11:01               ` Pali Rohár
2019-04-19 11:48                 ` Luiz Augusto von Dentz
2019-04-19 12:04                   ` Pali Rohár
2019-04-20  6:43                     ` Luiz Augusto von Dentz
2019-04-20  7:00                       ` Pali Rohár
2019-04-20  7:31                         ` Luiz Augusto von Dentz
2019-04-05 16:47       ` Pali Rohár
2019-04-08  9:41         ` Luiz Augusto von Dentz
2019-04-08  9:47           ` Luiz Augusto von Dentz
2019-04-08  9:53             ` Pali Rohár
2019-04-19  8:03           ` Pali Rohár
2019-05-03  6:17       ` Pali Rohár

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).