linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BlueZ RFC] media-api: Make QoS a single property
@ 2023-08-24 21:21 Luiz Augusto von Dentz
  2023-08-24 21:28 ` Luiz Augusto von Dentz
  2023-08-24 22:33 ` [BlueZ,RFC] " bluez.test.bot
  0 siblings, 2 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-24 21:21 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This moves QoS related propertis to a single dictionary.
---
 doc/media-api.txt | 44 +++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 3a0ec38e244d..9f2482e73ac1 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -816,42 +816,40 @@ Properties	object Device [readonly]
 			Linked transport objects which the transport is
 			associated with.
 
-		byte CIG [ISO only, optional, experimental]
+		dict QoS [ISO only, optional, experimental]
 
-			Indicates configured QoS CIG.
 			Only present when QoS is configured.
 
-		byte CIS [ISO only, optional, experimental]
+			Possible values for Unicast:
 
-			Indicates configured QoS CIS.
-			Only present when QoS is configured.
+			byte CIG
 
-		uint32 Interval [ISO only, optional, experimental]
+				Indicates configured QoS CIG.
 
-			Indicates configured QoS interval.
-			Only present when QoS is configured.
+			byte CIS
 
-		boolean Framing [ISO only, optional, experimental]
+				Only present when QoS is configured.
 
-			Indicates configured QoS framing.
-			Only present when QoS is configured.
+			uint32 Interval
 
-		byte PHY [ISO only, optional, experimental]
+				Indicates configured QoS interval.
 
-			Indicates configured QoS PHY.
-			Only present when QoS is configured.
+			boolean Framing
 
-		uint16 SDU [ISO only, optional, experimental]
+				Indicates configured QoS framing.
 
-			Indicates configured QoS SDU.
-			Only present when QoS is configured.
+			byte PHY
 
-		byte Retransmissions [ISO only, optional, experimental]
+				Indicates configured QoS PHY.
 
-			Indicates configured QoS retransmissions.
-			Only present when QoS is configured.
+			uint16 SDU
 
-		uint16 Latency [ISO only, optional, experimental]
+				Indicates configured QoS SDU.
 
-			Indicates configured QoS latency.
-			Only present when QoS is configured.
+			byte Retransmissions
+
+				Indicates configured QoS retransmissions.
+
+			uint16 Latency
+
+				Indicates configured QoS latency.
-- 
2.41.0


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

* Re: [BlueZ RFC] media-api: Make QoS a single property
  2023-08-24 21:21 [BlueZ RFC] media-api: Make QoS a single property Luiz Augusto von Dentz
@ 2023-08-24 21:28 ` Luiz Augusto von Dentz
  2023-08-25 15:44   ` Pauli Virtanen
  2023-08-24 22:33 ` [BlueZ,RFC] " bluez.test.bot
  1 sibling, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-24 21:28 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Pauli Virtanen, Claudia Draghicescu, iulia-tanasescu,
	Silviu Florian Barbulescu

Hi,

On Thu, Aug 24, 2023 at 2:21 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This moves QoS related propertis to a single dictionary.
> ---
>  doc/media-api.txt | 44 +++++++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index 3a0ec38e244d..9f2482e73ac1 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -816,42 +816,40 @@ Properties        object Device [readonly]
>                         Linked transport objects which the transport is
>                         associated with.
>
> -               byte CIG [ISO only, optional, experimental]
> +               dict QoS [ISO only, optional, experimental]
>
> -                       Indicates configured QoS CIG.
>                         Only present when QoS is configured.
>
> -               byte CIS [ISO only, optional, experimental]
> +                       Possible values for Unicast:
>
> -                       Indicates configured QoS CIS.
> -                       Only present when QoS is configured.
> +                       byte CIG
>
> -               uint32 Interval [ISO only, optional, experimental]
> +                               Indicates configured QoS CIG.
>
> -                       Indicates configured QoS interval.
> -                       Only present when QoS is configured.
> +                       byte CIS
>
> -               boolean Framing [ISO only, optional, experimental]
> +                               Only present when QoS is configured.
>
> -                       Indicates configured QoS framing.
> -                       Only present when QoS is configured.
> +                       uint32 Interval
>
> -               byte PHY [ISO only, optional, experimental]
> +                               Indicates configured QoS interval.
>
> -                       Indicates configured QoS PHY.
> -                       Only present when QoS is configured.
> +                       boolean Framing
>
> -               uint16 SDU [ISO only, optional, experimental]
> +                               Indicates configured QoS framing.
>
> -                       Indicates configured QoS SDU.
> -                       Only present when QoS is configured.
> +                       byte PHY
>
> -               byte Retransmissions [ISO only, optional, experimental]
> +                               Indicates configured QoS PHY.
>
> -                       Indicates configured QoS retransmissions.
> -                       Only present when QoS is configured.
> +                       uint16 SDU
>
> -               uint16 Latency [ISO only, optional, experimental]
> +                               Indicates configured QoS SDU.
>
> -                       Indicates configured QoS latency.
> -                       Only present when QoS is configured.
> +                       byte Retransmissions
> +
> +                               Indicates configured QoS retransmissions.
> +
> +                       uint16 Latency
> +
> +                               Indicates configured QoS latency.
> --
> 2.41.0

Let me know if you have anything against doing these changes, this
will break backward compatibility but since they are still marked as
experimental we can still do these changes.

-- 
Luiz Augusto von Dentz

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

* RE: [BlueZ,RFC] media-api: Make QoS a single property
  2023-08-24 21:21 [BlueZ RFC] media-api: Make QoS a single property Luiz Augusto von Dentz
  2023-08-24 21:28 ` Luiz Augusto von Dentz
@ 2023-08-24 22:33 ` bluez.test.bot
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2023-08-24 22:33 UTC (permalink / raw)
  To: linux-bluetooth, luiz.dentz

[-- Attachment #1: Type: text/plain, Size: 947 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=779177

---Test result---

Test Summary:
CheckPatch                    PASS      0.39 seconds
GitLint                       PASS      0.29 seconds
BuildEll                      PASS      26.84 seconds
BluezMake                     PASS      780.95 seconds
MakeCheck                     PASS      12.05 seconds
MakeDistcheck                 PASS      156.04 seconds
CheckValgrind                 PASS      251.09 seconds
CheckSmatch                   PASS      340.10 seconds
bluezmakeextell               PASS      102.47 seconds
IncrementalBuild              PASS      656.04 seconds
ScanBuild                     PASS      1031.51 seconds



---
Regards,
Linux Bluetooth


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

* Re: [BlueZ RFC] media-api: Make QoS a single property
  2023-08-24 21:28 ` Luiz Augusto von Dentz
@ 2023-08-25 15:44   ` Pauli Virtanen
  2023-08-25 18:34     ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 5+ messages in thread
From: Pauli Virtanen @ 2023-08-25 15:44 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, linux-bluetooth
  Cc: Claudia Draghicescu, iulia-tanasescu, Silviu Florian Barbulescu

Hi Luiz,

to, 2023-08-24 kello 14:28 -0700, Luiz Augusto von Dentz kirjoitti:
> Hi,
> 
> On Thu, Aug 24, 2023 at 2:21 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > 
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > 
> > This moves QoS related propertis to a single dictionary.
> > ---
> >  doc/media-api.txt | 44 +++++++++++++++++++++-----------------------
> >  1 file changed, 21 insertions(+), 23 deletions(-)
> > 
> > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > index 3a0ec38e244d..9f2482e73ac1 100644
> > --- a/doc/media-api.txt
> > +++ b/doc/media-api.txt
> > @@ -816,42 +816,40 @@ Properties        object Device [readonly]
> >                         Linked transport objects which the transport is
> >                         associated with.
> > 
> > -               byte CIG [ISO only, optional, experimental]
> > +               dict QoS [ISO only, optional, experimental]
> > 
> > -                       Indicates configured QoS CIG.
> >                         Only present when QoS is configured.
> > 
> > -               byte CIS [ISO only, optional, experimental]
> > +                       Possible values for Unicast:
> > 
> > -                       Indicates configured QoS CIS.
> > -                       Only present when QoS is configured.
> > +                       byte CIG
> > 
> > -               uint32 Interval [ISO only, optional, experimental]
> > +                               Indicates configured QoS CIG.
> > 
> > -                       Indicates configured QoS interval.
> > -                       Only present when QoS is configured.
> > +                       byte CIS
> > 
> > -               boolean Framing [ISO only, optional, experimental]
> > +                               Only present when QoS is configured.
> > 
> > -                       Indicates configured QoS framing.
> > -                       Only present when QoS is configured.
> > +                       uint32 Interval
> > 
> > -               byte PHY [ISO only, optional, experimental]
> > +                               Indicates configured QoS interval.
> > 
> > -                       Indicates configured QoS PHY.
> > -                       Only present when QoS is configured.
> > +                       boolean Framing

While we are breaking things, it would be useful to unify this API with
SelectProperties/SetConfiguration, where PHY is string "1M" or "2M" not
byte.

Can we make it byte everywhere? I think the enum ultimately comes from
Core spec, so not sure the string names are that useful.

> > 
> > -               uint16 SDU [ISO only, optional, experimental]
> > +                               Indicates configured QoS framing.
> > 
> > -                       Indicates configured QoS SDU.
> > -                       Only present when QoS is configured.
> > +                       byte PHY
> > 
> > -               byte Retransmissions [ISO only, optional, experimental]
> > +                               Indicates configured QoS PHY.
> > 
> > -                       Indicates configured QoS retransmissions.
> > -                       Only present when QoS is configured.
> > +                       uint16 SDU
> > 
> > -               uint16 Latency [ISO only, optional, experimental]
> > +                               Indicates configured QoS SDU.
> > 
> > -                       Indicates configured QoS latency.
> > -                       Only present when QoS is configured.
> > +                       byte Retransmissions
> > +
> > +                               Indicates configured QoS retransmissions.
> > +
> > +                       uint16 Latency
> > +
> > +                               Indicates configured QoS latency.
> > --
> > 2.41.0
> 
> Let me know if you have anything against doing these changes, this
> will break backward compatibility but since they are still marked as
> experimental we can still do these changes.

I think it's fine if Pipewire targets BlueZ master branch as long as
the feature is experimental.

-- 
Pauli Virtanen

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

* Re: [BlueZ RFC] media-api: Make QoS a single property
  2023-08-25 15:44   ` Pauli Virtanen
@ 2023-08-25 18:34     ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2023-08-25 18:34 UTC (permalink / raw)
  To: Pauli Virtanen
  Cc: linux-bluetooth, Claudia Draghicescu, iulia-tanasescu,
	Silviu Florian Barbulescu

Hi Pauli,

On Fri, Aug 25, 2023 at 8:44 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Luiz,
>
> to, 2023-08-24 kello 14:28 -0700, Luiz Augusto von Dentz kirjoitti:
> > Hi,
> >
> > On Thu, Aug 24, 2023 at 2:21 PM Luiz Augusto von Dentz
> > <luiz.dentz@gmail.com> wrote:
> > >
> > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > >
> > > This moves QoS related propertis to a single dictionary.
> > > ---
> > >  doc/media-api.txt | 44 +++++++++++++++++++++-----------------------
> > >  1 file changed, 21 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/doc/media-api.txt b/doc/media-api.txt
> > > index 3a0ec38e244d..9f2482e73ac1 100644
> > > --- a/doc/media-api.txt
> > > +++ b/doc/media-api.txt
> > > @@ -816,42 +816,40 @@ Properties        object Device [readonly]
> > >                         Linked transport objects which the transport is
> > >                         associated with.
> > >
> > > -               byte CIG [ISO only, optional, experimental]
> > > +               dict QoS [ISO only, optional, experimental]
> > >
> > > -                       Indicates configured QoS CIG.
> > >                         Only present when QoS is configured.
> > >
> > > -               byte CIS [ISO only, optional, experimental]
> > > +                       Possible values for Unicast:
> > >
> > > -                       Indicates configured QoS CIS.
> > > -                       Only present when QoS is configured.
> > > +                       byte CIG
> > >
> > > -               uint32 Interval [ISO only, optional, experimental]
> > > +                               Indicates configured QoS CIG.
> > >
> > > -                       Indicates configured QoS interval.
> > > -                       Only present when QoS is configured.
> > > +                       byte CIS
> > >
> > > -               boolean Framing [ISO only, optional, experimental]
> > > +                               Only present when QoS is configured.
> > >
> > > -                       Indicates configured QoS framing.
> > > -                       Only present when QoS is configured.
> > > +                       uint32 Interval
> > >
> > > -               byte PHY [ISO only, optional, experimental]
> > > +                               Indicates configured QoS interval.
> > >
> > > -                       Indicates configured QoS PHY.
> > > -                       Only present when QoS is configured.
> > > +                       boolean Framing
>
> While we are breaking things, it would be useful to unify this API with
> SelectProperties/SetConfiguration, where PHY is string "1M" or "2M" not
> byte.
>
> Can we make it byte everywhere? I think the enum ultimately comes from
> Core spec, so not sure the string names are that useful.

Fair enough, there was already some feedback that it doesn't
correspond to the values when read to the socket, so yeah I will
change that was well.

> > >
> > > -               uint16 SDU [ISO only, optional, experimental]
> > > +                               Indicates configured QoS framing.
> > >
> > > -                       Indicates configured QoS SDU.
> > > -                       Only present when QoS is configured.
> > > +                       byte PHY
> > >
> > > -               byte Retransmissions [ISO only, optional, experimental]
> > > +                               Indicates configured QoS PHY.
> > >
> > > -                       Indicates configured QoS retransmissions.
> > > -                       Only present when QoS is configured.
> > > +                       uint16 SDU
> > >
> > > -               uint16 Latency [ISO only, optional, experimental]
> > > +                               Indicates configured QoS SDU.
> > >
> > > -                       Indicates configured QoS latency.
> > > -                       Only present when QoS is configured.
> > > +                       byte Retransmissions
> > > +
> > > +                               Indicates configured QoS retransmissions.
> > > +
> > > +                       uint16 Latency
> > > +
> > > +                               Indicates configured QoS latency.
> > > --
> > > 2.41.0
> >
> > Let me know if you have anything against doing these changes, this
> > will break backward compatibility but since they are still marked as
> > experimental we can still do these changes.
>
> I think it's fine if Pipewire targets BlueZ master branch as long as
> the feature is experimental.

We are planning to do a release first before starting to do these changes.

>
> --
> Pauli Virtanen



-- 
Luiz Augusto von Dentz

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

end of thread, other threads:[~2023-08-25 18:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-24 21:21 [BlueZ RFC] media-api: Make QoS a single property Luiz Augusto von Dentz
2023-08-24 21:28 ` Luiz Augusto von Dentz
2023-08-25 15:44   ` Pauli Virtanen
2023-08-25 18:34     ` Luiz Augusto von Dentz
2023-08-24 22:33 ` [BlueZ,RFC] " bluez.test.bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).