[PATCH BlueZ 0/4] Add Modify Source initial implementation

[PATCH BlueZ 0/4] Add Modify Source initial implementation

[Iulia Tanasescu]

This patch implements a BASS fix for adding watches for closed
channels, adds btio support for binding a PA sync channel to
a number of BISes and adds an initial implementation for the
BASS Modify Source procedure.

This patch series depends on the kernel support added by
Bluetooth: ISO: Allow binding a PA sync socket

Iulia Tanasescu (4):
  btio: Handle closed channel in server_cb
  shared/bass: Handle G_IO_HUP on io channels
  btio: Allow binding a bcast listener before accept
  shared/bass: Add Modify Source initial implementation

 btio/btio.c          |  54 ++++++-
 btio/btio.h          |   2 +-
 profiles/audio/bap.c |   2 +-
 src/shared/bass.c    | 333 ++++++++++++++++++++++++++++++++++++++++---
 src/shared/bass.h    |   4 +
 5 files changed, 371 insertions(+), 24 deletions(-)


base-commit: e347792f41a8ab2cf4f789b6544a4f8ec8464a53
-- 
2.39.2

[PATCH BlueZ 1/4] btio: Handle closed channel in server_cb

[Iulia Tanasescu]

This handles G_IO_ERR and G_IO_HUP conditions in server_cb

---
 btio/btio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/btio/btio.c b/btio/btio.c
index d45b8240d..c63a6d1df 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io, GIOCondition cond,
 	GIOChannel *cli_io;
 
 	/* If the user closed the server */
-	if ((cond & G_IO_NVAL) || check_nval(io))
+	if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
+			check_nval(io))
 		return FALSE;
 
 	srv_sock = g_io_channel_unix_get_fd(io);
-- 
2.39.2

[PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io channels

[Iulia Tanasescu]

This adds watches to handle closed sockets

---
 src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 src/shared/bass.h |  2 ++
 2 files changed, 61 insertions(+)

diff --git a/src/shared/bass.c b/src/shared/bass.c
index 0ee3187d1..78103d463 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
 		g_io_channel_unref(bcast_src->listen_io);
 		bcast_src->listen_io = NULL;
 
+		if (bcast_src->listen_io_id > 0) {
+			g_source_remove(bcast_src->listen_io_id);
+			bcast_src->listen_io_id  = 0;
+		}
+
 		/* Close pa sync io */
 		if (bcast_src->pa_sync_io) {
 			g_io_channel_shutdown(bcast_src->pa_sync_io,
@@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
 			bcast_src->pa_sync_io = NULL;
 		}
 
+		if (bcast_src->pa_sync_io_id > 0) {
+			g_source_remove(bcast_src->pa_sync_io_id);
+			bcast_src->pa_sync_io_id  = 0;
+		}
+
 		for (i = 0; i < bcast_src->num_subgroups; i++)
 			bcast_src->subgroup_data[i].bis_sync =
 				BT_BASS_BIG_SYNC_FAILED_BITMASK;
@@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
 	return false;
 }
 
+static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+			gpointer data)
+{
+	struct bt_bcast_src *bcast_src = data;
+
+	DBG(bcast_src->bass, "PA sync io has been disconnected");
+
+	bcast_src->pa_sync_io_id = 0;
+	bcast_src->pa_sync_io = NULL;
+
+	return FALSE;
+}
 
 static void confirm_cb(GIOChannel *io, gpointer user_data)
 {
@@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
 	bcast_src->pa_sync_io = io;
 	g_io_channel_ref(bcast_src->pa_sync_io);
 
+	if (bcast_src->pa_sync_io_id > 0) {
+		g_source_remove(bcast_src->pa_sync_io_id);
+		bcast_src->pa_sync_io_id  = 0;
+	}
+
+	bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
+				G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
+				bcast_src);
+
 	len = sizeof(qos);
 	memset(&qos, 0, len);
 
@@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *value, size_t len)
 	return true;
 }
 
+static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
+			gpointer data)
+{
+	struct bt_bcast_src *bcast_src = data;
+
+	DBG(bcast_src->bass, "Listen io has been disconnected");
+
+	bcast_src->listen_io_id = 0;
+	bcast_src->listen_io = NULL;
+
+	return FALSE;
+}
+
 static void bass_handle_add_src_op(struct bt_bass *bass,
 					struct gatt_db_attribute *attrib,
 					uint8_t opcode,
@@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
 		bcast_src->listen_io = io;
 		g_io_channel_ref(bcast_src->listen_io);
 
+		bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
+					G_IO_HUP | G_IO_NVAL,
+					(GIOFunc)listen_io_disconnect_cb,
+					bcast_src);
+
 		if (num_bis > 0 && !bcast_src->bises)
 			bcast_src->bises = queue_new();
 	} else {
@@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
 		g_io_channel_unref(bcast_src->listen_io);
 	}
 
+	if (bcast_src->listen_io_id > 0) {
+		g_source_remove(bcast_src->listen_io_id);
+		bcast_src->listen_io_id  = 0;
+	}
+
 	if (bcast_src->pa_sync_io) {
 		g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
 		g_io_channel_unref(bcast_src->pa_sync_io);
 	}
 
+	if (bcast_src->pa_sync_io_id > 0) {
+		g_source_remove(bcast_src->pa_sync_io_id);
+		bcast_src->pa_sync_io_id  = 0;
+	}
+
 	queue_destroy(bcast_src->bises, bass_bis_unref);
 
 	free(bcast_src);
diff --git a/src/shared/bass.h b/src/shared/bass.h
index c4b5b76ba..bd3fe900b 100644
--- a/src/shared/bass.h
+++ b/src/shared/bass.h
@@ -57,7 +57,9 @@ struct bt_bcast_src {
 	uint8_t num_subgroups;
 	struct bt_bass_subgroup_data *subgroup_data;
 	GIOChannel *listen_io;
+	guint listen_io_id;
 	GIOChannel *pa_sync_io;
+	guint pa_sync_io_id;
 	struct queue *bises;
 };
 
-- 
2.39.2

[PATCH BlueZ 3/4] btio: Allow binding a bcast listener before accept

[Iulia Tanasescu]

This adds btio support for binding a PA sync io to a number of BISes,
before proceeding with BIG Create Sync

---
 btio/btio.c          | 51 +++++++++++++++++++++++++++++++++++++++++++-
 btio/btio.h          |  2 +-
 profiles/audio/bap.c |  2 +-
 src/shared/bass.c    |  9 ++++----
 4 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/btio/btio.c b/btio/btio.c
index c63a6d1df..338f24916 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -1797,14 +1797,63 @@ gboolean bt_io_accept(GIOChannel *io, BtIOConnect connect, gpointer user_data,
 
 gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
 				gpointer user_data, GDestroyNotify destroy,
-				GError * *err)
+				GError * *err, BtIOOption opt1, ...)
 {
 	int sock;
 	char c;
 	struct pollfd pfd;
+	va_list args;
+	struct sockaddr_iso *addr = NULL;
+	uint8_t bc_num_bis = 0;
+	uint8_t bc_bis[ISO_MAX_NUM_BIS] = {0};
+	BtIOOption opt = opt1;
+
+	va_start(args, opt1);
+
+	while (opt != BT_IO_OPT_INVALID) {
+		if (opt == BT_IO_OPT_ISO_BC_NUM_BIS)  {
+			bc_num_bis = va_arg(args, int);
+		} else if (opt == BT_IO_OPT_ISO_BC_BIS) {
+			memcpy(bc_bis, va_arg(args, uint8_t *),
+					bc_num_bis);
+		} else {
+			g_set_error(err, BT_IO_ERROR, EINVAL,
+					"Invalid option %d", opt);
+			break;
+		}
+
+		opt = va_arg(args, int);
+	}
+
+	va_end(args);
+
+	if (*err)
+		return FALSE;
 
 	sock = g_io_channel_unix_get_fd(io);
 
+	if (bc_num_bis) {
+		addr = malloc(sizeof(*addr) + sizeof(*addr->iso_bc));
+
+		if (!addr) {
+			ERROR_FAILED(err, "poll", ENOMEM);
+			return FALSE;
+		}
+
+		memset(addr, 0, sizeof(*addr) + sizeof(*addr->iso_bc));
+		addr->iso_family = AF_BLUETOOTH;
+
+		addr->iso_bc->bc_num_bis = bc_num_bis;
+		memcpy(addr->iso_bc->bc_bis, bc_bis,
+			addr->iso_bc->bc_num_bis);
+
+		if (bind(sock, (struct sockaddr *)addr,
+			sizeof(*addr) + sizeof(*addr->iso_bc)) < 0) {
+			ERROR_FAILED(err, "bind", errno);
+			return FALSE;
+		}
+	}
+
 	memset(&pfd, 0, sizeof(pfd));
 	pfd.fd = sock;
 	pfd.events = POLLOUT;
diff --git a/btio/btio.h b/btio/btio.h
index 3169bebf3..3e69092b1 100644
--- a/btio/btio.h
+++ b/btio/btio.h
@@ -77,7 +77,7 @@ gboolean bt_io_accept(GIOChannel *io, BtIOConnect connect, gpointer user_data,
 
 gboolean bt_io_bcast_accept(GIOChannel *io, BtIOConnect connect,
 				gpointer user_data, GDestroyNotify destroy,
-				GError **err);
+				GError **err, BtIOOption opt1, ...);
 
 gboolean bt_io_set(GIOChannel *io, GError **err, BtIOOption opt1, ...);
 
diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index fa5cf1f54..b74498c4c 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -981,7 +981,7 @@ static void iso_pa_sync_confirm_cb(GIOChannel *io, void *user_data)
 	GError *err = NULL;
 
 	if (!bt_io_bcast_accept(io, iso_bcast_confirm_cb,
-				user_data, NULL, &err)) {
+				user_data, NULL, &err, BT_IO_OPT_INVALID)) {
 		error("bt_io_bcast_accept: %s", err->message);
 		g_error_free(err);
 		g_io_channel_shutdown(io, TRUE, NULL);
diff --git a/src/shared/bass.c b/src/shared/bass.c
index 78103d463..326ce6dae 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -774,8 +774,9 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
 
 		if (bass_trigger_big_sync(bcast_src)) {
 			if (!bt_io_bcast_accept(bcast_src->pa_sync_io,
-				connect_cb, bcast_src, NULL, &gerr)) {
-				DBG(bcast_src->bass, "bt_io_accept: %s",
+				connect_cb, bcast_src, NULL, &gerr,
+				BT_IO_OPT_INVALID)) {
+				DBG(bcast_src->bass, "bt_io_bcast_accept: %s",
 				gerr->message);
 				g_error_free(gerr);
 			}
@@ -1178,8 +1179,8 @@ static void bass_handle_set_bcast_code_op(struct bt_bass *bass,
 	}
 
 	if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
-		bcast_src, NULL, &gerr)) {
-		DBG(bcast_src->bass, "bt_io_accept: %s", gerr->message);
+		bcast_src, NULL, &gerr,  BT_IO_OPT_INVALID)) {
+		DBG(bcast_src->bass, "bt_io_bcast_accept: %s", gerr->message);
 		g_error_free(gerr);
 	}
 }
-- 
2.39.2

[PATCH BlueZ 4/4] shared/bass: Add Modify Source initial implementation

[Iulia Tanasescu]

This adds an initial implementation for the BASS Modify Source
procedure

---
 src/shared/bass.c | 289 +++++++++++++++++++++++++++++++++++++++++-----
 src/shared/bass.h |   2 +
 2 files changed, 263 insertions(+), 28 deletions(-)

diff --git a/src/shared/bass.c b/src/shared/bass.c
index 326ce6dae..5765ef6ed 100644
--- a/src/shared/bass.c
+++ b/src/shared/bass.c
@@ -614,6 +614,9 @@ static void connect_cb(GIOChannel *io, GError *gerr,
 	g_io_channel_ref(io);
 	queue_push_tail(bcast_src->bises, io);
 
+	if (!bcast_src->bises)
+		bcast_src->bises = queue_new();
+
 	for (i = 0; i < bcast_src->num_subgroups; i++) {
 		struct bt_bass_subgroup_data *data =
 				&bcast_src->subgroup_data[i];
@@ -650,29 +653,6 @@ static void connect_cb(GIOChannel *io, GError *gerr,
 		queue_destroy(bcast_src->bises, bass_bis_unref);
 		bcast_src->bises = NULL;
 
-		/* Close listen io */
-		g_io_channel_shutdown(bcast_src->listen_io, TRUE, NULL);
-		g_io_channel_unref(bcast_src->listen_io);
-		bcast_src->listen_io = NULL;
-
-		if (bcast_src->listen_io_id > 0) {
-			g_source_remove(bcast_src->listen_io_id);
-			bcast_src->listen_io_id  = 0;
-		}
-
-		/* Close pa sync io */
-		if (bcast_src->pa_sync_io) {
-			g_io_channel_shutdown(bcast_src->pa_sync_io,
-					TRUE, NULL);
-			g_io_channel_unref(bcast_src->pa_sync_io);
-			bcast_src->pa_sync_io = NULL;
-		}
-
-		if (bcast_src->pa_sync_io_id > 0) {
-			g_source_remove(bcast_src->pa_sync_io_id);
-			bcast_src->pa_sync_io_id  = 0;
-		}
-
 		for (i = 0; i < bcast_src->num_subgroups; i++)
 			bcast_src->subgroup_data[i].bis_sync =
 				BT_BASS_BIG_SYNC_FAILED_BITMASK;
@@ -741,6 +721,17 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
 
 		/* Mark PA sync as failed and notify client */
 		bcast_src->sync_state = BT_BASS_FAILED_TO_SYNCHRONIZE_TO_PA;
+
+		/* Close listen io */
+		g_io_channel_shutdown(bcast_src->listen_io, TRUE, NULL);
+		g_io_channel_unref(bcast_src->listen_io);
+		bcast_src->listen_io = NULL;
+
+		if (bcast_src->listen_io_id > 0) {
+			g_source_remove(bcast_src->listen_io_id);
+			bcast_src->listen_io_id  = 0;
+		}
+
 		goto notify;
 	}
 
@@ -1037,7 +1028,7 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
 					data->meta_len), data->meta_len);
 	}
 
-	if (pa_sync != PA_SYNC_NO_SYNC) {
+	if (*pa_sync != PA_SYNC_NO_SYNC) {
 		/* Convert to three-value type */
 		if (bcast_src->addr_type)
 			addr_type = BDADDR_LE_RANDOM;
@@ -1072,9 +1063,6 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
 					G_IO_HUP | G_IO_NVAL,
 					(GIOFunc)listen_io_disconnect_cb,
 					bcast_src);
-
-		if (num_bis > 0 && !bcast_src->bises)
-			bcast_src->bises = queue_new();
 	} else {
 		for (int i = 0; i < bcast_src->num_subgroups; i++)
 			bcast_src->subgroup_data[i].bis_sync =
@@ -1179,12 +1167,255 @@ static void bass_handle_set_bcast_code_op(struct bt_bass *bass,
 	}
 
 	if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
-		bcast_src, NULL, &gerr,  BT_IO_OPT_INVALID)) {
+		bcast_src, NULL, &gerr, BT_IO_OPT_INVALID)) {
 		DBG(bcast_src->bass, "bt_io_bcast_accept: %s", gerr->message);
 		g_error_free(gerr);
 	}
 }
 
+static bool bass_validate_mod_src_params(struct bt_bass_mod_src_params *params,
+							struct iovec *iov)
+{
+	struct iovec subgroup_data = {
+		.iov_base = iov->iov_base,
+		.iov_len = iov->iov_len,
+	};
+
+	if (params->pa_sync > PA_SYNC_NO_PAST)
+		return false;
+
+	if (!bass_validate_bis_sync(params->num_subgroups,
+					&subgroup_data))
+		return false;
+
+	return true;
+}
+
+static void bass_handle_mod_src_op(struct bt_bass *bass,
+					struct gatt_db_attribute *attrib,
+					uint8_t opcode,
+					unsigned int id,
+					struct iovec *iov,
+					struct bt_att *att)
+{
+	struct bt_bass_mod_src_params *params;
+	struct bt_bcast_src *bcast_src;
+	uint8_t num_bis = 0;
+	uint8_t bis[ISO_MAX_NUM_BIS];
+	uint8_t addr_type;
+	GIOChannel *io;
+	GError *err = NULL;
+	struct bt_iso_qos iso_qos = default_qos;
+	struct bt_bass_subgroup_data *subgroup_data = NULL;
+	uint8_t *notify_data;
+	size_t notify_data_len;
+	bool changed = false;
+
+	/* Get Modify Source command parameters */
+	params = util_iov_pull_mem(iov, sizeof(*params));
+
+	bcast_src = queue_find(bass->ldb->bcast_srcs,
+						bass_src_id_match,
+						&params->id);
+
+	if (!bcast_src) {
+		/* No source matches the written source id */
+		gatt_db_attribute_write_result(attrib, id,
+					BT_BASS_ERROR_INVALID_SOURCE_ID);
+
+		return;
+	}
+
+	gatt_db_attribute_write_result(attrib, id, 0x00);
+
+	/* Ignore operation if parameters are invalid */
+	if (!bass_validate_mod_src_params(params, iov))
+		return;
+
+	/* Only subgroup metadata can change */
+	if (params->num_subgroups != bcast_src->num_subgroups)
+		return;
+
+	/* Extract subgroup information */
+	subgroup_data = malloc(params->num_subgroups * sizeof(*subgroup_data));
+	if (!subgroup_data)
+		return;
+
+	memset(subgroup_data, 0, params->num_subgroups *
+		sizeof(*subgroup_data));
+
+	for (int i = 0; i < params->num_subgroups; i++) {
+		struct bt_bass_subgroup_data *data = &subgroup_data[i];
+		struct bt_bass_subgroup_data *old_data =
+				&bcast_src->subgroup_data[i];
+
+		util_iov_pull_le32(iov, &data->pending_bis_sync);
+
+		if (old_data->pending_bis_sync)
+			goto err;
+
+		if (old_data->bis_sync && data->pending_bis_sync &&
+			data->pending_bis_sync != old_data->bis_sync)
+			/* The Server cannot resync to different BIS indxes */
+			goto err;
+
+		if (data->pending_bis_sync == BIS_SYNC_NO_PREF)
+			data->pending_bis_sync = DEFAULT_BIS_SYNC_BITMASK;
+
+		if (data->pending_bis_sync != 0) {
+			/* Client instructs Server to sync to some BISes */
+			for (int bis_idx = 0; bis_idx < 31; bis_idx++) {
+				if (data->pending_bis_sync & (1 << bis_idx)) {
+					bis[num_bis] = bis_idx + 1;
+					num_bis++;
+				}
+			}
+		}
+
+		data->meta_len = *(uint8_t *)util_iov_pull_mem(iov,
+						sizeof(data->meta_len));
+		if (!data->meta_len)
+			continue;
+
+		data->meta = malloc(data->meta_len);
+		if (!data->meta)
+			goto err;
+
+		memcpy(data->meta, (uint8_t *)util_iov_pull_mem(iov,
+					data->meta_len), data->meta_len);
+
+		if (old_data->meta_len != data->meta_len ||
+			memcmp(old_data->meta, data->meta, old_data->meta_len))
+			changed = true;
+	}
+
+	if (bcast_src->sync_state == BT_BASS_NOT_SYNCHRONIZED_TO_PA &&
+		bcast_src->listen_io)
+		return;
+
+	/* If subgroup information has been extracted successfully,
+	 * update broadcast source
+	 */
+	if (bcast_src->subgroup_data) {
+		for (int i = 0; i < bcast_src->num_subgroups; i++)
+			free(bcast_src->subgroup_data[i].meta);
+
+		free(bcast_src->subgroup_data);
+	}
+
+	bcast_src->subgroup_data = subgroup_data;
+
+	/* Client instructs Server to sync to PA */
+	if (bcast_src->sync_state == BT_BASS_NOT_SYNCHRONIZED_TO_PA) {
+		if (params->pa_sync == PA_SYNC_NO_PAST) {
+			/* Convert to three-value type */
+			if (bcast_src->addr_type)
+				addr_type = BDADDR_LE_RANDOM;
+			else
+				addr_type = BDADDR_LE_PUBLIC;
+
+			/* Try to synchronize to the source */
+			io = bt_io_listen(NULL, confirm_cb, bcast_src,
+						NULL, &err,
+						BT_IO_OPT_SOURCE_BDADDR,
+						&bass->ldb->adapter_bdaddr,
+						BT_IO_OPT_DEST_BDADDR,
+						&bcast_src->addr,
+						BT_IO_OPT_DEST_TYPE,
+						addr_type,
+						BT_IO_OPT_MODE, BT_IO_MODE_ISO,
+						BT_IO_OPT_QOS, &iso_qos,
+						BT_IO_OPT_ISO_BC_SID,
+						bcast_src->sid,
+						BT_IO_OPT_ISO_BC_NUM_BIS,
+						num_bis,
+						BT_IO_OPT_ISO_BC_BIS, bis,
+						BT_IO_OPT_INVALID);
+
+			if (!io) {
+				DBG(bass, "%s", err->message);
+				g_error_free(err);
+				return;
+			}
+
+			bcast_src->listen_io = io;
+			g_io_channel_ref(bcast_src->listen_io);
+
+			if (bcast_src->listen_io_id > 0) {
+				g_source_remove(bcast_src->listen_io_id);
+				bcast_src->listen_io_id  = 0;
+			}
+
+			bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
+					G_IO_HUP | G_IO_NVAL,
+					(GIOFunc)listen_io_disconnect_cb,
+					bcast_src);
+		}
+
+		if (changed)
+			goto notify;
+
+		return;
+	}
+
+	/* Client instructs Server to stop PA sync */
+	if (params->pa_sync == PA_SYNC_NO_SYNC) {
+		/* TODO */
+		return;
+	}
+
+	/* Server is already synchronized to PA, and Client now
+	 * instructs the Server to also sync to some BISes
+	 */
+	if (num_bis && !queue_length(bcast_src->bises)) {
+		/* Since the Server has never been synced to any BIS before,
+		 * the PA sync socket must be bound to the num_bis to sync with
+		 */
+		if (!bt_io_bcast_accept(bcast_src->pa_sync_io, connect_cb,
+					bcast_src, NULL, &err,
+					BT_IO_OPT_ISO_BC_NUM_BIS, num_bis,
+					BT_IO_OPT_ISO_BC_BIS, bis,
+					BT_IO_OPT_INVALID)) {
+			DBG(bcast_src->bass, "bt_io_bcast_accept: %s",
+				err->message);
+			g_error_free(err);
+		}
+
+		return;
+	}
+
+	/* Client instructs Server to keep PA sync but stop BIG sync */
+	if (!num_bis && queue_length(bcast_src->bises)) {
+		/* TODO */
+		return;
+	}
+
+	/* If no change has occurred, just return */
+	if (!changed)
+		return;
+
+notify:
+	notify_data = bass_build_notif_from_bcast_src(bcast_src,
+						&notify_data_len);
+
+	gatt_db_attribute_notify(bcast_src->attr,
+			(void *)notify_data,
+			notify_data_len,
+			bt_bass_get_att(bcast_src->bass));
+
+	free(notify_data);
+
+	return;
+
+err:
+	if (subgroup_data) {
+		for (int i = 0; i < params->num_subgroups; i++)
+			free(subgroup_data[i].meta);
+
+		free(subgroup_data);
+	}
+}
+
 #define BASS_OP(_str, _op, _size, _func) \
 	{ \
 		.str = _str, \
@@ -1214,6 +1445,8 @@ struct bass_op_handler {
 		0, bass_handle_add_src_op),
 	BASS_OP("Set Broadcast Code", BT_BASS_SET_BCAST_CODE,
 		0, bass_handle_set_bcast_code_op),
+	BASS_OP("Modify Source", BT_BASS_MOD_SRC,
+		0, bass_handle_mod_src_op),
 	{}
 };
 
diff --git a/src/shared/bass.h b/src/shared/bass.h
index bd3fe900b..ee958bf7a 100644
--- a/src/shared/bass.h
+++ b/src/shared/bass.h
@@ -32,6 +32,8 @@ struct bt_bass;
 #define BT_BASS_BIG_ENC_STATE_DEC			0x02
 #define BT_BASS_BIG_ENC_STATE_BAD_CODE			0x03
 
+#define DEFAULT_BIS_SYNC_BITMASK			0x00000001
+
 /* BASS subgroup field of the Broadcast
  * Receive State characteristic
  */
-- 
2.39.2

Re: [PATCH BlueZ 1/4] btio: Handle closed channel in server_cb

[Luiz Augusto von Dentz]

Hi Iulia,

On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>
> This handles G_IO_ERR and G_IO_HUP conditions in server_cb
>
> ---
>  btio/btio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index d45b8240d..c63a6d1df 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io, GIOCondition cond,
>         GIOChannel *cli_io;
>
>         /* If the user closed the server */
> -       if ((cond & G_IO_NVAL) || check_nval(io))
> +       if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> +                       check_nval(io))

I believe this was done on purpose to only check for NVAL, the other
conditions shall probably be notified back via callback so it can
process the error reported.

>                 return FALSE;
>
>         srv_sock = g_io_channel_unix_get_fd(io);
> --
> 2.39.2
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io channels

[Luiz Augusto von Dentz]

Hi Iulia,

On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>
> This adds watches to handle closed sockets
>
> ---
>  src/shared/bass.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/shared/bass.h |  2 ++
>  2 files changed, 61 insertions(+)
>
> diff --git a/src/shared/bass.c b/src/shared/bass.c
> index 0ee3187d1..78103d463 100644
> --- a/src/shared/bass.c
> +++ b/src/shared/bass.c
> @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                 g_io_channel_unref(bcast_src->listen_io);
>                 bcast_src->listen_io = NULL;
>
> +               if (bcast_src->listen_io_id > 0) {
> +                       g_source_remove(bcast_src->listen_io_id);
> +                       bcast_src->listen_io_id  = 0;
> +               }
> +
>                 /* Close pa sync io */
>                 if (bcast_src->pa_sync_io) {
>                         g_io_channel_shutdown(bcast_src->pa_sync_io,
> @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError *gerr,
>                         bcast_src->pa_sync_io = NULL;
>                 }
>
> +               if (bcast_src->pa_sync_io_id > 0) {
> +                       g_source_remove(bcast_src->pa_sync_io_id);
> +                       bcast_src->pa_sync_io_id  = 0;
> +               }
> +
>                 for (i = 0; i < bcast_src->num_subgroups; i++)
>                         bcast_src->subgroup_data[i].bis_sync =
>                                 BT_BASS_BIG_SYNC_FAILED_BITMASK;
> @@ -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src *bcast_src)
>         return false;
>  }
>
> +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src = data;
> +
> +       DBG(bcast_src->bass, "PA sync io has been disconnected");
> +
> +       bcast_src->pa_sync_io_id = 0;
> +       bcast_src->pa_sync_io = NULL;
> +
> +       return FALSE;
> +}
>
>  static void confirm_cb(GIOChannel *io, gpointer user_data)
>  {
> @@ -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer user_data)
>         bcast_src->pa_sync_io = io;
>         g_io_channel_ref(bcast_src->pa_sync_io);
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  = 0;
> +       }
> +
> +       bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR | G_IO_HUP |
> +                               G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> +                               bcast_src);
> +
>         len = sizeof(qos);
>         memset(&qos, 0, len);
>
> @@ -844,6 +875,19 @@ static bool bass_validate_add_src_params(uint8_t *value, size_t len)
>         return true;
>  }
>
> +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition cond,
> +                       gpointer data)
> +{
> +       struct bt_bcast_src *bcast_src = data;
> +
> +       DBG(bcast_src->bass, "Listen io has been disconnected");
> +
> +       bcast_src->listen_io_id = 0;
> +       bcast_src->listen_io = NULL;
> +
> +       return FALSE;
> +}
> +
>  static void bass_handle_add_src_op(struct bt_bass *bass,
>                                         struct gatt_db_attribute *attrib,
>                                         uint8_t opcode,
> @@ -1023,6 +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
>                 bcast_src->listen_io = io;
>                 g_io_channel_ref(bcast_src->listen_io);
>
> +               bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> +                                       G_IO_HUP | G_IO_NVAL,
> +                                       (GIOFunc)listen_io_disconnect_cb,
> +                                       bcast_src);

IO handling shall probably be kept outside of shared so we could
emulate it with use of socketpairs and unit test it, see how it was
done for bap.

>                 if (num_bis > 0 && !bcast_src->bises)
>                         bcast_src->bises = queue_new();
>         } else {
> @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
>                 g_io_channel_unref(bcast_src->listen_io);
>         }
>
> +       if (bcast_src->listen_io_id > 0) {
> +               g_source_remove(bcast_src->listen_io_id);
> +               bcast_src->listen_io_id  = 0;
> +       }
> +
>         if (bcast_src->pa_sync_io) {
>                 g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
>                 g_io_channel_unref(bcast_src->pa_sync_io);
>         }
>
> +       if (bcast_src->pa_sync_io_id > 0) {
> +               g_source_remove(bcast_src->pa_sync_io_id);
> +               bcast_src->pa_sync_io_id  = 0;
> +       }
> +
>         queue_destroy(bcast_src->bises, bass_bis_unref);
>
>         free(bcast_src);
> diff --git a/src/shared/bass.h b/src/shared/bass.h
> index c4b5b76ba..bd3fe900b 100644
> --- a/src/shared/bass.h
> +++ b/src/shared/bass.h
> @@ -57,7 +57,9 @@ struct bt_bcast_src {
>         uint8_t num_subgroups;
>         struct bt_bass_subgroup_data *subgroup_data;
>         GIOChannel *listen_io;
> +       guint listen_io_id;
>         GIOChannel *pa_sync_io;
> +       guint pa_sync_io_id;
>         struct queue *bises;
>  };
>
> --
> 2.39.2
>


-- 
Luiz Augusto von Dentz

RE: Add Modify Source initial implementation

[bluez.test.bot]

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

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=793607

---Test result---

Test Summary:
CheckPatch                    PASS      2.06 seconds
GitLint                       PASS      1.10 seconds
BuildEll                      PASS      27.73 seconds
BluezMake                     PASS      798.70 seconds
MakeCheck                     PASS      12.22 seconds
MakeDistcheck                 PASS      175.13 seconds
CheckValgrind                 PASS      267.39 seconds
CheckSmatch                   PASS      360.77 seconds
bluezmakeextell               PASS      115.59 seconds
IncrementalBuild              PASS      2814.91 seconds
ScanBuild                     WARNING   1102.61 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
btio/btio.c:1852:4: warning: Potential leak of memory pointed to by 'addr'
                        ERROR_FAILED(err, "bind", errno);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
btio/btio.c:41:3: note: expanded from macro 'ERROR_FAILED'
                g_set_error(gerr, BT_IO_ERROR, err, \
                ^~~~~~~~~~~
1 warning generated.
src/shared/bass.c:1294:3: warning: Potential leak of memory pointed to by 'subgroup_data'
                return;
                ^~~~~~
1 warning generated.



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/4] btio: Handle closed channel in server_cb

[Iulia Tanasescu]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Monday, October 16, 2023 8:01 PM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>;
> Andrei Istodorescu <andrei.istodorescu@nxp.com>
> Subject: Re: [PATCH BlueZ 1/4] btio: Handle closed channel in server_cb
> 
> Hi Iulia,
> 
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com>
> wrote:
> >
> > This handles G_IO_ERR and G_IO_HUP conditions in server_cb
> >
> > ---
> >  btio/btio.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/btio/btio.c b/btio/btio.c index d45b8240d..c63a6d1df
> > 100644
> > --- a/btio/btio.c
> > +++ b/btio/btio.c
> > @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io,
> GIOCondition cond,
> >         GIOChannel *cli_io;
> >
> >         /* If the user closed the server */
> > -       if ((cond & G_IO_NVAL) || check_nval(io))
> > +       if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> > +                       check_nval(io))
> 
> I believe this was done on purpose to only check for NVAL, the other
> conditions shall probably be notified back via callback so it can process the
> error reported.
> 

It seems that server_cb is supposed to accept an incoming connection on
server io and pass the child io through the confirm or connect callbacks.
But if the condition is G_IO_HUP for example, we shouldn't get to the point
of calling accept.

> >                 return FALSE;
> >
> >         srv_sock = g_io_channel_unix_get_fd(io);
> > --
> > 2.39.2
> >
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Iulia

Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io channels

[Iulia Tanasescu]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Monday, October 16, 2023 8:05 PM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>;
> Andrei Istodorescu <andrei.istodorescu@nxp.com>
> Subject: Re: [PATCH BlueZ 2/4] shared/bass: Handle G_IO_HUP on io
> channels
> 
> Hi Iulia,
> 
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com>
> wrote:
> >
> > This adds watches to handle closed sockets
> >
> > ---
> >  src/shared/bass.c | 59
> > +++++++++++++++++++++++++++++++++++++++++++++++
> >  src/shared/bass.h |  2 ++
> >  2 files changed, 61 insertions(+)
> >
> > diff --git a/src/shared/bass.c b/src/shared/bass.c index
> > 0ee3187d1..78103d463 100644
> > --- a/src/shared/bass.c
> > +++ b/src/shared/bass.c
> > @@ -655,6 +655,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> >                 g_io_channel_unref(bcast_src->listen_io);
> >                 bcast_src->listen_io = NULL;
> >
> > +               if (bcast_src->listen_io_id > 0) {
> > +                       g_source_remove(bcast_src->listen_io_id);
> > +                       bcast_src->listen_io_id  = 0;
> > +               }
> > +
> >                 /* Close pa sync io */
> >                 if (bcast_src->pa_sync_io) {
> >                         g_io_channel_shutdown(bcast_src->pa_sync_io,
> > @@ -663,6 +668,11 @@ static void connect_cb(GIOChannel *io, GError
> *gerr,
> >                         bcast_src->pa_sync_io = NULL;
> >                 }
> >
> > +               if (bcast_src->pa_sync_io_id > 0) {
> > +                       g_source_remove(bcast_src->pa_sync_io_id);
> > +                       bcast_src->pa_sync_io_id  = 0;
> > +               }
> > +
> >                 for (i = 0; i < bcast_src->num_subgroups; i++)
> >                         bcast_src->subgroup_data[i].bis_sync =
> >                                 BT_BASS_BIG_SYNC_FAILED_BITMASK; @@
> > -703,6 +713,18 @@ static bool bass_trigger_big_sync(struct bt_bcast_src
> *bcast_src)
> >         return false;
> >  }
> >
> > +static gboolean pa_sync_io_disconnect_cb(GIOChannel *io,
> GIOCondition cond,
> > +                       gpointer data) {
> > +       struct bt_bcast_src *bcast_src = data;
> > +
> > +       DBG(bcast_src->bass, "PA sync io has been disconnected");
> > +
> > +       bcast_src->pa_sync_io_id = 0;
> > +       bcast_src->pa_sync_io = NULL;
> > +
> > +       return FALSE;
> > +}
> >
> >  static void confirm_cb(GIOChannel *io, gpointer user_data)  { @@
> > -726,6 +748,15 @@ static void confirm_cb(GIOChannel *io, gpointer
> user_data)
> >         bcast_src->pa_sync_io = io;
> >         g_io_channel_ref(bcast_src->pa_sync_io);
> >
> > +       if (bcast_src->pa_sync_io_id > 0) {
> > +               g_source_remove(bcast_src->pa_sync_io_id);
> > +               bcast_src->pa_sync_io_id  = 0;
> > +       }
> > +
> > +       bcast_src->pa_sync_io_id = g_io_add_watch(io, G_IO_ERR |
> G_IO_HUP |
> > +                               G_IO_NVAL, (GIOFunc) pa_sync_io_disconnect_cb,
> > +                               bcast_src);
> > +
> >         len = sizeof(qos);
> >         memset(&qos, 0, len);
> >
> > @@ -844,6 +875,19 @@ static bool
> bass_validate_add_src_params(uint8_t *value, size_t len)
> >         return true;
> >  }
> >
> > +static gboolean listen_io_disconnect_cb(GIOChannel *io, GIOCondition
> cond,
> > +                       gpointer data) {
> > +       struct bt_bcast_src *bcast_src = data;
> > +
> > +       DBG(bcast_src->bass, "Listen io has been disconnected");
> > +
> > +       bcast_src->listen_io_id = 0;
> > +       bcast_src->listen_io = NULL;
> > +
> > +       return FALSE;
> > +}
> > +
> >  static void bass_handle_add_src_op(struct bt_bass *bass,
> >                                         struct gatt_db_attribute *attrib,
> >                                         uint8_t opcode, @@ -1023,6
> > +1067,11 @@ static void bass_handle_add_src_op(struct bt_bass *bass,
> >                 bcast_src->listen_io = io;
> >                 g_io_channel_ref(bcast_src->listen_io);
> >
> > +               bcast_src->listen_io_id = g_io_add_watch(io, G_IO_ERR |
> > +                                       G_IO_HUP | G_IO_NVAL,
> > +                                       (GIOFunc)listen_io_disconnect_cb,
> > +                                       bcast_src);
> 
> IO handling shall probably be kept outside of shared so we could emulate it
> with use of socketpairs and unit test it, see how it was done for bap.
> 

I submitted a patch to move all IO handling from src/shared to
profiles/audio, similar to bap.

> >                 if (num_bis > 0 && !bcast_src->bises)
> >                         bcast_src->bises = queue_new();
> >         } else {
> > @@ -1318,11 +1367,21 @@ static void bass_bcast_src_free(void *data)
> >                 g_io_channel_unref(bcast_src->listen_io);
> >         }
> >
> > +       if (bcast_src->listen_io_id > 0) {
> > +               g_source_remove(bcast_src->listen_io_id);
> > +               bcast_src->listen_io_id  = 0;
> > +       }
> > +
> >         if (bcast_src->pa_sync_io) {
> >                 g_io_channel_shutdown(bcast_src->pa_sync_io, TRUE, NULL);
> >                 g_io_channel_unref(bcast_src->pa_sync_io);
> >         }
> >
> > +       if (bcast_src->pa_sync_io_id > 0) {
> > +               g_source_remove(bcast_src->pa_sync_io_id);
> > +               bcast_src->pa_sync_io_id  = 0;
> > +       }
> > +
> >         queue_destroy(bcast_src->bises, bass_bis_unref);
> >
> >         free(bcast_src);
> > diff --git a/src/shared/bass.h b/src/shared/bass.h index
> > c4b5b76ba..bd3fe900b 100644
> > --- a/src/shared/bass.h
> > +++ b/src/shared/bass.h
> > @@ -57,7 +57,9 @@ struct bt_bcast_src {
> >         uint8_t num_subgroups;
> >         struct bt_bass_subgroup_data *subgroup_data;
> >         GIOChannel *listen_io;
> > +       guint listen_io_id;
> >         GIOChannel *pa_sync_io;
> > +       guint pa_sync_io_id;
> >         struct queue *bises;
> >  };
> >
> > --
> > 2.39.2
> >
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Iulia