diff mbox series

[BlueZ,1/1] bap: Fix bcast endpoint config

Message ID 20240119150443.3163-2-iulia.tanasescu@nxp.com (mailing list archive)
State New, archived
Headers show
Series bap: Fix bcast endpoint config | expand

Checks

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

Commit Message

Iulia Tanasescu Jan. 19, 2024, 3:04 p.m. UTC
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(-)

Comments

Luiz Augusto von Dentz Jan. 19, 2024, 3:11 p.m. UTC | #1
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.
Iulia Tanasescu Jan. 19, 2024, 3:47 p.m. UTC | #2
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
bluez.test.bot@gmail.com Jan. 19, 2024, 4:08 p.m. UTC | #3
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 mbox series

Patch

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);