Re: [PATCH v3 6/6] a2dp: Reword LastUsed

["Pali Rohár" <pali.rohar@gmail.com>]

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

On Tuesday 07 May 2019 13:13:17 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Tue, May 7, 2019 at 11:52 AM Pali Rohár <pali.rohar@gmail.com> wrote:
> >
> > On Monday 06 May 2019 15:02:25 Pali Rohár wrote:
> > > On Monday 06 May 2019 15:46:03 Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > I hope this fixes the problems you have been seeing, it should at
> > > > least take care of the issues with wrong order of SelectConfiguration
> > > > and restoring the exact same endpoint used last time.
> > >
> > > Hi Luiz! Great, this should make bluez to be more deterministic.
> >
> > Hi! Now I looked at it and in cache file I see:
> >
> > LastUsed=04:01
> >
> > What would happen when A2DP application (e.g. pulseaudio) register DBus
> > endpoints in different order? Or e.g. do not register some endpoints due
> > to missing codec librayr (aptX).
> >
> > I guess that in this case LastUsed information stops working...
> 
> It would most likely fail at SelectConfiguration and then try with the
> other endpoints.

Yes, that is truth. My point is just about validity of LastUsed value.

> >
> > Should not be there stored rather dbus endpoint name identifier?
> 
> Originally I tried to avoid having the local endpoints because of this
> problem, but now that SelectConfiguration can fail it shouldn't really
> be a problem. At least with the seid if you have a system that didn't
> changed the order or number of endpoints it will keep working as
> expected,

But this may happen. And such thing is allowed. Any application,
including unprivileged can register own new endpoint to bluez. It is not
specific to pulseaudio. And in my opinion central bluetooth daemon which
expose such functionality should be robust and be prepared that
application on "other side of dbus IPC" does not have to be well
behaved.

> if we choose to encode the D-Bus connection, etc, as soon as
> PA is restarted, the system is rebooted, etc, the D-Bus connection may
> have changed making the stored values in LastUsed invalid.

That is truth.

My idea was to encode just dbus path of local dbus endpoint. We can say
(in IPC API) that client application should preserve dbus path for one
same endpoint between dbus daemon / computer restarts. Because otherwise
functionality of "choose last used endpoint" would not work.

This is less strict requirement and current one (that registration order
of all existing applications in system must be same across reboots),
less error prone and still should be easily implemented.

> Anyway
> chances are the LastUsed is only invalidated if you update PA, in

Or once we include support for dynamic codec loading (based on encoder
library presence in system), installation of any irrelevant application
may bring a new supported codec and therefore a new endpoint. So codecs
can become in any order...

Or another example, when different application (not PA) register also
some endpoint.

> which case there could be new endpoints or a change in their order and
> package can also provide a script to clear the LastUsed if that
> happens, but then again LastUsed setting does not actually store the
> configuration just the endpoint so SelectConfiguration has control
> over the configuration.

Yes, whole thing is only about initial codec selection. In the worst
case user would have pre-selected different codec as which was last
used.

But I think that storing dbus path of endpoint as described above should
be more reliable solution.

Applications (e.g. PA) talking to bluetooth daemon does not register or
request directly SEID. Instead they register their dbus path and dbus
connection and bluetooth daemon later allocates SEID for that path.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]