All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties
@ 2023-03-15 17:53 Pauli Virtanen
  2023-03-15 17:53 ` [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation Pauli Virtanen
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Pauli Virtanen @ 2023-03-15 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Pauli Virtanen

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[] = {
 	{ }
 };
 
+static gboolean qos_exists(const GDBusPropertyTable *property, void *data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = transport->data;
+
+	return bap->qos.phy != 0x00;
+}
+
+static gboolean get_cig(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = 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 = data;
+	struct bap_transport *bap = 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 = data;
+	struct bap_transport *bap = transport->data;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy);
+
+	return TRUE;
+}
+
 static gboolean get_sdu(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = {
 	{ "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_transport *transport)
 
 	bap->qos = *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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation
  2023-03-15 17:53 [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Pauli Virtanen
@ 2023-03-15 17:53 ` Pauli Virtanen
  2023-03-15 18:32   ` Luiz Augusto von Dentz
  2023-03-15 18:19 ` [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Luiz Augusto von Dentz
  2023-03-15 19:27 ` [BlueZ,1/2] " bluez.test.bot
  2 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2023-03-15 17:53 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Pauli Virtanen

Document the transport QoS properties. Fix documentation of Delay, it's
microseconds for ISO.
---
 doc/media-api.txt | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 281f72c1e..eac7f081c 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -773,12 +773,17 @@ Properties	object Device [readonly]
 				"pending": streaming but not acquired
 				"active": streaming and acquired
 
-		uint16 Delay [readwrite]
+		uint16 Delay [A2DP only, readwrite, optional]
 
-			Optional. Transport delay in 1/10 of millisecond, this
+			For A2DP: transport delay in 1/10 of millisecond. This
 			property is only writeable when the transport was
 			acquired by the sender.
 
+		uint32 Delay [ISO only, optional]
+
+			For ISO, presentation delay in microseconds.
+			Note the value type is different for ISO and A2DP.
+
 		uint16 Volume [readwrite]
 
 			Optional. Indicates volume level of the transport,
@@ -804,3 +809,38 @@ Properties	object Device [readonly]
 
 			Linked transport objects which the transport is
 			associated with.
+
+		byte CIG [ISO only, optional, experimental]
+
+			Indicates configured QoS CIG.
+			Only present when QoS is configured.
+
+		byte CIS [ISO only, optional, experimental]
+
+			Indicates configured QoS CIS.
+			Only present when QoS is configured.
+
+		byte Interval [ISO only, optional, experimental]
+
+			Indicates configured QoS interval.
+			Only present when QoS is configured.
+
+		byte Framing [ISO only, optional, experimental]
+
+			Indicates configured QoS framing.
+			Only present when QoS is configured.
+
+		byte PHY [ISO only, optional, experimental]
+
+			Indicates configured QoS PHY.
+			Only present when QoS is configured.
+
+		uint32 Retransmissions [ISO only, optional, experimental]
+
+			Indicates configured QoS retransmissions.
+			Only present when QoS is configured.
+
+		uint32 Latency [ISO only, optional, experimental]
+
+			Indicates configured QoS latency.
+			Only present when QoS is configured.
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties
  2023-03-15 17:53 [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Pauli Virtanen
  2023-03-15 17:53 ` [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation Pauli Virtanen
@ 2023-03-15 18:19 ` Luiz Augusto von Dentz
  2023-03-15 18:49   ` Pauli Virtanen
  2023-03-15 19:27 ` [BlueZ,1/2] " bluez.test.bot
  2 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2023-03-15 18:19 UTC (permalink / raw)
  To: Pauli Virtanen; +Cc: linux-bluetooth

Hi Pauli,

On Wed, Mar 15, 2023 at 10:54 AM 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[] = {
>         { }
>  };
>
> +static gboolean qos_exists(const GDBusPropertyTable *property, void *data)
> +{
> +       struct media_transport *transport = data;
> +       struct bap_transport *bap = transport->data;
> +
> +       return bap->qos.phy != 0x00;
> +}
> +
> +static gboolean get_cig(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct media_transport *transport = data;
> +       struct bap_transport *bap = 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 = data;
> +       struct bap_transport *bap = 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 = data;
> +       struct bap_transport *bap = transport->data;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy);
> +
> +       return TRUE;
> +}
> +
>  static gboolean get_sdu(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = {
>         { "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_transport *transport)
>
>         bap->qos = *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.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation
  2023-03-15 17:53 ` [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation Pauli Virtanen
@ 2023-03-15 18:32   ` Luiz Augusto von Dentz
  2023-03-15 19:21     ` Pauli Virtanen
  0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2023-03-15 18:32 UTC (permalink / raw)
  To: Pauli Virtanen; +Cc: linux-bluetooth

HI Pauli,

On Wed, Mar 15, 2023 at 10:54 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Document the transport QoS properties. Fix documentation of Delay, it's
> microseconds for ISO.
> ---
>  doc/media-api.txt | 44 ++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index 281f72c1e..eac7f081c 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -773,12 +773,17 @@ Properties        object Device [readonly]
>                                 "pending": streaming but not acquired
>                                 "active": streaming and acquired
>
> -               uint16 Delay [readwrite]
> +               uint16 Delay [A2DP only, readwrite, optional]
>
> -                       Optional. Transport delay in 1/10 of millisecond, this
> +                       For A2DP: transport delay in 1/10 of millisecond. This
>                         property is only writeable when the transport was
>                         acquired by the sender.
>
> +               uint32 Delay [ISO only, optional]
> +
> +                       For ISO, presentation delay in microseconds.
> +                       Note the value type is different for ISO and A2DP.
> +

I don't think D-Bus introspection allows polymorphism of properties,
so either we just use uint16 and limit the maximum delay to 65K or we
have say it is in milliseconds given that in most cases the presets
have the Presentation Delay as a round number.

>                 uint16 Volume [readwrite]
>
>                         Optional. Indicates volume level of the transport,
> @@ -804,3 +809,38 @@ Properties object Device [readonly]
>
>                         Linked transport objects which the transport is
>                         associated with.
> +
> +               byte CIG [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS CIG.
> +                       Only present when QoS is configured.
> +
> +               byte CIS [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS CIS.
> +                       Only present when QoS is configured.
> +
> +               byte Interval [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS interval.
> +                       Only present when QoS is configured.

This should be uint32

> +               byte Framing [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS framing.
> +                       Only present when QoS is configured.
> +
> +               byte PHY [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS PHY.
> +                       Only present when QoS is configured.
> +
> +               uint32 Retransmissions [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS retransmissions.
> +                       Only present when QoS is configured.

And this should be byte

> +               uint32 Latency [ISO only, optional, experimental]
> +
> +                       Indicates configured QoS latency.
> +                       Only present when QoS is configured.

uint16

> --
> 2.39.2
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties
  2023-03-15 18:19 ` [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Luiz Augusto von Dentz
@ 2023-03-15 18:49   ` Pauli Virtanen
  0 siblings, 0 replies; 7+ messages in thread
From: Pauli Virtanen @ 2023-03-15 18:49 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

ke, 2023-03-15 kello 11:19 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Wed, Mar 15, 2023 at 10:54 AM 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[] = {
> >         { }
> >  };
> > 
> > +static gboolean qos_exists(const GDBusPropertyTable *property, void *data)
> > +{
> > +       struct media_transport *transport = data;
> > +       struct bap_transport *bap = transport->data;
> > +
> > +       return bap->qos.phy != 0x00;
> > +}
> > +
> > +static gboolean get_cig(const GDBusPropertyTable *property,
> > +                                       DBusMessageIter *iter, void *data)
> > +{
> > +       struct media_transport *transport = data;
> > +       struct bap_transport *bap = 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 = data;
> > +       struct bap_transport *bap = 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 = data;
> > +       struct bap_transport *bap = transport->data;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &bap->qos.phy);
> > +
> > +       return TRUE;
> > +}
> > +
> >  static gboolean get_sdu(const GDBusPropertyTable *property,
> >                                         DBusMessageIter *iter, void *data)
> >  {
> > @@ -962,12 +1005,15 @@ static const GDBusPropertyTable bap_properties[] = {
> >         { "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_transport *transport)
> > 
> >         bap->qos = *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.

Yes, we can do that once we have the fd.

But if client wants to do CIG+CIS allocation in its SelectProperties,
it needs to know already reserved CIG+CIS. Acquiring the fd moves ASEs
out from qos state, which is not wanted for this.

Also, no Acquire call will return until all CIS in the same CIG have
been connected. To acquire the right transports (only those in the same
CIG), we need to know CIGs of all transports before starting to acquire
any. So we don't have any fds at that point.

Managing that probably should be responsibility of BlueZ, it could
connect all CIS in the same CIG when one of the transports is acquired.
Or, try address it in kernel but maybe that has more constraints.

But I think these properties can be added regardless of the above,
given that they have some uses also otherwise.

-- 
Pauli Virtanen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation
  2023-03-15 18:32   ` Luiz Augusto von Dentz
@ 2023-03-15 19:21     ` Pauli Virtanen
  0 siblings, 0 replies; 7+ messages in thread
From: Pauli Virtanen @ 2023-03-15 19:21 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi,

ke, 2023-03-15 kello 11:32 -0700, Luiz Augusto von Dentz kirjoitti:
> On Wed, Mar 15, 2023 at 10:54 AM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > Document the transport QoS properties. Fix documentation of Delay, it's
> > microseconds for ISO.
> > ---
> >  doc/media-api.txt | 44 ++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > index 281f72c1e..eac7f081c 100644
> > --- a/doc/media-api.txt
> > +++ b/doc/media-api.txt
> > @@ -773,12 +773,17 @@ Properties        object Device [readonly]
> >                                 "pending": streaming but not acquired
> >                                 "active": streaming and acquired
> > 
> > -               uint16 Delay [readwrite]
> > +               uint16 Delay [A2DP only, readwrite, optional]
> > 
> > -                       Optional. Transport delay in 1/10 of millisecond, this
> > +                       For A2DP: transport delay in 1/10 of millisecond. This
> >                         property is only writeable when the transport was
> >                         acquired by the sender.
> > 
> > +               uint32 Delay [ISO only, optional]
> > +
> > +                       For ISO, presentation delay in microseconds.
> > +                       Note the value type is different for ISO and A2DP.
> > +
> 
> I don't think D-Bus introspection allows polymorphism of properties,
> so either we just use uint16 and limit the maximum delay to 65K or we
> have say it is in milliseconds given that in most cases the presets
> have the Presentation Delay as a round number.

How about renaming the property to PresentationDelay in ISO transports?
I think it would clearest to keep it as it is in BAP.

For A2DP changing away from the 1/10 ms unit breaks things.

> >                 uint16 Volume [readwrite]
> > 
> >                         Optional. Indicates volume level of the transport,
> > @@ -804,3 +809,38 @@ Properties object Device [readonly]
> > 
> >                         Linked transport objects which the transport is
> >                         associated with.
> > +
> > +               byte CIG [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS CIG.
> > +                       Only present when QoS is configured.
> > +
> > +               byte CIS [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS CIS.
> > +                       Only present when QoS is configured.
> > +
> > +               byte Interval [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS interval.
> > +                       Only present when QoS is configured.
> 
> This should be uint32

Indeed...

> 
> > +               byte Framing [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS framing.
> > +                       Only present when QoS is configured.
> > +
> > +               byte PHY [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS PHY.
> > +                       Only present when QoS is configured.
> > +
> > +               uint32 Retransmissions [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS retransmissions.
> > +                       Only present when QoS is configured.
> 
> And this should be byte
> 
> > +               uint32 Latency [ISO only, optional, experimental]
> > +
> > +                       Indicates configured QoS latency.
> > +                       Only present when QoS is configured.
> 
> uint16
> 
> > --
> > 2.39.2
> > 
> 
> 

-- 
Pauli Virtanen

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties
  2023-03-15 17:53 [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Pauli Virtanen
  2023-03-15 17:53 ` [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation Pauli Virtanen
  2023-03-15 18:19 ` [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Luiz Augusto von Dentz
@ 2023-03-15 19:27 ` bluez.test.bot
  2 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2023-03-15 19:27 UTC (permalink / raw)
  To: linux-bluetooth, pav

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

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=730450

---Test result---

Test Summary:
CheckPatch                    PASS      0.90 seconds
GitLint                       FAIL      0.88 seconds
BuildEll                      FAIL      21.98 seconds
BluezMake                     PASS      863.86 seconds
MakeCheck                     PASS      11.04 seconds
MakeDistcheck                 PASS      151.92 seconds
CheckValgrind                 PASS      248.09 seconds
CheckSmatch                   PASS      329.13 seconds
bluezmakeextell               FAIL      7.32 seconds
IncrementalBuild              PASS      1432.04 seconds
ScanBuild                     PASS      1036.93 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (82>80): "[BlueZ,1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties"
##############################
Test: BuildEll - FAIL
Desc: Build and Install ELL
Output:

writing RSA key
writing RSA key
writing RSA key
writing RSA key
writing RSA key
make[1]: *** [Makefile:3277: unit/cert-intca.pem] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1265: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:

configure.ac:21: installing './compile'
configure.ac:36: installing './config.guess'
configure.ac:36: installing './config.sub'
configure.ac:5: installing './install-sh'
configure.ac:5: installing './missing'
Makefile.am: installing './depcomp'
parallel-tests: installing './test-driver'
configure: error: Embedded Linux library >= 0.39 is required


---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-15 19:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-15 17:53 [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Pauli Virtanen
2023-03-15 17:53 ` [PATCH BlueZ 2/2] doc: update ISO Transport properties to match implementation Pauli Virtanen
2023-03-15 18:32   ` Luiz Augusto von Dentz
2023-03-15 19:21     ` Pauli Virtanen
2023-03-15 18:19 ` [PATCH BlueZ 1/2] transport: add CIG/CIS/PHY properties, don't show unset QoS properties Luiz Augusto von Dentz
2023-03-15 18:49   ` Pauli Virtanen
2023-03-15 19:27 ` [BlueZ,1/2] " bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.