Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Sent: Friday, January 19, 2024 5:12 PM
> To: Iulia Tanasescu <iulia.tanasescu@xxxxxxx>
> Cc: linux-bluetooth@xxxxxxxxxxxxxxx; Claudia Cristina Draghicescu
> <claudia.rosu@xxxxxxx>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@xxxxxxx>; Silviu Florian Barbulescu
> <silviu.barbulescu@xxxxxxx>; Vlad Pruteanu <vlad.pruteanu@xxxxxxx>;
> Andrei Istodorescu <andrei.istodorescu@xxxxxxx>
> Subject: Re: [PATCH BlueZ 1/1] bap: Fix bcast endpoint config
>
> Hi Iulia,
>
> On Fri, Jan 19, 2024 at 10:04 AM Iulia Tanasescu
> <iulia.tanasescu@xxxxxxx> wrote:
> >
> > This updates the way broadcast is differentiated from unicast
> > at endpoint configuration: Instead of checking if setup->base
> > is allocated, check lpac type.
> >
> > ---
> > profiles/audio/bap.c | 39 ++++++++++++++++++---------------------
> > 1 file changed, 18 insertions(+), 21 deletions(-)
> >
> > diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
> > index b88876485..137ed7d39 100644
> > --- a/profiles/audio/bap.c
> > +++ b/profiles/audio/bap.c
> > @@ -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
> > *
> > *
> > */
> > @@ -617,15 +617,16 @@ static int parse_bcast_qos(const char *key, int
> var, DBusMessageIter *iter,
> > return 0;
> > }
> >
> > -static int parse_qos(DBusMessageIter *iter, struct bt_bap_qos *qos,
> > - struct iovec **base)
> > +static int parse_qos(DBusMessageIter *iter, uint8_t pac_type,
> > + struct bt_bap_qos *qos)
> > {
> > DBusMessageIter array;
> > const char *key;
> > int (*parser)(const char *key, int var, DBusMessageIter *iter,
> > struct bt_bap_qos *qos);
> >
> > - if (*base)
> > + if ((pac_type == BT_BAP_BCAST_SOURCE) ||
> > + (pac_type == BT_BAP_BCAST_SINK))
> > parser = parse_bcast_qos;
> > else
> > parser = parse_ucast_qos;
> > @@ -656,9 +657,9 @@ static int parse_qos(DBusMessageIter *iter, struct
> bt_bap_qos *qos,
> > return 0;
> > }
> >
> > -static int parse_configuration(DBusMessageIter *props, struct iovec
> **caps,
> > - struct iovec **metadata, struct iovec **base,
> > - struct bt_bap_qos *qos)
> > +static int parse_configuration(DBusMessageIter *props, uint8_t pac_type,
> > + struct iovec **caps, struct iovec **metadata,
> > + struct iovec **base, struct bt_bap_qos *qos)
> > {
> > const char *key;
> > struct iovec iov;
> > @@ -686,6 +687,12 @@ static int parse_configuration(DBusMessageIter
> *props, struct iovec **caps,
> >
> > util_iov_free(*caps, 1);
> > *caps = util_iov_dup(&iov, 1);
> > +
> > + /* Currently, the base iovec only duplicates
> > + * setup->caps. TODO: Dynamically generate
> > + * base using received caps.
> > + */
> > + *base = util_iov_dup(*caps, 1);
> > } else if (!strcasecmp(key, "Metadata")) {
> > if (var != DBUS_TYPE_ARRAY)
> > goto fail;
> > @@ -699,24 +706,13 @@ static int parse_configuration(DBusMessageIter
> *props, struct iovec **caps,
> > if (var != DBUS_TYPE_ARRAY)
> > goto fail;
> >
> > - if (parse_qos(&value, qos, base))
> > + if (parse_qos(&value, pac_type, qos))
> > goto fail;
> > }
> >
> > dbus_message_iter_next(props);
> > }
> >
> > - if (*base) {
> > - uint32_t presDelay;
> > - uint8_t numSubgroups, numBis;
> > - struct bt_bap_codec codec;
> > -
> > - util_iov_memcpy(*base, (*caps)->iov_base, (*caps)->iov_len);
> > - parse_base((*caps)->iov_base, (*caps)->iov_len, bap_debug,
> > - &presDelay, &numSubgroups, &numBis, &codec,
> > - caps, NULL);
> > - }
> > -
> > return 0;
> >
> > fail:
> > @@ -882,8 +878,9 @@ static DBusMessage
> *set_configuration(DBusConnection *conn, DBusMessage *msg,
> > setup->qos.ucast.cis_id = BT_ISO_QOS_CIS_UNSET;
> > }
> >
> > - if (parse_configuration(&props, &setup->caps, &setup->metadata,
> > - &setup->base, &setup->qos) < 0) {
> > + if (parse_configuration(&props, bt_bap_pac_get_type(ep->lpac),
> > + &setup->caps, &setup->metadata, &setup->base,
> > + &setup->qos) < 0) {
> > DBG("Unable to parse configuration");
> > setup_free(setup);
> > return btd_error_invalid_args(msg);
> > --
> > 2.39.2
>
> I sort of did the same thing but end up refactoring the code in the process:
>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.kernel.org%2Fproject%2Fbluetooth%2Flist%2F%3Fseries%3D817943&d
> ata=05%7C02%7Ciulia.tanasescu%40nxp.com%7Cdb1ba6b2414f4919bd7008
> dc190103cf%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6384127
> 39344015066%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQ
> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata
> =ZlnSGD6GfP8JTCbNPkRck%2FAfUp4a5uKBnzg9ANpg7B4%3D&reserved=0
>
> So it's worth checking if I didn't break it further.
I tested and it looks ok.
>
> --
> Luiz Augusto von Dentz
Regards,
Iulia
