| Message ID | 20240119150443.3163-2-iulia.tanasescu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bap: Fix bcast endpoint config | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
Hi Iulia, On Fri, Jan 19, 2024 at 10:04 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> 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://patchwork.kernel.org/project/bluetooth/list/?series=817943 So it's worth checking if I didn't break it further.
Hi Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Friday, January 19, 2024 5:12 PM > To: Iulia Tanasescu <iulia.tanasescu@nxp.com> > Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu > <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai- > octavian.urzica@nxp.com>; Silviu Florian Barbulescu > <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; > Andrei Istodorescu <andrei.istodorescu@nxp.com> > 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@nxp.com> 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
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=818149 ---Test result--- Test Summary: CheckPatch PASS 0.48 seconds GitLint PASS 0.32 seconds BuildEll PASS 23.89 seconds BluezMake PASS 714.74 seconds MakeCheck PASS 11.55 seconds MakeDistcheck PASS 160.56 seconds CheckValgrind PASS 222.97 seconds CheckSmatch PASS 326.48 seconds bluezmakeextell PASS 106.23 seconds IncrementalBuild PASS 670.06 seconds ScanBuild PASS 937.69 seconds --- Regards, Linux Bluetooth
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);