Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-1204-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240119150443.3163-1-iulia.tanasescu@nxp.com> <20240119150443.3163-2-iulia.tanasescu@nxp.com>
In-Reply-To: <20240119150443.3163-2-iulia.tanasescu@nxp.com>
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: Fri, 19 Jan 2024 10:11:58 -0500
Message-ID: <CABBYNZJduvCkv3F5VNJDmUn1sBDAvV5uXFaTOskc0Feg0xU14w@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/1] bap: Fix bcast endpoint config
To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
Cc: linux-bluetooth@vger.kernel.org, claudia.rosu@nxp.com, 
	mihai-octavian.urzica@nxp.com, silviu.barbulescu@nxp.com, 
	vlad.pruteanu@nxp.com, andrei.istodorescu@nxp.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Iulia,

On Fri, Jan 19, 2024 at 10:04=E2=80=AFAM 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 =3D=3D BT_BAP_BCAST_SOURCE) ||
> +               (pac_type =3D=3D BT_BAP_BCAST_SINK))
>                 parser =3D parse_bcast_qos;
>         else
>                 parser =3D 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 **ca=
ps,
> -                               struct iovec **metadata, struct iovec **b=
ase,
> -                               struct bt_bap_qos *qos)
> +static int parse_configuration(DBusMessageIter *props, uint8_t pac_type,
> +                               struct iovec **caps, struct iovec **metad=
ata,
> +                               struct iovec **base, struct bt_bap_qos *q=
os)
>  {
>         const char *key;
>         struct iovec iov;
> @@ -686,6 +687,12 @@ static int parse_configuration(DBusMessageIter *prop=
s, struct iovec **caps,
>
>                         util_iov_free(*caps, 1);
>                         *caps =3D util_iov_dup(&iov, 1);
> +
> +                       /* Currently, the base iovec only duplicates
> +                        * setup->caps. TODO: Dynamically generate
> +                        * base using received caps.
> +                        */
> +                       *base =3D util_iov_dup(*caps, 1);
>                 } else if (!strcasecmp(key, "Metadata")) {
>                         if (var !=3D DBUS_TYPE_ARRAY)
>                                 goto fail;
> @@ -699,24 +706,13 @@ static int parse_configuration(DBusMessageIter *pro=
ps, struct iovec **caps,
>                         if (var !=3D 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_le=
n);
> -               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 =3D 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->b=
ase,
> +                               &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=3D817943

So it's worth checking if I didn't break it further.

--=20
Luiz Augusto von Dentz