[PATCH 4/4] audio: Add check for non-a2dp codec

[PATCH 4/4] audio: Add check for non-a2dp codec

[chanyeol.park]

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

This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
non-a2dp codec selection.
---
 audio/a2dp-codecs.h |    6 +++++
 audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
index 51c796a..e3d2cba 100644
--- a/audio/a2dp-codecs.h
+++ b/audio/a2dp-codecs.h
@@ -26,6 +26,7 @@
 #define A2DP_CODEC_MPEG12		0x01
 #define A2DP_CODEC_MPEG24		0x02
 #define A2DP_CODEC_ATRAC		0x03
+#define A2DP_CODEC_NON_A2DP		0xFF
 
 #define SBC_SAMPLING_FREQ_16000		(1 << 3)
 #define SBC_SAMPLING_FREQ_32000		(1 << 2)
@@ -114,3 +115,8 @@ typedef struct {
 #else
 #error "Unknown byte order"
 #endif
+
+typedef struct {
+	uint8_t vendor_id[4];
+	uint8_t codec_id[2];
+} __attribute__ ((packed)) non_a2dp_vendor_codec_t;
diff --git a/audio/a2dp.c b/audio/a2dp.c
index fd1c494..9b4adb1 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -44,6 +44,7 @@
 #include "sink.h"
 #include "source.h"
 #include "a2dp.h"
+#include "a2dp-codecs.h"
 #include "sdpd.h"
 
 /* The duration that streams without users are allowed to stay in
@@ -1427,11 +1428,66 @@ done:
 	finalize_select(setup);
 }
 
+static gboolean non_a2dp_codec_is_supported(uint8_t *remote_cap,
+				size_t remote_cap_len, struct a2dp_sep *sep)
+{
+	uint8_t *capabilities;
+	size_t	length;
+
+	non_a2dp_vendor_codec_t *local_codec;
+	non_a2dp_vendor_codec_t *remote_codec;
+
+	if (remote_cap_len < sizeof(non_a2dp_vendor_codec_t))
+		return FALSE;
+
+	remote_codec = (non_a2dp_vendor_codec_t *) remote_cap;
+
+	DBG("Remote vendor id %x %x %x %x", remote_codec->vendor_id[0],
+			remote_codec->vendor_id[1], remote_codec->vendor_id[2],
+						remote_codec->vendor_id[3]);
+
+	DBG("Remote vendor codec id %x %x", remote_codec->codec_id[0],
+						remote_codec->codec_id[1]);
+
+	if (sep->endpoint == NULL)
+		return FALSE;
+
+	length = sep->endpoint->get_capabilities(sep,
+				&capabilities, sep->user_data);
+
+	if (length < sizeof(non_a2dp_vendor_codec_t))
+		return FALSE;
+
+	local_codec = (non_a2dp_vendor_codec_t *) capabilities;
+
+	DBG("Registered vendor id %x %x %x %x", local_codec->vendor_id[0],
+			local_codec->vendor_id[1], local_codec->vendor_id[2],
+						local_codec->vendor_id[3]);
+
+	DBG("Registered vendor codec id %x %x", local_codec->codec_id[0],
+						local_codec->codec_id[1]);
+
+	if (memcmp(remote_codec->vendor_id, local_codec->vendor_id,
+					sizeof(local_codec->vendor_id)))
+		return FALSE;
+
+	if (memcmp(remote_codec->codec_id, local_codec->codec_id,
+					sizeof(local_codec->codec_id)))
+		return FALSE;
+
+	DBG("Remote non a2dp codec is supported");
+
+	return TRUE;
+}
+
 static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 					const char *sender)
 {
 	for (; list; list = list->next) {
 		struct a2dp_sep *sep = list->data;
+		struct avdtp_remote_sep *rsep;
+		struct avdtp_media_codec_capability *rsep_codec;
+		struct avdtp_service_capability *service;
 
 		/* Use sender's endpoint if available */
 		if (sender) {
@@ -1445,7 +1501,17 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
 				continue;
 		}
 
-		if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
+		rsep = avdtp_find_remote_sep(session, sep->lsep);
+		if (rsep == NULL)
+			continue;
+
+		service = avdtp_get_codec(rsep);
+		rsep_codec = (struct avdtp_media_codec_capability *)
+								service->data;
+
+		if (rsep_codec->media_codec_type == A2DP_CODEC_NON_A2DP &&
+			!non_a2dp_codec_is_supported(rsep_codec->data,
+				service->length - sizeof(*rsep_codec), sep))
 			continue;
 
 		return sep;
-- 
1.7.9.5

[PATCH 3/4] audio: Remove redundant procedure when a2dp connect

[chanyeol.park]

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

This patch fixes the bug that a2dp connection failure is handled like XCASE
when remote host is down.
---
 audio/avdtp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 54b3d08..e450ec7 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -2481,7 +2481,8 @@ failed:
 			avdtp_sep_set_state(session, stream->lsep,
 						AVDTP_STATE_IDLE);
 	} else
-		connection_lost(session, EIO);
+		connection_lost(session, err->code == EHOSTDOWN ?
+						EHOSTDOWN : EIO);
 }
 
 static void auth_cb(DBusError *derr, void *user_data)
-- 
1.7.9.5

[PATCH 2/4] audio: Remove unused function

[chanyeol.park]

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

---
 audio/avdtp.c |   39 ---------------------------------------
 audio/avdtp.h |    2 --
 2 files changed, 41 deletions(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 7db3c02..54b3d08 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -3581,45 +3581,6 @@ int avdtp_set_configuration(struct avdtp *session,
 	return err;
 }
 
-int avdtp_reconfigure(struct avdtp *session, GSList *caps,
-			struct avdtp_stream *stream)
-{
-	struct reconf_req *req;
-	unsigned char *ptr;
-	int caps_len, err;
-	GSList *l;
-	struct avdtp_service_capability *cap;
-
-	if (!g_slist_find(session->streams, stream))
-		return -EINVAL;
-
-	if (stream->lsep->state != AVDTP_STATE_OPEN)
-		return -EINVAL;
-
-	/* Calculate total size of request */
-	for (l = caps, caps_len = 0; l != NULL; l = g_slist_next(l)) {
-		cap = l->data;
-		caps_len += cap->length + 2;
-	}
-
-	req = g_malloc0(sizeof(struct reconf_req) + caps_len);
-
-	req->acp_seid = stream->rseid;
-
-	/* Copy the capabilities into the request */
-	for (l = caps, ptr = req->caps; l != NULL; l = g_slist_next(l)) {
-		cap = l->data;
-		memcpy(ptr, cap, cap->length + 2);
-		ptr += cap->length + 2;
-	}
-
-	err = send_request(session, FALSE, stream, AVDTP_RECONFIGURE, req,
-						sizeof(*req) + caps_len);
-	g_free(req);
-
-	return err;
-}
-
 int avdtp_open(struct avdtp *session, struct avdtp_stream *stream)
 {
 	struct seid_req req;
diff --git a/audio/avdtp.h b/audio/avdtp.h
index e294ded..7b330b9 100644
--- a/audio/avdtp.h
+++ b/audio/avdtp.h
@@ -274,8 +274,6 @@ int avdtp_get_configuration(struct avdtp *session,
 				struct avdtp_stream *stream);
 
 int avdtp_open(struct avdtp *session, struct avdtp_stream *stream);
-int avdtp_reconfigure(struct avdtp *session, GSList *caps,
-			struct avdtp_stream *stream);
 int avdtp_start(struct avdtp *session, struct avdtp_stream *stream);
 int avdtp_suspend(struct avdtp *session, struct avdtp_stream *stream);
 int avdtp_close(struct avdtp *session, struct avdtp_stream *stream,
-- 
1.7.9.5

[PATCH 1/4] mgmt: Add string for Passkey Notify Event

[chanyeol.park]

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

---
 lib/mgmt.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 3432f94..6c7e44a 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -522,6 +522,7 @@ static const char *mgmt_ev[] = {
 	"Device Blocked",
 	"Device Unblocked",
 	"Device Unpaired",
+	"Passkey Notify",
 };
 
 static const char *mgmt_status[] = {
-- 
1.7.9.5

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Luiz Augusto von Dentz]

Hi Chanyel,

On Thu, Oct 4, 2012 at 4:14 PM,  <chanyeol.park@samsung.com> wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
>
> This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
> non-a2dp codec selection.
> ---
>  audio/a2dp-codecs.h |    6 +++++
>  audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
> index 51c796a..e3d2cba 100644
> --- a/audio/a2dp-codecs.h
> +++ b/audio/a2dp-codecs.h
> @@ -26,6 +26,7 @@
>  #define A2DP_CODEC_MPEG12              0x01
>  #define A2DP_CODEC_MPEG24              0x02
>  #define A2DP_CODEC_ATRAC               0x03
> +#define A2DP_CODEC_NON_A2DP            0xFF

I prefer A2DP_CODEC_VENDOR

>  #define SBC_SAMPLING_FREQ_16000                (1 << 3)
>  #define SBC_SAMPLING_FREQ_32000                (1 << 2)
> @@ -114,3 +115,8 @@ typedef struct {
>  #else
>  #error "Unknown byte order"
>  #endif
> +
> +typedef struct {
> +       uint8_t vendor_id[4];
> +       uint8_t codec_id[2];
> +} __attribute__ ((packed)) non_a2dp_vendor_codec_t;

We normally don't typedef this type of structs, besides
a2dp_vendor_codec should be enough so this should be named struct
a2dp_vendor_codec.

> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index fd1c494..9b4adb1 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -44,6 +44,7 @@
>  #include "sink.h"
>  #include "source.h"
>  #include "a2dp.h"
> +#include "a2dp-codecs.h"
>  #include "sdpd.h"
>
>  /* The duration that streams without users are allowed to stay in
> @@ -1427,11 +1428,66 @@ done:
>         finalize_select(setup);
>  }
>
> +static gboolean non_a2dp_codec_is_supported(uint8_t *remote_cap,
> +                               size_t remote_cap_len, struct a2dp_sep *sep)

Use vendor_codec_is_supported as function name.

> +{
> +       uint8_t *capabilities;
> +       size_t  length;
> +
> +       non_a2dp_vendor_codec_t *local_codec;
> +       non_a2dp_vendor_codec_t *remote_codec;
> +
> +       if (remote_cap_len < sizeof(non_a2dp_vendor_codec_t))
> +               return FALSE;
> +
> +       remote_codec = (non_a2dp_vendor_codec_t *) remote_cap;
> +
> +       DBG("Remote vendor id %x %x %x %x", remote_codec->vendor_id[0],
> +                       remote_codec->vendor_id[1], remote_codec->vendor_id[2],
> +                                               remote_codec->vendor_id[3]);
> +
> +       DBG("Remote vendor codec id %x %x", remote_codec->codec_id[0],
> +                                               remote_codec->codec_id[1]);

You can probably join this 2 DBGs in one, also fix the printing format
for uint8_t it should be %02x and if is hex add 0x e.g. Remote vendor
0x%02x%02x%02x%02x codec 0x%02x%02x

> +       if (sep->endpoint == NULL)
> +               return FALSE;
> +
> +       length = sep->endpoint->get_capabilities(sep,
> +                               &capabilities, sep->user_data);
> +
> +       if (length < sizeof(non_a2dp_vendor_codec_t))
> +               return FALSE;
> +
> +       local_codec = (non_a2dp_vendor_codec_t *) capabilities;
> +
> +       DBG("Registered vendor id %x %x %x %x", local_codec->vendor_id[0],
> +                       local_codec->vendor_id[1], local_codec->vendor_id[2],
> +                                               local_codec->vendor_id[3]);
> +
> +       DBG("Registered vendor codec id %x %x", local_codec->codec_id[0],
> +                                               local_codec->codec_id[1]);

Same as above.

> +       if (memcmp(remote_codec->vendor_id, local_codec->vendor_id,
> +                                       sizeof(local_codec->vendor_id)))
> +               return FALSE;
> +
> +       if (memcmp(remote_codec->codec_id, local_codec->codec_id,
> +                                       sizeof(local_codec->codec_id)))
> +               return FALSE;
> +
> +       DBG("Remote non a2dp codec is supported");

Not sure if this DBG would be of much use since latter you select it,
so it is implicit whether it is supported or not.

> +       return TRUE;
> +}
> +
>  static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
>                                         const char *sender)
>  {
>         for (; list; list = list->next) {
>                 struct a2dp_sep *sep = list->data;
> +               struct avdtp_remote_sep *rsep;
> +               struct avdtp_media_codec_capability *rsep_codec;
> +               struct avdtp_service_capability *service;
>
>                 /* Use sender's endpoint if available */
>                 if (sender) {
> @@ -1445,7 +1501,17 @@ static struct a2dp_sep *a2dp_find_sep(struct avdtp *session, GSList *list,
>                                 continue;
>                 }
>
> -               if (avdtp_find_remote_sep(session, sep->lsep) == NULL)
> +               rsep = avdtp_find_remote_sep(session, sep->lsep);
> +               if (rsep == NULL)
> +                       continue;
> +
> +               service = avdtp_get_codec(rsep);
> +               rsep_codec = (struct avdtp_media_codec_capability *)
> +                                                               service->data;
> +
> +               if (rsep_codec->media_codec_type == A2DP_CODEC_NON_A2DP &&
> +                       !non_a2dp_codec_is_supported(rsep_codec->data,
> +                               service->length - sizeof(*rsep_codec), sep))
>                         continue;
>
>                 return sep;
> --
> 1.7.9.5

The rest looks okay.

-- 
Luiz Augusto von Dentz

Re: [PATCH 3/4] audio: Remove redundant procedure when a2dp connect

[Luiz Augusto von Dentz]

Hi Chanyeol,

On Thu, Oct 4, 2012 at 4:14 PM,  <chanyeol.park@samsung.com> wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
>
> This patch fixes the bug that a2dp connection failure is handled like XCASE
> when remote host is down.
> ---
>  audio/avdtp.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/audio/avdtp.c b/audio/avdtp.c
> index 54b3d08..e450ec7 100644
> --- a/audio/avdtp.c
> +++ b/audio/avdtp.c
> @@ -2481,7 +2481,8 @@ failed:
>                         avdtp_sep_set_state(session, stream->lsep,
>                                                 AVDTP_STATE_IDLE);
>         } else
> -               connection_lost(session, EIO);
> +               connection_lost(session, err->code == EHOSTDOWN ?
> +                                               EHOSTDOWN : EIO);
>  }

This rely on err being set which may not always be the case since
there are other places calling goto failed.


-- 
Luiz Augusto von Dentz

Re: [PATCH 1/4] mgmt: Add string for Passkey Notify Event

[Johan Hedberg]

Hi Chan-yeol,

On Thu, Oct 04, 2012, chanyeol.park@samsung.com wrote:
> From: Chan-yeol Park <chanyeol.park@samsung.com>
> 
> ---
>  lib/mgmt.h |    1 +
>  1 file changed, 1 insertion(+)

Patches 1/4 and 2/4 have been applied. Thanks.

Johan

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Marcel Holtmann]

Hi Luiz,

> > This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
> > non-a2dp codec selection.
> > ---
> >  audio/a2dp-codecs.h |    6 +++++
> >  audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 73 insertions(+), 1 deletion(-)
> >
> > diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
> > index 51c796a..e3d2cba 100644
> > --- a/audio/a2dp-codecs.h
> > +++ b/audio/a2dp-codecs.h
> > @@ -26,6 +26,7 @@
> >  #define A2DP_CODEC_MPEG12              0x01
> >  #define A2DP_CODEC_MPEG24              0x02
> >  #define A2DP_CODEC_ATRAC               0x03
> > +#define A2DP_CODEC_NON_A2DP            0xFF
> 
> I prefer A2DP_CODEC_VENDOR

what is the specification calling these? And yes, I would prefer VENDOR
as well.

Regards

Marcel

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Luiz Augusto von Dentz]

Hi Marcel,

On Thu, Oct 4, 2012 at 9:08 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> > This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
>> > non-a2dp codec selection.
>> > ---
>> >  audio/a2dp-codecs.h |    6 +++++
>> >  audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>> >  2 files changed, 73 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
>> > index 51c796a..e3d2cba 100644
>> > --- a/audio/a2dp-codecs.h
>> > +++ b/audio/a2dp-codecs.h
>> > @@ -26,6 +26,7 @@
>> >  #define A2DP_CODEC_MPEG12              0x01
>> >  #define A2DP_CODEC_MPEG24              0x02
>> >  #define A2DP_CODEC_ATRAC               0x03
>> > +#define A2DP_CODEC_NON_A2DP            0xFF
>>
>> I prefer A2DP_CODEC_VENDOR
>
> what is the specification calling these? And yes, I would prefer VENDOR
> as well.

Apparently it comes from the assigned numbers
http://www.bluetooth.org/Technical/AssignedNumbers/audio-video.htm:

1	1	1	1	1	1	1	1	non-A2DP	n/a	A2DP

But the spec refer to it as 4.7 Vendor Specific A2DP Codec, IMO the
term non-A2DP can be confused with not defined in A2DP spec when in
fact it is defined:

"4.2.3 Vendor Specific A2DP Codecs
The device may support other codecs as Vendor Specific A2DP codecs. A
user of a Vendor Specific A2DP codec (hereafter the Vendor) will need
to define parameters and other information necessary for use of the
codec in A2DP."

There is no such a thing as non-A2DP in the spec, so again to avoid
confusion I would stick to the terminology vendor or vendor specific.

-- 
Luiz Augusto von Dentz

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Marcel Holtmann]

Hi Luiz,

> >> > This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
> >> > non-a2dp codec selection.
> >> > ---
> >> >  audio/a2dp-codecs.h |    6 +++++
> >> >  audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> >  2 files changed, 73 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
> >> > index 51c796a..e3d2cba 100644
> >> > --- a/audio/a2dp-codecs.h
> >> > +++ b/audio/a2dp-codecs.h
> >> > @@ -26,6 +26,7 @@
> >> >  #define A2DP_CODEC_MPEG12              0x01
> >> >  #define A2DP_CODEC_MPEG24              0x02
> >> >  #define A2DP_CODEC_ATRAC               0x03
> >> > +#define A2DP_CODEC_NON_A2DP            0xFF
> >>
> >> I prefer A2DP_CODEC_VENDOR
> >
> > what is the specification calling these? And yes, I would prefer VENDOR
> > as well.
> 
> Apparently it comes from the assigned numbers
> http://www.bluetooth.org/Technical/AssignedNumbers/audio-video.htm:
> 
> 1	1	1	1	1	1	1	1	non-A2DP	n/a	A2DP
> 
> But the spec refer to it as 4.7 Vendor Specific A2DP Codec, IMO the
> term non-A2DP can be confused with not defined in A2DP spec when in
> fact it is defined:
> 
> "4.2.3 Vendor Specific A2DP Codecs
> The device may support other codecs as Vendor Specific A2DP codecs. A
> user of a Vendor Specific A2DP codec (hereafter the Vendor) will need
> to define parameters and other information necessary for use of the
> codec in A2DP."
> 
> There is no such a thing as non-A2DP in the spec, so again to avoid
> confusion I would stick to the terminology vendor or vendor specific.

lets use "vendor" for these. Otherwise the function and variable naming
gets too complicated.

Regards

Marcel

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Chan-yeol Park]

Hi All,

On 10/05/2012 05:30 AM, Marcel Holtmann wrote:
> Hi Luiz,
>
>>>>> This patch adds checks(vendor ID, vendor specific codec ID) to make sure of
>>>>> non-a2dp codec selection.
>>>>> ---
>>>>>   audio/a2dp-codecs.h |    6 +++++
>>>>>   audio/a2dp.c        |   68 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>   2 files changed, 73 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/audio/a2dp-codecs.h b/audio/a2dp-codecs.h
>>>>> index 51c796a..e3d2cba 100644
>>>>> --- a/audio/a2dp-codecs.h
>>>>> +++ b/audio/a2dp-codecs.h
>>>>> @@ -26,6 +26,7 @@
>>>>>   #define A2DP_CODEC_MPEG12              0x01
>>>>>   #define A2DP_CODEC_MPEG24              0x02
>>>>>   #define A2DP_CODEC_ATRAC               0x03
>>>>> +#define A2DP_CODEC_NON_A2DP            0xFF
>>>> I prefer A2DP_CODEC_VENDOR
>>> what is the specification calling these? And yes, I would prefer VENDOR
>>> as well.
>> Apparently it comes from the assigned numbers
>> http://www.bluetooth.org/Technical/AssignedNumbers/audio-video.htm:
>>
>> 1	1	1	1	1	1	1	1	non-A2DP	n/a	A2DP
>>
>> But the spec refer to it as 4.7 Vendor Specific A2DP Codec, IMO the
>> term non-A2DP can be confused with not defined in A2DP spec when in
>> fact it is defined:
>>
>> "4.2.3 Vendor Specific A2DP Codecs
>> The device may support other codecs as Vendor Specific A2DP codecs. A
>> user of a Vendor Specific A2DP codec (hereafter the Vendor) will need
>> to define parameters and other information necessary for use of the
>> codec in A2DP."
>>
>> There is no such a thing as non-A2DP in the spec, so again to avoid
>> confusion I would stick to the terminology vendor or vendor specific.
> lets use "vendor" for these. Otherwise the function and variable naming
> gets too complicated.
As you guided I prefer "vendor" codec rather than "non-a2dp" codec. I 
just follow the word in the spec 1.2 because current bluez A2DP version 
is 1.2.  Actually A2DP spec changes the name of 0xff codec from 1.2 to 
1.3 like the below.

1.4 Bluetooth A2DP Profile Change History
1.4.1 Changes from 1.2 to 1.3
1.4.1.1 General Changes
· Non-A2DP codecs from A2DP 1.2 are now referred to as Vendor-Specific 
A2DP codecs

Anyway I think "vendor" is more suitable than "non-a2dp".
I will raise a second patch.

Regards
Chanyeol

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Chan-yeol Park]

Hi Luiz,

On 10/04/2012 11:02 PM, Luiz Augusto von Dentz wrote:
>   #define SBC_SAMPLING_FREQ_16000                (1 << 3)
>   #define SBC_SAMPLING_FREQ_32000                (1 << 2)
> @@ -114,3 +115,8 @@ typedef struct {
>   #else
>   #error "Unknown byte order"
>   #endif
> +
> +typedef struct {
> +       uint8_t vendor_id[4];
> +       uint8_t codec_id[2];
> +} __attribute__ ((packed)) non_a2dp_vendor_codec_t;
> We normally don't typedef this type of structs, besides
> a2dp_vendor_codec should be enough so this should be named struct
> a2dp_vendor_codec.
You're right. originally I didn't use typedef.
but in order to achieve harmony in a2dp-codecs.h like sbc,mpeg, I've 
changed it.

If you make decision about this, I will raise a second patch.

Regards
Chanyeol

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Marcel Holtmann]

Hi Chan-yeol,

> >   #define SBC_SAMPLING_FREQ_16000                (1 << 3)
> >   #define SBC_SAMPLING_FREQ_32000                (1 << 2)
> > @@ -114,3 +115,8 @@ typedef struct {
> >   #else
> >   #error "Unknown byte order"
> >   #endif
> > +
> > +typedef struct {
> > +       uint8_t vendor_id[4];
> > +       uint8_t codec_id[2];
> > +} __attribute__ ((packed)) non_a2dp_vendor_codec_t;
> > We normally don't typedef this type of structs, besides
> > a2dp_vendor_codec should be enough so this should be named struct
> > a2dp_vendor_codec.
> You're right. originally I didn't use typedef.
> but in order to achieve harmony in a2dp-codecs.h like sbc,mpeg, I've 
> changed it.
> 
> If you make decision about this, I will raise a second patch.

hold on for a second, is this the file that is shared with PulseAudio?

Regards

Marcel

Re: [PATCH 4/4] audio: Add check for non-a2dp codec

[Chan-yeol Park]

Hi Marcel,
On 10/05/2012 05:41 PM, Marcel Holtmann wrote:
> Hi Chan-yeol,
>
>>>    #define SBC_SAMPLING_FREQ_16000                (1 << 3)
>>>    #define SBC_SAMPLING_FREQ_32000                (1 << 2)
>>> @@ -114,3 +115,8 @@ typedef struct {
>>>    #else
>>>    #error "Unknown byte order"
>>>    #endif
>>> +
>>> +typedef struct {
>>> +       uint8_t vendor_id[4];
>>> +       uint8_t codec_id[2];
>>> +} __attribute__ ((packed)) non_a2dp_vendor_codec_t;
>>> We normally don't typedef this type of structs, besides
>>> a2dp_vendor_codec should be enough so this should be named struct
>>> a2dp_vendor_codec.
>> You're right. originally I didn't use typedef.
>> but in order to achieve harmony in a2dp-codecs.h like sbc,mpeg, I've
>> changed it.
>>
>> If you make decision about this, I will raise a second patch.
> hold on for a second, is this the file that is shared with PulseAudio?
Yes right. PulseAudio copy it as far as I know.

Regards