Hi, Luiz
On Fri, 2015-05-08 at 16:56 +0300, Luiz Augusto von Dentz wrote:
> Hi Chan-yeol,
>
> On Thu, May 7, 2015 at 1:02 PM, <chanyeol.park@xxxxxxxxxxx> wrote:
> > From: Chan-yeol Park <chanyeol.park@xxxxxxxxxxx>
> >
> > When matching remote SEP to local SEP we do not match vendor codecs
> > properly, i.e. neither vendor ID not codec ID are checked, which may
> > cause wrong endpoint to be selected in case there are more that one
> > endpoints using vendor codec on remote.
> >
> > This patch fixes this by assinging vendor codec indentification to
> > local SEP after it's registered and uses this information when matching
> > SEPs.
> >
> > This patch is made based on e1c7dddd0dd2f5e23e4d4cf98a9dde713fe6dd53
> > written by Andrzej Kaczmarek <andrzej.kaczmarek@xxxxxxxxx>.
> > ---
> > profiles/audio/a2dp.c | 11 +++++++++++
> > profiles/audio/avdtp.c | 22 ++++++++++++++++++++++
> > profiles/audio/avdtp.h | 3 ++-
> > 3 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index 31c5086..85ec753 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -1597,6 +1597,17 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
> > return NULL;
> > }
> >
> > + if (codec == A2DP_CODEC_VENDOR) {
> > + uint8_t *capabilities;
> > + a2dp_vendor_codec_t *vndcodec;
> > +
> > + endpoint->get_capabilities(sep, &capabilities, user_data);
>
> This sounds much better than what we have in Android actually, but I
> just wonder why we need to store this in the first place? Instead I
> would just do what you are doing above when matching which saves
> memory and you can also skip the byte order conversion.
Originally I consider what you suggest. but I can not find how avdtp.c
access to a2dp's capabilities except avdtp_getcap_cmd(). If we use
avdtp_getcap_cmd(), it would make confusion with original usage.
If we want to follow your guidance I think we should make new call back
which allows avdtp.c access to a2dp's capabilities.
I think this was the best way I could.
> > + vndcodec = (a2dp_vendor_codec_t *) capabilities;
> > + avdtp_sep_set_vendor_codec(sep->lsep,
> > + btohl(vndcodec->vendor_id),
> > + btohs(vndcodec->codec_id));
> > + }
> > +
> > sep->server = server;
> > sep->endpoint = endpoint;
> > sep->codec = codec;
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index f38188f..df6fd42 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -49,6 +49,7 @@
> > #include "src/device.h"
> >
> > #include "avdtp.h"
> > +#include "a2dp-codecs.h"
> > #include "sink.h"
> > #include "source.h"
> >
> > @@ -328,6 +329,8 @@ struct avdtp_local_sep {
> > struct avdtp_stream *stream;
> > struct seid_info info;
> > uint8_t codec;
> > + uint32_t vndcodec_vendor;
> > + uint16_t vndcodec_codec;
> > gboolean delay_reporting;
> > GSList *caps;
> > struct avdtp_sep_ind *ind;
> > @@ -1197,6 +1200,13 @@ static struct avdtp_local_sep *find_local_sep_by_seid(struct avdtp *session,
> > return queue_find(session->lseps, match_by_seid, INT_TO_PTR(seid));
> > }
> >
> > +void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
> > + uint16_t codec_id)
> > +{
> > + sep->vndcodec_vendor = vendor_id;
> > + sep->vndcodec_codec = codec_id;
> > +}
> > +
> > struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> > struct avdtp_local_sep *lsep)
> > {
> > @@ -1226,6 +1236,18 @@ struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> > if (codec_data->media_codec_type != lsep->codec)
> > continue;
> >
> > + /* FIXME: Add Vendor Specific Codec match to SEP callback */
> > + if (lsep->codec == A2DP_CODEC_VENDOR) {
> > + a2dp_vendor_codec_t *vndcodec =
> > + (void *) codec_data->data;
> > +
> > + if (btohl(vndcodec->vendor_id) != lsep->vndcodec_vendor)
> > + continue;
> > +
> > + if (btohs(vndcodec->codec_id) != lsep->vndcodec_codec)
> > + continue;
> > + }
> > +
> > if (sep->stream == NULL)
> > return sep;
> > }
> > diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
> > index 1b02232..e237b10 100644
> > --- a/profiles/audio/avdtp.h
> > +++ b/profiles/audio/avdtp.h
> > @@ -276,7 +276,8 @@ struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
> > struct avdtp_sep_ind *ind,
> > struct avdtp_sep_cfm *cfm,
> > void *user_data);
> > -
> > +void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
> > + uint16_t codec_id);
> > /* Find a matching pair of local and remote SEP ID's */
> > struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> > struct avdtp_local_sep *lsep);
> > --
> > 2.1.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
Thanks
Chanyeol
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
