Hi Luiz,
I would like to propose and discuss how to share the code between between
android/avdtp.[ch] and profile/audio/avdtp.[ch]. This mail is written based on
your proposal in IRC yesterday. I agree we need to consider AVRCP also.
As far as I understand they are very similar except profile/audio dependencies.
So I consider the below steps.
1. Merge patches in android/avdtp.[ch] into profile/audio/avdtp.[ch]
*This step would merge the patches made during PTS verification.
To discuss I raised some of patches. After discussion I would raise remain
patches.
2. Move the same codes between android/ and profile/. to the common files.
For example:
src/shared/avdtp-common.c
src/shared/avdtp-common.h
src/shared/avdtp-internal.h
-> this file would have local definition in avdtp.c[android, profile/audio]
3. Handle dependencies in profile/audio/avdtp.[ch]
Actually regarding this part I have no idea until now.
I guess this could be discussed easily once we complete 1,2 tasks because at
that time most of the same code are moved into src/shared so it would be easy
to see the dependencies are remained.
Chan-yeol Park (6):
audio/avdtp: Fix not calling callback if DISCOVER is rejected
audio/avdtp: Fix not calling callback if GET_CAPABILITIES is rejected
audio/avdtp: Fix not calling callback if GET_CONFIGURATION is rejected
audio/avdtp:-Fix-not-removing-start_timer-source
audio/a2dp: Move PSM definition to AVDTP header
Move avdtp common code into src/shared
Makefile.am | 1 +
android/avdtp.c | 240 +------------------------------------------
profiles/audio/a2dp.c | 2 -
profiles/audio/avdtp.c | 272 +++++--------------------------------------------
profiles/audio/avdtp.h | 2 +
5 files changed, 28 insertions(+), 489 deletions(-)
--
2.1.4
This patch moves avdtp common definition into src/shared.
---
Makefile.am | 1 +
android/avdtp.c | 240 +------------------------------------------------
profiles/audio/avdtp.c | 240 +------------------------------------------------
3 files changed, 3 insertions(+), 478 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 9e94648..2db0a4d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -108,6 +108,7 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
src/shared/hci.h src/shared/hci.c \
src/shared/hci-crypto.h src/shared/hci-crypto.c \
src/shared/hfp.h src/shared/hfp.c \
+ src/shared/avdtp-common.h \
src/shared/uhid.h src/shared/uhid.c \
src/shared/pcap.h src/shared/pcap.c \
src/shared/btsnoop.h src/shared/btsnoop.c \
diff --git a/android/avdtp.c b/android/avdtp.c
index 7e61280..3bf2f23 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -45,245 +45,7 @@
#include "src/shared/queue.h"
#include "avdtp.h"
#include "../profiles/audio/a2dp-codecs.h"
-
-#define MAX_SEID 0x3E
-static unsigned int seids;
-
-#ifndef MAX
-# define MAX(x, y) ((x) > (y) ? (x) : (y))
-#endif
-
-#define AVDTP_DISCOVER 0x01
-#define AVDTP_GET_CAPABILITIES 0x02
-#define AVDTP_SET_CONFIGURATION 0x03
-#define AVDTP_GET_CONFIGURATION 0x04
-#define AVDTP_RECONFIGURE 0x05
-#define AVDTP_OPEN 0x06
-#define AVDTP_START 0x07
-#define AVDTP_CLOSE 0x08
-#define AVDTP_SUSPEND 0x09
-#define AVDTP_ABORT 0x0A
-#define AVDTP_SECURITY_CONTROL 0x0B
-#define AVDTP_GET_ALL_CAPABILITIES 0x0C
-#define AVDTP_DELAY_REPORT 0x0D
-
-#define AVDTP_PKT_TYPE_SINGLE 0x00
-#define AVDTP_PKT_TYPE_START 0x01
-#define AVDTP_PKT_TYPE_CONTINUE 0x02
-#define AVDTP_PKT_TYPE_END 0x03
-
-#define AVDTP_MSG_TYPE_COMMAND 0x00
-#define AVDTP_MSG_TYPE_GEN_REJECT 0x01
-#define AVDTP_MSG_TYPE_ACCEPT 0x02
-#define AVDTP_MSG_TYPE_REJECT 0x03
-
-#define REQ_TIMEOUT 6
-#define ABORT_TIMEOUT 2
-#define DISCONNECT_TIMEOUT 1
-#define START_TIMEOUT 1
-
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct avdtp_common_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
-} __attribute__ ((packed));
-
-struct avdtp_single_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
- uint8_t signal_id:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct avdtp_start_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
- uint8_t no_of_packets;
- uint8_t signal_id:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct avdtp_continue_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
-} __attribute__ ((packed));
-
-struct seid_info {
- uint8_t rfa0:1;
- uint8_t inuse:1;
- uint8_t seid:6;
- uint8_t rfa2:3;
- uint8_t type:1;
- uint8_t media_type:4;
-} __attribute__ ((packed));
-
-struct seid {
- uint8_t rfa0:2;
- uint8_t seid:6;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct avdtp_common_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
-} __attribute__ ((packed));
-
-struct avdtp_single_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
- uint8_t rfa0:2;
- uint8_t signal_id:6;
-} __attribute__ ((packed));
-
-struct avdtp_start_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
- uint8_t no_of_packets;
- uint8_t rfa0:2;
- uint8_t signal_id:6;
-} __attribute__ ((packed));
-
-struct avdtp_continue_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
-} __attribute__ ((packed));
-
-struct seid_info {
- uint8_t seid:6;
- uint8_t inuse:1;
- uint8_t rfa0:1;
- uint8_t media_type:4;
- uint8_t type:1;
- uint8_t rfa2:3;
-} __attribute__ ((packed));
-
-struct seid {
- uint8_t seid:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
-
-/* packets */
-
-struct discover_resp {
- struct seid_info seps[0];
-} __attribute__ ((packed));
-
-struct getcap_resp {
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct start_req {
- struct seid first_seid;
- struct seid other_seids[0];
-} __attribute__ ((packed));
-
-struct suspend_req {
- struct seid first_seid;
- struct seid other_seids[0];
-} __attribute__ ((packed));
-
-struct seid_rej {
- uint8_t error;
-} __attribute__ ((packed));
-
-struct conf_rej {
- uint8_t category;
- uint8_t error;
-} __attribute__ ((packed));
-
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct seid_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
-} __attribute__ ((packed));
-
-struct setconf_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint8_t rfa1:2;
- uint8_t int_seid:6;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct stream_rej {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint8_t error;
-} __attribute__ ((packed));
-
-struct reconf_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
-
- uint8_t serv_cap;
- uint8_t serv_cap_len;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct delay_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint16_t delay;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct seid_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct setconf_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint8_t int_seid:6;
- uint8_t rfa1:2;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct stream_rej {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint8_t error;
-} __attribute__ ((packed));
-
-struct reconf_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
-
- uint8_t serv_cap;
- uint8_t serv_cap_len;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct delay_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint16_t delay;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
+#include "src/shared/avdtp-internal.h"
struct in_buf {
gboolean active;
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index b26f1ba..fa91cf6 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -45,6 +45,7 @@
#include "src/log.h"
#include "src/shared/util.h"
#include "src/shared/queue.h"
+#include "src/shared/avdtp-internal.h"
#include "src/adapter.h"
#include "src/device.h"
@@ -52,245 +53,6 @@
#include "sink.h"
#include "source.h"
-#define MAX_SEID 0x3E
-static unsigned int seids;
-
-#ifndef MAX
-# define MAX(x, y) ((x) > (y) ? (x) : (y))
-#endif
-
-#define AVDTP_DISCOVER 0x01
-#define AVDTP_GET_CAPABILITIES 0x02
-#define AVDTP_SET_CONFIGURATION 0x03
-#define AVDTP_GET_CONFIGURATION 0x04
-#define AVDTP_RECONFIGURE 0x05
-#define AVDTP_OPEN 0x06
-#define AVDTP_START 0x07
-#define AVDTP_CLOSE 0x08
-#define AVDTP_SUSPEND 0x09
-#define AVDTP_ABORT 0x0A
-#define AVDTP_SECURITY_CONTROL 0x0B
-#define AVDTP_GET_ALL_CAPABILITIES 0x0C
-#define AVDTP_DELAY_REPORT 0x0D
-
-#define AVDTP_PKT_TYPE_SINGLE 0x00
-#define AVDTP_PKT_TYPE_START 0x01
-#define AVDTP_PKT_TYPE_CONTINUE 0x02
-#define AVDTP_PKT_TYPE_END 0x03
-
-#define AVDTP_MSG_TYPE_COMMAND 0x00
-#define AVDTP_MSG_TYPE_GEN_REJECT 0x01
-#define AVDTP_MSG_TYPE_ACCEPT 0x02
-#define AVDTP_MSG_TYPE_REJECT 0x03
-
-#define REQ_TIMEOUT 6
-#define ABORT_TIMEOUT 2
-#define DISCONNECT_TIMEOUT 1
-#define START_TIMEOUT 1
-
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct avdtp_common_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
-} __attribute__ ((packed));
-
-struct avdtp_single_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
- uint8_t signal_id:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct avdtp_start_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
- uint8_t no_of_packets;
- uint8_t signal_id:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct avdtp_continue_header {
- uint8_t message_type:2;
- uint8_t packet_type:2;
- uint8_t transaction:4;
-} __attribute__ ((packed));
-
-struct seid_info {
- uint8_t rfa0:1;
- uint8_t inuse:1;
- uint8_t seid:6;
- uint8_t rfa2:3;
- uint8_t type:1;
- uint8_t media_type:4;
-} __attribute__ ((packed));
-
-struct seid {
- uint8_t rfa0:2;
- uint8_t seid:6;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct avdtp_common_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
-} __attribute__ ((packed));
-
-struct avdtp_single_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
- uint8_t rfa0:2;
- uint8_t signal_id:6;
-} __attribute__ ((packed));
-
-struct avdtp_start_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
- uint8_t no_of_packets;
- uint8_t rfa0:2;
- uint8_t signal_id:6;
-} __attribute__ ((packed));
-
-struct avdtp_continue_header {
- uint8_t transaction:4;
- uint8_t packet_type:2;
- uint8_t message_type:2;
-} __attribute__ ((packed));
-
-struct seid_info {
- uint8_t seid:6;
- uint8_t inuse:1;
- uint8_t rfa0:1;
- uint8_t media_type:4;
- uint8_t type:1;
- uint8_t rfa2:3;
-} __attribute__ ((packed));
-
-struct seid {
- uint8_t seid:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
-
-/* packets */
-
-struct discover_resp {
- struct seid_info seps[0];
-} __attribute__ ((packed));
-
-struct getcap_resp {
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct start_req {
- struct seid first_seid;
- struct seid other_seids[0];
-} __attribute__ ((packed));
-
-struct suspend_req {
- struct seid first_seid;
- struct seid other_seids[0];
-} __attribute__ ((packed));
-
-struct seid_rej {
- uint8_t error;
-} __attribute__ ((packed));
-
-struct conf_rej {
- uint8_t category;
- uint8_t error;
-} __attribute__ ((packed));
-
-#if __BYTE_ORDER == __LITTLE_ENDIAN
-
-struct seid_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
-} __attribute__ ((packed));
-
-struct setconf_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint8_t rfa1:2;
- uint8_t int_seid:6;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct stream_rej {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint8_t error;
-} __attribute__ ((packed));
-
-struct reconf_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
-
- uint8_t serv_cap;
- uint8_t serv_cap_len;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct delay_req {
- uint8_t rfa0:2;
- uint8_t acp_seid:6;
- uint16_t delay;
-} __attribute__ ((packed));
-
-#elif __BYTE_ORDER == __BIG_ENDIAN
-
-struct seid_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
-} __attribute__ ((packed));
-
-struct setconf_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint8_t int_seid:6;
- uint8_t rfa1:2;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct stream_rej {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint8_t error;
-} __attribute__ ((packed));
-
-struct reconf_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
-
- uint8_t serv_cap;
- uint8_t serv_cap_len;
-
- uint8_t caps[0];
-} __attribute__ ((packed));
-
-struct delay_req {
- uint8_t acp_seid:6;
- uint8_t rfa0:2;
- uint16_t delay;
-} __attribute__ ((packed));
-
-#else
-#error "Unknown byte order"
-#endif
-
struct in_buf {
gboolean active;
int no_of_packets;
--
2.1.4
If GET_CONFIGURATION command is rejected and a callback exists it should
be called with the reject error, this fixes test /TP/SIG/SMG/BI-10-C.
This patch is made based on 1ce1db5002f3b7714d1788a68ca06fd1849b68a0.
---
profiles/audio/avdtp.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 57f2d16..ac89370 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2949,6 +2949,15 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
sep->cfm->set_configuration(session, sep, stream,
&err, sep->user_data);
return TRUE;
+ case AVDTP_GET_CONFIGURATION:
+ if (!seid_rej_to_err(buf, size, &err))
+ return FALSE;
+ error("GET_CONFIGURATION request rejected: %s (%d)",
+ avdtp_strerror(&err), err.err.error_code);
+ if (sep && sep->cfm && sep->cfm->get_configuration)
+ sep->cfm->get_configuration(session, sep, stream, &err,
+ sep->user_data);
+ return TRUE;
case AVDTP_RECONFIGURE:
if (!conf_rej_to_err(buf, size, &err))
return FALSE;
--
2.1.4
If GET_CAPABILITIES command is rejected and a callback exists it should
be called with the reject error, this fixes test /TP/SIG/SMG/BI-04-C.
This patch is made based on cb5058cf26e4ad6fd857fd480675cd6fcd4b7985.
---
profiles/audio/avdtp.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 14f9a61..57f2d16 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2915,10 +2915,15 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
switch (signal_id) {
case AVDTP_DISCOVER:
+ case AVDTP_GET_CAPABILITIES:
+ case AVDTP_GET_ALL_CAPABILITIES:
if (!seid_rej_to_err(buf, size, &err))
return FALSE;
- error("DISCOVER request rejected: %s (%d)",
- avdtp_strerror(&err), err.err.error_code);
+ error("%s request rejected: %s (%d)",
+ signal_id == AVDTP_DISCOVER ? "DISCOVER" :
+ signal_id == AVDTP_GET_CAPABILITIES ?
+ "GET_CAPABILITIES" : "GET_ALL_CAPABILITIES",
+ avdtp_strerror(&err), err.err.error_code);
if (session->discover) {
session->discover->cb(session, session->seps, &err,
session->discover->user_data);
@@ -2926,13 +2931,6 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
session->discover = NULL;
}
return TRUE;
- case AVDTP_GET_CAPABILITIES:
- case AVDTP_GET_ALL_CAPABILITIES:
- if (!seid_rej_to_err(buf, size, &err))
- return FALSE;
- error("GET_CAPABILITIES request rejected: %s (%d)",
- avdtp_strerror(&err), err.err.error_code);
- return TRUE;
case AVDTP_OPEN:
if (!seid_rej_to_err(buf, size, &err))
return FALSE;
--
2.1.4
If DISCOVER command is rejected and a callback exists it should be
called with the reject error, this fixes test /TP/SIG/SMG/BI-01-C
This patches is made based on
262e11982bfb94d0da66a862facf6e71330ef23fjk.
---
profiles/audio/avdtp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index f38188f..14f9a61 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2919,6 +2919,12 @@ static gboolean avdtp_parse_rej(struct avdtp *session,
return FALSE;
error("DISCOVER request rejected: %s (%d)",
avdtp_strerror(&err), err.err.error_code);
+ if (session->discover) {
+ session->discover->cb(session, session->seps, &err,
+ session->discover->user_data);
+ g_free(session->discover);
+ session->discover = NULL;
+ }
return TRUE;
case AVDTP_GET_CAPABILITIES:
case AVDTP_GET_ALL_CAPABILITIES:
--
2.1.4
This patch moves PSM definition to AVDTP header based on
7e518f2f9a7dfd195ec39825b41a58d8fcc4a849.
---
profiles/audio/a2dp.c | 2 --
profiles/audio/avdtp.c | 2 --
profiles/audio/avdtp.h | 2 ++
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 31c5086..0405f58 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -61,8 +61,6 @@
#define SUSPEND_TIMEOUT 5
#define RECONFIGURE_TIMEOUT 500
-#define AVDTP_PSM 25
-
struct a2dp_sep {
struct a2dp_server *server;
struct a2dp_endpoint *endpoint;
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index eed3f88..b26f1ba 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -52,8 +52,6 @@
#include "sink.h"
#include "source.h"
-#define AVDTP_PSM 25
-
#define MAX_SEID 0x3E
static unsigned int seids;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index 1b02232..81705fd 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -41,6 +41,8 @@ struct avdtp_error {
} err;
};
+#define AVDTP_PSM 25
+
/* SEP capability categories */
#define AVDTP_MEDIA_TRANSPORT 0x01
#define AVDTP_REPORTING 0x02
--
2.1.4
It is possible that stream->start_timer can be set on stream_free which
should then should take care of remove it properly otherwise it can
trigger which would very likely cause a crash.
Thanks to Hannu Mallat <[email protected]> for reporting it.
This patch is made based on 24fc2b639e6fe567633a60dd4d99eca36a0f1323.
---
profiles/audio/avdtp.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index ac89370..eed3f88 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -714,6 +714,9 @@ static void stream_free(void *data)
if (stream->timer)
g_source_remove(stream->timer);
+ if (stream->start_timer > 0)
+ g_source_remove(stream->start_timer);
+
if (stream->io)
close_stream(stream);
--
2.1.4