This updates the way broadcast is differentiated from unicast
at endpoint configuration: Instead of checking if setup->base
is allocated, check lpac type.
This fixes the transport acquire crash currently visible for
broadcast source.
Iulia Tanasescu (1):
bap: Fix bcast endpoint config
profiles/audio/bap.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
base-commit: e108c744fa8dfa1c4f54257532f3433a47179869
--
2.39.2
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
Hi Iulia,
On Fri, Jan 19, 2024 at 10:04 AM Iulia Tanasescu
<[email protected]> 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.
--
Luiz Augusto von Dentz
Hi Luiz,
> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Friday, January 19, 2024 5:12 PM
> To: Iulia Tanasescu <[email protected]>
> Cc: [email protected]; Claudia Cristina Draghicescu
> <[email protected]>; Mihai-Octavian Urzica <mihai-
> [email protected]>; Silviu Florian Barbulescu
> <[email protected]>; Vlad Pruteanu <[email protected]>;
> Andrei Istodorescu <[email protected]>
> Subject: Re: [PATCH BlueZ 1/1] bap: Fix bcast endpoint config
>
> Hi Iulia,
>
> On Fri, Jan 19, 2024 at 10:04 AM Iulia Tanasescu
> <[email protected]> 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