2024-02-23 15:35:05

by Andrei Istodorescu

[permalink] [raw]
Subject: [PATCH BlueZ v4 0/2] Update Sink BASE management

This patch refactors the flow for the BAP Broadcast Sink.
After discovering a new Broadcast Source a short PA Sync will be tried.
Depending on the BASE information present in the PA and the local PACS
capabilities of the Sink, one remote media endpoint will be created for
each compatible BIS.
Configuring one of these endpoints results in creating a stream and a
media transport for the corresponding BIS.

Idle timer.
This patch introduces an idle timer, responsible for processing PA Sync
requests. One request is possible at a time. When a broadcast source is
discovered a new request for short PA sync is enqueued. Also, when a
SetConfiguration is requested for an existing BAP broadcast source
endpoint another request is enqueued.
The timer runs periodically and peeks for requests. In case there is no
request in progress the timer marks the already peeked request to
pending and starts processing it. When the request is completed, it is
removed from the timer's queue, so that the next request can be
processed.

Andrei Istodorescu (2):
shared/bap: Add API to add an observed BIS
bap: Do PA Sync for each BAP Broadcast source discovered

profiles/audio/bap.c | 651 +++++++++++++++++++++++++++----------------
src/shared/bap.c | 304 +++++++++++++++++++-
src/shared/bap.h | 13 +-
3 files changed, 714 insertions(+), 254 deletions(-)


base-commit: a16c2ccf9c256285188f4549b7b767cf31b100eb
--
2.40.1



2024-02-23 15:36:06

by Andrei Istodorescu

[permalink] [raw]
Subject: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an observed BIS

Add API to add a PAC for each observed BIS that is supported by the
local PACS data.
Each BIS observed capabilities LTV is compared to the local capabilities
and when we have a full LTVs match a PAC record is created for that BIS.
Also a MediaEndpoint is registered over DBUS and the stream can be
enabled using the SetConfiguration DBUS call.
---
src/shared/bap.c | 304 ++++++++++++++++++++++++++++++++++++++++++++---
src/shared/bap.h | 13 +-
2 files changed, 302 insertions(+), 15 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index f5fc1402701c..853919111835 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -48,6 +48,15 @@

#define BAP_PROCESS_TIMEOUT 10

+#define BAP_FREQ_LTV_TYPE 1
+#define BAP_DURATION_LTV_TYPE 2
+#define BAP_CHANNEL_ALLOCATION_LTV_TYPE 3
+#define BAP_FRAME_LEN_LTV_TYPE 4
+#define CODEC_SPECIFIC_CONFIGURATION_MASK (\
+ (1<<BAP_FREQ_LTV_TYPE)|\
+ (1<<BAP_DURATION_LTV_TYPE)|\
+ (1<<BAP_FRAME_LEN_LTV_TYPE))
+
struct bt_bap_pac_changed {
unsigned int id;
bt_bap_pac_func_t added;
@@ -1692,11 +1701,8 @@ static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
bt_bap_stream_func_t func, void *user_data)
{
stream->qos = *qos;
- if (stream->lpac->type == BT_BAP_BCAST_SINK) {
- if (data)
- stream_config(stream, data, NULL);
- stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
- }
+ stream->lpac->ops->config(stream, stream->cc, &stream->qos,
+ ep_config_cb, stream->lpac->user_data);

return 1;
}
@@ -3302,6 +3308,13 @@ static void bap_add_broadcast_source(struct bt_bap_pac *pac)
static void bap_add_broadcast_sink(struct bt_bap_pac *pac)
{
queue_push_tail(pac->bdb->broadcast_sinks, pac);
+
+ /* Update local PACS for broadcast sink also, when registering an
+ * endpoint
+ */
+ pacs_add_sink_location(pac->bdb->pacs, pac->qos.location);
+ pacs_add_sink_supported_context(pac->bdb->pacs,
+ pac->qos.supported_context);
}

static void notify_pac_added(void *data, void *user_data)
@@ -3453,6 +3466,16 @@ struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac)
return &pac->qos;
}

+struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac)
+{
+ return pac->data;
+}
+
+struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac)
+{
+ return pac->metadata;
+}
+
uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
{
if (!stream)
@@ -5253,10 +5276,6 @@ bool bt_bap_stream_set_user_data(struct bt_bap_stream *stream, void *user_data)

stream->user_data = user_data;

- if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST)
- stream->lpac->ops->config(stream, stream->cc, &stream->qos,
- ep_config_cb, stream->lpac->user_data);
-
return true;
}

@@ -5892,8 +5911,9 @@ static void add_new_subgroup(struct bt_base *base,

struct bt_ltv_match {
uint8_t l;
- uint8_t *v;
+ void *data;
bool found;
+ uint32_t data32;
};

struct bt_ltv_search {
@@ -5912,7 +5932,7 @@ static void match_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
if (ltv_match->l != l)
return;

- if (!memcmp(v, ltv_match->v, l))
+ if (!memcmp(v, ltv_match->data, l))
ltv_match->found = true;
}

@@ -5924,7 +5944,7 @@ static void search_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,

ltv_match.found = false;
ltv_match.l = l;
- ltv_match.v = v;
+ ltv_match.data = v;

util_ltv_foreach(ltv_search->iov->iov_base,
ltv_search->iov->iov_len, &t,
@@ -5965,8 +5985,10 @@ static bool compare_ltv(struct iovec *iov1,
}

struct bt_ltv_extract {
- struct iovec *result;
struct iovec *src;
+ void *value;
+ uint8_t len;
+ struct iovec *result;
};

static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
@@ -5978,7 +6000,7 @@ static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,

ltv_match.found = false;
ltv_match.l = l;
- ltv_match.v = v;
+ ltv_match.data = v;

/* Search each BIS caps ltv in subgroup caps
* to extract the one that are BIS specific
@@ -6111,3 +6133,257 @@ struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream)

return base_iov;
}
+
+static void bap_sink_get_allocation(size_t i, uint8_t l, uint8_t t,
+ uint8_t *v, void *user_data)
+{
+ uint32_t location32;
+
+ if (!v)
+ return;
+
+ memcpy(&location32, v, l);
+ *((uint32_t *)user_data) = le32_to_cpu(location32);
+}
+
+/*
+ * This function compares PAC Codec Specific Capabilities, with the Codec
+ * Specific Configuration LTVs received in the BASE of the BAP Source. The
+ * result is accumulated in data32 which is a bitmask of types.
+ */
+static void check_pac_caps_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
+ void *user_data)
+{
+ struct bt_ltv_match *compare_data = user_data;
+ uint8_t *bis_v = compare_data->data;
+
+ switch (t) {
+ case BAP_FREQ_LTV_TYPE:
+ {
+ uint16_t mask = *((uint16_t *)v);
+
+ mask = le16_to_cpu(mask);
+ if (mask & (1 << (bis_v[0] - 1)))
+ compare_data->data32 |= 1<<t;
+ }
+ break;
+ case BAP_DURATION_LTV_TYPE:
+ if ((v[0]) & (1 << bis_v[0]))
+ compare_data->data32 |= 1<<t;
+ break;
+ case BAP_FRAME_LEN_LTV_TYPE:
+ {
+ uint16_t min = *((uint16_t *)v);
+ uint16_t max = *((uint16_t *)(&v[2]));
+ uint16_t frame_len = *((uint16_t *)bis_v);
+
+ min = le16_to_cpu(min);
+ max = le16_to_cpu(max);
+ frame_len = le16_to_cpu(frame_len);
+ if ((frame_len >= min) &&
+ (frame_len <= max))
+ compare_data->data32 |= 1<<t;
+ }
+ break;
+ }
+}
+
+static void check_source_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
+ void *user_data)
+{
+ struct bt_ltv_match *local_data = user_data;
+ struct iovec *pac_caps = local_data->data;
+ struct bt_ltv_match compare_data;
+
+ compare_data.data = v;
+
+ /* Search inside local PAC's caps for LTV of type t */
+ util_ltv_foreach(pac_caps->iov_base, pac_caps->iov_len, &t,
+ check_pac_caps_ltv, &compare_data);
+
+ local_data->data32 |= compare_data.data32;
+}
+
+static void bap_sink_check_level3_ltv(size_t i, uint8_t l, uint8_t t,
+ uint8_t *v, void *user_data)
+{
+ struct bt_ltv_extract *merge_data = user_data;
+
+ merge_data->value = v;
+ merge_data->len = l;
+}
+
+static void bap_push_ltv(struct iovec *output, uint8_t l, uint8_t t, void *v)
+{
+ l++;
+ iov_append(output, 1, &l);
+ iov_append(output, 1, &t);
+ iov_append(output, l - 1, v);
+}
+
+static void bap_sink_check_level2_ltv(size_t i, uint8_t l, uint8_t t,
+ uint8_t *v, void *user_data)
+{
+ struct bt_ltv_extract *merge_data = user_data;
+
+ merge_data->value = NULL;
+ util_ltv_foreach(merge_data->src->iov_base,
+ merge_data->src->iov_len,
+ &t,
+ bap_sink_check_level3_ltv, user_data);
+
+ /* If the LTV at level 2 was found at level 3 add the one from level 3,
+ * otherwise add the one at level 2
+ */
+ if (merge_data->value)
+ bap_push_ltv(merge_data->result, merge_data->len,
+ t, merge_data->value);
+ else
+ bap_push_ltv(merge_data->result, l, t, v);
+}
+
+static void check_local_pac(void *data, void *user_data)
+{
+ struct bt_ltv_match *compare_data = user_data;
+ struct iovec *bis_data = (struct iovec *)compare_data->data;
+ const struct bt_bap_pac *pac = data;
+
+ /* Keep searching for a matching PAC if one wasn't found
+ * in previous PAC element
+ */
+ if (compare_data->found == false) {
+ struct bt_ltv_match bis_compare_data = {
+ .data = pac->data,
+ .data32 = 0, /* LTVs bitmask result */
+ .found = false
+ };
+
+ /* loop each BIS LTV */
+ util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, NULL,
+ check_source_ltv, &bis_compare_data);
+
+ /* We have a match if all selected LTVs have a match */
+ if ((bis_compare_data.data32 &
+ CODEC_SPECIFIC_CONFIGURATION_MASK) ==
+ CODEC_SPECIFIC_CONFIGURATION_MASK)
+ compare_data->found = true;
+ }
+}
+
+static void bap_sink_match_allocation(size_t i, uint8_t l, uint8_t t,
+ uint8_t *v, void *user_data)
+{
+ struct bt_ltv_match *data = user_data;
+ uint32_t location32;
+
+ if (!v)
+ return;
+
+ memcpy(&location32, v, l);
+ location32 = le32_to_cpu(location32);
+
+ /* If all the bits in the received bitmask are found in
+ * the local bitmask then we have a match
+ */
+ if ((location32 & data->data32) == location32)
+ data->found = true;
+ else
+ data->found = false;
+}
+
+static bool bap_check_bis(struct bt_bap_db *ldb, struct iovec *bis_data)
+{
+ struct bt_ltv_match compare_data = {};
+
+ /* Check channel allocation against the PACS location.
+ * If we don't have a location set we can accept any BIS location.
+ * If the BIS doesn't have a location set we also accept it
+ */
+ compare_data.found = true;
+
+ if (ldb->pacs->sink_loc_value) {
+ uint8_t type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
+
+ compare_data.data32 = ldb->pacs->sink_loc_value;
+ util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, &type,
+ bap_sink_match_allocation, &compare_data);
+ }
+
+ /* Check remaining LTVs against the PACs list */
+ if (compare_data.found) {
+ compare_data.data = bis_data;
+ compare_data.found = false;
+ queue_foreach(ldb->broadcast_sinks, check_local_pac,
+ &compare_data);
+ }
+
+ return compare_data.found;
+}
+
+void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
+ struct bt_bap_codec *codec,
+ struct iovec *l2_caps,
+ struct iovec *l3_caps,
+ struct iovec *meta)
+{
+ struct bt_bap_pac *pac_source_bis;
+ struct bt_bap_endpoint *ep;
+ int err = 0;
+ struct bt_bap_pac_qos bis_qos = {0};
+ uint8_t type = 0;
+ struct bt_ltv_extract merge_data = {0};
+
+ merge_data.src = l3_caps;
+ merge_data.result = new0(struct iovec, 1);
+
+ /* Create a Codec Specific Configuration with LTVs at level 2 (subgroup)
+ * overwritten by LTVs at level 3 (BIS)
+ */
+ util_ltv_foreach(l2_caps->iov_base,
+ l2_caps->iov_len,
+ NULL,
+ bap_sink_check_level2_ltv, &merge_data);
+
+ /* Check each BIS Codec Specific Configuration LTVs against our Codec
+ * Specific Capabilities and if the BIS matches create a PAC with it
+ */
+ if (bap_check_bis(bap->ldb, merge_data.result) == false)
+ goto cleanup;
+
+ DBG(bap, "Matching BIS %i", bis_index);
+
+ /* Create a QoS structure based on the received BIS information to
+ * specify the desired channel for this BIS/PAC
+ */
+ type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
+ util_ltv_foreach(merge_data.result->iov_base,
+ merge_data.result->iov_len, &type,
+ bap_sink_get_allocation, &bis_qos.location);
+
+ /* Create a remote PAC */
+ pac_source_bis = bap_pac_new(bap->rdb, NULL,
+ BT_BAP_BCAST_SOURCE, codec, &bis_qos,
+ merge_data.result, meta);
+
+ err = asprintf(&pac_source_bis->name, "%d", bis_index);
+
+ if (err < 0) {
+ DBG(bap, "error in asprintf");
+ goto cleanup;
+ }
+
+ /* Add remote source endpoint */
+ if (!bap->rdb->broadcast_sources)
+ bap->rdb->broadcast_sources = queue_new();
+ queue_push_tail(bap->rdb->broadcast_sources, pac_source_bis);
+
+ queue_foreach(bap->pac_cbs, notify_pac_added, pac_source_bis);
+ /* Push remote endpoint with direction sink */
+ ep = bap_endpoint_new_broadcast(bap->rdb, BT_BAP_BCAST_SINK);
+
+ if (ep)
+ queue_push_tail(bap->remote_eps, ep);
+
+cleanup:
+ util_iov_free(merge_data.result, 1);
+}
diff --git a/src/shared/bap.h b/src/shared/bap.h
index 2c3550921f07..b2826719f84f 100644
--- a/src/shared/bap.h
+++ b/src/shared/bap.h
@@ -4,7 +4,7 @@
* BlueZ - Bluetooth protocol stack for Linux
*
* Copyright (C) 2022 Intel Corporation. All rights reserved.
- * Copyright 2023 NXP
+ * Copyright 2023-2024 NXP
*
*/

@@ -175,6 +175,10 @@ uint16_t bt_bap_pac_get_context(struct bt_bap_pac *pac);

struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac);

+struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac);
+
+struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
+
uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);

struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
@@ -323,3 +327,10 @@ void bt_bap_update_bcast_source(struct bt_bap_pac *pac,
bool bt_bap_pac_bcast_is_local(struct bt_bap *bap, struct bt_bap_pac *pac);

struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream);
+
+void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
+ struct bt_bap_codec *codec,
+ struct iovec *l2_caps,
+ struct iovec *l3_caps,
+ struct iovec *meta);
+
--
2.40.1


2024-02-23 15:36:12

by Andrei Istodorescu

[permalink] [raw]
Subject: [PATCH BlueZ v4 2/2] bap: Do PA Sync for each BAP Broadcast source discovered

After discovering a BAP Broadcast Source do a short PA sync first to learn
the BASE. After discovering the BASE check how many BISes are matching
the sink capabilities and create endpoints for them. Allow user to
configure one endpoint using "SetConfiguration" causing BIG
synchronization to the corresponding BIS; also this results in creating a
stream and the corresponding transport.
---
profiles/audio/bap.c | 651 +++++++++++++++++++++++++++----------------
1 file changed, 412 insertions(+), 239 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index b4acdd9ef40c..6daa3640c21a 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -62,6 +62,13 @@
#define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
#define MEDIA_INTERFACE "org.bluez.Media1"

+/* Periodic advertisments are performed by an idle timer, which,
+ * at every tick, checks a queue for pending PA requests.
+ * When there is no pending requests, an item is popped from the
+ * queue, marked as pending and then it gets processed.
+ */
+#define PA_IDLE_TIMEOUT 2
+
struct bap_setup {
struct bap_ep *ep;
struct bt_bap_stream *stream;
@@ -105,7 +112,62 @@ struct bap_data {
void *user_data;
};

+enum {
+ BAP_PA_SHORT_REQ = 0, /* Request for short PA sync */
+ BAP_PA_BIG_SYNC_REQ, /* Request for PA Sync and BIG Sync */
+};
+
+struct bap_bcast_pa_req {
+ uint8_t type;
+ bool in_progress;
+ union {
+ struct btd_service *service;
+ struct bap_setup *setup;
+ } data;
+};
+
static struct queue *sessions;
+static struct queue *bcast_pa_requests;
+static unsigned int pa_timer_id;
+
+/* Structure holding the parameters for Periodic Advertisement create sync.
+ * The full QOS is populated at the time the user selects and endpoint and
+ * configures it using SetConfiguration.
+ */
+static struct bt_iso_qos bap_sink_pa_qos = {
+ .bcast = {
+ .options = 0x00,
+ .skip = 0x0000,
+ .sync_timeout = 0x4000,
+ .sync_cte_type = 0x00,
+ /* TODO: The following parameters are not needed for PA Sync.
+ * They will be removed when the kernel checks will be removed.
+ */
+ .big = BT_ISO_QOS_BIG_UNSET,
+ .bis = BT_ISO_QOS_BIS_UNSET,
+ .encryption = 0x00,
+ .bcode = {0x00},
+ .mse = 0x00,
+ .timeout = 0x4000,
+ .sync_factor = 0x07,
+ .packing = 0x00,
+ .framing = 0x00,
+ .in = {
+ .interval = 10000,
+ .latency = 10,
+ .sdu = 40,
+ .phy = 0x02,
+ .rtn = 2,
+ },
+ .out = {
+ .interval = 10000,
+ .latency = 10,
+ .sdu = 40,
+ .phy = 0x02,
+ .rtn = 2,
+ }
+ }
+};

static bool bap_data_set_user_data(struct bap_data *data, void *user_data)
{
@@ -422,113 +484,6 @@ static int parse_array(DBusMessageIter *iter, struct iovec *iov)
return 0;
}

-static bool parse_base(void *data, size_t len, util_debug_func_t func,
- uint32_t *presDelay, uint8_t *numSubgroups, uint8_t *numBis,
- struct bt_bap_codec *codec, struct iovec **caps,
- struct iovec **meta)
-{
- struct iovec iov = {
- .iov_base = data,
- .iov_len = len,
- };
-
- uint8_t capsLen, metaLen;
- struct iovec cc;
- struct iovec metadata;
-
- if (presDelay) {
- if (!util_iov_pull_le24(&iov, presDelay))
- return false;
- util_debug(func, NULL, "PresentationDelay %d", *presDelay);
- }
-
- if (numSubgroups) {
- if (!util_iov_pull_u8(&iov, numSubgroups))
- return false;
- util_debug(func, NULL, "NumSubgroups %d", *numSubgroups);
- }
-
- if (numBis) {
- if (!util_iov_pull_u8(&iov, numBis))
- return false;
- util_debug(func, NULL, "NumBis %d", *numBis);
- }
-
- if (codec) {
- codec = util_iov_pull_mem(&iov, sizeof(*codec));
- if (!codec)
- return false;
- util_debug(func, NULL, "%s: ID %d CID 0x%2.2x VID 0x%2.2x",
- "Codec", codec->id, codec->cid, codec->vid);
- }
-
- if (!util_iov_pull_u8(&iov, &capsLen))
- return false;
- util_debug(func, NULL, "CC Len %d", capsLen);
-
- if (!capsLen)
- return false;
-
- cc.iov_len = capsLen;
- cc.iov_base = util_iov_pull_mem(&iov, capsLen);
- if (!cc.iov_base)
- return false;
-
- if (caps) {
- if (*caps)
- util_iov_free(*caps, 1);
-
- *caps = util_iov_dup(&cc, 1);
- }
-
- for (int i = 0; capsLen > 1; i++) {
- struct bt_ltv *ltv = util_iov_pull_mem(&cc, sizeof(*ltv));
- uint8_t *caps;
-
- if (!ltv) {
- util_debug(func, NULL, "Unable to parse %s",
- "Capabilities");
- return false;
- }
-
- util_debug(func, NULL, "%s #%u: len %u type %u",
- "CC", i, ltv->len, ltv->type);
-
- caps = util_iov_pull_mem(&cc, ltv->len - 1);
- if (!caps) {
- util_debug(func, NULL, "Unable to parse %s",
- "CC");
- return false;
- }
- util_hexdump(' ', caps, ltv->len - 1, func, NULL);
-
- capsLen -= (ltv->len + 1);
- }
-
- if (!util_iov_pull_u8(&iov, &metaLen))
- return false;
- util_debug(func, NULL, "Metadata Len %d", metaLen);
-
- if (!metaLen)
- return false;
-
- metadata.iov_len = metaLen;
- metadata.iov_base = util_iov_pull_mem(&iov, metaLen);
- if (!metadata.iov_base)
- return false;
-
- if (meta) {
- if (*meta)
- util_iov_free(*meta, 1);
-
- *meta = util_iov_dup(&metadata, 1);
- }
-
- util_hexdump(' ', metadata.iov_base, metaLen, func, NULL);
-
- return true;
-}
-
static int parse_io_qos(const char *key, int var, DBusMessageIter *iter,
struct bt_bap_io_qos *qos)
{
@@ -954,9 +909,20 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
return btd_error_invalid_args(msg);
}

+ /* For BAP Broadcast Sink, the capabilities and metadata are coming
+ * from the source's BIS, which are present in the remote PAC
+ */
+ if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SINK) {
+ util_iov_free(setup->caps, 1);
+ setup->caps = util_iov_dup(bt_bap_pac_get_data(ep->rpac), 1);
+ util_iov_free(setup->metadata, 1);
+ setup->metadata = util_iov_dup(
+ bt_bap_pac_get_metadata(ep->rpac), 1);
+ }
+
setup->stream = bt_bap_stream_new(ep->data->bap, ep->lpac, ep->rpac,
&setup->qos, setup->caps);
-
+ bt_bap_stream_set_user_data(setup->stream, ep->path);
setup->id = bt_bap_stream_config(setup->stream, &setup->qos,
setup->caps, config_cb, setup);
if (!setup->id) {
@@ -965,8 +931,6 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
return btd_error_invalid_args(msg);
}

- bt_bap_stream_set_user_data(setup->stream, ep->path);
-
if (setup->metadata && setup->metadata->iov_len)
bt_bap_stream_metadata(setup->stream, setup->metadata, NULL,
NULL);
@@ -977,95 +941,29 @@ static DBusMessage *set_configuration(DBusConnection *conn, DBusMessage *msg,
break;
case BT_BAP_STREAM_TYPE_BCAST:
/* No message sent over the air for broadcast */
- if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SINK)
- setup->msg = dbus_message_ref(msg);
- else {
+ if (bt_bap_pac_get_type(ep->lpac) == BT_BAP_BCAST_SOURCE)
setup->base = bt_bap_stream_get_base(setup->stream);
- setup->id = 0;
}
+ setup->id = 0;

if (ep->data->service)
service_set_connecting(ep->data->service);

return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
- }

return NULL;
}

-static void update_bcast_qos(struct bt_iso_qos *qos,
- struct bt_bap_qos *bap_qos)
-{
- bap_qos->bcast.big = qos->bcast.big;
- bap_qos->bcast.bis = qos->bcast.bis;
- bap_qos->bcast.sync_factor = qos->bcast.sync_factor;
- bap_qos->bcast.packing = qos->bcast.packing;
- bap_qos->bcast.framing = qos->bcast.framing;
- bap_qos->bcast.encryption = qos->bcast.encryption;
- bap_qos->bcast.options = qos->bcast.options;
- bap_qos->bcast.skip = qos->bcast.skip;
- bap_qos->bcast.sync_timeout = qos->bcast.sync_timeout;
- bap_qos->bcast.sync_cte_type = qos->bcast.sync_cte_type;
- bap_qos->bcast.mse = qos->bcast.mse;
- bap_qos->bcast.timeout = qos->bcast.timeout;
- bap_qos->bcast.io_qos.interval = qos->bcast.in.interval;
- bap_qos->bcast.io_qos.latency = qos->bcast.in.latency;
- bap_qos->bcast.io_qos.phy = qos->bcast.in.phy;
- bap_qos->bcast.io_qos.sdu = qos->bcast.in.sdu;
- bap_qos->bcast.io_qos.rtn = qos->bcast.in.rtn;
- if (!bap_qos->bcast.bcode)
- bap_qos->bcast.bcode = new0(struct iovec, 1);
- util_iov_memcpy(bap_qos->bcast.bcode, qos->bcast.bcode,
- sizeof(qos->bcast.bcode));
-}
-
static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data)
{
- struct bap_setup *setup = user_data;
- struct bap_ep *ep = setup->ep;
- struct bap_data *data = ep->data;
- struct bt_iso_qos qos;
- struct bt_iso_base base;
- char address[18];
+ struct bap_bcast_pa_req *req = user_data;
+ struct bap_setup *setup = req->data.setup;
int fd;
- struct iovec *base_io;
- uint32_t presDelay;
- uint8_t numSubgroups;
- uint8_t numBis;
- struct bt_bap_codec codec;

- bt_io_get(io, &err,
- BT_IO_OPT_DEST, address,
- BT_IO_OPT_QOS, &qos,
- BT_IO_OPT_BASE, &base,
- BT_IO_OPT_INVALID);
- if (err) {
- error("%s", err->message);
- g_error_free(err);
- goto drop;
- }
-
- g_io_channel_ref(io);
- btd_service_connecting_complete(data->service, 0);
- DBG("BCAST ISO: sync with %s (BIG 0x%02x BIS 0x%02x)",
- address, qos.bcast.big, qos.bcast.bis);
+ queue_remove(bcast_pa_requests, req);

- update_bcast_qos(&qos, &setup->qos);
-
- base_io = new0(struct iovec, 1);
- util_iov_memcpy(base_io, base.base, base.base_len);
-
- parse_base(base_io->iov_base, base_io->iov_len, bap_debug,
- &presDelay, &numSubgroups, &numBis,
- &codec, &setup->caps, &setup->metadata);
-
- /* Update pac with BASE information */
- bt_bap_update_bcast_source(ep->rpac, &codec, setup->caps,
- setup->metadata);
- setup->id = bt_bap_stream_config(setup->stream, &setup->qos,
- setup->caps, NULL, NULL);
-
- bt_bap_stream_set_user_data(setup->stream, ep->path);
+ /* This device is no longer needed */
+ btd_service_connecting_complete(setup->ep->data->service, 0);

fd = g_io_channel_unix_get_fd(io);

@@ -1074,26 +972,157 @@ static void iso_bcast_confirm_cb(GIOChannel *io, GError *err, void *user_data)
g_io_channel_set_close_on_unref(io, FALSE);
return;
}
+}

+static void print_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
+ void *user_data)
+{
+ util_debug(user_data, NULL, "CC #%zu: l:%u t:%u", i, l, t);
+ util_hexdump(' ', v, l, user_data, NULL);
+}

- return;
+static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
+ util_debug_func_t func)
+{
+ struct iovec iov = {
+ .iov_base = base->base,
+ .iov_len = base->base_len,
+ };
+ uint32_t pres_delay;
+ uint8_t num_subgroups;

-drop:
- g_io_channel_shutdown(io, TRUE, NULL);
+ util_debug(func, NULL, "BASE len: %ld", iov.iov_len);
+
+ if (!util_iov_pull_le24(&iov, &pres_delay))
+ return false;
+ util_debug(func, NULL, "PresentationDelay: %d", pres_delay);
+
+ if (!util_iov_pull_u8(&iov, &num_subgroups))
+ return false;
+ util_debug(func, NULL, "Number of Subgroups: %d", num_subgroups);
+
+ /* Loop subgroups */
+ for (int idx = 0; idx < num_subgroups; idx++) {
+ uint8_t num_bis;
+ struct bt_bap_codec codec;
+ struct iovec *l2_caps;
+ struct iovec *meta;

+ util_debug(func, NULL, "Subgroup #%d", idx);
+
+ if (!util_iov_pull_u8(&iov, &num_bis))
+ goto fail;
+ util_debug(func, NULL, "Number of BISes: %d", num_bis);
+
+ memcpy(&codec,
+ util_iov_pull_mem(&iov,
+ sizeof(struct bt_bap_codec)),
+ sizeof(struct bt_bap_codec));
+ util_debug(func, NULL, "Codec: ID %d CID 0x%2.2x VID 0x%2.2x",
+ codec.id, codec.cid, codec.vid);
+
+ /* Level 2 */
+ /* Read Codec Specific Configuration */
+ l2_caps = new0(struct iovec, 1);
+ if (!util_iov_pull_u8(&iov, (void *)&l2_caps->iov_len))
+ goto fail;
+
+ util_iov_memcpy(l2_caps, util_iov_pull_mem(&iov,
+ l2_caps->iov_len),
+ l2_caps->iov_len);
+
+ /* Print Codec Specific Configuration */
+ util_debug(func, NULL, "CC len: %ld", l2_caps->iov_len);
+ util_ltv_foreach(l2_caps->iov_base, l2_caps->iov_len, NULL,
+ print_ltv, func);
+
+ /* Read Metadata */
+ meta = new0(struct iovec, 1);
+ if (!util_iov_pull_u8(&iov, (void *)&meta->iov_len))
+ goto fail;
+
+ util_iov_memcpy(meta,
+ util_iov_pull_mem(&iov, meta->iov_len),
+ meta->iov_len);
+
+ /* Print Metadata */
+ util_debug(func, NULL, "Metadata len: %i",
+ (uint8_t)meta->iov_len);
+ util_hexdump(' ', meta->iov_base, meta->iov_len, func, NULL);
+
+ /* Level 3 */
+ for (; num_bis; num_bis--) {
+ uint8_t bis_index;
+ struct iovec *l3_caps;
+
+ if (!util_iov_pull_u8(&iov, &bis_index))
+ goto fail;
+
+ util_debug(func, NULL, "BIS #%d", bis_index);
+
+ /* Read Codec Specific Configuration */
+ l3_caps = new0(struct iovec, 1);
+ if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len))
+ goto fail;
+
+ util_iov_memcpy(l3_caps,
+ util_iov_pull_mem(&iov,
+ l3_caps->iov_len),
+ l3_caps->iov_len);
+
+ /* Print Codec Specific Configuration */
+ util_debug(func, NULL, "CC Len: %d",
+ (uint8_t)l3_caps->iov_len);
+ util_ltv_foreach(l3_caps->iov_base,
+ l3_caps->iov_len, NULL, print_ltv,
+ func);
+
+ /* Try to create a PAC using this BIS information */
+ bt_bap_add_bis(bap, bis_index, &codec, l2_caps, l3_caps,
+ meta);
+ }
+
+ }
+ return true;
+
+fail:
+ util_debug(func, NULL, "Unable to parse Base");
+
+ return false;
}

static void iso_pa_sync_confirm_cb(GIOChannel *io, void *user_data)
{
GError *err = NULL;
+ struct bap_bcast_pa_req *pa_req = user_data;
+ struct bap_data *data = btd_service_get_user_data(pa_req->data.service);
+ struct bt_iso_base base;
+ struct bt_iso_qos qos;

- if (!bt_io_bcast_accept(io, iso_bcast_confirm_cb,
- user_data, NULL, &err, BT_IO_OPT_INVALID)) {
- error("bt_io_bcast_accept: %s", err->message);
+ DBG("PA Sync done");
+
+ bt_io_get(io, &err,
+ BT_IO_OPT_BASE, &base,
+ BT_IO_OPT_QOS, &qos,
+ BT_IO_OPT_INVALID);
+ if (err) {
+ error("%s", err->message);
g_error_free(err);
g_io_channel_shutdown(io, TRUE, NULL);
+ return;
}

+ /* Close the io and remove the queue request for another PA Sync */
+ g_io_channel_shutdown(data->listen_io, TRUE, NULL);
+ g_io_channel_unref(data->listen_io);
+ g_io_channel_shutdown(io, TRUE, NULL);
+ data->listen_io = NULL;
+ queue_remove(bcast_pa_requests, pa_req);
+
+ /* Analyze received BASE data and create remote media endpoints for each
+ * BIS matching our capabilities
+ */
+ parse_base(data->bap, &base, bap_debug);
}

static bool match_data_bap_data(const void *data, const void *match_data)
@@ -1932,47 +1961,58 @@ static void setup_listen_io(struct bap_data *data, struct bt_bap_stream *stream,
data->listen_io = io;
}

-static void setup_listen_io_broadcast(struct bap_data *data,
- struct bap_setup *setup,
- struct bt_bap_stream *stream,
- struct bt_iso_qos *qos)
+static void check_pa_req_in_progress(void *data, void *user_data)
{
- GIOChannel *io;
- GError *err = NULL;
- struct sockaddr_iso_bc iso_bc_addr;
+ struct bap_bcast_pa_req *req = data;

- iso_bc_addr.bc_bdaddr_type = btd_device_get_bdaddr_type(data->device);
- memcpy(&iso_bc_addr.bc_bdaddr, device_get_address(data->device),
- sizeof(bdaddr_t));
- iso_bc_addr.bc_bis[0] = 1;
- iso_bc_addr.bc_num_bis = 1;
+ if (req->in_progress == TRUE)
+ *((bool *)user_data) = TRUE;
+}

- DBG("stream %p", stream);
+static int short_lived_pa_sync(struct bap_bcast_pa_req *req);
+static void pa_and_big_sync(struct bap_bcast_pa_req *req);

- /* If IO already set skip creating it again */
- if (bt_bap_stream_get_io(stream) || data->listen_io)
- return;
+static gboolean pa_idle_timer(gpointer user_data)
+{
+ struct bap_bcast_pa_req *req = user_data;
+ bool in_progress = FALSE;

- io = bt_io_listen(NULL, iso_pa_sync_confirm_cb, setup, NULL, &err,
- BT_IO_OPT_SOURCE_BDADDR,
- btd_adapter_get_address(data->adapter),
- BT_IO_OPT_DEST_BDADDR,
- device_get_address(data->device),
- BT_IO_OPT_DEST_TYPE,
- btd_device_get_bdaddr_type(data->device),
- BT_IO_OPT_MODE, BT_IO_MODE_ISO,
- BT_IO_OPT_QOS, &qos->bcast,
- BT_IO_OPT_ISO_BC_NUM_BIS, iso_bc_addr.bc_num_bis,
- BT_IO_OPT_ISO_BC_BIS, iso_bc_addr.bc_bis,
- BT_IO_OPT_INVALID);
- if (!io) {
- error("%s", err->message);
- g_error_free(err);
+ /* Handle timer if no request is in progress */
+ queue_foreach(bcast_pa_requests, check_pa_req_in_progress,
+ &in_progress);
+ if (in_progress == FALSE) {
+ req = queue_peek_head(bcast_pa_requests);
+ if (req != NULL)
+ switch (req->type) {
+ case BAP_PA_SHORT_REQ:
+ DBG("do short lived PA Sync");
+ short_lived_pa_sync(req);
+ break;
+ case BAP_PA_BIG_SYNC_REQ:
+ DBG("do PA Sync and BIG Sync");
+ pa_and_big_sync(req);
+ break;
+ }
}
- setup->io = io;
- data->listen_io = io;

+ return TRUE;
}
+
+static void setup_accept_io_broadcast(struct bap_data *data,
+ struct bap_setup *setup)
+{
+ struct bap_bcast_pa_req *pa_req = new0(struct bap_bcast_pa_req, 1);
+
+ /* Add this request to the PA queue.
+ * We don't need to check the queue here and the timer, as we cannot
+ * have BAP_PA_BIG_SYNC_REQ before a short PA (BAP_PA_SHORT_REQ)
+ */
+ pa_req->type = BAP_PA_BIG_SYNC_REQ;
+ pa_req->in_progress = FALSE;
+ pa_req->data.setup = setup;
+ queue_push_tail(bcast_pa_requests, pa_req);
+}
+
static void setup_create_ucast_io(struct bap_data *data,
struct bap_setup *setup,
struct bt_bap_stream *stream,
@@ -2035,7 +2075,7 @@ done:
if (bt_bap_pac_get_type(setup->ep->lpac) == BT_BAP_BCAST_SOURCE)
setup_connect_io_broadcast(data, setup, stream, &iso_qos);
else
- setup_listen_io_broadcast(data, setup, stream, &iso_qos);
+ setup_accept_io_broadcast(data, setup);
}

static void setup_create_io(struct bap_data *data, struct bap_setup *setup,
@@ -2148,7 +2188,10 @@ static void bap_state_bcast(struct bt_bap_stream *stream, uint8_t old_state,
case BT_BAP_STREAM_STATE_CONFIG:
if (setup && !setup->id) {
setup_create_io(data, setup, stream, true);
- if (!setup->io) {
+ /* BAP Broadcast SInk creates the io asynchronously */
+ if ((!setup->io) &&
+ (bt_bap_pac_get_type(setup->ep->lpac)
+ != BT_BAP_BCAST_SINK)) {
error("Unable to create io");
if (old_state != BT_BAP_STREAM_STATE_RELEASING)
bt_bap_stream_release(stream, NULL,
@@ -2470,32 +2513,23 @@ static void bap_detached(struct bt_bap *bap, void *user_data)
bap_data_remove(data);
}

-static int bap_bcast_probe(struct btd_service *service)
+static int short_lived_pa_sync(struct bap_bcast_pa_req *req)
{
+ struct btd_service *service = req->data.service;
struct btd_device *device = btd_service_get_device(service);
struct btd_adapter *adapter = device_get_adapter(device);
struct btd_gatt_database *database = btd_adapter_get_database(adapter);
struct bap_data *data = btd_service_get_user_data(service);
- char addr[18];
-
- ba2str(device_get_address(device), addr);
-
- if (!btd_adapter_has_exp_feature(adapter, EXP_FEAT_ISO_SOCKET)) {
- error("BAP requires ISO Socket which is not enabled");
- return -ENOTSUP;
- }
+ GError *err = NULL;

- /* Ignore, if we were probed for this device already */
if (data) {
- error("Profile probed twice for the same device!");
- return -EINVAL;
+ DBG("Already probed");
+ return -1;
}
-
data = bap_data_new(device);
data->service = service;
data->adapter = adapter;
data->device = device;
-
data->bap = bt_bap_new(btd_gatt_database_get_db(database),
btd_gatt_database_get_db(database));
if (!data->bap) {
@@ -2520,7 +2554,146 @@ static int bap_bcast_probe(struct btd_service *service)

bt_bap_set_user_data(data->bap, service);

- bt_bap_new_bcast_source(data->bap, device_get_path(device));
+ DBG("Create PA sync with this source");
+ req->in_progress = TRUE;
+ data->listen_io = bt_io_listen(NULL, iso_pa_sync_confirm_cb, req,
+ NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR,
+ btd_adapter_get_address(data->adapter),
+ BT_IO_OPT_DEST_BDADDR,
+ device_get_address(data->device),
+ BT_IO_OPT_DEST_TYPE,
+ btd_device_get_bdaddr_type(data->device),
+ BT_IO_OPT_MODE, BT_IO_MODE_ISO,
+ BT_IO_OPT_QOS, &bap_sink_pa_qos,
+ BT_IO_OPT_INVALID);
+ if (!data->listen_io) {
+ error("%s", err->message);
+ g_error_free(err);
+ }
+
+ return 0;
+}
+
+static void iso_do_big_sync(GIOChannel *io, void *user_data)
+{
+ GError *err = NULL;
+ struct bap_bcast_pa_req *req = user_data;
+ struct bap_setup *setup = req->data.setup;
+ struct bap_data *data = setup->ep->data;
+ struct sockaddr_iso_bc iso_bc_addr;
+ struct bt_iso_qos qos;
+
+ DBG("PA Sync done, do BIG Sync");
+ g_io_channel_unref(setup->io);
+ setup->io = NULL;
+
+ setup->io = io;
+ g_io_channel_ref(setup->io);
+
+ /* TODO
+ * We can only synchronize with a single BIS to a BIG.
+ * In order to have multiple BISes targeting this BIG we need to have
+ * all the BISes before doing this request. This request is triggered
+ * by an endpoint "SetConfiguration" command. For multiple BISes
+ * we need another way to specify which BISes user is requesting
+ */
+ iso_bc_addr.bc_bdaddr_type = btd_device_get_bdaddr_type(data->device);
+ memcpy(&iso_bc_addr.bc_bdaddr, device_get_address(data->device),
+ sizeof(bdaddr_t));
+ iso_bc_addr.bc_bis[0] = 1;
+ iso_bc_addr.bc_num_bis = 1;
+
+ /* Set the user requested QOS */
+ qos.bcast.big = setup->qos.bcast.big;
+ qos.bcast.bis = setup->qos.bcast.bis;
+ qos.bcast.sync_factor = setup->qos.bcast.sync_factor;
+ qos.bcast.packing = setup->qos.bcast.packing;
+ qos.bcast.framing = setup->qos.bcast.framing;
+ qos.bcast.encryption = setup->qos.bcast.encryption;
+ if (setup->qos.bcast.bcode && setup->qos.bcast.bcode->iov_base)
+ memcpy(qos.bcast.bcode, setup->qos.bcast.bcode->iov_base,
+ setup->qos.bcast.bcode->iov_len);
+ qos.bcast.options = setup->qos.bcast.options;
+ qos.bcast.skip = setup->qos.bcast.skip;
+ qos.bcast.sync_timeout = setup->qos.bcast.sync_timeout;
+ qos.bcast.sync_cte_type = setup->qos.bcast.sync_cte_type;
+ qos.bcast.mse = setup->qos.bcast.mse;
+ qos.bcast.timeout = setup->qos.bcast.timeout;
+ memcpy(&qos.bcast.out, &setup->qos.bcast.io_qos,
+ sizeof(struct bt_iso_io_qos));
+
+ if (!bt_io_set(setup->io, &err,
+ BT_IO_OPT_QOS, &qos,
+ BT_IO_OPT_INVALID)) {
+ error("bt_io_set: %s", err->message);
+ g_error_free(err);
+ }
+
+ if (!bt_io_bcast_accept(setup->io,
+ iso_bcast_confirm_cb,
+ req, NULL, &err,
+ BT_IO_OPT_ISO_BC_NUM_BIS,
+ iso_bc_addr.bc_num_bis, BT_IO_OPT_ISO_BC_BIS,
+ iso_bc_addr.bc_bis, BT_IO_OPT_INVALID)) {
+ error("bt_io_bcast_accept: %s", err->message);
+ g_error_free(err);
+ }
+}
+
+static void pa_and_big_sync(struct bap_bcast_pa_req *req)
+{
+ GError *err = NULL;
+ struct bap_setup *setup = req->data.setup;
+ struct bap_data *data = setup->ep->data;
+
+ req->in_progress = TRUE;
+
+ DBG("Create PA sync with this source");
+ setup->io = bt_io_listen(NULL, iso_do_big_sync, req,
+ NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR,
+ btd_adapter_get_address(data->adapter),
+ BT_IO_OPT_DEST_BDADDR,
+ device_get_address(data->device),
+ BT_IO_OPT_DEST_TYPE,
+ btd_device_get_bdaddr_type(data->device),
+ BT_IO_OPT_MODE, BT_IO_MODE_ISO,
+ BT_IO_OPT_QOS, &bap_sink_pa_qos,
+ BT_IO_OPT_INVALID);
+ if (!setup->io) {
+ error("%s", err->message);
+ g_error_free(err);
+ }
+}
+
+static int bap_bcast_probe(struct btd_service *service)
+{
+ struct btd_device *device = btd_service_get_device(service);
+ struct btd_adapter *adapter = device_get_adapter(device);
+ struct bap_bcast_pa_req *pa_req =
+ new0(struct bap_bcast_pa_req, 1);
+
+ if (!btd_adapter_has_exp_feature(adapter, EXP_FEAT_ISO_SOCKET)) {
+ error("BAP requires ISO Socket which is not enabled");
+ return -ENOTSUP;
+ }
+
+ /* First time initialize the queue and start the idle timer */
+ if (bcast_pa_requests == NULL) {
+ bcast_pa_requests = queue_new();
+ pa_timer_id = g_timeout_add_seconds(PA_IDLE_TIMEOUT,
+ pa_idle_timer, NULL);
+ }
+
+ /* Enqueue this device advertisement so that we can do short-lived
+ */
+ DBG("enqueue service: %p", service);
+ pa_req->type = BAP_PA_SHORT_REQ;
+ pa_req->in_progress = FALSE;
+ pa_req->data.service = service;
+ queue_push_tail(bcast_pa_requests, pa_req);
+
return 0;
}

--
2.40.1


2024-02-23 17:33:10

by bluez.test.bot

[permalink] [raw]
Subject: RE: Update Sink BASE management

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=829146

---Test result---

Test Summary:
CheckPatch PASS 1.43 seconds
GitLint PASS 0.63 seconds
BuildEll PASS 24.47 seconds
BluezMake PASS 715.60 seconds
MakeCheck PASS 11.57 seconds
MakeDistcheck PASS 165.54 seconds
CheckValgrind PASS 229.04 seconds
CheckSmatch PASS 333.01 seconds
bluezmakeextell PASS 108.83 seconds
IncrementalBuild PASS 1352.76 seconds
ScanBuild WARNING 968.20 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
src/shared/bap.c:1154:2: warning: Use of memory after it is freed
DBG(stream->bap, "stream %p", stream);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/bap.c:40:2: note: expanded from macro 'DBG'
bap_debug(_bap, "%s:%s() " fmt, __FILE__, __func__, ## arg)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.



---
Regards,
Linux Bluetooth

2024-02-27 15:02:46

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an observed BIS

Hi Andrei,

On Fri, Feb 23, 2024 at 10:34 AM Andrei Istodorescu
<[email protected]> wrote:
>
> Add API to add a PAC for each observed BIS that is supported by the
> local PACS data.
> Each BIS observed capabilities LTV is compared to the local capabilities
> and when we have a full LTVs match a PAC record is created for that BIS.
> Also a MediaEndpoint is registered over DBUS and the stream can be
> enabled using the SetConfiguration DBUS call.
> ---
> src/shared/bap.c | 304 ++++++++++++++++++++++++++++++++++++++++++++---
> src/shared/bap.h | 13 +-
> 2 files changed, 302 insertions(+), 15 deletions(-)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index f5fc1402701c..853919111835 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -48,6 +48,15 @@
>
> #define BAP_PROCESS_TIMEOUT 10
>
> +#define BAP_FREQ_LTV_TYPE 1
> +#define BAP_DURATION_LTV_TYPE 2
> +#define BAP_CHANNEL_ALLOCATION_LTV_TYPE 3
> +#define BAP_FRAME_LEN_LTV_TYPE 4
> +#define CODEC_SPECIFIC_CONFIGURATION_MASK (\
> + (1<<BAP_FREQ_LTV_TYPE)|\
> + (1<<BAP_DURATION_LTV_TYPE)|\
> + (1<<BAP_FRAME_LEN_LTV_TYPE))
> +
> struct bt_bap_pac_changed {
> unsigned int id;
> bt_bap_pac_func_t added;
> @@ -1692,11 +1701,8 @@ static unsigned int bap_bcast_config(struct bt_bap_stream *stream,
> bt_bap_stream_func_t func, void *user_data)
> {
> stream->qos = *qos;
> - if (stream->lpac->type == BT_BAP_BCAST_SINK) {
> - if (data)
> - stream_config(stream, data, NULL);
> - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> - }
> + stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> + ep_config_cb, stream->lpac->user_data);
>
> return 1;
> }
> @@ -3302,6 +3308,13 @@ static void bap_add_broadcast_source(struct bt_bap_pac *pac)
> static void bap_add_broadcast_sink(struct bt_bap_pac *pac)
> {
> queue_push_tail(pac->bdb->broadcast_sinks, pac);
> +
> + /* Update local PACS for broadcast sink also, when registering an
> + * endpoint
> + */
> + pacs_add_sink_location(pac->bdb->pacs, pac->qos.location);
> + pacs_add_sink_supported_context(pac->bdb->pacs,
> + pac->qos.supported_context);
> }
>
> static void notify_pac_added(void *data, void *user_data)
> @@ -3453,6 +3466,16 @@ struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac)
> return &pac->qos;
> }
>
> +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac)
> +{
> + return pac->data;
> +}
> +
> +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac)
> +{
> + return pac->metadata;
> +}
> +
> uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
> {
> if (!stream)
> @@ -5253,10 +5276,6 @@ bool bt_bap_stream_set_user_data(struct bt_bap_stream *stream, void *user_data)
>
> stream->user_data = user_data;
>
> - if (bt_bap_stream_get_type(stream) == BT_BAP_STREAM_TYPE_BCAST)
> - stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> - ep_config_cb, stream->lpac->user_data);
> -
> return true;
> }
>
> @@ -5892,8 +5911,9 @@ static void add_new_subgroup(struct bt_base *base,
>
> struct bt_ltv_match {
> uint8_t l;
> - uint8_t *v;
> + void *data;
> bool found;
> + uint32_t data32;
> };
>
> struct bt_ltv_search {
> @@ -5912,7 +5932,7 @@ static void match_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> if (ltv_match->l != l)
> return;
>
> - if (!memcmp(v, ltv_match->v, l))
> + if (!memcmp(v, ltv_match->data, l))
> ltv_match->found = true;
> }
>
> @@ -5924,7 +5944,7 @@ static void search_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
>
> ltv_match.found = false;
> ltv_match.l = l;
> - ltv_match.v = v;
> + ltv_match.data = v;
>
> util_ltv_foreach(ltv_search->iov->iov_base,
> ltv_search->iov->iov_len, &t,
> @@ -5965,8 +5985,10 @@ static bool compare_ltv(struct iovec *iov1,
> }
>
> struct bt_ltv_extract {
> - struct iovec *result;
> struct iovec *src;
> + void *value;
> + uint8_t len;
> + struct iovec *result;
> };
>
> static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> @@ -5978,7 +6000,7 @@ static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
>
> ltv_match.found = false;
> ltv_match.l = l;
> - ltv_match.v = v;
> + ltv_match.data = v;
>
> /* Search each BIS caps ltv in subgroup caps
> * to extract the one that are BIS specific
> @@ -6111,3 +6133,257 @@ struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream)
>
> return base_iov;
> }
> +
> +static void bap_sink_get_allocation(size_t i, uint8_t l, uint8_t t,
> + uint8_t *v, void *user_data)
> +{
> + uint32_t location32;
> +
> + if (!v)
> + return;
> +
> + memcpy(&location32, v, l);
> + *((uint32_t *)user_data) = le32_to_cpu(location32);
> +}
> +
> +/*
> + * This function compares PAC Codec Specific Capabilities, with the Codec
> + * Specific Configuration LTVs received in the BASE of the BAP Source. The
> + * result is accumulated in data32 which is a bitmask of types.
> + */
> +static void check_pac_caps_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> + void *user_data)
> +{
> + struct bt_ltv_match *compare_data = user_data;
> + uint8_t *bis_v = compare_data->data;
> +
> + switch (t) {
> + case BAP_FREQ_LTV_TYPE:
> + {
> + uint16_t mask = *((uint16_t *)v);
> +
> + mask = le16_to_cpu(mask);
> + if (mask & (1 << (bis_v[0] - 1)))
> + compare_data->data32 |= 1<<t;
> + }
> + break;
> + case BAP_DURATION_LTV_TYPE:
> + if ((v[0]) & (1 << bis_v[0]))
> + compare_data->data32 |= 1<<t;
> + break;
> + case BAP_FRAME_LEN_LTV_TYPE:
> + {
> + uint16_t min = *((uint16_t *)v);
> + uint16_t max = *((uint16_t *)(&v[2]));
> + uint16_t frame_len = *((uint16_t *)bis_v);
> +
> + min = le16_to_cpu(min);
> + max = le16_to_cpu(max);
> + frame_len = le16_to_cpu(frame_len);
> + if ((frame_len >= min) &&
> + (frame_len <= max))
> + compare_data->data32 |= 1<<t;
> + }

Don't create new scopes inside a switch statement, that makes it hard
to follow the code, if you really think that would help have a helper
function to check each field.

> + break;
> + }
> +}
> +
> +static void check_source_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> + void *user_data)
> +{
> + struct bt_ltv_match *local_data = user_data;
> + struct iovec *pac_caps = local_data->data;
> + struct bt_ltv_match compare_data;
> +
> + compare_data.data = v;
> +
> + /* Search inside local PAC's caps for LTV of type t */
> + util_ltv_foreach(pac_caps->iov_base, pac_caps->iov_len, &t,
> + check_pac_caps_ltv, &compare_data);
> +
> + local_data->data32 |= compare_data.data32;
> +}
> +
> +static void bap_sink_check_level3_ltv(size_t i, uint8_t l, uint8_t t,
> + uint8_t *v, void *user_data)
> +{
> + struct bt_ltv_extract *merge_data = user_data;
> +
> + merge_data->value = v;
> + merge_data->len = l;
> +}
> +
> +static void bap_push_ltv(struct iovec *output, uint8_t l, uint8_t t, void *v)
> +{
> + l++;
> + iov_append(output, 1, &l);
> + iov_append(output, 1, &t);
> + iov_append(output, l - 1, v);
> +}

Perhaps we should create a helper function to do this sort of thing,
maybe util_ltv_push?

> +static void bap_sink_check_level2_ltv(size_t i, uint8_t l, uint8_t t,
> + uint8_t *v, void *user_data)
> +{
> + struct bt_ltv_extract *merge_data = user_data;
> +
> + merge_data->value = NULL;
> + util_ltv_foreach(merge_data->src->iov_base,
> + merge_data->src->iov_len,
> + &t,
> + bap_sink_check_level3_ltv, user_data);
> +
> + /* If the LTV at level 2 was found at level 3 add the one from level 3,
> + * otherwise add the one at level 2
> + */
> + if (merge_data->value)
> + bap_push_ltv(merge_data->result, merge_data->len,
> + t, merge_data->value);
> + else
> + bap_push_ltv(merge_data->result, l, t, v);
> +}
> +
> +static void check_local_pac(void *data, void *user_data)
> +{
> + struct bt_ltv_match *compare_data = user_data;
> + struct iovec *bis_data = (struct iovec *)compare_data->data;
> + const struct bt_bap_pac *pac = data;
> +
> + /* Keep searching for a matching PAC if one wasn't found
> + * in previous PAC element
> + */
> + if (compare_data->found == false) {
> + struct bt_ltv_match bis_compare_data = {
> + .data = pac->data,
> + .data32 = 0, /* LTVs bitmask result */
> + .found = false
> + };
> +
> + /* loop each BIS LTV */
> + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, NULL,
> + check_source_ltv, &bis_compare_data);
> +
> + /* We have a match if all selected LTVs have a match */
> + if ((bis_compare_data.data32 &
> + CODEC_SPECIFIC_CONFIGURATION_MASK) ==
> + CODEC_SPECIFIC_CONFIGURATION_MASK)
> + compare_data->found = true;
> + }
> +}
> +
> +static void bap_sink_match_allocation(size_t i, uint8_t l, uint8_t t,
> + uint8_t *v, void *user_data)
> +{
> + struct bt_ltv_match *data = user_data;
> + uint32_t location32;
> +
> + if (!v)
> + return;
> +
> + memcpy(&location32, v, l);
> + location32 = le32_to_cpu(location32);
> +
> + /* If all the bits in the received bitmask are found in
> + * the local bitmask then we have a match
> + */
> + if ((location32 & data->data32) == location32)
> + data->found = true;
> + else
> + data->found = false;
> +}
> +
> +static bool bap_check_bis(struct bt_bap_db *ldb, struct iovec *bis_data)
> +{
> + struct bt_ltv_match compare_data = {};
> +
> + /* Check channel allocation against the PACS location.
> + * If we don't have a location set we can accept any BIS location.
> + * If the BIS doesn't have a location set we also accept it
> + */
> + compare_data.found = true;
> +
> + if (ldb->pacs->sink_loc_value) {
> + uint8_t type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> +
> + compare_data.data32 = ldb->pacs->sink_loc_value;
> + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, &type,
> + bap_sink_match_allocation, &compare_data);
> + }
> +
> + /* Check remaining LTVs against the PACs list */
> + if (compare_data.found) {
> + compare_data.data = bis_data;
> + compare_data.found = false;
> + queue_foreach(ldb->broadcast_sinks, check_local_pac,
> + &compare_data);
> + }
> +
> + return compare_data.found;
> +}
> +
> +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> + struct bt_bap_codec *codec,
> + struct iovec *l2_caps,
> + struct iovec *l3_caps,
> + struct iovec *meta)
> +{
> + struct bt_bap_pac *pac_source_bis;
> + struct bt_bap_endpoint *ep;
> + int err = 0;
> + struct bt_bap_pac_qos bis_qos = {0};
> + uint8_t type = 0;
> + struct bt_ltv_extract merge_data = {0};
> +
> + merge_data.src = l3_caps;
> + merge_data.result = new0(struct iovec, 1);
> +
> + /* Create a Codec Specific Configuration with LTVs at level 2 (subgroup)
> + * overwritten by LTVs at level 3 (BIS)
> + */
> + util_ltv_foreach(l2_caps->iov_base,
> + l2_caps->iov_len,
> + NULL,
> + bap_sink_check_level2_ltv, &merge_data);
> +
> + /* Check each BIS Codec Specific Configuration LTVs against our Codec
> + * Specific Capabilities and if the BIS matches create a PAC with it
> + */
> + if (bap_check_bis(bap->ldb, merge_data.result) == false)
> + goto cleanup;
> +
> + DBG(bap, "Matching BIS %i", bis_index);
> +
> + /* Create a QoS structure based on the received BIS information to
> + * specify the desired channel for this BIS/PAC
> + */
> + type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> + util_ltv_foreach(merge_data.result->iov_base,
> + merge_data.result->iov_len, &type,
> + bap_sink_get_allocation, &bis_qos.location);

Isn't it better to parse this inline with the use of util_iov_pull_*?
If you don't want to change the iovec passed, which shall be made a
const if that is intention, then just create a dup and parse it.

> + /* Create a remote PAC */
> + pac_source_bis = bap_pac_new(bap->rdb, NULL,
> + BT_BAP_BCAST_SOURCE, codec, &bis_qos,
> + merge_data.result, meta);
> +
> + err = asprintf(&pac_source_bis->name, "%d", bis_index);
> +
> + if (err < 0) {
> + DBG(bap, "error in asprintf");
> + goto cleanup;
> + }
> +
> + /* Add remote source endpoint */
> + if (!bap->rdb->broadcast_sources)
> + bap->rdb->broadcast_sources = queue_new();
> + queue_push_tail(bap->rdb->broadcast_sources, pac_source_bis);
> +
> + queue_foreach(bap->pac_cbs, notify_pac_added, pac_source_bis);
> + /* Push remote endpoint with direction sink */
> + ep = bap_endpoint_new_broadcast(bap->rdb, BT_BAP_BCAST_SINK);
> +
> + if (ep)
> + queue_push_tail(bap->remote_eps, ep);
> +
> +cleanup:
> + util_iov_free(merge_data.result, 1);
> +}
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index 2c3550921f07..b2826719f84f 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -4,7 +4,7 @@
> * BlueZ - Bluetooth protocol stack for Linux
> *
> * Copyright (C) 2022 Intel Corporation. All rights reserved.
> - * Copyright 2023 NXP
> + * Copyright 2023-2024 NXP
> *
> */
>
> @@ -175,6 +175,10 @@ uint16_t bt_bap_pac_get_context(struct bt_bap_pac *pac);
>
> struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac);
>
> +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac);
> +
> +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);

Have these 2 functions above in a separate patch.

> uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
>
> struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> @@ -323,3 +327,10 @@ void bt_bap_update_bcast_source(struct bt_bap_pac *pac,
> bool bt_bap_pac_bcast_is_local(struct bt_bap *bap, struct bt_bap_pac *pac);
>
> struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream);
> +
> +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> + struct bt_bap_codec *codec,
> + struct iovec *l2_caps,
> + struct iovec *l3_caps,
> + struct iovec *meta);
> +
> --
> 2.40.1
>


--
Luiz Augusto von Dentz

2024-02-28 15:12:56

by Andrei Istodorescu

[permalink] [raw]
Subject: Re: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an observed BIS

Hi Luiz,

I have fixed all the comments excepting one. Please see inline.

> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Tuesday, February 27, 2024 4:39 PM
> To: Andrei Istodorescu <[email protected]>
> Cc: [email protected]; Mihai-Octavian Urzica <mihai-
> [email protected]>; Silviu Florian Barbulescu
> <[email protected]>; Vlad Pruteanu <[email protected]>;
> Iulia Tanasescu <[email protected]>
> Subject: Re: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an
> observed BIS
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> Hi Andrei,
>
> On Fri, Feb 23, 2024 at 10:34 AM Andrei Istodorescu
> <[email protected]> wrote:
> >
> > Add API to add a PAC for each observed BIS that is supported by the
> > local PACS data.
> > Each BIS observed capabilities LTV is compared to the local
> > capabilities and when we have a full LTVs match a PAC record is created for
> that BIS.
> > Also a MediaEndpoint is registered over DBUS and the stream can be
> > enabled using the SetConfiguration DBUS call.
> > ---
> > src/shared/bap.c | 304
> > ++++++++++++++++++++++++++++++++++++++++++++---
> > src/shared/bap.h | 13 +-
> > 2 files changed, 302 insertions(+), 15 deletions(-)
> >
> > diff --git a/src/shared/bap.c b/src/shared/bap.c index
> > f5fc1402701c..853919111835 100644
> > --- a/src/shared/bap.c
> > +++ b/src/shared/bap.c
> > @@ -48,6 +48,15 @@
> >
> > #define BAP_PROCESS_TIMEOUT 10
> >
> > +#define BAP_FREQ_LTV_TYPE 1
> > +#define BAP_DURATION_LTV_TYPE 2
> > +#define BAP_CHANNEL_ALLOCATION_LTV_TYPE 3 #define
> > +BAP_FRAME_LEN_LTV_TYPE 4 #define
> CODEC_SPECIFIC_CONFIGURATION_MASK (\
> > + (1<<BAP_FREQ_LTV_TYPE)|\
> > + (1<<BAP_DURATION_LTV_TYPE)|\
> > + (1<<BAP_FRAME_LEN_LTV_TYPE))
> > +
> > struct bt_bap_pac_changed {
> > unsigned int id;
> > bt_bap_pac_func_t added;
> > @@ -1692,11 +1701,8 @@ static unsigned int bap_bcast_config(struct
> bt_bap_stream *stream,
> > bt_bap_stream_func_t func, void
> > *user_data) {
> > stream->qos = *qos;
> > - if (stream->lpac->type == BT_BAP_BCAST_SINK) {
> > - if (data)
> > - stream_config(stream, data, NULL);
> > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > - }
> > + stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> > + ep_config_cb, stream->lpac->user_data);
> >
> > return 1;
> > }
> > @@ -3302,6 +3308,13 @@ static void bap_add_broadcast_source(struct
> > bt_bap_pac *pac) static void bap_add_broadcast_sink(struct bt_bap_pac
> > *pac) {
> > queue_push_tail(pac->bdb->broadcast_sinks, pac);
> > +
> > + /* Update local PACS for broadcast sink also, when registering an
> > + * endpoint
> > + */
> > + pacs_add_sink_location(pac->bdb->pacs, pac->qos.location);
> > + pacs_add_sink_supported_context(pac->bdb->pacs,
> > + pac->qos.supported_context);
> > }
> >
> > static void notify_pac_added(void *data, void *user_data) @@ -3453,6
> > +3466,16 @@ struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct
> bt_bap_pac *pac)
> > return &pac->qos;
> > }
> >
> > +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac) {
> > + return pac->data;
> > +}
> > +
> > +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac) {
> > + return pac->metadata;
> > +}
> > +
> > uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream) {
> > if (!stream)
> > @@ -5253,10 +5276,6 @@ bool bt_bap_stream_set_user_data(struct
> > bt_bap_stream *stream, void *user_data)
> >
> > stream->user_data = user_data;
> >
> > - if (bt_bap_stream_get_type(stream) ==
> BT_BAP_STREAM_TYPE_BCAST)
> > - stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> > - ep_config_cb, stream->lpac->user_data);
> > -
> > return true;
> > }
> >
> > @@ -5892,8 +5911,9 @@ static void add_new_subgroup(struct bt_base
> > *base,
> >
> > struct bt_ltv_match {
> > uint8_t l;
> > - uint8_t *v;
> > + void *data;
> > bool found;
> > + uint32_t data32;
> > };
> >
> > struct bt_ltv_search {
> > @@ -5912,7 +5932,7 @@ static void match_ltv(size_t i, uint8_t l, uint8_t t,
> uint8_t *v,
> > if (ltv_match->l != l)
> > return;
> >
> > - if (!memcmp(v, ltv_match->v, l))
> > + if (!memcmp(v, ltv_match->data, l))
> > ltv_match->found = true; }
> >
> > @@ -5924,7 +5944,7 @@ static void search_ltv(size_t i, uint8_t l,
> > uint8_t t, uint8_t *v,
> >
> > ltv_match.found = false;
> > ltv_match.l = l;
> > - ltv_match.v = v;
> > + ltv_match.data = v;
> >
> > util_ltv_foreach(ltv_search->iov->iov_base,
> > ltv_search->iov->iov_len, &t, @@ -5965,8
> > +5985,10 @@ static bool compare_ltv(struct iovec *iov1, }
> >
> > struct bt_ltv_extract {
> > - struct iovec *result;
> > struct iovec *src;
> > + void *value;
> > + uint8_t len;
> > + struct iovec *result;
> > };
> >
> > static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > @@ -5978,7 +6000,7 @@ static void extract_ltv(size_t i, uint8_t l,
> > uint8_t t, uint8_t *v,
> >
> > ltv_match.found = false;
> > ltv_match.l = l;
> > - ltv_match.v = v;
> > + ltv_match.data = v;
> >
> > /* Search each BIS caps ltv in subgroup caps
> > * to extract the one that are BIS specific @@ -6111,3
> > +6133,257 @@ struct iovec *bt_bap_stream_get_base(struct
> bt_bap_stream
> > *stream)
> >
> > return base_iov;
> > }
> > +
> > +static void bap_sink_get_allocation(size_t i, uint8_t l, uint8_t t,
> > + uint8_t *v, void *user_data) {
> > + uint32_t location32;
> > +
> > + if (!v)
> > + return;
> > +
> > + memcpy(&location32, v, l);
> > + *((uint32_t *)user_data) = le32_to_cpu(location32); }
> > +
> > +/*
> > + * This function compares PAC Codec Specific Capabilities, with the
> > +Codec
> > + * Specific Configuration LTVs received in the BASE of the BAP
> > +Source. The
> > + * result is accumulated in data32 which is a bitmask of types.
> > + */
> > +static void check_pac_caps_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > + void *user_data) {
> > + struct bt_ltv_match *compare_data = user_data;
> > + uint8_t *bis_v = compare_data->data;
> > +
> > + switch (t) {
> > + case BAP_FREQ_LTV_TYPE:
> > + {
> > + uint16_t mask = *((uint16_t *)v);
> > +
> > + mask = le16_to_cpu(mask);
> > + if (mask & (1 << (bis_v[0] - 1)))
> > + compare_data->data32 |= 1<<t;
> > + }
> > + break;
> > + case BAP_DURATION_LTV_TYPE:
> > + if ((v[0]) & (1 << bis_v[0]))
> > + compare_data->data32 |= 1<<t;
> > + break;
> > + case BAP_FRAME_LEN_LTV_TYPE:
> > + {
> > + uint16_t min = *((uint16_t *)v);
> > + uint16_t max = *((uint16_t *)(&v[2]));
> > + uint16_t frame_len = *((uint16_t *)bis_v);
> > +
> > + min = le16_to_cpu(min);
> > + max = le16_to_cpu(max);
> > + frame_len = le16_to_cpu(frame_len);
> > + if ((frame_len >= min) &&
> > + (frame_len <= max))
> > + compare_data->data32 |= 1<<t;
> > + }
>
> Don't create new scopes inside a switch statement, that makes it hard to
> follow the code, if you really think that would help have a helper function to
> check each field.
>
> > + break;
> > + }
> > +}
> > +
> > +static void check_source_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > + void *user_data) {
> > + struct bt_ltv_match *local_data = user_data;
> > + struct iovec *pac_caps = local_data->data;
> > + struct bt_ltv_match compare_data;
> > +
> > + compare_data.data = v;
> > +
> > + /* Search inside local PAC's caps for LTV of type t */
> > + util_ltv_foreach(pac_caps->iov_base, pac_caps->iov_len, &t,
> > + check_pac_caps_ltv,
> > + &compare_data);
> > +
> > + local_data->data32 |= compare_data.data32; }
> > +
> > +static void bap_sink_check_level3_ltv(size_t i, uint8_t l, uint8_t t,
> > + uint8_t *v, void *user_data) {
> > + struct bt_ltv_extract *merge_data = user_data;
> > +
> > + merge_data->value = v;
> > + merge_data->len = l;
> > +}
> > +
> > +static void bap_push_ltv(struct iovec *output, uint8_t l, uint8_t t,
> > +void *v) {
> > + l++;
> > + iov_append(output, 1, &l);
> > + iov_append(output, 1, &t);
> > + iov_append(output, l - 1, v);
> > +}
>
> Perhaps we should create a helper function to do this sort of thing, maybe
> util_ltv_push?
>
> > +static void bap_sink_check_level2_ltv(size_t i, uint8_t l, uint8_t t,
> > + uint8_t *v, void *user_data) {
> > + struct bt_ltv_extract *merge_data = user_data;
> > +
> > + merge_data->value = NULL;
> > + util_ltv_foreach(merge_data->src->iov_base,
> > + merge_data->src->iov_len,
> > + &t,
> > + bap_sink_check_level3_ltv, user_data);
> > +
> > + /* If the LTV at level 2 was found at level 3 add the one from level 3,
> > + * otherwise add the one at level 2
> > + */
> > + if (merge_data->value)
> > + bap_push_ltv(merge_data->result, merge_data->len,
> > + t, merge_data->value);
> > + else
> > + bap_push_ltv(merge_data->result, l, t, v); }
> > +
> > +static void check_local_pac(void *data, void *user_data) {
> > + struct bt_ltv_match *compare_data = user_data;
> > + struct iovec *bis_data = (struct iovec *)compare_data->data;
> > + const struct bt_bap_pac *pac = data;
> > +
> > + /* Keep searching for a matching PAC if one wasn't found
> > + * in previous PAC element
> > + */
> > + if (compare_data->found == false) {
> > + struct bt_ltv_match bis_compare_data = {
> > + .data = pac->data,
> > + .data32 = 0, /* LTVs bitmask result */
> > + .found = false
> > + };
> > +
> > + /* loop each BIS LTV */
> > + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, NULL,
> > + check_source_ltv, &bis_compare_data);
> > +
> > + /* We have a match if all selected LTVs have a match */
> > + if ((bis_compare_data.data32 &
> > + CODEC_SPECIFIC_CONFIGURATION_MASK) ==
> > + CODEC_SPECIFIC_CONFIGURATION_MASK)
> > + compare_data->found = true;
> > + }
> > +}
> > +
> > +static void bap_sink_match_allocation(size_t i, uint8_t l, uint8_t t,
> > + uint8_t *v, void *user_data) {
> > + struct bt_ltv_match *data = user_data;
> > + uint32_t location32;
> > +
> > + if (!v)
> > + return;
> > +
> > + memcpy(&location32, v, l);
> > + location32 = le32_to_cpu(location32);
> > +
> > + /* If all the bits in the received bitmask are found in
> > + * the local bitmask then we have a match
> > + */
> > + if ((location32 & data->data32) == location32)
> > + data->found = true;
> > + else
> > + data->found = false;
> > +}
> > +
> > +static bool bap_check_bis(struct bt_bap_db *ldb, struct iovec
> > +*bis_data) {
> > + struct bt_ltv_match compare_data = {};
> > +
> > + /* Check channel allocation against the PACS location.
> > + * If we don't have a location set we can accept any BIS location.
> > + * If the BIS doesn't have a location set we also accept it
> > + */
> > + compare_data.found = true;
> > +
> > + if (ldb->pacs->sink_loc_value) {
> > + uint8_t type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> > +
> > + compare_data.data32 = ldb->pacs->sink_loc_value;
> > + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, &type,
> > + bap_sink_match_allocation, &compare_data);
> > + }
> > +
> > + /* Check remaining LTVs against the PACs list */
> > + if (compare_data.found) {
> > + compare_data.data = bis_data;
> > + compare_data.found = false;
> > + queue_foreach(ldb->broadcast_sinks, check_local_pac,
> > + &compare_data);
> > + }
> > +
> > + return compare_data.found;
> > +}
> > +
> > +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> > + struct bt_bap_codec *codec,
> > + struct iovec *l2_caps,
> > + struct iovec *l3_caps,
> > + struct iovec *meta)
> > +{
> > + struct bt_bap_pac *pac_source_bis;
> > + struct bt_bap_endpoint *ep;
> > + int err = 0;
> > + struct bt_bap_pac_qos bis_qos = {0};
> > + uint8_t type = 0;
> > + struct bt_ltv_extract merge_data = {0};
> > +
> > + merge_data.src = l3_caps;
> > + merge_data.result = new0(struct iovec, 1);
> > +
> > + /* Create a Codec Specific Configuration with LTVs at level 2 (subgroup)
> > + * overwritten by LTVs at level 3 (BIS)
> > + */
> > + util_ltv_foreach(l2_caps->iov_base,
> > + l2_caps->iov_len,
> > + NULL,
> > + bap_sink_check_level2_ltv, &merge_data);
> > +
> > + /* Check each BIS Codec Specific Configuration LTVs against our Codec
> > + * Specific Capabilities and if the BIS matches create a PAC with it
> > + */
> > + if (bap_check_bis(bap->ldb, merge_data.result) == false)
> > + goto cleanup;
> > +
> > + DBG(bap, "Matching BIS %i", bis_index);
> > +
> > + /* Create a QoS structure based on the received BIS information to
> > + * specify the desired channel for this BIS/PAC
> > + */
> > + type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> > + util_ltv_foreach(merge_data.result->iov_base,
> > + merge_data.result->iov_len, &type,
> > + bap_sink_get_allocation, &bis_qos.location);
>
> Isn't it better to parse this inline with the use of util_iov_pull_*?
> If you don't want to change the iovec passed, which shall be made a const if
> that is intention, then just create a dup and parse it.
merge_data.result is the concatenated list for Capabilities LTVs. We need to
extract the value for the Allocation LTV so that we pass it to the stream qos.
I used util_ltv_foreach to easily access the ltv. Do you see another way?
Maybe I didn't understand your comment.

>
> > + /* Create a remote PAC */
> > + pac_source_bis = bap_pac_new(bap->rdb, NULL,
> > + BT_BAP_BCAST_SOURCE, codec, &bis_qos,
> > + merge_data.result, meta);
> > +
> > + err = asprintf(&pac_source_bis->name, "%d", bis_index);
> > +
> > + if (err < 0) {
> > + DBG(bap, "error in asprintf");
> > + goto cleanup;
> > + }
> > +
> > + /* Add remote source endpoint */
> > + if (!bap->rdb->broadcast_sources)
> > + bap->rdb->broadcast_sources = queue_new();
> > + queue_push_tail(bap->rdb->broadcast_sources, pac_source_bis);
> > +
> > + queue_foreach(bap->pac_cbs, notify_pac_added, pac_source_bis);
> > + /* Push remote endpoint with direction sink */
> > + ep = bap_endpoint_new_broadcast(bap->rdb, BT_BAP_BCAST_SINK);
> > +
> > + if (ep)
> > + queue_push_tail(bap->remote_eps, ep);
> > +
> > +cleanup:
> > + util_iov_free(merge_data.result, 1); }
> > diff --git a/src/shared/bap.h b/src/shared/bap.h index
> > 2c3550921f07..b2826719f84f 100644
> > --- a/src/shared/bap.h
> > +++ b/src/shared/bap.h
> > @@ -4,7 +4,7 @@
> > * BlueZ - Bluetooth protocol stack for Linux
> > *
> > * Copyright (C) 2022 Intel Corporation. All rights reserved.
> > - * Copyright 2023 NXP
> > + * Copyright 2023-2024 NXP
> > *
> > */
> >
> > @@ -175,6 +175,10 @@ uint16_t bt_bap_pac_get_context(struct
> bt_bap_pac
> > *pac);
> >
> > struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac);
> >
> > +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac);
> > +
> > +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
>
> Have these 2 functions above in a separate patch.
>
> > uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
> >
> > struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> > @@ -323,3 +327,10 @@ void bt_bap_update_bcast_source(struct
> bt_bap_pac
> > *pac, bool bt_bap_pac_bcast_is_local(struct bt_bap *bap, struct
> > bt_bap_pac *pac);
> >
> > struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream);
> > +
> > +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> > + struct bt_bap_codec *codec,
> > + struct iovec *l2_caps,
> > + struct iovec *l3_caps,
> > + struct iovec *meta);
> > +
> > --
> > 2.40.1
> >
>
>
> --
> Luiz Augusto von Dentz

Regards,
Andrei

2024-02-28 15:42:39

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an observed BIS

Hi Andrei,

On Wed, Feb 28, 2024 at 10:12 AM Andrei Istodorescu
<[email protected]> wrote:
>
> Hi Luiz,
>
> I have fixed all the comments excepting one. Please see inline.
>
> > -----Original Message-----
> > From: Luiz Augusto von Dentz <[email protected]>
> > Sent: Tuesday, February 27, 2024 4:39 PM
> > To: Andrei Istodorescu <[email protected]>
> > Cc: [email protected]; Mihai-Octavian Urzica <mihai-
> > [email protected]>; Silviu Florian Barbulescu
> > <[email protected]>; Vlad Pruteanu <[email protected]>;
> > Iulia Tanasescu <[email protected]>
> > Subject: Re: [PATCH BlueZ v4 1/2] shared/bap: Add API to add an
> > observed BIS
> >
> > Caution: This is an external email. Please take care when clicking links or
> > opening attachments. When in doubt, report the message using the 'Report
> > this email' button
> >
> >
> > Hi Andrei,
> >
> > On Fri, Feb 23, 2024 at 10:34 AM Andrei Istodorescu
> > <[email protected]> wrote:
> > >
> > > Add API to add a PAC for each observed BIS that is supported by the
> > > local PACS data.
> > > Each BIS observed capabilities LTV is compared to the local
> > > capabilities and when we have a full LTVs match a PAC record is created for
> > that BIS.
> > > Also a MediaEndpoint is registered over DBUS and the stream can be
> > > enabled using the SetConfiguration DBUS call.
> > > ---
> > > src/shared/bap.c | 304
> > > ++++++++++++++++++++++++++++++++++++++++++++---
> > > src/shared/bap.h | 13 +-
> > > 2 files changed, 302 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/src/shared/bap.c b/src/shared/bap.c index
> > > f5fc1402701c..853919111835 100644
> > > --- a/src/shared/bap.c
> > > +++ b/src/shared/bap.c
> > > @@ -48,6 +48,15 @@
> > >
> > > #define BAP_PROCESS_TIMEOUT 10
> > >
> > > +#define BAP_FREQ_LTV_TYPE 1
> > > +#define BAP_DURATION_LTV_TYPE 2
> > > +#define BAP_CHANNEL_ALLOCATION_LTV_TYPE 3 #define
> > > +BAP_FRAME_LEN_LTV_TYPE 4 #define
> > CODEC_SPECIFIC_CONFIGURATION_MASK (\
> > > + (1<<BAP_FREQ_LTV_TYPE)|\
> > > + (1<<BAP_DURATION_LTV_TYPE)|\
> > > + (1<<BAP_FRAME_LEN_LTV_TYPE))
> > > +
> > > struct bt_bap_pac_changed {
> > > unsigned int id;
> > > bt_bap_pac_func_t added;
> > > @@ -1692,11 +1701,8 @@ static unsigned int bap_bcast_config(struct
> > bt_bap_stream *stream,
> > > bt_bap_stream_func_t func, void
> > > *user_data) {
> > > stream->qos = *qos;
> > > - if (stream->lpac->type == BT_BAP_BCAST_SINK) {
> > > - if (data)
> > > - stream_config(stream, data, NULL);
> > > - stream_set_state(stream, BT_BAP_STREAM_STATE_CONFIG);
> > > - }
> > > + stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> > > + ep_config_cb, stream->lpac->user_data);
> > >
> > > return 1;
> > > }
> > > @@ -3302,6 +3308,13 @@ static void bap_add_broadcast_source(struct
> > > bt_bap_pac *pac) static void bap_add_broadcast_sink(struct bt_bap_pac
> > > *pac) {
> > > queue_push_tail(pac->bdb->broadcast_sinks, pac);
> > > +
> > > + /* Update local PACS for broadcast sink also, when registering an
> > > + * endpoint
> > > + */
> > > + pacs_add_sink_location(pac->bdb->pacs, pac->qos.location);
> > > + pacs_add_sink_supported_context(pac->bdb->pacs,
> > > + pac->qos.supported_context);
> > > }
> > >
> > > static void notify_pac_added(void *data, void *user_data) @@ -3453,6
> > > +3466,16 @@ struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct
> > bt_bap_pac *pac)
> > > return &pac->qos;
> > > }
> > >
> > > +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac) {
> > > + return pac->data;
> > > +}
> > > +
> > > +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac) {
> > > + return pac->metadata;
> > > +}
> > > +
> > > uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream) {
> > > if (!stream)
> > > @@ -5253,10 +5276,6 @@ bool bt_bap_stream_set_user_data(struct
> > > bt_bap_stream *stream, void *user_data)
> > >
> > > stream->user_data = user_data;
> > >
> > > - if (bt_bap_stream_get_type(stream) ==
> > BT_BAP_STREAM_TYPE_BCAST)
> > > - stream->lpac->ops->config(stream, stream->cc, &stream->qos,
> > > - ep_config_cb, stream->lpac->user_data);
> > > -
> > > return true;
> > > }
> > >
> > > @@ -5892,8 +5911,9 @@ static void add_new_subgroup(struct bt_base
> > > *base,
> > >
> > > struct bt_ltv_match {
> > > uint8_t l;
> > > - uint8_t *v;
> > > + void *data;
> > > bool found;
> > > + uint32_t data32;
> > > };
> > >
> > > struct bt_ltv_search {
> > > @@ -5912,7 +5932,7 @@ static void match_ltv(size_t i, uint8_t l, uint8_t t,
> > uint8_t *v,
> > > if (ltv_match->l != l)
> > > return;
> > >
> > > - if (!memcmp(v, ltv_match->v, l))
> > > + if (!memcmp(v, ltv_match->data, l))
> > > ltv_match->found = true; }
> > >
> > > @@ -5924,7 +5944,7 @@ static void search_ltv(size_t i, uint8_t l,
> > > uint8_t t, uint8_t *v,
> > >
> > > ltv_match.found = false;
> > > ltv_match.l = l;
> > > - ltv_match.v = v;
> > > + ltv_match.data = v;
> > >
> > > util_ltv_foreach(ltv_search->iov->iov_base,
> > > ltv_search->iov->iov_len, &t, @@ -5965,8
> > > +5985,10 @@ static bool compare_ltv(struct iovec *iov1, }
> > >
> > > struct bt_ltv_extract {
> > > - struct iovec *result;
> > > struct iovec *src;
> > > + void *value;
> > > + uint8_t len;
> > > + struct iovec *result;
> > > };
> > >
> > > static void extract_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > > @@ -5978,7 +6000,7 @@ static void extract_ltv(size_t i, uint8_t l,
> > > uint8_t t, uint8_t *v,
> > >
> > > ltv_match.found = false;
> > > ltv_match.l = l;
> > > - ltv_match.v = v;
> > > + ltv_match.data = v;
> > >
> > > /* Search each BIS caps ltv in subgroup caps
> > > * to extract the one that are BIS specific @@ -6111,3
> > > +6133,257 @@ struct iovec *bt_bap_stream_get_base(struct
> > bt_bap_stream
> > > *stream)
> > >
> > > return base_iov;
> > > }
> > > +
> > > +static void bap_sink_get_allocation(size_t i, uint8_t l, uint8_t t,
> > > + uint8_t *v, void *user_data) {
> > > + uint32_t location32;
> > > +
> > > + if (!v)
> > > + return;
> > > +
> > > + memcpy(&location32, v, l);
> > > + *((uint32_t *)user_data) = le32_to_cpu(location32); }
> > > +
> > > +/*
> > > + * This function compares PAC Codec Specific Capabilities, with the
> > > +Codec
> > > + * Specific Configuration LTVs received in the BASE of the BAP
> > > +Source. The
> > > + * result is accumulated in data32 which is a bitmask of types.
> > > + */
> > > +static void check_pac_caps_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > > + void *user_data) {
> > > + struct bt_ltv_match *compare_data = user_data;
> > > + uint8_t *bis_v = compare_data->data;
> > > +
> > > + switch (t) {
> > > + case BAP_FREQ_LTV_TYPE:
> > > + {
> > > + uint16_t mask = *((uint16_t *)v);
> > > +
> > > + mask = le16_to_cpu(mask);
> > > + if (mask & (1 << (bis_v[0] - 1)))
> > > + compare_data->data32 |= 1<<t;
> > > + }
> > > + break;
> > > + case BAP_DURATION_LTV_TYPE:
> > > + if ((v[0]) & (1 << bis_v[0]))
> > > + compare_data->data32 |= 1<<t;
> > > + break;
> > > + case BAP_FRAME_LEN_LTV_TYPE:
> > > + {
> > > + uint16_t min = *((uint16_t *)v);
> > > + uint16_t max = *((uint16_t *)(&v[2]));
> > > + uint16_t frame_len = *((uint16_t *)bis_v);
> > > +
> > > + min = le16_to_cpu(min);
> > > + max = le16_to_cpu(max);
> > > + frame_len = le16_to_cpu(frame_len);
> > > + if ((frame_len >= min) &&
> > > + (frame_len <= max))
> > > + compare_data->data32 |= 1<<t;
> > > + }
> >
> > Don't create new scopes inside a switch statement, that makes it hard to
> > follow the code, if you really think that would help have a helper function to
> > check each field.
> >
> > > + break;
> > > + }
> > > +}
> > > +
> > > +static void check_source_ltv(size_t i, uint8_t l, uint8_t t, uint8_t *v,
> > > + void *user_data) {
> > > + struct bt_ltv_match *local_data = user_data;
> > > + struct iovec *pac_caps = local_data->data;
> > > + struct bt_ltv_match compare_data;
> > > +
> > > + compare_data.data = v;
> > > +
> > > + /* Search inside local PAC's caps for LTV of type t */
> > > + util_ltv_foreach(pac_caps->iov_base, pac_caps->iov_len, &t,
> > > + check_pac_caps_ltv,
> > > + &compare_data);
> > > +
> > > + local_data->data32 |= compare_data.data32; }
> > > +
> > > +static void bap_sink_check_level3_ltv(size_t i, uint8_t l, uint8_t t,
> > > + uint8_t *v, void *user_data) {
> > > + struct bt_ltv_extract *merge_data = user_data;
> > > +
> > > + merge_data->value = v;
> > > + merge_data->len = l;
> > > +}
> > > +
> > > +static void bap_push_ltv(struct iovec *output, uint8_t l, uint8_t t,
> > > +void *v) {
> > > + l++;
> > > + iov_append(output, 1, &l);
> > > + iov_append(output, 1, &t);
> > > + iov_append(output, l - 1, v);
> > > +}
> >
> > Perhaps we should create a helper function to do this sort of thing, maybe
> > util_ltv_push?
> >
> > > +static void bap_sink_check_level2_ltv(size_t i, uint8_t l, uint8_t t,
> > > + uint8_t *v, void *user_data) {
> > > + struct bt_ltv_extract *merge_data = user_data;
> > > +
> > > + merge_data->value = NULL;
> > > + util_ltv_foreach(merge_data->src->iov_base,
> > > + merge_data->src->iov_len,
> > > + &t,
> > > + bap_sink_check_level3_ltv, user_data);
> > > +
> > > + /* If the LTV at level 2 was found at level 3 add the one from level 3,
> > > + * otherwise add the one at level 2
> > > + */
> > > + if (merge_data->value)
> > > + bap_push_ltv(merge_data->result, merge_data->len,
> > > + t, merge_data->value);
> > > + else
> > > + bap_push_ltv(merge_data->result, l, t, v); }
> > > +
> > > +static void check_local_pac(void *data, void *user_data) {
> > > + struct bt_ltv_match *compare_data = user_data;
> > > + struct iovec *bis_data = (struct iovec *)compare_data->data;
> > > + const struct bt_bap_pac *pac = data;
> > > +
> > > + /* Keep searching for a matching PAC if one wasn't found
> > > + * in previous PAC element
> > > + */
> > > + if (compare_data->found == false) {
> > > + struct bt_ltv_match bis_compare_data = {
> > > + .data = pac->data,
> > > + .data32 = 0, /* LTVs bitmask result */
> > > + .found = false
> > > + };
> > > +
> > > + /* loop each BIS LTV */
> > > + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, NULL,
> > > + check_source_ltv, &bis_compare_data);
> > > +
> > > + /* We have a match if all selected LTVs have a match */
> > > + if ((bis_compare_data.data32 &
> > > + CODEC_SPECIFIC_CONFIGURATION_MASK) ==
> > > + CODEC_SPECIFIC_CONFIGURATION_MASK)
> > > + compare_data->found = true;
> > > + }
> > > +}
> > > +
> > > +static void bap_sink_match_allocation(size_t i, uint8_t l, uint8_t t,
> > > + uint8_t *v, void *user_data) {
> > > + struct bt_ltv_match *data = user_data;
> > > + uint32_t location32;
> > > +
> > > + if (!v)
> > > + return;
> > > +
> > > + memcpy(&location32, v, l);
> > > + location32 = le32_to_cpu(location32);
> > > +
> > > + /* If all the bits in the received bitmask are found in
> > > + * the local bitmask then we have a match
> > > + */
> > > + if ((location32 & data->data32) == location32)
> > > + data->found = true;
> > > + else
> > > + data->found = false;
> > > +}
> > > +
> > > +static bool bap_check_bis(struct bt_bap_db *ldb, struct iovec
> > > +*bis_data) {
> > > + struct bt_ltv_match compare_data = {};
> > > +
> > > + /* Check channel allocation against the PACS location.
> > > + * If we don't have a location set we can accept any BIS location.
> > > + * If the BIS doesn't have a location set we also accept it
> > > + */
> > > + compare_data.found = true;
> > > +
> > > + if (ldb->pacs->sink_loc_value) {
> > > + uint8_t type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> > > +
> > > + compare_data.data32 = ldb->pacs->sink_loc_value;
> > > + util_ltv_foreach(bis_data->iov_base, bis_data->iov_len, &type,
> > > + bap_sink_match_allocation, &compare_data);
> > > + }
> > > +
> > > + /* Check remaining LTVs against the PACs list */
> > > + if (compare_data.found) {
> > > + compare_data.data = bis_data;
> > > + compare_data.found = false;
> > > + queue_foreach(ldb->broadcast_sinks, check_local_pac,
> > > + &compare_data);
> > > + }
> > > +
> > > + return compare_data.found;
> > > +}
> > > +
> > > +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> > > + struct bt_bap_codec *codec,
> > > + struct iovec *l2_caps,
> > > + struct iovec *l3_caps,
> > > + struct iovec *meta)
> > > +{
> > > + struct bt_bap_pac *pac_source_bis;
> > > + struct bt_bap_endpoint *ep;
> > > + int err = 0;
> > > + struct bt_bap_pac_qos bis_qos = {0};
> > > + uint8_t type = 0;
> > > + struct bt_ltv_extract merge_data = {0};
> > > +
> > > + merge_data.src = l3_caps;
> > > + merge_data.result = new0(struct iovec, 1);
> > > +
> > > + /* Create a Codec Specific Configuration with LTVs at level 2 (subgroup)
> > > + * overwritten by LTVs at level 3 (BIS)
> > > + */
> > > + util_ltv_foreach(l2_caps->iov_base,
> > > + l2_caps->iov_len,
> > > + NULL,
> > > + bap_sink_check_level2_ltv, &merge_data);
> > > +
> > > + /* Check each BIS Codec Specific Configuration LTVs against our Codec
> > > + * Specific Capabilities and if the BIS matches create a PAC with it
> > > + */
> > > + if (bap_check_bis(bap->ldb, merge_data.result) == false)
> > > + goto cleanup;
> > > +
> > > + DBG(bap, "Matching BIS %i", bis_index);
> > > +
> > > + /* Create a QoS structure based on the received BIS information to
> > > + * specify the desired channel for this BIS/PAC
> > > + */
> > > + type = BAP_CHANNEL_ALLOCATION_LTV_TYPE;
> > > + util_ltv_foreach(merge_data.result->iov_base,
> > > + merge_data.result->iov_len, &type,
> > > + bap_sink_get_allocation, &bis_qos.location);
> >
> > Isn't it better to parse this inline with the use of util_iov_pull_*?
> > If you don't want to change the iovec passed, which shall be made a const if
> > that is intention, then just create a dup and parse it.
> merge_data.result is the concatenated list for Capabilities LTVs. We need to
> extract the value for the Allocation LTV so that we pass it to the stream qos.
> I used util_ltv_foreach to easily access the ltv. Do you see another way?
> Maybe I didn't understand your comment.

My bad, I thought you were iterating over the same iov but the
merge_data is different, so please disconsider this comment.

>
> >
> > > + /* Create a remote PAC */
> > > + pac_source_bis = bap_pac_new(bap->rdb, NULL,
> > > + BT_BAP_BCAST_SOURCE, codec, &bis_qos,
> > > + merge_data.result, meta);
> > > +
> > > + err = asprintf(&pac_source_bis->name, "%d", bis_index);
> > > +
> > > + if (err < 0) {
> > > + DBG(bap, "error in asprintf");
> > > + goto cleanup;
> > > + }
> > > +
> > > + /* Add remote source endpoint */
> > > + if (!bap->rdb->broadcast_sources)
> > > + bap->rdb->broadcast_sources = queue_new();
> > > + queue_push_tail(bap->rdb->broadcast_sources, pac_source_bis);
> > > +
> > > + queue_foreach(bap->pac_cbs, notify_pac_added, pac_source_bis);
> > > + /* Push remote endpoint with direction sink */
> > > + ep = bap_endpoint_new_broadcast(bap->rdb, BT_BAP_BCAST_SINK);
> > > +
> > > + if (ep)
> > > + queue_push_tail(bap->remote_eps, ep);
> > > +
> > > +cleanup:
> > > + util_iov_free(merge_data.result, 1); }
> > > diff --git a/src/shared/bap.h b/src/shared/bap.h index
> > > 2c3550921f07..b2826719f84f 100644
> > > --- a/src/shared/bap.h
> > > +++ b/src/shared/bap.h
> > > @@ -4,7 +4,7 @@
> > > * BlueZ - Bluetooth protocol stack for Linux
> > > *
> > > * Copyright (C) 2022 Intel Corporation. All rights reserved.
> > > - * Copyright 2023 NXP
> > > + * Copyright 2023-2024 NXP
> > > *
> > > */
> > >
> > > @@ -175,6 +175,10 @@ uint16_t bt_bap_pac_get_context(struct
> > bt_bap_pac
> > > *pac);
> > >
> > > struct bt_bap_pac_qos *bt_bap_pac_get_qos(struct bt_bap_pac *pac);
> > >
> > > +struct iovec *bt_bap_pac_get_data(struct bt_bap_pac *pac);
> > > +
> > > +struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
> >
> > Have these 2 functions above in a separate patch.
> >
> > > uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
> > >
> > > struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> > > @@ -323,3 +327,10 @@ void bt_bap_update_bcast_source(struct
> > bt_bap_pac
> > > *pac, bool bt_bap_pac_bcast_is_local(struct bt_bap *bap, struct
> > > bt_bap_pac *pac);
> > >
> > > struct iovec *bt_bap_stream_get_base(struct bt_bap_stream *stream);
> > > +
> > > +void bt_bap_add_bis(struct bt_bap *bap, uint8_t bis_index,
> > > + struct bt_bap_codec *codec,
> > > + struct iovec *l2_caps,
> > > + struct iovec *l3_caps,
> > > + struct iovec *meta);
> > > +
> > > --
> > > 2.40.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
> Regards,
> Andrei



--
Luiz Augusto von Dentz