Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 29 Apr 2019 18:39:16 +0200
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH v3 3/3] a2dp: Fix not calling SelectConfiguration on
 other available endpoints
Message-ID: <20190429163916.ngdqk437xbuahl4k@pali>
References: <20190429120259.17880-1-luiz.dentz@gmail.com>
 <20190429120259.17880-3-luiz.dentz@gmail.com>
 <CABBYNZK1LRANfR6cwu+xbLsvy4EnB+ZPS78JNK04mJ=pwTGSDw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="lv4xcf2gga6zz6ag"
Content-Disposition: inline
In-Reply-To: <CABBYNZK1LRANfR6cwu+xbLsvy4EnB+ZPS78JNK04mJ=pwTGSDw@mail.gmail.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk


--lv4xcf2gga6zz6ag
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Monday 29 April 2019 19:35:24 Luiz Augusto von Dentz wrote:
> Hi Pali,
>=20
> On Mon, Apr 29, 2019 at 3:03 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > Endpoint may not be able to select a valid configuration but there could
> > be other endpoints available that could be used, so instead of just
> > using the first match this collects all the matching endpoints and put
> > them into a queue (ordered by priority) then proceed to next endpoint
> > only failing if none of the available endpoits can select a valid
> > configuration.
> > ---
> >  profiles/audio/a2dp.c | 77 ++++++++++++++++++++++++++++---------------
> >  1 file changed, 50 insertions(+), 27 deletions(-)
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index a23abdd97..4d378a91a 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -105,6 +105,7 @@ struct a2dp_setup_cb {
> >  struct a2dp_setup {
> >         struct a2dp_channel *chan;
> >         struct avdtp *session;
> > +       struct queue *eps;
> >         struct a2dp_sep *sep;
> >         struct a2dp_remote_sep *rsep;
> >         struct avdtp_stream *stream;
> > @@ -2406,23 +2407,44 @@ void a2dp_remove_sep(struct a2dp_sep *sep)
> >
> >  static void select_cb(struct a2dp_setup *setup, void *ret, int size)
> >  {
> > -       if (size < 0) {
> > -               DBG("Endpoint replied an invalid configuration");
> > +       struct avdtp_service_capability *service;
> > +       struct avdtp_media_codec_capability *codec;
> > +       int err;
> > +
> > +       if (size) {
> > +               caps_add_codec(&setup->caps, setup->sep->codec, ret, si=
ze);
> >                 goto done;
> >         }
> >
> > -       caps_add_codec(&setup->caps, setup->sep->codec, ret, size);
> > +       setup->sep =3D queue_pop_head(setup->eps);
> > +       if (!setup->sep) {
> > +               error("Unable to select a valid configuration");
> > +               queue_destroy(setup->eps, NULL);
> > +               goto done;
> > +       }
> > +
> > +       setup->rsep =3D find_remote_sep(setup->chan, setup->sep);
> > +       service =3D avdtp_get_codec(setup->rsep->sep);
> > +       codec =3D (struct avdtp_media_codec_capability *) service->data;
> > +
> > +       err =3D setup->sep->endpoint->select_configuration(setup->sep,
> > +                                       codec->data,
> > +                                       service->length - sizeof(*codec=
),
> > +                                       setup_ref(setup),
> > +                                       select_cb, setup->sep->user_dat=
a);
> > +       if (err =3D=3D 0)
> > +               return;
> >
> >  done:
> >         finalize_select(setup);
> >         setup_unref(setup);
> >  }
> >
> > -static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *l=
ist,
> > +static struct queue *a2dp_find_eps(struct avdtp *session, GSList *list,
> >                                         const char *sender)
> >  {
> > -       struct a2dp_sep *first =3D NULL;
> >         struct a2dp_channel *chan =3D find_channel(session);
> > +       struct queue *seps =3D NULL;
> >
> >         for (; list; list =3D list->next) {
> >                 struct a2dp_sep *sep =3D list->data;
> > @@ -2444,26 +2466,25 @@ static struct a2dp_sep *a2dp_find_sep(struct av=
dtp *session, GSList *list,
> >                 if (!rsep)
> >                         continue;
> >
> > -               /* Locate last used if set */
> > -               if (chan->last_used) {
> > -                       if (chan->last_used->sep =3D=3D rsep)
> > -                               return sep;
> > -                       first =3D sep;
> > -                       continue;
> > -               }
> > +               if (!seps)
> > +                       seps =3D queue_new();
> >
> > -               return sep;
> > +               /* Prepend last used so it is preferred over others */
> > +               if (chan->last_used && chan->last_used->sep =3D=3D rsep)
> > +                       queue_push_head(seps, sep);
> > +               else
> > +                       queue_push_tail(seps, sep);
> >
> >         }
> >
> > -       return first;
> > +       return seps;
> >  }
> >
> > -static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t=
 type,
> > +static struct queue *a2dp_select_eps(struct avdtp *session, uint8_t ty=
pe,
> >                                         const char *sender)
> >  {
> >         struct a2dp_server *server;
> > -       struct a2dp_sep *sep;
> > +       struct queue *seps;
> >         GSList *l;
> >
> >         server =3D find_server(servers, avdtp_get_adapter(session));
> > @@ -2473,11 +2494,11 @@ static struct a2dp_sep *a2dp_select_sep(struct =
avdtp *session, uint8_t type,
> >         l =3D type =3D=3D AVDTP_SEP_TYPE_SINK ? server->sources : serve=
r->sinks;
> >
> >         /* Check sender's seps first */
> > -       sep =3D a2dp_find_sep(session, l, sender);
> > -       if (sep !=3D NULL)
> > -               return sep;
> > +       seps =3D a2dp_find_eps(session, l, sender);
> > +       if (seps !=3D NULL)
> > +               return seps;
> >
> > -       return a2dp_find_sep(session, l, NULL);
> > +       return a2dp_find_eps(session, l, NULL);
> >  }
> >
> >  static void store_remote_sep(void *data, void *user_data)
> > @@ -2580,13 +2601,13 @@ unsigned int a2dp_select_capabilities(struct av=
dtp *session,
> >  {
> >         struct a2dp_setup *setup;
> >         struct a2dp_setup_cb *cb_data;
> > -       struct a2dp_sep *sep;
> > +       struct queue *eps;
> >         struct avdtp_service_capability *service;
> >         struct avdtp_media_codec_capability *codec;
> >         int err;
> >
> > -       sep =3D a2dp_select_sep(session, type, sender);
> > -       if (!sep) {
> > +       eps =3D a2dp_select_eps(session, type, sender);
> > +       if (!eps) {
> >                 error("Unable to select SEP");
> >                 return 0;
> >         }
> > @@ -2599,8 +2620,9 @@ unsigned int a2dp_select_capabilities(struct avdt=
p *session,
> >         cb_data->select_cb =3D cb;
> >         cb_data->user_data =3D user_data;
> >
> > -       setup->sep =3D sep;
> > -       setup->rsep =3D find_remote_sep(setup->chan, sep);
> > +       setup->eps =3D eps;
> > +       setup->sep =3D queue_pop_head(eps);
> > +       setup->rsep =3D find_remote_sep(setup->chan, setup->sep);
> >
> >         if (setup->rsep =3D=3D NULL) {
> >                 error("Could not find remote sep");
> > @@ -2610,10 +2632,11 @@ unsigned int a2dp_select_capabilities(struct av=
dtp *session,
> >         service =3D avdtp_get_codec(setup->rsep->sep);
> >         codec =3D (struct avdtp_media_codec_capability *) service->data;
> >
> > -       err =3D sep->endpoint->select_configuration(sep, codec->data,
> > +       err =3D setup->sep->endpoint->select_configuration(setup->sep,
> > +                                       codec->data,
> >                                         service->length - sizeof(*codec=
),
> >                                         setup_ref(setup),
> > -                                       select_cb, sep->user_data);
> > +                                       select_cb, setup->sep->user_dat=
a);
> >         if (err =3D=3D 0)
> >                 return cb_data->id;
> >
> > --
> > 2.20.1
>=20
> Le me know if you find any problem with this change, my headsets seems
> to always succeed the first try so I cannot really reproduce the
> problem.

Ok, I will try it in next days. I can register PA endpoints which always
"fails" so I can test if this is working correctly as expected.

Btw, for future patches please directly CC them to me as I'm not
subscribed to list and either extracting them from '> ' quotes or
finding them in web archive is quite impractical.

--=20
Pali Roh=C3=A1r
pali.rohar@gmail.com

--lv4xcf2gga6zz6ag
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQS4VrIQdKium2krgIWL8Mk9A+RDUgUCXMcosAAKCRCL8Mk9A+RD
Uo0PAJ9Sm5XK6j/r8ZmCBo3t6Y58qZLbowCfXfqQF7DhCeNUxfbMNKGHt0ZYj9U=
=mPSy
-----END PGP SIGNATURE-----

--lv4xcf2gga6zz6ag--