* [RFC BlueZ 2/3] audio/a2dp: Fix SEP selection
2015-05-06 15:10 [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible chanyeol.park
@ 2015-05-06 15:10 ` chanyeol.park
2015-05-06 15:10 ` [RFC BlueZ 3/3] audio/a2dp: Remove useless check_vendor_codec() chanyeol.park
2015-05-07 8:20 ` [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible Luiz Augusto von Dentz
2 siblings, 0 replies; 6+ messages in thread
From: chanyeol.park @ 2015-05-06 15:10 UTC (permalink / raw)
To: linux-bluetooth
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [RFC BlueZ 3/3] audio/a2dp: Remove useless check_vendor_codec()
2015-05-06 15:10 [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible chanyeol.park
2015-05-06 15:10 ` [RFC BlueZ 2/3] audio/a2dp: Fix SEP selection chanyeol.park
@ 2015-05-06 15:10 ` chanyeol.park
2015-05-07 8:20 ` [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible Luiz Augusto von Dentz
2 siblings, 0 replies; 6+ messages in thread
From: chanyeol.park @ 2015-05-06 15:10 UTC (permalink / raw)
To: linux-bluetooth
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible
2015-05-06 15:10 [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible chanyeol.park
2015-05-06 15:10 ` [RFC BlueZ 2/3] audio/a2dp: Fix SEP selection chanyeol.park
2015-05-06 15:10 ` [RFC BlueZ 3/3] audio/a2dp: Remove useless check_vendor_codec() chanyeol.park
@ 2015-05-07 8:20 ` Luiz Augusto von Dentz
2015-05-26 6:28 ` Chan-yeol Park
2 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-05-07 8:20 UTC (permalink / raw)
To: Chan-yeol Park; +Cc: linux-bluetooth@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible
2015-05-07 8:20 ` [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible Luiz Augusto von Dentz
@ 2015-05-26 6:28 ` Chan-yeol Park
2015-05-26 14:30 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 6+ messages in thread
From: Chan-yeol Park @ 2015-05-26 6:28 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC BlueZ 1/3] audio/a2dp: connect A2DP vendor codec if possible
2015-05-26 6:28 ` Chan-yeol Park
@ 2015-05-26 14:30 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2015-05-26 14:30 UTC (permalink / raw)
To: Chan-yeol Park; +Cc: linux-bluetooth@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread