Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1205-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Iulia Tanasescu <iulia.tanasescu@nxp.com>
To: luiz.dentz@gmail.com
Cc: andrei.istodorescu@nxp.com,
	claudia.rosu@nxp.com,
	iulia.tanasescu@nxp.com,
	linux-bluetooth@vger.kernel.org,
	mihai-octavian.urzica@nxp.com,
	silviu.barbulescu@nxp.com,
	vlad.pruteanu@nxp.com
Subject: Re: [PATCH BlueZ 1/1] bap: Fix bcast endpoint config
Date: Fri, 19 Jan 2024 17:47:23 +0200
Message-Id: <20240119154723.4737-1-iulia.tanasescu@nxp.com>
In-Reply-To: <CABBYNZJduvCkv3F5VNJDmUn1sBDAvV5uXFaTOskc0Feg0xU14w@mail.gmail.com>
References: <CABBYNZJduvCkv3F5VNJDmUn1sBDAvV5uXFaTOskc0Feg0xU14w@mail.gmail.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?a6hhdbNpoVjYrFX9CjG7Yjzttyy66VjjKtBbYp7Mt8ylYx4enDha2khHuiyQ?=
 =?us-ascii?Q?JDa+SRrMGfZ87xb1zbQ+Z7M1con67YC94JB8BV6BGhEp2xK+yYA2WhAzDb26?=
 =?us-ascii?Q?AGMzzK9OQfM+j9O4f7mrTukR2rguK5gj0N1K4oLtwjINfgCAHnfbJVqL6TNd?=
 =?us-ascii?Q?6kw0ZbcObhgzwDoHuL/2qzM20rkezeM8ffNh+Le/AMVxxU0iMPJmgDSpzGY5?=
 =?us-ascii?Q?US/ZFHiHwLycIYSNfDrvI0EI8OEdp1ypEkIHOi5l9LmfQ2NH3Deuz/h2ORx/?=
 =?us-ascii?Q?yW+oXK1C+O1IcnJvk/x/MTGlqNH1ANo3y6MVm3CVP7B7aa4mO5mJOH3jbgOV?=
 =?us-ascii?Q?TOcToUrIkIe/OcpZp1lB9QI7rnzmKaJ2f7zXkb7MKk+0LifGYR1aJFR5xH8b?=
 =?us-ascii?Q?h0btx+uAPPHZXDym2x3AvztFvv7rW1RUtHrqckeEyTCdOzyhQHoGpMYWjqH5?=
 =?us-ascii?Q?Zq9/mz/lFd82vfqX4kpG75zAuG7Exo+f+jeMn1j2dDVApFTtku/sq9yieu+S?=
 =?us-ascii?Q?dTQk52AKlbP55eYos+f2xUqprXmn64PXESqNSwUr3GLK4Iev/ny/EYRhm6AU?=
 =?us-ascii?Q?qdPTo8lqCobNChr9ASB8zufkqpzOcmvr8NQKCo/eQ9Z9IOBZ7aRDuyPZQGGJ?=
 =?us-ascii?Q?yWObb4bZUM+j0pRTadRZ3Etl2YtFYs5Tvh9dGlNabB+VNjaAUN5uS2AWedX9?=
 =?us-ascii?Q?k1MFl/Lsj5kpDlheTlitCZID9EYz0a/2ePfdj7RGXx61MNs5V7LJIWE5LN40?=
 =?us-ascii?Q?4T4NAE6ToAWmjAzr1G6kDB/C40ZhT7/jUussZXWV8Txhe5Xd4r14zWY83oqe?=
 =?us-ascii?Q?zgLohNHLW3IO8FKI1Z8WQnyeFHcd16Z3l9YrU3Nw8gABhK+ocnJ0s4sKFo1K?=
 =?us-ascii?Q?Z/45OmPzHyAiAJDH1pxs+VvH7MChrK9IOhbKew/FUVNBSWYOGHe9fQg5RID6?=
 =?us-ascii?Q?FhuHH8PPFXPWw2V3xyLKJMXwyz9NGBGyHAAQyOJMK0/ujrmypQxIQlrX7mr/?=
 =?us-ascii?Q?Gv9oVsXo4F3aZi2YzT+r+Y62Qek8dFd22DlWu8KIaLqEp4vJYXBr0w9ZBFSs?=
 =?us-ascii?Q?pHOvznUxBIxq4HlCioDns8oZ7KRtcVDp8sizD8iNqCNuzrZKsXd+n//LK09t?=
 =?us-ascii?Q?b61GnlaMeBgl83a6rSJbSSS7qoP9Q9rxJSwirLbatA/wCO3/CkUhSKDHmtJP?=
 =?us-ascii?Q?vNsNin5bemMEm+6X4W0i40Gi/6PYfLGhMPT6JkUL8HYXcjpV/YIW8jZCRD6z?=
 =?us-ascii?Q?WPBSMkCSHAsR7kB43iYPxr+8107Zj/+GJ33hDIWRnhuR3ggfHWK6ICEHT9Rv?=
 =?us-ascii?Q?Bp+S2GXSSvFWP36gVQUzRKPWCflYmeTcvxY22Tlzim1XAlGtTI/6NU5PJ0aM?=
 =?us-ascii?Q?D+Ox7IPMy9aUmpSqhaVBqdTUMMXVOUrcv5mWm1AtmF9i9n+Ym9HUM8PNnq/l?=
 =?us-ascii?Q?PBTJM3+e3E0ykna1n7j7PJUdIQpq/IXSG40KvUm/DHNi9byGJUxHXfk5bhvC?=
 =?us-ascii?Q?uaQq09Zpz6eJM+0FtMKMWczS3rH7euOH353FzbfUW0WOgBAXN8sNwbqG+0wh?=
 =?us-ascii?Q?3q7EtXkylQgGZVYHjWfEON4J3aykK5E4p+Qd+7o0C0+h0Go9BF2n//QRHLs+?=
 =?us-ascii?Q?rA=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 87a7f5b6-0954-465a-7fd6-08dc1905ef18
X-MS-Exchange-CrossTenant-AuthSource: AS8PR04MB8898.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2024 15:47:25.8407
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Na90Tcmq7Bt7PxPJyVVGP3BbKLoHEBgb/k0qu1WbCXBDPpmgHkKLbqBB9LE5apxPv/o5EHC8RXNhCx1yoAdgBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB9091

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