| Message ID | 20240223153450.86694-2-andrei.istodorescu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Update Sink BASE management | expand |
Hi Andrei, On Fri, Feb 23, 2024 at 10:34 AM Andrei Istodorescu <andrei.istodorescu@nxp.com> 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 >
Hi Luiz, I have fixed all the comments excepting one. Please see inline. > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Tuesday, February 27, 2024 4:39 PM > To: Andrei Istodorescu <andrei.istodorescu@nxp.com> > Cc: linux-bluetooth@vger.kernel.org; Mihai-Octavian Urzica <mihai- > octavian.urzica@nxp.com>; Silviu Florian Barbulescu > <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; > Iulia Tanasescu <iulia.tanasescu@nxp.com> > 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 > <andrei.istodorescu@nxp.com> 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
Hi Andrei, On Wed, Feb 28, 2024 at 10:12 AM Andrei Istodorescu <andrei.istodorescu@nxp.com> wrote: > > Hi Luiz, > > I have fixed all the comments excepting one. Please see inline. > > > -----Original Message----- > > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > > Sent: Tuesday, February 27, 2024 4:39 PM > > To: Andrei Istodorescu <andrei.istodorescu@nxp.com> > > Cc: linux-bluetooth@vger.kernel.org; Mihai-Octavian Urzica <mihai- > > octavian.urzica@nxp.com>; Silviu Florian Barbulescu > > <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; > > Iulia Tanasescu <iulia.tanasescu@nxp.com> > > 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 > > <andrei.istodorescu@nxp.com> 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
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); +