diff mbox series

[BlueZ,v4,4/6] shared/bap: Make bt_bap_select select a location

Message ID 20231211212516.577426-4-luiz.dentz@gmail.com
State Superseded
Headers show
Series [BlueZ,v4,1/6] bap: Allow setup of multiple stream per endpoint | expand

Commit Message

Luiz Augusto von Dentz Dec. 11, 2023, 9:25 p.m. UTC
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This makes bt_bap_select select a location based on the PAC channel
count and PACS locations, this is then passed to the Endpoint as a
recommended ChannelAllocation.
---
 profiles/audio/media.c |  6 ++-
 src/shared/bap.c       | 88 +++++++++++++++++++++++++++---------------
 src/shared/bap.h       |  2 +-
 3 files changed, 63 insertions(+), 33 deletions(-)

Comments

Pauli Virtanen Dec. 14, 2023, 4:55 p.m. UTC | #1
Hi,

ma, 2023-12-11 kello 16:25 -0500, Luiz Augusto von Dentz kirjoitti:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > 
> > This makes bt_bap_select select a location based on the PAC channel
> > count and PACS locations, this is then passed to the Endpoint as a
> > recommended ChannelAllocation.
> > ---
> >  profiles/audio/media.c |  6 ++-
> >  src/shared/bap.c       | 88 +++++++++++++++++++++++++++---------------
> >  src/shared/bap.h       |  2 +-
> >  3 files changed, 63 insertions(+), 33 deletions(-)
> > 
> > diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> > index 62f53defa7af..b17c555b63e4 100644
> > --- a/profiles/audio/media.c
> > +++ b/profiles/audio/media.c
> > @@ -921,7 +921,7 @@ done:
> >  }
> >  
> >  static int pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> > -			struct bt_bap_pac_qos *qos,
> > +			uint32_t chan_alloc, struct bt_bap_pac_qos *qos,
> >  			bt_bap_pac_select_t cb, void *cb_data, void *user_data)
> >  {
> >  	struct media_endpoint *endpoint = user_data;
> > @@ -969,6 +969,10 @@ static int pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> >  		g_dbus_dict_append_entry(&dict, "Locations", DBUS_TYPE_UINT32,
> >  									&loc);
> >  
> > +	if (chan_alloc)
> > +		g_dbus_dict_append_entry(&dict, "ChannelAllocation",
> > +					 DBUS_TYPE_UINT32, &chan_alloc);
> > +
> >  	if (metadata) {
> >  		key = "Metadata";
> >  		g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
> > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > index cb505d1564d6..e1dad95aca99 100644
> > --- a/src/shared/bap.c
> > +++ b/src/shared/bap.c
> > @@ -177,6 +177,11 @@ struct bt_bap {
> >  	void *user_data;
> >  };
> >  
> > +struct bt_bap_chan {
> > +	uint8_t count;
> > +	uint32_t location;
> > +};
> > +
> >  struct bt_bap_pac {
> >  	struct bt_bap_db *bdb;
> >  	char *name;
> > @@ -185,7 +190,7 @@ struct bt_bap_pac {
> >  	struct bt_bap_pac_qos qos;
> >  	struct iovec *data;
> >  	struct iovec *metadata;
> > -	struct queue *chan_map;
> > +	struct queue *channels;
> >  	struct bt_bap_pac_ops *ops;
> >  	void *user_data;
> >  };
> > @@ -2422,19 +2427,22 @@ static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> >  					void *user_data)
> >  {
> >  	struct bt_bap_pac *pac = user_data;
> > +	struct bt_bap_chan *chan;
> >  
> >  	if (!v)
> >  		return;
> >  
> > -	if (!pac->chan_map)
> > -		pac->chan_map = queue_new();
> > +	if (!pac->channels)
> > +		pac->channels = queue_new();
> >  
> > -	printf("PAC %p chan_map 0x%02x\n", pac, *v);
> > +	chan = new0(struct bt_bap_chan, 1);
> > +	chan->count = *v;
> > +	chan->location = bt_bap_pac_get_locations(pac) ? : pac->qos.location;
> >  
> > -	queue_push_tail(pac->chan_map, UINT_TO_PTR(*v));
> > +	queue_push_tail(pac->channels, chan);
> >  }
> >  
> > -static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data)
> > +static void bap_pac_update_channels(struct bt_bap_pac *pac, struct iovec *data)
> >  {
> >  	uint8_t type = 0x03;
> >  
> > @@ -2454,8 +2462,8 @@ static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data,
> >  	else
> >  		pac->data = util_iov_dup(data, 1);
> >  
> > -	/* Update channel map */
> > -	bap_pac_update_chan_map(pac, data);
> > +	/* Update channels */
> > +	bap_pac_update_channels(pac, data);
> >  
> >  	/* Merge metadata into existing record */
> >  	if (pac->metadata)
> > @@ -2495,7 +2503,7 @@ static void bap_pac_free(void *data)
> >  	free(pac->name);
> >  	util_iov_free(pac->metadata, 1);
> >  	util_iov_free(pac->data, 1);
> > -	queue_destroy(pac->chan_map, NULL);
> > +	queue_destroy(pac->channels, free);
> >  	free(pac);
> >  }
> >  
> > @@ -4677,34 +4685,52 @@ int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> >  	if (!lpac->ops || !lpac->ops->select)
> >  		return -EOPNOTSUPP;
> >  
> > -	for (lchan = queue_get_entries(lpac->chan_map); lchan;
> > +	for (lchan = queue_get_entries(lpac->channels); lchan;
> >  					lchan = lchan->next) {
> > -		uint8_t lmap = PTR_TO_UINT(lchan->data);
> > +		struct bt_bap_chan *lc = lchan->data;
> > +		uint8_t lmap = lc->count;
> > +		int i;
> >  
> > -		for (rchan = queue_get_entries(rpac->chan_map); rchan;
> > -					rchan = rchan->next) {
> > -			uint8_t rmap = PTR_TO_UINT(rchan->data);
> > +		printf("lmap 0x%02x\n", lmap);
> >  
> > -			printf("lmap 0x%02x rmap 0x%02x\n", lmap, rmap);
> > +		for (i = 0, rchan = queue_get_entries(rpac->channels); rchan;
> > +					rchan = rchan->next, i++) {
> > +			struct bt_bap_chan *rc = rchan->data;
> >  
> > -			/* Try matching the channel mapping */
> > -			if (lmap & rmap) {
> > -				lpac->ops->select(lpac, rpac, &rpac->qos,
> > -							func, user_data,
> > -							lpac->user_data);
> > -				if (count)
> > -					(*count)++;
> > +			printf("rc->count 0x%02x\n", rc->count);
> >  
> > -				/* Check if there are any channels left */
> > -				lmap &= ~(lmap & rmap);
> > -				if (!lmap)
> > -					break;
> > -
> > -				/* Check if device require AC*(i) settings */
> > -				if (rmap == 0x01)
> > -					lmap = lmap >> 1;
> > -			} else
> > +			/* Try matching the channel count */
> > +			if (!(lmap & rc->count))
> >  				break;
> > +
> > +			/* Check if location was set otherwise attempt to
> > +			 * assign one based on the number of channels it
> > +			 * supports.
> > +			 */
> > +			if (!rc->location) {
> > +				rc->location = bt_bap_pac_get_locations(rpac);
> > +				/* If channel count is 1 use a single
> > +				 * location
> > +				 */
> > +				if (rc->count == 0x01)
> > +					rc->location &= BIT(i);
> > +			}
> > +
> > +			lpac->ops->select(lpac, rpac, lc->location &
> > +						rc->location, &rpac->qos,
> > +						func, user_data,
> > +						lpac->user_data);
> > +			if (count)
> > +				(*count)++;

If sound server PAC supports both 1 and 2 channels, and remote PAC only
support 1 channel, and two streams are set up: then it looks like sound
server would be called two times with same ChannelAllocation so it'd
likely set up two left channels.

This code perhaps should first decide the bitmask of locations that
will be set up, and then in each call to select() pass only the so far
unallocated location bits.

I'll try to find some time to set things up to test this in Pipewire,
where we need some changes to enable multiple streams per device but it
should be straightforward.

> > +
> > +			/* Check if there are any channels left to select */
> > +			lmap &= ~(lmap & rc->count);
> > +			if (!lmap)
> > +				break;
> > +
> > +			/* Check if device require AC*(i) settings */
> > +			if (rc->count == 0x01)
> > +				lmap = lmap >> 1;
> >  		}
> >  	}
> >  
> > diff --git a/src/shared/bap.h b/src/shared/bap.h
> > index 470313e66fc0..9be198cec72c 100644
> > --- a/src/shared/bap.h
> > +++ b/src/shared/bap.h
> > @@ -151,7 +151,7 @@ struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
> >  
> >  struct bt_bap_pac_ops {
> >  	int (*select)(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
> > -			struct bt_bap_pac_qos *qos,
> > +			uint32_t chan_alloc, struct bt_bap_pac_qos *qos,
> >  			bt_bap_pac_select_t cb, void *cb_data, void *user_data);
> >  	int (*config)(struct bt_bap_stream *stream, struct iovec *cfg,
> >  			struct bt_bap_qos *qos, bt_bap_pac_config_t cb,
diff mbox series

Patch

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 62f53defa7af..b17c555b63e4 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -921,7 +921,7 @@  done:
 }
 
 static int pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
-			struct bt_bap_pac_qos *qos,
+			uint32_t chan_alloc, struct bt_bap_pac_qos *qos,
 			bt_bap_pac_select_t cb, void *cb_data, void *user_data)
 {
 	struct media_endpoint *endpoint = user_data;
@@ -969,6 +969,10 @@  static int pac_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 		g_dbus_dict_append_entry(&dict, "Locations", DBUS_TYPE_UINT32,
 									&loc);
 
+	if (chan_alloc)
+		g_dbus_dict_append_entry(&dict, "ChannelAllocation",
+					 DBUS_TYPE_UINT32, &chan_alloc);
+
 	if (metadata) {
 		key = "Metadata";
 		g_dbus_dict_append_basic_array(&dict, DBUS_TYPE_STRING, &key,
diff --git a/src/shared/bap.c b/src/shared/bap.c
index cb505d1564d6..e1dad95aca99 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -177,6 +177,11 @@  struct bt_bap {
 	void *user_data;
 };
 
+struct bt_bap_chan {
+	uint8_t count;
+	uint32_t location;
+};
+
 struct bt_bap_pac {
 	struct bt_bap_db *bdb;
 	char *name;
@@ -185,7 +190,7 @@  struct bt_bap_pac {
 	struct bt_bap_pac_qos qos;
 	struct iovec *data;
 	struct iovec *metadata;
-	struct queue *chan_map;
+	struct queue *channels;
 	struct bt_bap_pac_ops *ops;
 	void *user_data;
 };
@@ -2422,19 +2427,22 @@  static void bap_pac_foreach_channel(size_t i, uint8_t l, uint8_t t, uint8_t *v,
 					void *user_data)
 {
 	struct bt_bap_pac *pac = user_data;
+	struct bt_bap_chan *chan;
 
 	if (!v)
 		return;
 
-	if (!pac->chan_map)
-		pac->chan_map = queue_new();
+	if (!pac->channels)
+		pac->channels = queue_new();
 
-	printf("PAC %p chan_map 0x%02x\n", pac, *v);
+	chan = new0(struct bt_bap_chan, 1);
+	chan->count = *v;
+	chan->location = bt_bap_pac_get_locations(pac) ? : pac->qos.location;
 
-	queue_push_tail(pac->chan_map, UINT_TO_PTR(*v));
+	queue_push_tail(pac->channels, chan);
 }
 
-static void bap_pac_update_chan_map(struct bt_bap_pac *pac, struct iovec *data)
+static void bap_pac_update_channels(struct bt_bap_pac *pac, struct iovec *data)
 {
 	uint8_t type = 0x03;
 
@@ -2454,8 +2462,8 @@  static void bap_pac_merge(struct bt_bap_pac *pac, struct iovec *data,
 	else
 		pac->data = util_iov_dup(data, 1);
 
-	/* Update channel map */
-	bap_pac_update_chan_map(pac, data);
+	/* Update channels */
+	bap_pac_update_channels(pac, data);
 
 	/* Merge metadata into existing record */
 	if (pac->metadata)
@@ -2495,7 +2503,7 @@  static void bap_pac_free(void *data)
 	free(pac->name);
 	util_iov_free(pac->metadata, 1);
 	util_iov_free(pac->data, 1);
-	queue_destroy(pac->chan_map, NULL);
+	queue_destroy(pac->channels, free);
 	free(pac);
 }
 
@@ -4677,34 +4685,52 @@  int bt_bap_select(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
 	if (!lpac->ops || !lpac->ops->select)
 		return -EOPNOTSUPP;
 
-	for (lchan = queue_get_entries(lpac->chan_map); lchan;
+	for (lchan = queue_get_entries(lpac->channels); lchan;
 					lchan = lchan->next) {
-		uint8_t lmap = PTR_TO_UINT(lchan->data);
+		struct bt_bap_chan *lc = lchan->data;
+		uint8_t lmap = lc->count;
+		int i;
 
-		for (rchan = queue_get_entries(rpac->chan_map); rchan;
-					rchan = rchan->next) {
-			uint8_t rmap = PTR_TO_UINT(rchan->data);
+		printf("lmap 0x%02x\n", lmap);
 
-			printf("lmap 0x%02x rmap 0x%02x\n", lmap, rmap);
+		for (i = 0, rchan = queue_get_entries(rpac->channels); rchan;
+					rchan = rchan->next, i++) {
+			struct bt_bap_chan *rc = rchan->data;
 
-			/* Try matching the channel mapping */
-			if (lmap & rmap) {
-				lpac->ops->select(lpac, rpac, &rpac->qos,
-							func, user_data,
-							lpac->user_data);
-				if (count)
-					(*count)++;
+			printf("rc->count 0x%02x\n", rc->count);
 
-				/* Check if there are any channels left */
-				lmap &= ~(lmap & rmap);
-				if (!lmap)
-					break;
-
-				/* Check if device require AC*(i) settings */
-				if (rmap == 0x01)
-					lmap = lmap >> 1;
-			} else
+			/* Try matching the channel count */
+			if (!(lmap & rc->count))
 				break;
+
+			/* Check if location was set otherwise attempt to
+			 * assign one based on the number of channels it
+			 * supports.
+			 */
+			if (!rc->location) {
+				rc->location = bt_bap_pac_get_locations(rpac);
+				/* If channel count is 1 use a single
+				 * location
+				 */
+				if (rc->count == 0x01)
+					rc->location &= BIT(i);
+			}
+
+			lpac->ops->select(lpac, rpac, lc->location &
+						rc->location, &rpac->qos,
+						func, user_data,
+						lpac->user_data);
+			if (count)
+				(*count)++;
+
+			/* Check if there are any channels left to select */
+			lmap &= ~(lmap & rc->count);
+			if (!lmap)
+				break;
+
+			/* Check if device require AC*(i) settings */
+			if (rc->count == 0x01)
+				lmap = lmap >> 1;
 		}
 	}
 
diff --git a/src/shared/bap.h b/src/shared/bap.h
index 470313e66fc0..9be198cec72c 100644
--- a/src/shared/bap.h
+++ b/src/shared/bap.h
@@ -151,7 +151,7 @@  struct bt_bap_pac *bt_bap_add_pac(struct gatt_db *db, const char *name,
 
 struct bt_bap_pac_ops {
 	int (*select)(struct bt_bap_pac *lpac, struct bt_bap_pac *rpac,
-			struct bt_bap_pac_qos *qos,
+			uint32_t chan_alloc, struct bt_bap_pac_qos *qos,
 			bt_bap_pac_select_t cb, void *cb_data, void *user_data);
 	int (*config)(struct bt_bap_stream *stream, struct iovec *cfg,
 			struct bt_bap_qos *qos, bt_bap_pac_config_t cb,