Hi Chan-yeol,
On Mon, May 18, 2015 at 4:33 PM, <chanyeol.park@xxxxxxxxxxx> wrote:
> From: Chan-yeol Park <chanyeol.park@xxxxxxxxx>
>
> When matching remote SEP to local SEP avdtp does not compare 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 it by making avdtp matching a2dp vendor codec by
> a2dp's call back.
> ---
> profiles/audio/a2dp.c | 41 +++++++++++++++++++++++++++++++++++++----
> profiles/audio/avdtp.c | 7 ++++++-
> profiles/audio/avdtp.h | 6 +++++-
> 3 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index 31c5086..291be35 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -925,6 +925,35 @@ static gboolean close_ind(struct avdtp *session, struct avdtp_local_sep *sep,
> return TRUE;
> }
>
> +static gboolean a2dp_vndcodec_matched(void *remote_vndcodec, void *user_data)
> +{
> + struct a2dp_sep *sep = user_data;
> + a2dp_vendor_codec_t *remote_codec = remote_vndcodec;
> + a2dp_vendor_codec_t *local_codec;
> + uint8_t *capabilities;
> + size_t length;
> +
> + if (sep->endpoint == NULL)
> + return FALSE;
> +
> + length = sep->endpoint->get_capabilities(sep, &capabilities,
> + sep->user_data);
> + if (length < sizeof(a2dp_vendor_codec_t))
> + return FALSE;
> +
> + local_codec = (a2dp_vendor_codec_t *) capabilities;
> +
> + if (remote_codec->vendor_id != local_codec->vendor_id)
> + return FALSE;
> +
> + if (remote_codec->codec_id != local_codec->codec_id)
> + return FALSE;
> +
> + DBG("vendor 0x%08x codec 0x%04x", btohl(remote_codec->vendor_id),
> + btohs(remote_codec->codec_id));
> + return TRUE;
> +}
That was not what I has initially in mind, anyway Im not against
adding a callback to interact with avdtp_find_remote_sep but perhaps
it should happen when registering the endpoint so AVDTP code don't
need to check if codec is 0xff, instead it just call the callback if
it exists after checking the codec. That means adding a match callback
avdtp_sep_ind:
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index 1b02232..2d55a20 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -172,6 +172,8 @@ struct avdtp_sep_cfm {
/* Callbacks for indicating when we received a new command. The return value
* indicates whether the command should be rejected or accepted */
struct avdtp_sep_ind {
+ gboolean (*match) (struct avdtp *session, struct avdtp_local_sep *lsep,
+ struct avdtp_remote_sep *rsep);
gboolean (*get_capability) (struct avdtp *session,
struct avdtp_local_sep *sep,
gboolean get_all,
> static gboolean a2dp_reconfigure(gpointer data)
> {
> struct a2dp_setup *setup = data;
> @@ -939,7 +968,8 @@ static gboolean a2dp_reconfigure(gpointer data)
> }
>
> if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
> - setup->rsep = avdtp_find_remote_sep(setup->session, sep->lsep);
> + setup->rsep = avdtp_find_remote_sep(setup->session, sep->lsep,
> + a2dp_vndcodec_matched);
>
> posix_err = avdtp_set_configuration(setup->session, setup->rsep,
> sep->lsep,
> @@ -1769,7 +1799,8 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
> continue;
> }
>
> - rsep = avdtp_find_remote_sep(session, sep->lsep);
> + rsep = avdtp_find_remote_sep(session, sep->lsep,
> + a2dp_vndcodec_matched);
> if (rsep == NULL)
> continue;
>
> @@ -1835,7 +1866,8 @@ unsigned int a2dp_select_capabilities(struct avdtp *session,
> cb_data->user_data = user_data;
>
> setup->sep = sep;
> - setup->rsep = avdtp_find_remote_sep(session, sep->lsep);
> + setup->rsep = avdtp_find_remote_sep(session, sep->lsep,
> + a2dp_vndcodec_matched);
>
> if (setup->rsep == NULL) {
> error("Could not find remote sep");
> @@ -1934,7 +1966,8 @@ unsigned int a2dp_config(struct avdtp *session, struct a2dp_sep *sep,
> break;
> }
>
> - setup->rsep = avdtp_find_remote_sep(session, sep->lsep);
> + setup->rsep = avdtp_find_remote_sep(session, sep->lsep,
> + a2dp_vndcodec_matched);
> if (setup->rsep == NULL) {
> error("No matching ACP and INT SEPs found");
> goto failed;
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index f38188f..ab07128 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -1198,7 +1198,8 @@ static struct avdtp_local_sep *find_local_sep_by_seid(struct avdtp *session,
> }
>
> struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> - struct avdtp_local_sep *lsep)
> + struct avdtp_local_sep *lsep,
> + avdtp_vndcodec_matched_cb cb)
> {
> GSList *l;
>
> @@ -1226,6 +1227,10 @@ struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> if (codec_data->media_codec_type != lsep->codec)
> continue;
>
> + if (codec_data->media_codec_type == 0xff && cb)
> + if (cb(codec_data->data, lsep->user_data) == FALSE)
> + continue;
> +
> if (sep->stream == NULL)
> return sep;
> }
> diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
> index 1b02232..5886691 100644
> --- a/profiles/audio/avdtp.h
> +++ b/profiles/audio/avdtp.h
> @@ -277,9 +277,13 @@ struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
> struct avdtp_sep_cfm *cfm,
> void *user_data);
>
> +typedef gboolean (*avdtp_vndcodec_matched_cb) (void *remote_vndcodec,
> + void *user_data);
> +
> /* 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);
> + struct avdtp_local_sep *lsep,
> + avdtp_vndcodec_matched_cb cb);
>
> int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep);
>
> --
> 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
--
Luiz Augusto von Dentz
--
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
