Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <9100eb19ee8fb16e78cfee86e953dfe2afd6efe9.camel@iki.fi>
Subject: Re: [PATCH v2 6/6] doc/media: Convert media-api.rst into manpages
From:   Pauli Virtanen <pav@iki.fi>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        linux-bluetooth@vger.kernel.org
Date:   Sat, 23 Sep 2023 12:14:05 +0300
In-Reply-To: <20230922192335.1060601-6-luiz.dentz@gmail.com>
References: <20230922192335.1060601-1-luiz.dentz@gmail.com>
         <20230922192335.1060601-6-luiz.dentz@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

Hi Luiz,

pe, 2023-09-22 kello 12:23 -0700, Luiz Augusto von Dentz kirjoitti:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>=20
> This splits media-api.rst into org.bluez.Media<interface>.rst and
> generate manpages for them.

I made a pass looking at the implementations of the transport/endpoint
and SetConfiguration/SelectProperties/SelectConfiguration and comparing
to docs.

Note that the SelectProperties API has a problem with BAP endpoint
configuration. We maybe should redesign it while we still can?

I think generally at least from Pipewire side there's no need to keep
backward compatibility while this is experimental. We can target BlueZ
master branch.


SelectProperties API problem:

As BAP client one first does Config Codec (ASCS =C2=A75.1) which takes
Target_Latency, Target_PHY and Codec.

After this, the ASE properties (ACSS Table 4.3) in Codec Configured
state contain the server supported values for Max_Transport_Latency,
Presentation_Delay_Min/Max etc.

SelectProperties is called before Config Codec, so it cannot
necessarily know the server supported values. In this case the sound
server cannot fill in QoS correctly.

The client-invoked SetConfiguration API also seems to have similar
issue.

Probably: SelectProperties should be called twice, once to get
parameters for Config Codec, and then again to get parameters for
Config QoS. Or, there should be a separate "SelectCodec" and
"SelectQoS" calls. Calling "SelectProperties" twice could be simpler
for everyone.=C2=A0

In client-invoked "SetConfiguration" API, one probably should only pass
in the parameters needed for Config Codec, and BlueZ should then make a
SelectProperties call to get the QoS ones once the server-side values
are known.

I can take a look at this...


MediaTransport:

"Delay" field only exists for A2DP in code.

"Volume" field only exists for A2DP in code.

"Links" is ucast only in code.

"QoS.TargetLatency" does not exist in code. Maybe it should be exposed
for consistency, since it's expected as return from SelectProperties.


SetConfiguration:

SetConfiguration properties dict when called by BlueZ contains exactly
the properties of the transport. The documentation probably should say
that this is so.

When called by client, the contents of the properties dict are expected
to be different. QoS parameters are not packed in a "QoS" dict. Maybe
they should be. The "PHY" key is also still a string here.

The documented input parameters expected from client are wrong, the
documentation explains fields of struct bt_bap_pac_qos, but what the
code parses are that of struct bt_bap_qos.

"MaximumLatency": no such field in code, seems to be called "Latency"
everywhere.


SelectProperties:

In code, the return parameter expects QoS parameters (struct
bt_bap_qos) to be packed in "QoS" dict.

They are not packed in "QoS" dict in the input parameters. These are
not exactly QoS, but instead struct bt_bap_pac_qos, so not clear if
they should be packed.

The input and return parameters are not documented. The input QoS
contains fields of struct bt_bap_pac_qos. The return QoS has fields of
struct bt_bap_qos.


MediaEndpoint:

The endpoints published by BlueZ only have the properties
"UUID", "Codec", "Capabilities", "Device".

The documentation lists various other things, but they are not
implemented as server endpoint properties.

As client endpoint properties:

"RTN" is expected to be present in code, but is not documented.

"MaximumLatency" "Location" "SupportedContext" "Context" are documented
but not present in code.


--=20
Pauli Virtanen