Re: bluez: dbus method call for switching endpoint

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

Hi Pali, Pasi,

On Sat, Dec 29, 2018 at 10:08 AM Pali Rohár <pali.rohar@gmail.com> wrote:
>
> On Friday 28 December 2018 19:10:11 Luiz Augusto von Dentz wrote:
> > Hi Pasi,
> >
> > On Fri, Dec 28, 2018 at 4:46 PM Pasi Kärkkäinen <pasik@iki.fi> wrote:
> > >
> > > Hi Luiz,
> > >
> > > On Tue, Dec 18, 2018 at 01:02:39PM -0300, Luiz Augusto von Dentz wrote:
> > > > Hi Pali,
> > > >
> > > > On Sat, Dec 15, 2018 at 5:29 PM Pali Rohár <pali.rohar@gmail.com> wrote:
> > > > >
> > > > > On Wednesday 11 July 2018 16:45:01 Pali Rohár wrote:
> > > > > > On Wednesday 11 July 2018 16:27:07 Luiz Augusto von Dentz wrote:
> > > > > > > One way to solve all of these is that we would expose the remote
> > > > > > > endpoints using MediaEndpoint1
> > > > > >
> > > > > > So client application (like pulseadio) would see list of remote
> > > > > > endpoints (one for each codec) and choose one for connection?
> > > > > >
> > > > > > Looks like this should solve this problem.
> > > > >
> > > > > Are there any progress on such API in bluez?
> > > > >
> > > > > On pulseaudio mailing list other people proposed patches for other A2DP
> > > > > codecs and basically bluez API for choosing A2DP codec is still missing
> > > > > part...
> > > >
> > > > Im currenlty on vacation but will try to find time for that, that said
> > > > that shouldn't stop us to include support for other codecs, the last
> > > > set seems to have some sort of priority for ordering the registration
> > > > of the codecs.
> > > >
> > >
> > > Let us know when you have something to see/try!
> > > I'm happy to do testing of the patches.
> > >
> > > People are currently eager to get support for multiple BT audio codecs working and merged upstream :)
> >
> > Note that it is very unlikely that this API would allow multiple
> > codecs at the same time, headsets normally only allow one
> > configuration at time.
>
> Multiple codecs at the same time is not needed.
>
> > Making it possible to switch codecs is more of
> > a workaround for headsets that don't honor the codec priority since
> > they normally codec back, although some model no longer configure a
> > stream just wait the source.
>
> First thing is to provide list of supported codecs via dbus, so
> pulseaudio would know which codecs remote device (headset) support. And
> ideally tell it also to user.
>
> Second thing which is needed, is ability to switch to different
> supported codec. If headset supports 4 codecs, then user wants to switch
> from first codec to second and compare which is "better for him".
>
> And third thing, some headsets remember last used codec for particular
> notebook and when headset initialize connection it uses this codec.
> So if pulseaudio (or any other application) adds support for a new
> codec, which is also supported by that headset, then this codec would
> not be used -- even when it has higher priority in pulseaudio.
>
> I think probably all headsets (or at least majority of them) initialize
> connection to last "notebook/phone" device after starting it up.
>
> So priority defined by pulseaudio is not going to be used in above cases
> and we need a way in pulseaudio to switch from one codec to another.
>
> And forth thing, people lot of times listen music and their music is
> stored in some lossy compression codecs (MP3, AAC, ...). A2DP supports
> of these codecs and pulseaudio has already support for pass-through
> different codec payloads to sound card (if sound card driver support
> is). So this would allow us to e.g. pass-through MP3 or AAC data to
> supported headset without need to decode and encode again. Pulseaudio in
> this case can take care for switching from "better A2DP codec" to AAC
> when input application source is already in AAC; and then back to that
> previous "better A2DP codec" after AAC playback file finish.
>
> I think that similar technique is already used in Apple products which
> propagates AAC format.
>
> Therefore I do not think switching codec is some king of workaround or
> hack, but fully valid use case.

Except that we can't do that because we cannot garanteed there wont be
other sources active e.g. system notifications, etc, so in practice
complex/expensive codecs such as MP3 and AAC are hard to use since
that would mean we would have to encode on the fly. Perhaps there we
could have a setting for these type of codecs so the user would have
to opt-in if he want to just listen to those files, though if they are
using streaming services they normally decode on their own so AAC and
MP3 endpoint are not that useful with the likes of youtube, spotify,
etc.

> > Regarding the API I still didn't have time to start it, so it will
> > take a little longer than I antecipated.

Ive just sent the patches adding support to switch the endpoints, Ive
only tested with a couple of sony headsets so I would appreciate if
you guys could try it as well. Note that the SetConfiguration must
come from the same D-Bus connection as the endpoint that would be
used, also if there is already an stream in place it must also be from
the same client since it would be terminated in the process, this is
to prevent entities fighting to configure with its own priority though
usually we only PA endpoints, if you want to bypass this just for now
I have you can use the following changes:

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index cda2b6c64..e5e1e0d1c 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1574,10 +1574,11 @@ static int a2dp_reconfig(struct a2dp_channel
*chan, const char *sender,

                /* Attempt to reconfigure if a stream already exists */
                if (tmp->stream) {
-                       /* Only allow switching sep from the same sender */
-                       if (strcmp(sender, lsep->endpoint->get_name(tmp,
+                       /* Only allow switching sep from the same sender
+                       if (strcmp(sender, tmp->endpoint->get_name(tmp,
                                                        tmp->user_data)))
                                return -EPERM;
+                       */

                        err = avdtp_close(chan->session, tmp->stream, FALSE);
                        if (err < 0) {
@@ -1627,12 +1628,31 @@ static DBusMessage
*set_configuration(DBusConnection *conn, DBusMessage *msg,
        dbus_message_iter_get_basic(&args, &path);
        dbus_message_iter_next(&args);

-       lsep = find_sep(chan->server, avdtp_get_type(rsep->sep), sender, path);
+       service = avdtp_get_codec(rsep->sep);
+       codec = (struct avdtp_media_codec_capability *) service->data;
+
+       if (!strcmp(path, "/")) {
+               GSList *l;
+
+               l = avdtp_get_type(rsep->sep) == AVDTP_SEP_TYPE_SINK ?
+                                       chan->server->sources :
+                                       chan->server->sinks;
+
+               for (; l; l = g_slist_next(l)) {
+                       if (endpoint_match_codec_ind(chan->session, codec,
+                                                               l->data))
+                               break;
+               }
+
+               if (l)
+                       lsep = l->data;
+       } else
+               lsep = find_sep(chan->server, avdtp_get_type(rsep->sep), sender,
+                               path);
+
        if (!lsep)
                return btd_error_invalid_args(msg);

-       service = avdtp_get_codec(rsep->sep);
-       codec = (struct avdtp_media_codec_capability *) service->data;

        /* Check if codec really matches */
        if (!endpoint_match_codec_ind(chan->session, codec, lsep))


-- 
Luiz Augusto von Dentz