bluez5: simplify BAP settings parsing and use device settings for them

Parse BAP settings in a single place, and simplify QoS customization a
bit.

Ensure the selected preset gets selected.

For all the BAP codec settings, use device settings instead of global
monitor ones.

doc/dox/config/pipewire-props.7.md

@@ -997,17 +997,30 @@ PipeWire Opus Pro Audio duplex encoding mode: audio, voip, lowdelay
 @PAR@ device-prop  bluez5.bap.cig = "auto"   # integer, or 'auto'
 Set CIG ID for BAP unicast streams of the device.
 
-@PAR@ device-prop  bluez5.bap.rtn = "48_2_1"  # string
+@PAR@ device-prop  bluez5.bap.preset = "auto"  # string
-BAP QoS preset name that needed to be used with vendor config
+BAP QoS preset name that needed to be used with vendor config.
+This property is experimental.
+Available: "48_2_1", ... as in the BAP specification.
 
-@PAR@ device-prop  bluez5.bap.latency = 20 # integer
+@PAR@ device-prop  bluez5.bap.rtn  # integer
+BAP QoS preset name that needed to be used with vendor config.
+This property is experimental.
+Default: as per QoS preset.
+
+@PAR@ device-prop  bluez5.bap.latency  # integer
 BAP QoS latency that needs to be applied for vendor defined preset
+This property is experimental.
+Default: as QoS preset.
 
 @PAR@ device-prop  bluez5.bap.delay = 40000 # integer
 BAP QoS delay that needs to be applied for vendor defined preset
+This property is experimental.
+Default: as per QoS preset.
 
 @PAR@ device-prop  bluez5.framing = false # boolean
 BAP QoS framing that needs to be applied for vendor defined preset
+This property is experimental.
+Default: as per QoS preset.
 
 ## Node properties
 

spa/plugins/bluez5/bap-codec-lc3.c

@@ -8,6 +8,7 @@
 #include <unistd.h>
 #include <stddef.h>
 #include <errno.h>
+#include <limits.h>
 #include <arpa/inet.h>
 #include <bluetooth/bluetooth.h>
 
@@ -38,14 +39,23 @@ struct impl {
 	unsigned int codesize;
 };
 
-struct pac_data {
+struct settings {
-	const uint8_t *data;
-	size_t size;
-	int index;
 	uint32_t locations;
 	uint32_t channel_allocation;
 	bool sink;
 	bool duplex;
+	const char *qos_name;
+	int retransmission;
+	int latency;
+	int64_t delay;
+	int framing;
+};
+
+struct pac_data {
+	const uint8_t *data;
+	size_t size;
+	int index;
+	const struct settings *settings;
 };
 
 struct bap_qos {
@@ -470,110 +480,47 @@ static bool supports_channel_count(uint8_t mask, uint8_t count)
 	return mask & (1u << (count - 1));
 }
 
-static bool parse_qos_settings(struct bap_qos *qos_conf,
+static bool select_bap_qos(struct bap_qos *conf,
-		const struct spa_dict *settings, unsigned int rate_mask,
+		const struct settings *s, unsigned int rate_mask,
-		unsigned int duration_mask, uint16_t framelen_min,
-		uint16_t framelen_max)
- {
-	bool found = false;
-	const char *str;
-	uint32_t value = 0;
-
-	if ((str = spa_dict_lookup(settings, "bluez5.bap.set_name"))) {
-		spa_log_info(log_, "Parsing for vendor defined Configuration\n\n");
-
-		SPA_FOR_EACH_ELEMENT_VAR(bap_qos_configs, c)
-		{
-			if (spa_streq(c->name, str)) {
-				/* We set the user defined preset configuration for vendor as
-				 * default. This helps to avoid using invalid params in scenario
-				 * were user has not configured or has missed to add any items. In
-				 * case if user has added the config it will be overwritten if found */
-				memcpy(qos_conf, c, sizeof(struct bap_qos));
-
-				if (settings && (str = spa_dict_lookup(settings, "bluez5.bap.rtn")))
-					if (spa_atou32(str, &value, 0))
-						qos_conf->retransmission = value;
-
-				if (settings && (str = spa_dict_lookup(settings, "bluez5.bap.latency")))
-					if (spa_atou32(str, &value, 0))
-						qos_conf->latency = value;
-
-				if (settings && (str = spa_dict_lookup(settings, "bluez5.bap.delay")))
-					if (spa_atou32(str, &value, 0))
-						qos_conf->delay = value;
-
-				if (settings && (str = spa_dict_lookup(settings, "bluez5.framing")))
-					qos_conf->framing = spa_atob(str);
-
-				/*We check if the vendor defined config is compatible with the
-				 * remote device. If not, we fallback to use default preset config*/
-				if (!(get_rate_mask(c->rate) & rate_mask))
-					break;
-				if (!(get_duration_mask(c->frame_duration) & duration_mask))
-					break;
-				if (c->framing)
-					break; /* XXX: framing not supported */
-				if (c->framelen < framelen_min || c->framelen > framelen_max)
-					break;
-
-				/* At this point, the vendor configured settings is
-				 * compatible with the remote device and hence we can
-				 * use this configuration to establish transport*/
-				found = true;
-				break;
-			}
-		}
-	}
-
-	return found;
-}
-
-static bool select_bap_qos(struct bap_qos *qos_conf,
-		const struct spa_dict *settings, unsigned int rate_mask,
 		unsigned int duration_mask, uint16_t framelen_min,
 		uint16_t framelen_max)
 {
-	const struct bap_qos *best = NULL;
+	conf->name = NULL;
-	unsigned int best_priority = 0;
+	conf->priority = 0;
-	bool found = false;
 
-	if(!qos_conf){
+	SPA_FOR_EACH_ELEMENT_VAR(bap_qos_configs, cur_conf) {
-		spa_log_error(log_, "Invalid qos config\n");
+		struct bap_qos c = *cur_conf;
-		return found;
+
-	}
+		/* Check if custom QoS settings are configured. If so, we check if
-	/* We will check if vendor defined QoS settings are configured. If so, we check if the
+		 * the configured settings are compatible with unicast server
-	 * configured settings are compatible with unicast server*/
+		 */
-	if (settings) {
+		if (spa_streq(c.name, s->qos_name))
-		found = parse_qos_settings(qos_conf, settings, rate_mask,
+			c.priority = UINT_MAX;
-				duration_mask, framelen_min, framelen_max);
+		else if (c.priority < conf->priority)
+			continue;
+
+		if (s->retransmission >= 0)
+			c.retransmission = s->retransmission;
+		if (s->latency >= 0)
+			c.latency = s->latency;
+		if (s->delay >= 0)
+			c.delay = s->delay;
+		if (s->framing >= 0)
+			c.framing = s->framing;
+
+		if (!(get_rate_mask(c.rate) & rate_mask))
+			continue;
+		if (!(get_duration_mask(c.frame_duration) & duration_mask))
+			continue;
+		if (c.framing)
+			continue;  /* XXX: framing not supported */
+		if (c.framelen < framelen_min || c.framelen > framelen_max)
+			continue;
+
+		*conf = c;
 	}
 
-	if (!found) {
+	return conf->name;
-		SPA_FOR_EACH_ELEMENT_VAR(bap_qos_configs, c)
-		{
-			if (c->priority < best_priority)
-				continue;
-			if (!(get_rate_mask(c->rate) & rate_mask))
-				continue;
-			if (!(get_duration_mask(c->frame_duration) & duration_mask))
-				continue;
-			if (c->framing)
-				continue; /* XXX: framing not supported */
-			if (c->framelen < framelen_min || c->framelen > framelen_max)
-				continue;
-
-			best = c;
-			best_priority = c->priority;
-		}
-
-		if (best) {
-			memcpy(qos_conf, best, sizeof(struct bap_qos));
-			found = true;
-		}
-	}
-
-	return found;
 }
 
 static int select_channels(uint8_t channel_counts, uint32_t locations, uint32_t channel_allocation,
@@ -632,7 +579,7 @@ static int select_channels(uint8_t channel_counts, uint32_t locations, uint32_t
 	return 0;
 }
 
-static bool select_config(bap_lc3_t *conf, const struct pac_data *pac,	struct spa_debug_context *debug_ctx, const struct spa_dict *settings)
+static bool select_config(bap_lc3_t *conf, const struct pac_data *pac,	struct spa_debug_context *debug_ctx)
 {
 	const uint8_t *data = pac->data;
 	size_t data_size = pac->size;
@@ -650,8 +597,8 @@ static bool select_config(bap_lc3_t *conf, const struct pac_data *pac,	struct sp
 		return false;
 	memset(conf, 0, sizeof(*conf));
 
-	conf->sink = pac->sink;
+	conf->sink = pac->settings->sink;
-	conf->duplex = pac->duplex;
+	conf->duplex = pac->settings->duplex;
 
 	/* XXX: we always use one frame block */
 	conf->n_blks = 1;
@@ -713,7 +660,7 @@ static bool select_config(bap_lc3_t *conf, const struct pac_data *pac,	struct sp
 		max_frames = max_channels;
 	}
 
-	if (select_channels(channel_counts, pac->locations, pac->channel_allocation, &conf->channels) < 0) {
+	if (select_channels(channel_counts, pac->settings->locations, pac->settings->channel_allocation, &conf->channels) < 0) {
 		spa_debugc(debug_ctx, "invalid channel configuration: 0x%02x %u",
 				channel_counts, max_frames);
 		return false;
@@ -730,16 +677,16 @@ static bool select_config(bap_lc3_t *conf, const struct pac_data *pac,	struct sp
 	 * Frame length is not limited by ISO MTU, as kernel will fragment
 	 * and reassemble SDUs as needed.
 	 */
-	if (pac->sink && pac->duplex) {
+	if (pac->settings->sink && pac->settings->duplex) {
 		/* 16KHz input is mandatory in BAP v1.0.1 Table 3.5, so prefer
 		 * it for now for input rate in duplex configuration.
 		 *
 		 * Devices may list other values but not certain they will work properly.
 		 */
-		found = select_bap_qos(&bap_qos, settings, rate_mask & LC3_FREQ_16KHZ, duration_mask, framelen_min, framelen_max);
+		found = select_bap_qos(&bap_qos, pac->settings, rate_mask & LC3_FREQ_16KHZ, duration_mask, framelen_min, framelen_max);
 	}
 	if (!found)
-		found = select_bap_qos(&bap_qos, settings, rate_mask, duration_mask, framelen_min, framelen_max);
+		found = select_bap_qos(&bap_qos, pac->settings, rate_mask, duration_mask, framelen_min, framelen_max);
 
 	if (!found) {
 		spa_debugc(debug_ctx, "no compatible configuration found, rate:0x%08x, duration:0x%08x frame:%u-%u",
@@ -832,6 +779,8 @@ static int conf_cmp(const bap_lc3_t *conf1, int res1, const bap_lc3_t *conf2, in
 	if (!a || !b)
 		return b - a;
 
+	PREFER_EXPR(conf->priority == UINT_MAX);
+
 	PREFER_BOOL(conf->channels & LC3_CHAN_2);
 	PREFER_BOOL(conf->channels & LC3_CHAN_1);
 
@@ -854,12 +803,68 @@ static int pac_cmp(const void *p1, const void *p2)
 	bap_lc3_t conf1, conf2;
 	int res1, res2;
 
-	res1 = select_config(&conf1, pac1, &debug_ctx.ctx, NULL) ? (int)sizeof(bap_lc3_t) : -EINVAL;
+	res1 = select_config(&conf1, pac1, &debug_ctx.ctx) ? (int)sizeof(bap_lc3_t) : -EINVAL;
-	res2 = select_config(&conf2, pac2, &debug_ctx.ctx, NULL) ? (int)sizeof(bap_lc3_t) : -EINVAL;
+	res2 = select_config(&conf2, pac2, &debug_ctx.ctx) ? (int)sizeof(bap_lc3_t) : -EINVAL;
 
 	return conf_cmp(&conf1, res1, &conf2, res2);
 }
 
+static void parse_settings(struct settings *s, const struct spa_dict *settings,
+		struct spa_debug_log_ctx *debug_ctx)
+{
+	const char *str;
+	uint32_t value;
+
+	spa_zero(*s);
+	s->retransmission = -1;
+	s->latency = -1;
+	s->delay = -1;
+	s->framing = -1;
+
+	if (!settings)
+		return;
+
+	if ((str = spa_dict_lookup(settings, "bluez5.bap.preset")))
+		s->qos_name = str;
+
+	if (spa_atou32(spa_dict_lookup(settings, "bluez5.bap.rtn"), &value, 0))
+		s->retransmission = value;
+
+	if (spa_atou32(spa_dict_lookup(settings, "bluez5.bap.latency"), &value, 0))
+		s->latency = value;
+
+	if (spa_atou32(spa_dict_lookup(settings, "bluez5.bap.delay"), &value, 0))
+		s->delay = value;
+
+	if ((str = spa_dict_lookup(settings, "bluez5.bap.framing")))
+		s->framing = spa_atob(str);
+
+	if (spa_atou32(spa_dict_lookup(settings, "bluez5.bap.locations"), &value, 0))
+		s->locations = value;
+
+	if (spa_atou32(spa_dict_lookup(settings, "bluez5.bap.channel-allocation"), &value, 0))
+		s->channel_allocation = value;
+
+	if (spa_atob(spa_dict_lookup(settings, "bluez5.bap.debug")))
+		*debug_ctx = SPA_LOG_DEBUG_INIT(log_, SPA_LOG_LEVEL_DEBUG);
+	else
+		*debug_ctx = SPA_LOG_DEBUG_INIT(NULL, SPA_LOG_LEVEL_TRACE);
+
+	/* Is remote endpoint sink or source */
+	s->sink = spa_atob(spa_dict_lookup(settings, "bluez5.bap.sink"));
+
+	/* Is remote endpoint duplex */
+	s->duplex = spa_atob(spa_dict_lookup(settings, "bluez5.bap.duplex"));
+
+	spa_debugc(&debug_ctx->ctx,
+			"BAP LC3 settings: preset:%s rtn:%d latency:%d delay:%d framing:%d "
+			"locations:%x chnalloc:%x sink:%d duplex:%d",
+			s->qos_name ? s->qos_name : "auto",
+			s->retransmission, s->latency, (int)s->delay, s->framing,
+			(unsigned int)s->locations, (unsigned int)s->channel_allocation,
+			(int)s->sink, (int)s->duplex);
+}
+
 static int codec_select_config(const struct media_codec *codec, uint32_t flags,
 		const void *caps, size_t caps_size,
 		const struct media_codec_audio_info *info,
@@ -869,33 +874,14 @@ static int codec_select_config(const struct media_codec *codec, uint32_t flags,
 	int npacs;
 	bap_lc3_t conf;
 	uint8_t *data = config;
-	uint32_t locations = 0;
+	struct spa_debug_log_ctx debug_ctx;
-	uint32_t channel_allocation = 0;
+	struct settings s;
-	bool sink = false, duplex = false;
-	struct spa_debug_log_ctx debug_ctx = SPA_LOG_DEBUG_INIT(log_, SPA_LOG_LEVEL_TRACE);
 	int i;
 
 	if (caps == NULL)
 		return -EINVAL;
 
-	if (settings) {
+	parse_settings(&s, settings, &debug_ctx);
-		for (i = 0; i < (int)settings->n_items; ++i) {
-			if (spa_streq(settings->items[i].key, "bluez5.bap.locations"))
-				sscanf(settings->items[i].value, "%"PRIu32, &locations);
-			if (spa_streq(settings->items[i].key, "bluez5.bap.channel-allocation"))
-				sscanf(settings->items[i].value, "%"PRIu32, &channel_allocation);
-		}
-
-		if (spa_atob(spa_dict_lookup(settings, "bluez5.bap.debug")))
-			debug_ctx = SPA_LOG_DEBUG_INIT(log_, SPA_LOG_LEVEL_DEBUG);
-
-		/* Is remote endpoint sink or source */
-		sink = spa_atob(spa_dict_lookup(settings, "bluez5.bap.sink"));
-
-		/* Is remote endpoint duplex */
-		duplex = spa_atob(spa_dict_lookup(settings, "bluez5.bap.duplex"));
-	}
-
 
 	/* Select best conf from those possible */
 	npacs = parse_bluez_pacs(caps, caps_size, pacs, &debug_ctx.ctx);
@@ -907,18 +893,14 @@ static int codec_select_config(const struct media_codec *codec, uint32_t flags,
 		return -EINVAL;
 	}
 
-	for (i = 0; i < npacs; ++i) {
+	for (i = 0; i < npacs; ++i)
-		pacs[i].locations = locations;
+		pacs[i].settings = &s;
-		pacs[i].channel_allocation = channel_allocation;
-		pacs[i].sink = sink;
-		pacs[i].duplex = duplex;
-	}
 
 	qsort(pacs, npacs, sizeof(struct pac_data), pac_cmp);
 
 	spa_debugc(&debug_ctx.ctx, "selected PAC %d", pacs[0].index);
 
-	if (!select_config(&conf, &pacs[0], &debug_ctx.ctx, settings))
+	if (!select_config(&conf, &pacs[0], &debug_ctx.ctx))
 		return -ENOTSUP;
 
 	data += write_ltv_uint8(data, LC3_TYPE_FREQ, conf.rate);
@@ -1106,13 +1088,17 @@ static int codec_get_qos(const struct media_codec *codec,
 	struct bap_qos bap_qos;
 	bap_lc3_t conf;
 	bool found = false;
+	struct settings s;
+	struct spa_debug_log_ctx debug_ctx;
 
 	spa_zero(*qos);
 
 	if (!parse_conf(&conf, config, config_size))
 		return -EINVAL;
 
-	found = select_bap_qos(&bap_qos, settings, get_rate_mask(conf.rate), get_duration_mask(conf.frame_duration),
+	parse_settings(&s, settings, &debug_ctx);
+
+	found = select_bap_qos(&bap_qos, &s, get_rate_mask(conf.rate), get_duration_mask(conf.frame_duration),
 			conf.framelen, conf.framelen);
 	if (!found) {
 		/* shouldn't happen: select_config should pick existing one */

spa/plugins/bluez5/bluez5-dbus.c

@@ -926,8 +926,8 @@ static DBusHandlerResult endpoint_select_properties(DBusConnection *conn, DBusMe
 	bool sink, duplex;
 	const char *err_msg = "Unknown error";
 	struct spa_dict settings;
-	struct spa_dict_item setting_items[SPA_N_ELEMENTS(monitor->global_setting_items) + 5];
+	struct spa_dict_item setting_items[128];
-	int i;
+	unsigned int i, j;
 
 	const char *endpoint_path = NULL;
 	uint8_t caps[A2DP_MAX_CAPS_SIZE];
@@ -986,15 +986,16 @@ static DBusHandlerResult endpoint_select_properties(DBusConnection *conn, DBusMe
 	 */
 	ep->acceptor = true;
 
-	for (i = 0; i < (int)monitor->global_settings.n_items; ++i)
+	i = 0;
-		setting_items[i] = monitor->global_settings.items[i];
 	setting_items[i++] = SPA_DICT_ITEM_INIT("bluez5.bap.locations", locations);
 	setting_items[i++] = SPA_DICT_ITEM_INIT("bluez5.bap.channel-allocation", channel_allocation);
 	setting_items[i++] = SPA_DICT_ITEM_INIT("bluez5.bap.sink", sink ? "true" : "false");
 	setting_items[i++] = SPA_DICT_ITEM_INIT("bluez5.bap.duplex", duplex ? "true" : "false");
 	setting_items[i++] = SPA_DICT_ITEM_INIT("bluez5.bap.debug", "true");
+	if (ep->device->settings)
+		for (j = 0; j < ep->device->settings->n_items && i < SPA_N_ELEMENTS(setting_items); ++i, ++j)
+			setting_items[i] = ep->device->settings->items[j];
 	settings = SPA_DICT_INIT(setting_items, i);
-	spa_assert((size_t)i <= SPA_N_ELEMENTS(setting_items));
 
 	conf_size = codec->select_config(codec, 0, caps, caps_size, &monitor->default_audio_info, &settings, config);
 	if (conf_size < 0) {

spa/plugins/bluez5/media-codecs.h

@@ -26,7 +26,7 @@
 
 #define SPA_TYPE_INTERFACE_Bluez5CodecMedia	SPA_TYPE_INFO_INTERFACE_BASE "Bluez5:Codec:Media:Private"
 
-#define SPA_VERSION_BLUEZ5_CODEC_MEDIA		12
+#define SPA_VERSION_BLUEZ5_CODEC_MEDIA		13
 
 struct spa_bluez5_codec_a2dp {
 	struct spa_interface iface;