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

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Pali,

On Mon, Jun 10, 2019 at 3:35 PM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Friday 07 June 2019 18:18:04 Luiz Augusto von Dentz wrote:
> > Hi Pali,
> >
> > On Fri, Jun 7, 2019 at 4:04 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > >
> > > On Tuesday 07 May 2019 20:57:40 Pali Rohár wrote:
> > > > 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.
> > >
> > > So what about this my idea? Is there anything wrong with such thing?
> >
> > That makes the assumption that the path doesn't ever change which is
> > something we never required for the endpoints, besides can be clashes
> > if the same path is used in which case the stored value would be
> > ambiguous, so I prefer to just have the endpoint id, if we are so
> > eager to detect endpoint changes we can save some hash/unique id
> > generated but I think it is a bit overkill if this is working as it
> > currently is and we can always fallback if we cannot recover the last
> > used, anyway this is not even supported by the spec since it has no
> > guarantees regarding peers endpoints uniqueness.
>
> My main objection against endpoint id is that it is less stable then
> application dbus path. Endpoint id depends on ordering which is defined
> by all application which registers to dbus. And so it does not depend on
> application itself. On the other hand endpoint dbus path is fully under
> application control, so independently of other applications, dbus path
> can be stable -- if application make it stable.

Yes, it may be more stable but it is also not unique so in the event 2
endpoints happen to have the same path we would have to disambiguate
them somehow and because of that I don't think we should be using
that. Im not sure why this would be a big deal since we should call
SelectConfiguration it is not like we would end up in a situation
where the stream would fail to setup since if that fails we fallback
to the priority ordering, if the user felt something out of order it
was either the system has changed (PA update, etc) or the device has
changed (new firmware, etc).

> > > > > 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
>
> --
> Pali Rohár
> pali.rohar@gmail.com



-- 
Luiz Augusto von Dentz