[RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible

[RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

This patch gives the priority on vendor codec during A2DP codec
negotiation.
---
 profiles/audio/a2dp.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 31c5086..d9a2f72 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1751,6 +1751,8 @@ static gboolean check_vendor_codec(struct a2dp_sep *sep, uint8_t *cap,
 static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 					const char *sender)
 {
+	struct a2dp_sep *selected_sep = NULL;
+
 	for (; list; list = list->next) {
 		struct a2dp_sep *sep = list->data;
 		struct avdtp_remote_sep *rsep;
@@ -1776,15 +1778,20 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 		service = avdtp_get_codec(rsep);
 		cap = (struct avdtp_media_codec_capability *) service->data;
 
-		if (cap->media_codec_type != A2DP_CODEC_VENDOR)
-			return sep;
+		if (cap->media_codec_type != A2DP_CODEC_VENDOR) {
+			selected_sep = sep;
+			continue;
+		}
 
 		if (check_vendor_codec(sep, cap->data,
 					service->length - sizeof(*cap)))
 			return sep;
 	}
 
-	return NULL;
+	if (selected_sep)
+		return selected_sep;
+	else
+		return NULL;
 }
 
 static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
-- 
2.1.0

[RFC BlueZ 2/3] audio/a2dp: Fix SEP selection

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

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@tieto.com>.
---
 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 d9a2f72..c832048 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);
+		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

[RFC BlueZ 3/3] audio/a2dp: Remove useless check_vendor_codec()

[chanyeol.park]

From: Chan-yeol Park <chanyeol.park@samsung.com>

This function could be removed because A2DP vendor codec match is already
verified in avdtp_find_remote_sep().
---
 profiles/audio/a2dp.c | 42 ++----------------------------------------
 1 file changed, 2 insertions(+), 40 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index c832048..856b19b 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1723,42 +1723,6 @@ done:
 	finalize_select(setup);
 }
 
-static gboolean check_vendor_codec(struct a2dp_sep *sep, uint8_t *cap,
-								size_t len)
-{
-	uint8_t *capabilities;
-	size_t length;
-	a2dp_vendor_codec_t *local_codec;
-	a2dp_vendor_codec_t *remote_codec;
-
-	if (len < sizeof(a2dp_vendor_codec_t))
-		return FALSE;
-
-	remote_codec = (a2dp_vendor_codec_t *) cap;
-
-	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 (btohl(remote_codec->vendor_id) != btohl(local_codec->vendor_id))
-		return FALSE;
-
-	if (btohs(remote_codec->codec_id) != btohs(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;
-}
-
 static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 					const char *sender)
 {
@@ -1792,11 +1756,9 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 		if (cap->media_codec_type != A2DP_CODEC_VENDOR) {
 			selected_sep = sep;
 			continue;
-		}
-
-		if (check_vendor_codec(sep, cap->data,
-					service->length - sizeof(*cap)))
+		} else {
 			return sep;
+		}
 	}
 
 	if (selected_sep)
-- 
2.1.0

Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible

[Luiz Augusto von Dentz]

Hi Chan-yeol,

On Wed, May 6, 2015 at 6:10 PM,  <chanyeol.park@samsung.com> wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
>
> This patch gives the priority on vendor codec during A2DP codec
> negotiation.

There is already a prioritization based on the sender and in the order
the endpoints are registered, so by doing this you would be breaking
the prioritization instead register the codecs in order of preference
that should have the same results.

> ---
>  profiles/audio/a2dp.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index 31c5086..d9a2f72 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -1751,6 +1751,8 @@ static gboolean check_vendor_codec(struct a2dp_sep *sep, uint8_t *cap,
>  static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
>                                         const char *sender)
>  {
> +       struct a2dp_sep *selected_sep = NULL;
> +
>         for (; list; list = list->next) {
>                 struct a2dp_sep *sep = list->data;
>                 struct avdtp_remote_sep *rsep;
> @@ -1776,15 +1778,20 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
>                 service = avdtp_get_codec(rsep);
>                 cap = (struct avdtp_media_codec_capability *) service->data;
>
> -               if (cap->media_codec_type != A2DP_CODEC_VENDOR)
> -                       return sep;
> +               if (cap->media_codec_type != A2DP_CODEC_VENDOR) {
> +                       selected_sep = sep;
> +                       continue;
> +               }
>
>                 if (check_vendor_codec(sep, cap->data,
>                                         service->length - sizeof(*cap)))
>                         return sep;
>         }
>
> -       return NULL;
> +       if (selected_sep)
> +               return selected_sep;
> +       else
> +               return NULL;
>  }
>
>  static struct a2dp_sep *a2dp_select_sep(struct avdtp *session, uint8_t type,
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible

[Chan-yeol Park]

Hi Luiz,
On 05/07/2015 05:20 PM, Luiz Augusto von Dentz wrote:
> Hi Chan-yeol,
>
> On Wed, May 6, 2015 at 6:10 PM,  <chanyeol.park@samsung.com> wrote:
>> From: Chan-yeol Park <chanyeol.park@samsung.com>
>>
>> This patch gives the priority on vendor codec during A2DP codec
>> negotiation.
>
> There is already a prioritization based on the sender and in the order
> the endpoints are registered, so by doing this you would be breaking
> the prioritization instead register the codecs in order of preference
> that should have the same results.
>

Yes you're right. but most of phone(Android/iOS) register their SBC in 
the first SEP to support compatibility because some of headset try to 
connect first SEP without codec comparison. As a result I could not 
register vendor codec in the first SEP. So without this patch vendor 
codec establishment would not be made.

I am not clear how to give the priority on vendor codec in this case.

Thanks
Chanyeol

Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible

[Luiz Augusto von Dentz]

Hi Chanyeol,

On Tue, May 26, 2015 at 9:28 AM, Chan-yeol Park
<chanyeol.park@samsung.com> wrote:
> Hi Luiz,
> On 05/07/2015 05:20 PM, Luiz Augusto von Dentz wrote:
>>
>> Hi Chan-yeol,
>>
>> On Wed, May 6, 2015 at 6:10 PM,  <chanyeol.park@samsung.com> wrote:
>>>
>>> From: Chan-yeol Park <chanyeol.park@samsung.com>
>>>
>>> This patch gives the priority on vendor codec during A2DP codec
>>> negotiation.
>>
>>
>> There is already a prioritization based on the sender and in the order
>> the endpoints are registered, so by doing this you would be breaking
>> the prioritization instead register the codecs in order of preference
>> that should have the same results.
>>
>
> Yes you're right. but most of phone(Android/iOS) register their SBC in the
> first SEP to support compatibility because some of headset try to connect
> first SEP without codec comparison. As a result I could not register vendor
> codec in the first SEP. So without this patch vendor codec establishment
> would not be made.

Hmm, I did not know such devices exists, so they connect with first
set no matter what? Than perhaps we need a priority flag which we can
then check while selecting.

-- 
Luiz Augusto von Dentz