Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <b5dc735326b930322ca3dbfb50addc6de314e329.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 14:28:51 +0300
In-Reply-To: <9100eb19ee8fb16e78cfee86e953dfe2afd6efe9.camel@iki.fi>
References: <20230922192335.1060601-1-luiz.dentz@gmail.com>
         <20230922192335.1060601-6-luiz.dentz@gmail.com>
         <9100eb19ee8fb16e78cfee86e953dfe2afd6efe9.camel@iki.fi>
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,

la, 2023-09-23 kello 12:14 +0300, Pauli Virtanen kirjoitti:
> Hi Luiz,
>=20
> 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.
>=20
> I made a pass looking at the implementations of the transport/endpoint
> and SetConfiguration/SelectProperties/SelectConfiguration and comparing
> to docs.
>=20
> Note that the SelectProperties API has a problem with BAP endpoint
> configuration. We maybe should redesign it while we still can?
>=20
> 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.
>=20
>=20
> SelectProperties API problem:
>=20
> As BAP client one first does Config Codec (ASCS =C2=A75.1) which takes
> Target_Latency, Target_PHY and Codec.
>=20
> 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.
>=20
> 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.
>=20
> The client-invoked SetConfiguration API also seems to have similar
> issue.
>=20
> 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
>=20
> 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.
>=20
> I can take a look at this...
>=20
>=20
> MediaTransport:
>=20
> "Delay" field only exists for A2DP in code.
>=20
> "Volume" field only exists for A2DP in code.
>=20
> "Links" is ucast only in code.
>=20
> "QoS.TargetLatency" does not exist in code. Maybe it should be exposed
> for consistency, since it's expected as return from SelectProperties.
>=20
>=20
> SetConfiguration:
>=20
> SetConfiguration properties dict when called by BlueZ contains exactly
> the properties of the transport. The documentation probably should say
> that this is so.
>=20
> 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.

Also, "Framing" is boolean here (elsewhere it's byte).

>=20
> 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.
>=20
> "MaximumLatency": no such field in code, seems to be called "Latency"
> everywhere.
>=20
>=20
> SelectProperties:
>=20
> In code, the return parameter expects QoS parameters (struct
> bt_bap_qos) to be packed in "QoS" dict.
>=20
> 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.
>=20
> 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.
>=20
>=20
> MediaEndpoint:
>=20
> The endpoints published by BlueZ only have the properties
> "UUID", "Codec", "Capabilities", "Device".
>=20
> The documentation lists various other things, but they are not
> implemented as server endpoint properties.
>=20
> As client endpoint properties:
>=20
> "RTN" is expected to be present in code, but is not documented.

"RTN" is called "Retransmissions" elsewhere.

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