MIME-Version: 1.0
References: <e856ad3174024bb61113217cb889005a0bf0ad1c.1678902782.git.pav@iki.fi>
In-Reply-To: <e856ad3174024bb61113217cb889005a0bf0ad1c.1678902782.git.pav@iki.fi>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Wed, 15 Mar 2023 11:19:51 -0700
Message-ID: <CABBYNZJeJkR18gAdy9bLW0aCkj9t3eAQ_Vpvz7whnjeaeY+2cw@mail.gmail.com>
Subject: Re: [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't
 show unset QoS properties
To:     Pauli Virtanen <pav@iki.fi>
Cc:     linux-bluetooth@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Pauli,

On Wed, Mar 15, 2023 at 10:54=E2=80=AFAM Pauli Virtanen <pav@iki.fi> wrote:
>
> Add CIG, CIS, and PHY properties to BAP transport.  The other QoS
> properties are there, and these may also be useful to clients, e.g.  to
> manage CIG/CIS allocation as client.
>
> Hide transport QoS properties when they are not configured.
> ---
>  profiles/audio/transport.c | 67 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 457590746..53bf13175 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -811,6 +811,38 @@ static const GDBusPropertyTable a2dp_properties[] =
=3D {
>         { }
>  };
>
> +static gboolean qos_exists(const GDBusPropertyTable *property, void *dat=
a)
> +{
> +       struct media_transport *transport =3D data;
> +       struct bap_transport *bap =3D transport->data;
> +
> +       return bap->qos.phy !=3D 0x00;
> +}
> +
> +static gboolean get_cig(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data=
)
> +{
> +       struct media_transport *transport =3D data;
> +       struct bap_transport *bap =3D transport->data;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
> +                                                       &bap->qos.cig_id)=
;
> +
> +       return TRUE;
> +}
> +
> +static gboolean get_cis(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data=
)
> +{
> +       struct media_transport *transport =3D data;
> +       struct bap_transport *bap =3D transport->data;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE,
> +                                                       &bap->qos.cis_id)=
;
> +
> +       return TRUE;
> +}
> +
>  static gboolean get_interval(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data=
)
>  {
> @@ -835,6 +867,17 @@ static gboolean get_framing(const GDBusPropertyTable=
 *property,
>         return TRUE;
>  }
>
> +static gboolean get_phy(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data=
)
> +{
> +       struct media_transport *transport =3D data;
> +       struct bap_transport *bap =3D transport->data;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.ph=
y);
> +
> +       return TRUE;
> +}
> +
>  static gboolean get_sdu(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data=
)
>  {
> @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] =
=3D {
>         { "Codec", "y", get_codec },
>         { "Configuration", "ay", get_configuration },
>         { "State", "s", get_state },
> -       { "Interval", "u", get_interval },
> -       { "Framing", "b", get_framing },
> -       { "SDU", "q", get_sdu },
> -       { "Retransmissions", "y", get_retransmissions },
> -       { "Latency", "q", get_latency },
> -       { "Delay", "u", get_delay },
> +       { "CIG", "y", get_cig, NULL, qos_exists },
> +       { "CIS", "y", get_cis, NULL, qos_exists },
> +       { "Interval", "u", get_interval, NULL, qos_exists },
> +       { "Framing", "b", get_framing, NULL, qos_exists },
> +       { "PHY", "y", get_phy, NULL, qos_exists },
> +       { "SDU", "q", get_sdu, NULL, qos_exists },
> +       { "Retransmissions", "y", get_retransmissions, NULL, qos_exists }=
,
> +       { "Latency", "q", get_latency, NULL, qos_exists },
> +       { "Delay", "u", get_delay, NULL, qos_exists },
>         { "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
>         { "Location", "u", get_location },
>         { "Metadata", "ay", get_metadata },
> @@ -1191,12 +1237,21 @@ static void bap_update_qos(const struct media_tra=
nsport *transport)
>
>         bap->qos =3D *qos;
>
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(),
> +                       transport->path, MEDIA_TRANSPORT_INTERFACE,
> +                       "CIG");
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(),
> +                       transport->path, MEDIA_TRANSPORT_INTERFACE,
> +                       "CIS");
>         g_dbus_emit_property_changed(btd_get_dbus_connection(),
>                         transport->path, MEDIA_TRANSPORT_INTERFACE,
>                         "Interval");
>         g_dbus_emit_property_changed(btd_get_dbus_connection(),
>                         transport->path, MEDIA_TRANSPORT_INTERFACE,
>                         "Framing");
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(),
> +                       transport->path, MEDIA_TRANSPORT_INTERFACE,
> +                       "PHY");
>         g_dbus_emit_property_changed(btd_get_dbus_connection(),
>                         transport->path, MEDIA_TRANSPORT_INTERFACE,
>                         "SDU");
> --
> 2.39.2

I'm fine adding these but you could also have used BT_ISO_QOS
socketopt to read it directly from the socket in case you want to use
it on pipewire.

--=20
Luiz Augusto von Dentz