2015-02-19 15:46:43

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 1/5] android/avdtp: Refactor local SEP list handling

From: Andrei Emeltchenko <[email protected]>

In order to deduplicate similar code in android/ and profiles/ we need
to handle local SEP queue outside of avdtp.c. GSList is transformed to
queue.
---
android/Android.mk | 1 +
android/Makefile.am | 1 +
android/a2dp.c | 17 +++++++---
android/avdtp.c | 90 ++++++++++++++++++++++++++++++--------------------
android/avdtp.h | 8 +++--
android/avdtptest.c | 10 ++++--
unit/test-avdtp.c | 95 ++++++++++++++++++++++++++++++-----------------------
7 files changed, 136 insertions(+), 86 deletions(-)

diff --git a/android/Android.mk b/android/Android.mk
index 1f2095d..065032f 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -262,6 +262,7 @@ LOCAL_SRC_FILES := \
bluez/lib/bluetooth.c \
bluez/lib/hci.c \
bluez/src/shared/util.c \
+ bluez/src/shared/queue.c \

LOCAL_C_INCLUDES += \
$(LOCAL_PATH)/bluez \
diff --git a/android/Makefile.am b/android/Makefile.am
index f8c76b3..72fec73 100644
--- a/android/Makefile.am
+++ b/android/Makefile.am
@@ -103,6 +103,7 @@ android_avdtptest_SOURCES = android/avdtptest.c \
src/log.h src/log.c \
btio/btio.h btio/btio.c \
src/shared/util.h src/shared/util.c \
+ src/shared/queue.h src/shared/queue.c \
android/avdtp.h android/avdtp.c
android_avdtptest_CFLAGS = $(AM_CFLAGS)
android_avdtptest_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
diff --git a/android/a2dp.c b/android/a2dp.c
index 10f5523..021d781 100644
--- a/android/a2dp.c
+++ b/android/a2dp.c
@@ -38,6 +38,7 @@
#include "lib/sdp.h"
#include "lib/sdp_lib.h"
#include "profiles/audio/a2dp-codecs.h"
+#include "src/shared/queue.h"
#include "src/log.h"
#include "hal-msg.h"
#include "ipc-common.h"
@@ -52,6 +53,7 @@
#define SVC_HINT_CAPTURING 0x08
#define IDLE_TIMEOUT 1
#define AUDIO_RETRY_TIMEOUT 2
+#define MAX_SEID 0x3E

static GIOChannel *server = NULL;
static GSList *devices = NULL;
@@ -65,6 +67,8 @@ static bool audio_retrying = false;
static struct ipc *hal_ipc = NULL;
static struct ipc *audio_ipc = NULL;

+static struct queue *lseps = NULL;
+
struct a2dp_preset {
void *data;
int8_t len;
@@ -115,7 +119,7 @@ static void unregister_endpoint(void *data)
struct a2dp_endpoint *endpoint = data;

if (endpoint->sep)
- avdtp_unregister_sep(endpoint->sep);
+ avdtp_unregister_sep(lseps, endpoint->sep);

if (endpoint->caps)
preset_free(endpoint->caps);
@@ -620,6 +624,7 @@ static void signaling_connect_cb(GIOChannel *chan, GError *err,
gpointer user_data)
{
struct a2dp_device *dev = user_data;
+ struct avdtp *session;
uint16_t imtu, omtu;
GError *gerr = NULL;
int fd;
@@ -643,10 +648,12 @@ static void signaling_connect_cb(GIOChannel *chan, GError *err,
fd = g_io_channel_unix_get_fd(chan);

/* FIXME: Add proper version */
- dev->session = avdtp_new(fd, imtu, omtu, 0x0100);
- if (!dev->session)
+ session = avdtp_new(fd, imtu, omtu, 0x0100, lseps);
+ if (!session)
goto failed;

+ dev->session = session;
+
avdtp_add_disconnect_cb(dev->session, disconnect_cb, dev);

/* Proceed to stream setup if initiator */
@@ -1327,7 +1334,7 @@ static uint8_t register_endpoint(const uint8_t *uuid, uint8_t codec,
endpoint = g_new0(struct a2dp_endpoint, 1);
endpoint->id = g_slist_length(endpoints) + 1;
endpoint->codec = codec;
- endpoint->sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE,
+ endpoint->sep = avdtp_register_sep(lseps, AVDTP_SEP_TYPE_SOURCE,
AVDTP_MEDIA_TYPE_AUDIO,
codec, FALSE, &sep_ind,
&sep_cfm, endpoint);
@@ -1691,6 +1698,8 @@ bool bt_a2dp_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)

bacpy(&adapter_addr, addr);

+ lseps = queue_new();
+
server = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
BT_IO_OPT_PSM, AVDTP_PSM,
diff --git a/android/avdtp.c b/android/avdtp.c
index b290feb..35bc548 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -42,6 +42,7 @@
#include "lib/bluetooth.h"
#include "src/log.h"
#include "src/shared/util.h"
+#include "src/shared/queue.h"
#include "avdtp.h"
#include "../profiles/audio/a2dp-codecs.h"

@@ -383,6 +384,7 @@ struct avdtp {
guint io_id;

GSList *seps; /* Elements of type struct avdtp_remote_sep * */
+ struct queue *lseps; /* Elements of type struct avdtp_local_sep * */

GSList *streams; /* Elements of type struct avdtp_stream * */

@@ -406,8 +408,6 @@ struct avdtp {
bool shutdown;
};

-static GSList *lseps = NULL;
-
static int send_request(struct avdtp *session, gboolean priority,
struct avdtp_stream *stream, uint8_t signal_id,
void *buffer, size_t size);
@@ -993,6 +993,9 @@ static void avdtp_free(void *data)
g_slist_free_full(session->seps, sep_free);
g_slist_free_full(session->disconnect, g_free);

+ /* Free copy of the SEP list */
+ session->lseps = NULL;
+
g_free(session->buf);

g_free(session);
@@ -1050,18 +1053,18 @@ struct avdtp *avdtp_ref(struct avdtp *session)
return session;
}

-static struct avdtp_local_sep *find_local_sep_by_seid(uint8_t seid)
+static bool match_by_seid(const void *data, const void *user_data)
{
- GSList *l;
+ const struct avdtp_local_sep *sep = data;
+ uint8_t seid = PTR_TO_UINT(user_data);

- for (l = lseps; l != NULL; l = g_slist_next(l)) {
- struct avdtp_local_sep *sep = l->data;
-
- if (sep->info.seid == seid)
- return sep;
- }
+ return sep->info.seid == seid;
+}

- return NULL;
+static struct avdtp_local_sep *find_local_sep_by_seid(struct avdtp *session,
+ uint8_t seid)
+{
+ return queue_find(session->lseps, match_by_seid, INT_TO_PTR(seid));
}

struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
@@ -1161,15 +1164,23 @@ static gboolean avdtp_unknown_cmd(struct avdtp *session, uint8_t transaction,
signal_id, NULL, 0);
}

+static void copy_seps(void *data, void *user_data)
+{
+ struct avdtp_local_sep *sep = data;
+ struct seid_info **p = user_data;
+
+ memcpy(*p, &sep->info, sizeof(struct seid_info));
+ *p = *p + 1;
+}
+
static gboolean avdtp_discover_cmd(struct avdtp *session, uint8_t transaction,
void *buf, int size)
{
- GSList *l;
- unsigned int rsp_size, sep_count, i;
- struct seid_info *seps;
+ unsigned int rsp_size, sep_count;
+ struct seid_info *seps, *p;
gboolean ret;

- sep_count = g_slist_length(lseps);
+ sep_count = queue_length(session->lseps);

if (sep_count == 0) {
uint8_t err = AVDTP_NOT_SUPPORTED_COMMAND;
@@ -1180,12 +1191,9 @@ static gboolean avdtp_discover_cmd(struct avdtp *session, uint8_t transaction,
rsp_size = sep_count * sizeof(struct seid_info);

seps = g_new0(struct seid_info, sep_count);
+ p = seps;

- for (l = lseps, i = 0; l != NULL; l = l->next, i++) {
- struct avdtp_local_sep *sep = l->data;
-
- memcpy(&seps[i], &sep->info, sizeof(struct seid_info));
- }
+ queue_foreach(session->lseps, copy_seps, &p);

ret = avdtp_send(session, transaction, AVDTP_MSG_TYPE_ACCEPT,
AVDTP_DISCOVER, seps, rsp_size);
@@ -1211,7 +1219,7 @@ static gboolean avdtp_getcap_cmd(struct avdtp *session, uint8_t transaction,
goto failed;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1293,7 +1301,7 @@ static gboolean avdtp_setconf_cmd(struct avdtp *session, uint8_t transaction,
return FALSE;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1381,7 +1389,7 @@ static gboolean avdtp_getconf_cmd(struct avdtp *session, uint8_t transaction,

memset(buf, 0, sizeof(buf));

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1497,7 +1505,7 @@ static gboolean avdtp_open_cmd(struct avdtp *session, uint8_t transaction,
return FALSE;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1557,7 +1565,7 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
for (i = 0; i < seid_count; i++, seid++) {
failed_seid = seid->seid;

- sep = find_local_sep_by_seid(req->first_seid.seid);
+ sep = find_local_sep_by_seid(session, req->first_seid.seid);
if (!sep || !sep->stream) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1607,7 +1615,7 @@ static gboolean avdtp_close_cmd(struct avdtp *session, uint8_t transaction,
return FALSE;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep || !sep->stream) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1668,7 +1676,7 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
for (i = 0; i < seid_count; i++, seid++) {
failed_seid = seid->seid;

- sep = find_local_sep_by_seid(req->first_seid.seid);
+ sep = find_local_sep_by_seid(session, req->first_seid.seid);
if (!sep || !sep->stream) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -1715,7 +1723,7 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
return FALSE;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep || !sep->stream)
return TRUE;

@@ -1753,7 +1761,7 @@ static gboolean avdtp_delayreport_cmd(struct avdtp *session,
return FALSE;
}

- sep = find_local_sep_by_seid(req->acp_seid);
+ sep = find_local_sep_by_seid(session, req->acp_seid);
if (!sep || !sep->stream) {
err = AVDTP_BAD_ACP_SEID;
goto failed;
@@ -2105,12 +2113,16 @@ static int set_priority(int fd, int priority)
return err;
}

-struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
+struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version,
+ struct queue *lseps)
{
struct avdtp *session;
GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
int new_fd;

+ if (!lseps)
+ return NULL;
+
new_fd = dup(fd);
if (new_fd < 0) {
error("dup(): %s (%d)", strerror(errno), errno);
@@ -2138,6 +2150,8 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
(GIOFunc) session_cb, session,
NULL);

+ session->lseps = lseps;
+
return avdtp_ref(session);
}

@@ -3344,7 +3358,8 @@ int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
&req, sizeof(req));
}

-struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
+struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
+ uint8_t media_type,
uint8_t codec_type,
gboolean delay_reporting,
struct avdtp_sep_ind *ind,
@@ -3357,7 +3372,7 @@ struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
if (!seid)
return NULL;

- if (g_slist_length(lseps) > MAX_SEID)
+ if (queue_length(lseps) > MAX_SEID)
return NULL;

sep = g_new0(struct avdtp_local_sep, 1);
@@ -3374,7 +3389,11 @@ struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,

DBG("SEP %p registered: type:%d codec:%d seid:%d", sep,
sep->info.type, sep->codec, sep->info.seid);
- lseps = g_slist_append(lseps, sep);
+
+ if (!queue_push_tail(lseps, sep)) {
+ g_free(sep);
+ return NULL;
+ }

return sep;
}
@@ -3386,13 +3405,11 @@ void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
sep->vndcodec_codec = codec_id;
}

-int avdtp_unregister_sep(struct avdtp_local_sep *sep)
+int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep)
{
if (!sep)
return -EINVAL;

- lseps = g_slist_remove(lseps, sep);
-
if (sep->stream)
release_stream(sep->stream, sep->stream->session);

@@ -3400,6 +3417,7 @@ int avdtp_unregister_sep(struct avdtp_local_sep *sep)
sep->info.type, sep->codec, sep->info.seid);

util_clear_uid(&seids, sep->info.seid);
+ queue_remove(lseps, sep);
g_free(sep);

return 0;
diff --git a/android/avdtp.h b/android/avdtp.h
index d5335e4..07516a8 100644
--- a/android/avdtp.h
+++ b/android/avdtp.h
@@ -205,7 +205,8 @@ typedef void (*avdtp_discover_cb_t) (struct avdtp *session, GSList *seps,
struct avdtp_error *err, void *user_data);
typedef void (*avdtp_disconnect_cb_t) (void *user_data);

-struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
+struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version,
+ struct queue *lseps);

unsigned int avdtp_add_disconnect_cb(struct avdtp *session,
avdtp_disconnect_cb_t cb,
@@ -265,7 +266,8 @@ int avdtp_abort(struct avdtp *session, struct avdtp_stream *stream);
int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
uint16_t delay);

-struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
+struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
+ uint8_t media_type,
uint8_t codec_type,
gboolean delay_reporting,
struct avdtp_sep_ind *ind,
@@ -278,7 +280,7 @@ void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
struct avdtp_local_sep *lsep);

-int avdtp_unregister_sep(struct avdtp_local_sep *sep);
+int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep);

avdtp_state_t avdtp_sep_get_state(struct avdtp_local_sep *sep);

diff --git a/android/avdtptest.c b/android/avdtptest.c
index ce39519..6529423 100644
--- a/android/avdtptest.c
+++ b/android/avdtptest.c
@@ -40,6 +40,7 @@

#include "src/shared/util.h"
#include "btio/btio.h"
+#include "src/shared/queue.h"
#include "avdtp.h"

static GMainLoop *mainloop = NULL;
@@ -57,6 +58,7 @@ static uint16_t version = 0x0103;
static guint media_player = 0;
static guint media_recorder = 0;
static guint idle_id = 0;
+static struct queue *lseps = NULL;

static bool fragment = false;

@@ -411,7 +413,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
return;
}

- avdtp = avdtp_new(fd, imtu, omtu, version);
+ avdtp = avdtp_new(fd, imtu, omtu, version, lseps);
if (!avdtp) {
printf("Failed to create avdtp instance\n");
g_main_loop_quit(mainloop);
@@ -864,13 +866,15 @@ int main(int argc, char *argv[])
}
}

- local_sep = avdtp_register_sep(dev_role, AVDTP_MEDIA_TYPE_AUDIO,
+ local_sep = avdtp_register_sep(lseps, dev_role, AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, &sep_cfm, NULL);
if (!local_sep) {
printf("Failed to register sep\n");
exit(0);
}

+ queue_push_tail(lseps, local_sep);
+
if (!bacmp(&dst, BDADDR_ANY)) {
printf("Listening...\n");
io = do_listen(&err);
@@ -889,6 +893,8 @@ int main(int argc, char *argv[])

printf("Done\n");

+ queue_destroy(lseps, NULL);
+
avdtp_unref(avdtp);
avdtp = NULL;

diff --git a/unit/test-avdtp.c b/unit/test-avdtp.c
index 34820d1..a293a1d 100644
--- a/unit/test-avdtp.c
+++ b/unit/test-avdtp.c
@@ -36,13 +36,12 @@
#include <glib.h>

#include "src/shared/util.h"
+#include "src/shared/queue.h"
#include "src/log.h"
#include "android/avdtp.h"

#define MAX_SEID 0x3E

-GSList *lseps = NULL;
-
struct test_pdu {
bool valid;
bool fragmented;
@@ -89,6 +88,7 @@ struct context {
struct avdtp *session;
struct avdtp_local_sep *sep;
struct avdtp_stream *stream;
+ struct queue *lseps;
guint source;
guint process;
int fd;
@@ -233,7 +233,11 @@ static struct context *context_new(uint16_t version, uint16_t imtu,
err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
g_assert(err == 0);

- context->session = avdtp_new(sv[0], imtu, omtu, version);
+ context->lseps = queue_new();
+ g_assert(context->lseps);
+
+ context->session = avdtp_new(sv[0], imtu, omtu, version,
+ context->lseps);
g_assert(context->session != NULL);

channel = g_io_channel_unix_new(sv[1]);
@@ -505,7 +509,8 @@ static void test_server(gconstpointer data)
struct context *context = create_context(0x0100, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
+ AVDTP_MEDIA_TYPE_AUDIO,
0x00, FALSE, &sep_ind, &sep_cfm,
context);

@@ -513,7 +518,8 @@ static void test_server(gconstpointer data)

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void test_server_1_3(gconstpointer data)
@@ -521,14 +527,16 @@ static void test_server_1_3(gconstpointer data)
struct context *context = create_context(0x0103, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
+ AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL, context);

g_idle_add(send_pdu, context);

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void test_server_1_3_sink(gconstpointer data)
@@ -536,14 +544,16 @@ static void test_server_1_3_sink(gconstpointer data)
struct context *context = create_context(0x0103, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
+ AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL, context);

g_idle_add(send_pdu, context);

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void test_server_0_sep(gconstpointer data)
@@ -559,7 +569,8 @@ static void unregister_sep(void *data)
{
struct avdtp_local_sep *sep = data;

- avdtp_unregister_sep(sep);
+ /* Removed from the queue by caller */
+ avdtp_unregister_sep(NULL, sep);
}

static void test_server_seid(gconstpointer data)
@@ -569,25 +580,22 @@ static void test_server_seid(gconstpointer data)
unsigned int i;

for (i = 0; i < sizeof(int) * 8; i++) {
- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL,
context);
g_assert(sep);
-
- lseps = g_slist_append(lseps, sep);
}

/* Now add (MAX_SEID + 1) SEP -> it shall fail */
- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL,
context);
g_assert(!sep);

/* Remove all SEPs */
- g_slist_free_full(lseps, unregister_sep);
- lseps = NULL;
+ queue_destroy(context->lseps, unregister_sep);
}

static void test_server_seid_duplicate(gconstpointer data)
@@ -597,37 +605,34 @@ static void test_server_seid_duplicate(gconstpointer data)
int i;

for (i = 0; i < 2; i++) {
- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL,
context);
g_assert(sep);
-
- lseps = g_slist_append(lseps, sep);
}

/* Remove 1st element */
- sep = g_slist_nth_data(lseps, 0);
- lseps = g_slist_remove(lseps, sep);
- avdtp_unregister_sep(sep);
+ sep = queue_peek_head(context->lseps);
+ g_assert(sep);
+
+ avdtp_unregister_sep(context->lseps, sep);

/* Now register new element */
- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind, NULL,
context);
g_assert(sep);
- lseps = g_slist_append(lseps, sep);

- /* Check SEID ids */
+ /* Check SEID ids with DISCOVER */

g_idle_add(send_pdu, context);

execute_context(context);

/* Remove all SEPs */
- g_slist_free_full(lseps, unregister_sep);
- lseps = NULL;
+ queue_destroy(context->lseps, unregister_sep);
}

static gboolean sep_getcap_ind_frg(struct avdtp *session,
@@ -675,7 +680,8 @@ static void test_server_frg(gconstpointer data)
struct context *context = context_new(0x0100, 48, 48, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
+ AVDTP_MEDIA_TYPE_AUDIO,
0x00, TRUE, &sep_ind_frg,
NULL, context);

@@ -683,7 +689,8 @@ static void test_server_frg(gconstpointer data)

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void discover_cb(struct avdtp *session, GSList *seps,
@@ -758,16 +765,18 @@ static void test_client(gconstpointer data)
struct context *context = create_context(0x0100, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
- 0x00, FALSE, NULL, &sep_cfm,
- context);
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
+ AVDTP_MEDIA_TYPE_AUDIO,
+ 0x00, FALSE, NULL, &sep_cfm, context);
+
context->sep = sep;

avdtp_discover(context->session, discover_cb, context);

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void test_client_1_3(gconstpointer data)
@@ -775,16 +784,18 @@ static void test_client_1_3(gconstpointer data)
struct context *context = create_context(0x0103, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
- 0x00, TRUE, NULL, &sep_cfm,
- context);
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
+ AVDTP_MEDIA_TYPE_AUDIO,
+ 0x00, TRUE, NULL, &sep_cfm, context);
+
context->sep = sep;

avdtp_discover(context->session, discover_cb, context);

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

static void test_client_frg(gconstpointer data)
@@ -792,16 +803,18 @@ static void test_client_frg(gconstpointer data)
struct context *context = context_new(0x0100, 48, 48, data);
struct avdtp_local_sep *sep;

- sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
- 0x00, TRUE, NULL, &sep_cfm,
- context);
+ sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
+ AVDTP_MEDIA_TYPE_AUDIO,
+ 0x00, TRUE, NULL, &sep_cfm, context);
+
context->sep = sep;

avdtp_discover(context->session, discover_cb, context);

execute_context(context);

- avdtp_unregister_sep(sep);
+ avdtp_unregister_sep(context->lseps, sep);
+ queue_destroy(context->lseps, NULL);
}

int main(int argc, char *argv[])
--
2.1.0



2015-02-20 11:48:33

by Szymon Janc

[permalink] [raw]
Subject: Re: [RFCv2 1/5] android/avdtp: Refactor local SEP list handling

Hi Andrei,

On Thursday 19 of February 2015 17:46:43 Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> In order to deduplicate similar code in android/ and profiles/ we need
> to handle local SEP queue outside of avdtp.c. GSList is transformed to
> queue.
> ---
> android/Android.mk | 1 +
> android/Makefile.am | 1 +
> android/a2dp.c | 17 +++++++---
> android/avdtp.c | 90 ++++++++++++++++++++++++++++++--------------------
> android/avdtp.h | 8 +++--
> android/avdtptest.c | 10 ++++--
> unit/test-avdtp.c | 95 ++++++++++++++++++++++++++++++-----------------------
> 7 files changed, 136 insertions(+), 86 deletions(-)
>
> diff --git a/android/Android.mk b/android/Android.mk
> index 1f2095d..065032f 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -262,6 +262,7 @@ LOCAL_SRC_FILES := \
> bluez/lib/bluetooth.c \
> bluez/lib/hci.c \
> bluez/src/shared/util.c \
> + bluez/src/shared/queue.c \
>
> LOCAL_C_INCLUDES += \
> $(LOCAL_PATH)/bluez \
> diff --git a/android/Makefile.am b/android/Makefile.am
> index f8c76b3..72fec73 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -103,6 +103,7 @@ android_avdtptest_SOURCES = android/avdtptest.c \
> src/log.h src/log.c \
> btio/btio.h btio/btio.c \
> src/shared/util.h src/shared/util.c \
> + src/shared/queue.h src/shared/queue.c \
> android/avdtp.h android/avdtp.c
> android_avdtptest_CFLAGS = $(AM_CFLAGS)
> android_avdtptest_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
> diff --git a/android/a2dp.c b/android/a2dp.c
> index 10f5523..021d781 100644
> --- a/android/a2dp.c
> +++ b/android/a2dp.c
> @@ -38,6 +38,7 @@
> #include "lib/sdp.h"
> #include "lib/sdp_lib.h"
> #include "profiles/audio/a2dp-codecs.h"
> +#include "src/shared/queue.h"
> #include "src/log.h"
> #include "hal-msg.h"
> #include "ipc-common.h"
> @@ -52,6 +53,7 @@
> #define SVC_HINT_CAPTURING 0x08
> #define IDLE_TIMEOUT 1
> #define AUDIO_RETRY_TIMEOUT 2
> +#define MAX_SEID 0x3E

I'm reluctant to moving AVDTP logic to A2DP code.
This should be verified inside avdtp_register_sep().
lseps queue should be just a context for AVDTP operations.

>
> static GIOChannel *server = NULL;
> static GSList *devices = NULL;
> @@ -65,6 +67,8 @@ static bool audio_retrying = false;
> static struct ipc *hal_ipc = NULL;
> static struct ipc *audio_ipc = NULL;
>
> +static struct queue *lseps = NULL;
> +
> struct a2dp_preset {
> void *data;
> int8_t len;
> @@ -115,7 +119,7 @@ static void unregister_endpoint(void *data)
> struct a2dp_endpoint *endpoint = data;
>
> if (endpoint->sep)
> - avdtp_unregister_sep(endpoint->sep);
> + avdtp_unregister_sep(lseps, endpoint->sep);
>
> if (endpoint->caps)
> preset_free(endpoint->caps);
> @@ -620,6 +624,7 @@ static void signaling_connect_cb(GIOChannel *chan, GError *err,
> gpointer user_data)
> {
> struct a2dp_device *dev = user_data;
> + struct avdtp *session;
> uint16_t imtu, omtu;
> GError *gerr = NULL;
> int fd;
> @@ -643,10 +648,12 @@ static void signaling_connect_cb(GIOChannel *chan, GError *err,
> fd = g_io_channel_unix_get_fd(chan);
>
> /* FIXME: Add proper version */
> - dev->session = avdtp_new(fd, imtu, omtu, 0x0100);
> - if (!dev->session)
> + session = avdtp_new(fd, imtu, omtu, 0x0100, lseps);
> + if (!session)
> goto failed;
>
> + dev->session = session;
> +
> avdtp_add_disconnect_cb(dev->session, disconnect_cb, dev);
>
> /* Proceed to stream setup if initiator */
> @@ -1327,7 +1334,7 @@ static uint8_t register_endpoint(const uint8_t *uuid, uint8_t codec,
> endpoint = g_new0(struct a2dp_endpoint, 1);
> endpoint->id = g_slist_length(endpoints) + 1;
> endpoint->codec = codec;
> - endpoint->sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE,
> + endpoint->sep = avdtp_register_sep(lseps, AVDTP_SEP_TYPE_SOURCE,
> AVDTP_MEDIA_TYPE_AUDIO,
> codec, FALSE, &sep_ind,
> &sep_cfm, endpoint);
> @@ -1691,6 +1698,8 @@ bool bt_a2dp_register(struct ipc *ipc, const bdaddr_t *addr, uint8_t mode)
>
> bacpy(&adapter_addr, addr);
>
> + lseps = queue_new();
> +
> server = bt_io_listen(connect_cb, NULL, NULL, NULL, &err,
> BT_IO_OPT_SOURCE_BDADDR, &adapter_addr,
> BT_IO_OPT_PSM, AVDTP_PSM,
> diff --git a/android/avdtp.c b/android/avdtp.c
> index b290feb..35bc548 100644
> --- a/android/avdtp.c
> +++ b/android/avdtp.c
> @@ -42,6 +42,7 @@
> #include "lib/bluetooth.h"
> #include "src/log.h"
> #include "src/shared/util.h"
> +#include "src/shared/queue.h"
> #include "avdtp.h"
> #include "../profiles/audio/a2dp-codecs.h"
>
> @@ -383,6 +384,7 @@ struct avdtp {
> guint io_id;
>
> GSList *seps; /* Elements of type struct avdtp_remote_sep * */
> + struct queue *lseps; /* Elements of type struct avdtp_local_sep * */
>
> GSList *streams; /* Elements of type struct avdtp_stream * */
>
> @@ -406,8 +408,6 @@ struct avdtp {
> bool shutdown;
> };
>
> -static GSList *lseps = NULL;
> -
> static int send_request(struct avdtp *session, gboolean priority,
> struct avdtp_stream *stream, uint8_t signal_id,
> void *buffer, size_t size);
> @@ -993,6 +993,9 @@ static void avdtp_free(void *data)
> g_slist_free_full(session->seps, sep_free);
> g_slist_free_full(session->disconnect, g_free);
>
> + /* Free copy of the SEP list */
> + session->lseps = NULL;
> +
> g_free(session->buf);
>
> g_free(session);
> @@ -1050,18 +1053,18 @@ struct avdtp *avdtp_ref(struct avdtp *session)
> return session;
> }
>
> -static struct avdtp_local_sep *find_local_sep_by_seid(uint8_t seid)
> +static bool match_by_seid(const void *data, const void *user_data)
> {
> - GSList *l;
> + const struct avdtp_local_sep *sep = data;
> + uint8_t seid = PTR_TO_UINT(user_data);
>
> - for (l = lseps; l != NULL; l = g_slist_next(l)) {
> - struct avdtp_local_sep *sep = l->data;
> -
> - if (sep->info.seid == seid)
> - return sep;
> - }
> + return sep->info.seid == seid;
> +}
>
> - return NULL;
> +static struct avdtp_local_sep *find_local_sep_by_seid(struct avdtp *session,
> + uint8_t seid)
> +{
> + return queue_find(session->lseps, match_by_seid, INT_TO_PTR(seid));
> }
>
> struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> @@ -1161,15 +1164,23 @@ static gboolean avdtp_unknown_cmd(struct avdtp *session, uint8_t transaction,
> signal_id, NULL, 0);
> }
>
> +static void copy_seps(void *data, void *user_data)
> +{
> + struct avdtp_local_sep *sep = data;
> + struct seid_info **p = user_data;
> +
> + memcpy(*p, &sep->info, sizeof(struct seid_info));
> + *p = *p + 1;
> +}
> +
> static gboolean avdtp_discover_cmd(struct avdtp *session, uint8_t transaction,
> void *buf, int size)
> {
> - GSList *l;
> - unsigned int rsp_size, sep_count, i;
> - struct seid_info *seps;
> + unsigned int rsp_size, sep_count;
> + struct seid_info *seps, *p;
> gboolean ret;
>
> - sep_count = g_slist_length(lseps);
> + sep_count = queue_length(session->lseps);
>
> if (sep_count == 0) {
> uint8_t err = AVDTP_NOT_SUPPORTED_COMMAND;
> @@ -1180,12 +1191,9 @@ static gboolean avdtp_discover_cmd(struct avdtp *session, uint8_t transaction,
> rsp_size = sep_count * sizeof(struct seid_info);
>
> seps = g_new0(struct seid_info, sep_count);
> + p = seps;
>
> - for (l = lseps, i = 0; l != NULL; l = l->next, i++) {
> - struct avdtp_local_sep *sep = l->data;
> -
> - memcpy(&seps[i], &sep->info, sizeof(struct seid_info));
> - }
> + queue_foreach(session->lseps, copy_seps, &p);
>
> ret = avdtp_send(session, transaction, AVDTP_MSG_TYPE_ACCEPT,
> AVDTP_DISCOVER, seps, rsp_size);
> @@ -1211,7 +1219,7 @@ static gboolean avdtp_getcap_cmd(struct avdtp *session, uint8_t transaction,
> goto failed;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1293,7 +1301,7 @@ static gboolean avdtp_setconf_cmd(struct avdtp *session, uint8_t transaction,
> return FALSE;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1381,7 +1389,7 @@ static gboolean avdtp_getconf_cmd(struct avdtp *session, uint8_t transaction,
>
> memset(buf, 0, sizeof(buf));
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1497,7 +1505,7 @@ static gboolean avdtp_open_cmd(struct avdtp *session, uint8_t transaction,
> return FALSE;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1557,7 +1565,7 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
> for (i = 0; i < seid_count; i++, seid++) {
> failed_seid = seid->seid;
>
> - sep = find_local_sep_by_seid(req->first_seid.seid);
> + sep = find_local_sep_by_seid(session, req->first_seid.seid);
> if (!sep || !sep->stream) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1607,7 +1615,7 @@ static gboolean avdtp_close_cmd(struct avdtp *session, uint8_t transaction,
> return FALSE;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep || !sep->stream) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1668,7 +1676,7 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
> for (i = 0; i < seid_count; i++, seid++) {
> failed_seid = seid->seid;
>
> - sep = find_local_sep_by_seid(req->first_seid.seid);
> + sep = find_local_sep_by_seid(session, req->first_seid.seid);
> if (!sep || !sep->stream) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -1715,7 +1723,7 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
> return FALSE;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep || !sep->stream)
> return TRUE;
>
> @@ -1753,7 +1761,7 @@ static gboolean avdtp_delayreport_cmd(struct avdtp *session,
> return FALSE;
> }
>
> - sep = find_local_sep_by_seid(req->acp_seid);
> + sep = find_local_sep_by_seid(session, req->acp_seid);
> if (!sep || !sep->stream) {
> err = AVDTP_BAD_ACP_SEID;
> goto failed;
> @@ -2105,12 +2113,16 @@ static int set_priority(int fd, int priority)
> return err;
> }
>
> -struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> +struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version,
> + struct queue *lseps)
> {
> struct avdtp *session;
> GIOCondition cond = G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL;
> int new_fd;
>
> + if (!lseps)
> + return NULL;
> +
> new_fd = dup(fd);
> if (new_fd < 0) {
> error("dup(): %s (%d)", strerror(errno), errno);
> @@ -2138,6 +2150,8 @@ struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version)
> (GIOFunc) session_cb, session,
> NULL);
>
> + session->lseps = lseps;
> +
> return avdtp_ref(session);
> }
>
> @@ -3344,7 +3358,8 @@ int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
> &req, sizeof(req));
> }
>
> -struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
> +struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
> + uint8_t media_type,
> uint8_t codec_type,
> gboolean delay_reporting,
> struct avdtp_sep_ind *ind,
> @@ -3357,7 +3372,7 @@ struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
> if (!seid)
> return NULL;
>
> - if (g_slist_length(lseps) > MAX_SEID)
> + if (queue_length(lseps) > MAX_SEID)
> return NULL;
>
> sep = g_new0(struct avdtp_local_sep, 1);
> @@ -3374,7 +3389,11 @@ struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
>
> DBG("SEP %p registered: type:%d codec:%d seid:%d", sep,
> sep->info.type, sep->codec, sep->info.seid);
> - lseps = g_slist_append(lseps, sep);
> +
> + if (!queue_push_tail(lseps, sep)) {
> + g_free(sep);
> + return NULL;
> + }
>
> return sep;
> }
> @@ -3386,13 +3405,11 @@ void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
> sep->vndcodec_codec = codec_id;
> }
>
> -int avdtp_unregister_sep(struct avdtp_local_sep *sep)
> +int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep)
> {
> if (!sep)
> return -EINVAL;
>
> - lseps = g_slist_remove(lseps, sep);
> -
> if (sep->stream)
> release_stream(sep->stream, sep->stream->session);
>
> @@ -3400,6 +3417,7 @@ int avdtp_unregister_sep(struct avdtp_local_sep *sep)
> sep->info.type, sep->codec, sep->info.seid);
>
> util_clear_uid(&seids, sep->info.seid);
> + queue_remove(lseps, sep);
> g_free(sep);
>
> return 0;
> diff --git a/android/avdtp.h b/android/avdtp.h
> index d5335e4..07516a8 100644
> --- a/android/avdtp.h
> +++ b/android/avdtp.h
> @@ -205,7 +205,8 @@ typedef void (*avdtp_discover_cb_t) (struct avdtp *session, GSList *seps,
> struct avdtp_error *err, void *user_data);
> typedef void (*avdtp_disconnect_cb_t) (void *user_data);
>
> -struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version);
> +struct avdtp *avdtp_new(int fd, size_t imtu, size_t omtu, uint16_t version,
> + struct queue *lseps);
>
> unsigned int avdtp_add_disconnect_cb(struct avdtp *session,
> avdtp_disconnect_cb_t cb,
> @@ -265,7 +266,8 @@ int avdtp_abort(struct avdtp *session, struct avdtp_stream *stream);
> int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
> uint16_t delay);
>
> -struct avdtp_local_sep *avdtp_register_sep(uint8_t type, uint8_t media_type,
> +struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
> + uint8_t media_type,
> uint8_t codec_type,
> gboolean delay_reporting,
> struct avdtp_sep_ind *ind,
> @@ -278,7 +280,7 @@ void avdtp_sep_set_vendor_codec(struct avdtp_local_sep *sep, uint32_t vendor_id,
> struct avdtp_remote_sep *avdtp_find_remote_sep(struct avdtp *session,
> struct avdtp_local_sep *lsep);
>
> -int avdtp_unregister_sep(struct avdtp_local_sep *sep);
> +int avdtp_unregister_sep(struct queue *lseps, struct avdtp_local_sep *sep);
>
> avdtp_state_t avdtp_sep_get_state(struct avdtp_local_sep *sep);
>
> diff --git a/android/avdtptest.c b/android/avdtptest.c
> index ce39519..6529423 100644
> --- a/android/avdtptest.c
> +++ b/android/avdtptest.c
> @@ -40,6 +40,7 @@
>
> #include "src/shared/util.h"
> #include "btio/btio.h"
> +#include "src/shared/queue.h"
> #include "avdtp.h"
>
> static GMainLoop *mainloop = NULL;
> @@ -57,6 +58,7 @@ static uint16_t version = 0x0103;
> static guint media_player = 0;
> static guint media_recorder = 0;
> static guint idle_id = 0;
> +static struct queue *lseps = NULL;
>
> static bool fragment = false;
>
> @@ -411,7 +413,7 @@ static void connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
> return;
> }
>
> - avdtp = avdtp_new(fd, imtu, omtu, version);
> + avdtp = avdtp_new(fd, imtu, omtu, version, lseps);
> if (!avdtp) {
> printf("Failed to create avdtp instance\n");
> g_main_loop_quit(mainloop);
> @@ -864,13 +866,15 @@ int main(int argc, char *argv[])
> }
> }
>
> - local_sep = avdtp_register_sep(dev_role, AVDTP_MEDIA_TYPE_AUDIO,
> + local_sep = avdtp_register_sep(lseps, dev_role, AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, &sep_cfm, NULL);
> if (!local_sep) {
> printf("Failed to register sep\n");
> exit(0);
> }
>
> + queue_push_tail(lseps, local_sep);
> +
> if (!bacmp(&dst, BDADDR_ANY)) {
> printf("Listening...\n");
> io = do_listen(&err);
> @@ -889,6 +893,8 @@ int main(int argc, char *argv[])
>
> printf("Done\n");
>
> + queue_destroy(lseps, NULL);
> +
> avdtp_unref(avdtp);
> avdtp = NULL;
>
> diff --git a/unit/test-avdtp.c b/unit/test-avdtp.c
> index 34820d1..a293a1d 100644
> --- a/unit/test-avdtp.c
> +++ b/unit/test-avdtp.c
> @@ -36,13 +36,12 @@
> #include <glib.h>
>
> #include "src/shared/util.h"
> +#include "src/shared/queue.h"
> #include "src/log.h"
> #include "android/avdtp.h"
>
> #define MAX_SEID 0x3E
>
> -GSList *lseps = NULL;
> -
> struct test_pdu {
> bool valid;
> bool fragmented;
> @@ -89,6 +88,7 @@ struct context {
> struct avdtp *session;
> struct avdtp_local_sep *sep;
> struct avdtp_stream *stream;
> + struct queue *lseps;
> guint source;
> guint process;
> int fd;
> @@ -233,7 +233,11 @@ static struct context *context_new(uint16_t version, uint16_t imtu,
> err = socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, sv);
> g_assert(err == 0);
>
> - context->session = avdtp_new(sv[0], imtu, omtu, version);
> + context->lseps = queue_new();
> + g_assert(context->lseps);
> +
> + context->session = avdtp_new(sv[0], imtu, omtu, version,
> + context->lseps);
> g_assert(context->session != NULL);
>
> channel = g_io_channel_unix_new(sv[1]);
> @@ -505,7 +509,8 @@ static void test_server(gconstpointer data)
> struct context *context = create_context(0x0100, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
> + AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, FALSE, &sep_ind, &sep_cfm,
> context);
>
> @@ -513,7 +518,8 @@ static void test_server(gconstpointer data)
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void test_server_1_3(gconstpointer data)
> @@ -521,14 +527,16 @@ static void test_server_1_3(gconstpointer data)
> struct context *context = create_context(0x0103, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
> + AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL, context);
>
> g_idle_add(send_pdu, context);
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void test_server_1_3_sink(gconstpointer data)
> @@ -536,14 +544,16 @@ static void test_server_1_3_sink(gconstpointer data)
> struct context *context = create_context(0x0103, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> + AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL, context);
>
> g_idle_add(send_pdu, context);
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void test_server_0_sep(gconstpointer data)
> @@ -559,7 +569,8 @@ static void unregister_sep(void *data)
> {
> struct avdtp_local_sep *sep = data;
>
> - avdtp_unregister_sep(sep);
> + /* Removed from the queue by caller */
> + avdtp_unregister_sep(NULL, sep);
> }
>
> static void test_server_seid(gconstpointer data)
> @@ -569,25 +580,22 @@ static void test_server_seid(gconstpointer data)
> unsigned int i;
>
> for (i = 0; i < sizeof(int) * 8; i++) {
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL,
> context);
> g_assert(sep);
> -
> - lseps = g_slist_append(lseps, sep);
> }
>
> /* Now add (MAX_SEID + 1) SEP -> it shall fail */
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL,
> context);
> g_assert(!sep);
>
> /* Remove all SEPs */
> - g_slist_free_full(lseps, unregister_sep);
> - lseps = NULL;
> + queue_destroy(context->lseps, unregister_sep);
> }
>
> static void test_server_seid_duplicate(gconstpointer data)
> @@ -597,37 +605,34 @@ static void test_server_seid_duplicate(gconstpointer data)
> int i;
>
> for (i = 0; i < 2; i++) {
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL,
> context);
> g_assert(sep);
> -
> - lseps = g_slist_append(lseps, sep);
> }
>
> /* Remove 1st element */
> - sep = g_slist_nth_data(lseps, 0);
> - lseps = g_slist_remove(lseps, sep);
> - avdtp_unregister_sep(sep);
> + sep = queue_peek_head(context->lseps);
> + g_assert(sep);
> +
> + avdtp_unregister_sep(context->lseps, sep);
>
> /* Now register new element */
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind, NULL,
> context);
> g_assert(sep);
> - lseps = g_slist_append(lseps, sep);
>
> - /* Check SEID ids */
> + /* Check SEID ids with DISCOVER */
>
> g_idle_add(send_pdu, context);
>
> execute_context(context);
>
> /* Remove all SEPs */
> - g_slist_free_full(lseps, unregister_sep);
> - lseps = NULL;
> + queue_destroy(context->lseps, unregister_sep);
> }
>
> static gboolean sep_getcap_ind_frg(struct avdtp *session,
> @@ -675,7 +680,8 @@ static void test_server_frg(gconstpointer data)
> struct context *context = context_new(0x0100, 48, 48, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SOURCE, AVDTP_MEDIA_TYPE_AUDIO,
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SOURCE,
> + AVDTP_MEDIA_TYPE_AUDIO,
> 0x00, TRUE, &sep_ind_frg,
> NULL, context);
>
> @@ -683,7 +689,8 @@ static void test_server_frg(gconstpointer data)
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void discover_cb(struct avdtp *session, GSList *seps,
> @@ -758,16 +765,18 @@ static void test_client(gconstpointer data)
> struct context *context = create_context(0x0100, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
> - 0x00, FALSE, NULL, &sep_cfm,
> - context);
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> + AVDTP_MEDIA_TYPE_AUDIO,
> + 0x00, FALSE, NULL, &sep_cfm, context);
> +
> context->sep = sep;
>
> avdtp_discover(context->session, discover_cb, context);
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void test_client_1_3(gconstpointer data)
> @@ -775,16 +784,18 @@ static void test_client_1_3(gconstpointer data)
> struct context *context = create_context(0x0103, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
> - 0x00, TRUE, NULL, &sep_cfm,
> - context);
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> + AVDTP_MEDIA_TYPE_AUDIO,
> + 0x00, TRUE, NULL, &sep_cfm, context);
> +
> context->sep = sep;
>
> avdtp_discover(context->session, discover_cb, context);
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> static void test_client_frg(gconstpointer data)
> @@ -792,16 +803,18 @@ static void test_client_frg(gconstpointer data)
> struct context *context = context_new(0x0100, 48, 48, data);
> struct avdtp_local_sep *sep;
>
> - sep = avdtp_register_sep(AVDTP_SEP_TYPE_SINK, AVDTP_MEDIA_TYPE_AUDIO,
> - 0x00, TRUE, NULL, &sep_cfm,
> - context);
> + sep = avdtp_register_sep(context->lseps, AVDTP_SEP_TYPE_SINK,
> + AVDTP_MEDIA_TYPE_AUDIO,
> + 0x00, TRUE, NULL, &sep_cfm, context);
> +
> context->sep = sep;
>
> avdtp_discover(context->session, discover_cb, context);
>
> execute_context(context);
>
> - avdtp_unregister_sep(sep);
> + avdtp_unregister_sep(context->lseps, sep);
> + queue_destroy(context->lseps, NULL);
> }
>
> int main(int argc, char *argv[])
>

--
Best regards,
Szymon Janc

2015-02-19 15:46:47

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 5/5] audio/avdtp: Refactor avdtp and a2dp code

From: Andrei Emeltchenko <[email protected]>

Move connection-related code from avdtp to a2dp. This shall help to use
same avdtp library in profiles/ and android/ code.
---
profiles/audio/a2dp.c | 163 +++++++++++++++++++++++++++++++++++++++++++++--
profiles/audio/avdtp.c | 168 ++++++++++++++-----------------------------------
profiles/audio/avdtp.h | 14 ++++-
3 files changed, 219 insertions(+), 126 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 75503f3..33e66a7 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -46,6 +46,8 @@
#include "src/log.h"
#include "src/sdpd.h"

+#include "btio/btio.h"
+
#include "avdtp.h"
#include "sink.h"
#include "source.h"
@@ -58,6 +60,8 @@
#define SUSPEND_TIMEOUT 5
#define RECONFIGURE_TIMEOUT 500

+#define AVDTP_PSM 25
+
struct a2dp_sep {
struct a2dp_server *server;
struct a2dp_endpoint *endpoint;
@@ -110,7 +114,15 @@ struct a2dp_server {
gboolean source_enabled;
};

+struct avdtp_server {
+ struct btd_adapter *adapter;
+ GIOChannel *io;
+ GSList *seps;
+ GSList *sessions;
+};
+
static GSList *servers = NULL;
+static GSList *avdtp_servers = NULL;
static GSList *setups = NULL;
static unsigned int cb_id = 0;

@@ -1186,6 +1198,19 @@ static struct a2dp_server *find_server(GSList *list, struct btd_adapter *a)
return NULL;
}

+static struct avdtp_server *find_avdtp_server(GSList *list,
+ struct btd_adapter *a)
+{
+ for (; list; list = list->next) {
+ struct avdtp_server *server = list->data;
+
+ if (server->adapter == a)
+ return server;
+ }
+
+ return NULL;
+}
+
static struct a2dp_server *a2dp_server_register(struct btd_adapter *adapter)
{
struct a2dp_server *server;
@@ -1197,6 +1222,30 @@ static struct a2dp_server *a2dp_server_register(struct btd_adapter *adapter)
return server;
}

+static void avdtp_server_destroy(struct avdtp_server *server)
+{
+ g_slist_free_full(server->sessions, avdtp_free);
+
+ avdtp_servers = g_slist_remove(avdtp_servers, server);
+
+ g_io_channel_shutdown(server->io, TRUE, NULL);
+ g_io_channel_unref(server->io);
+ btd_adapter_unref(server->adapter);
+ g_free(server);
+}
+
+static void a2dp_clean_lsep(struct avdtp_local_sep *lsep)
+{
+ struct avdtp_server *server = avdtp_get_server(lsep);
+
+
+ server->seps = g_slist_remove(server->seps, lsep);
+ if (!server->seps)
+ avdtp_server_destroy(server);
+
+ avdtp_unregister_sep(lsep);
+}
+
static void a2dp_unregister_sep(struct a2dp_sep *sep)
{
if (sep->destroy) {
@@ -1204,7 +1253,8 @@ static void a2dp_unregister_sep(struct a2dp_sep *sep)
sep->endpoint = NULL;
}

- avdtp_unregister_sep(sep->lsep);
+ a2dp_clean_lsep(sep->lsep);
+
g_free(sep);
}

@@ -1215,6 +1265,103 @@ static void a2dp_server_unregister(struct a2dp_server *server)
g_free(server);
}

+static void auth_cb(DBusError *derr, void *user_data)
+{
+ struct avdtp *session = user_data;
+
+ if (derr && dbus_error_is_set(derr)) {
+ error("Access denied: %s", derr->message);
+ connection_lost(session, EACCES);
+ return;
+ }
+
+ avdtp_accept(session);
+}
+
+static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
+{
+ struct avdtp *session;
+ char address[18];
+ bdaddr_t src, dst;
+ GError *err = NULL;
+ struct btd_device *device;
+ struct avdtp_server *avdtp_server;
+
+ bt_io_get(chan, &err,
+ BT_IO_OPT_SOURCE_BDADDR, &src,
+ BT_IO_OPT_DEST_BDADDR, &dst,
+ BT_IO_OPT_DEST, address,
+ BT_IO_OPT_INVALID);
+ if (err) {
+ error("%s", err->message);
+ g_error_free(err);
+ goto drop;
+ }
+
+ DBG("AVDTP: incoming connect from %s", address);
+
+ device = btd_adapter_find_device(adapter_find(&src), &dst,
+ BDADDR_BREDR);
+ if (!device)
+ goto drop;
+
+ avdtp_server = find_avdtp_server(avdtp_servers,
+ device_get_adapter(device));
+ if (!avdtp_server)
+ goto drop;
+
+ session = avdtp_new(avdtp_server, avdtp_server->sessions, chan, device);
+ if (!session)
+ goto drop;
+
+ avdtp_request_authorization(session, &src, &dst, auth_cb);
+
+ return;
+
+drop:
+ g_io_channel_shutdown(chan, TRUE, NULL);
+}
+
+static GIOChannel *avdtp_server_socket(const bdaddr_t *src, gboolean master)
+{
+ GError *err = NULL;
+ GIOChannel *io;
+
+ io = bt_io_listen(NULL, avdtp_confirm_cb,
+ NULL, NULL, &err,
+ BT_IO_OPT_SOURCE_BDADDR, src,
+ BT_IO_OPT_PSM, AVDTP_PSM,
+ BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
+ BT_IO_OPT_MASTER, master,
+ BT_IO_OPT_INVALID);
+ if (!io) {
+ error("%s", err->message);
+ g_error_free(err);
+ }
+
+ return io;
+}
+
+static struct avdtp_server *avdtp_server_init(struct btd_adapter *adapter)
+{
+ struct avdtp_server *server;
+
+ server = g_new0(struct avdtp_server, 1);
+
+ server->io = avdtp_server_socket(btd_adapter_get_address(adapter),
+ TRUE);
+ if (!server->io) {
+ g_free(server);
+ return NULL;
+ }
+
+ server->adapter = btd_adapter_ref(adapter);
+
+ avdtp_servers = g_slist_append(avdtp_servers, server);
+
+ return server;
+}
+
struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
uint8_t codec, gboolean delay_reporting,
struct a2dp_endpoint *endpoint,
@@ -1222,6 +1369,7 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
int *err)
{
struct a2dp_server *server;
+ struct avdtp_server *avdtp_server;
struct a2dp_sep *sep;
GSList **l;
uint32_t *record_id;
@@ -1248,7 +1396,14 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,

sep = g_new0(struct a2dp_sep, 1);

- sep->lsep = avdtp_register_sep(adapter, type,
+ avdtp_server = find_avdtp_server(avdtp_servers, adapter);
+ if (!avdtp_server) {
+ avdtp_server = avdtp_server_init(adapter);
+ if (!server)
+ return NULL;
+ }
+
+ sep->lsep = avdtp_register_sep(avdtp_server, type,
AVDTP_MEDIA_TYPE_AUDIO, codec,
delay_reporting, &endpoint_ind,
&cfm, sep);
@@ -1282,7 +1437,7 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
record = a2dp_record(type);
if (!record) {
error("Unable to allocate new service record");
- avdtp_unregister_sep(sep->lsep);
+ a2dp_clean_lsep(sep->lsep);
g_free(sep);
if (err)
*err = -EINVAL;
@@ -1292,7 +1447,7 @@ struct a2dp_sep *a2dp_add_sep(struct btd_adapter *adapter, uint8_t type,
if (adapter_service_add(server->adapter, record) < 0) {
error("Unable to register A2DP service record");
sdp_record_free(record);
- avdtp_unregister_sep(sep->lsep);
+ a2dp_clean_lsep(sep->lsep);
g_free(sep);
if (err)
*err = -EINVAL;
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 825fb79..1f996db 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -1107,7 +1107,7 @@ static void remove_disconnect_timer(struct avdtp *session)
session->stream_setup = FALSE;
}

-static void avdtp_free(void *data)
+void avdtp_free(void *data)
{
struct avdtp *session = data;

@@ -1141,7 +1141,7 @@ static void avdtp_free(void *data)
g_free(session);
}

-static void connection_lost(struct avdtp *session, int err)
+void connection_lost(struct avdtp *session, int err)
{
struct avdtp_server *server = session->server;
char address[18];
@@ -2424,59 +2424,27 @@ failed:
connection_lost(session, err_no);
}

-static void auth_cb(DBusError *derr, void *user_data)
-{
- struct avdtp *session = user_data;
- GError *err = NULL;
-
- if (derr && dbus_error_is_set(derr)) {
- error("Access denied: %s", derr->message);
- connection_lost(session, EACCES);
- return;
- }
-
- if (!bt_io_accept(session->io, avdtp_connect_cb, session, NULL,
- &err)) {
- error("bt_io_accept: %s", err->message);
- connection_lost(session, EACCES);
- g_error_free(err);
- return;
- }
-
- /* This is so that avdtp_connect_cb will know to do the right thing
- * with respect to the disconnect timer */
- session->stream_setup = TRUE;
-}
-
-static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
+struct avdtp *avdtp_new(struct avdtp_server *server, GSList *sessions,
+ GIOChannel *chan,
+ struct btd_device *device)
{
struct avdtp *session;
- char address[18];
- bdaddr_t src, dst;
- GError *err = NULL;
- struct btd_device *device;

- bt_io_get(chan, &err,
- BT_IO_OPT_SOURCE_BDADDR, &src,
- BT_IO_OPT_DEST_BDADDR, &dst,
- BT_IO_OPT_DEST, address,
- BT_IO_OPT_INVALID);
- if (err) {
- error("%s", err->message);
- g_error_free(err);
- goto drop;
- }
+ session = find_session(sessions, device);
+ if (session)
+ return session;

- DBG("AVDTP: incoming connect from %s", address);
+ session = g_new0(struct avdtp, 1);

- device = btd_adapter_find_device(adapter_find(&src), &dst,
- BDADDR_BREDR);
- if (!device)
- goto drop;
+ session->server = server;
+ session->device = btd_device_ref(device);
+ /* We don't use avdtp_set_state() here since this isn't a state change
+ * but just setting of the initial state */
+ session->state = AVDTP_SESSION_STATE_DISCONNECTED;

- session = avdtp_get_internal(device);
- if (!session)
- goto drop;
+ session->version = get_version(session);
+
+ sessions = g_slist_append(sessions, session);

/* This state (ie, session is already *connecting*) happens when the
* device initiates a connect (really a config'd L2CAP channel) even
@@ -2492,11 +2460,12 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
if (session->pending_open && session->pending_open->open_acp) {
if (!bt_io_accept(chan, avdtp_connect_cb, session, NULL, NULL))
goto drop;
- return;
+
+ return NULL;
}

if (session->io) {
- error("Refusing unexpected connect from %s", address);
+ error("Refusing unexpected connect");
goto drop;
}

@@ -2508,18 +2477,23 @@ static void avdtp_confirm_cb(GIOChannel *chan, gpointer data)
session->io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);

- session->auth_id = btd_request_authorization(&src, &dst,
+ return session;
+drop:
+ return NULL;
+}
+
+bool avdtp_request_authorization(struct avdtp *session, const bdaddr_t *src,
+ const bdaddr_t *dst, service_auth_cb cb)
+{
+ session->auth_id = btd_request_authorization(src, dst,
ADVANCED_AUDIO_UUID,
- auth_cb, session);
+ cb, session);
if (session->auth_id == 0) {
avdtp_set_state(session, AVDTP_SESSION_STATE_DISCONNECTED);
- goto drop;
+ return false;
}

- return;
-
-drop:
- g_io_channel_shutdown(chan, TRUE, NULL);
+ return true;
}

static GIOChannel *l2cap_connect(struct avdtp *session)
@@ -3654,47 +3628,24 @@ int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
&req, sizeof(req));
}

-static GIOChannel *avdtp_server_socket(const bdaddr_t *src, gboolean master)
+void avdtp_accept(struct avdtp *session)
{
GError *err = NULL;
- GIOChannel *io;

- io = bt_io_listen(NULL, avdtp_confirm_cb,
- NULL, NULL, &err,
- BT_IO_OPT_SOURCE_BDADDR, src,
- BT_IO_OPT_PSM, AVDTP_PSM,
- BT_IO_OPT_SEC_LEVEL, BT_IO_SEC_MEDIUM,
- BT_IO_OPT_MASTER, master,
- BT_IO_OPT_INVALID);
- if (!io) {
- error("%s", err->message);
+ if (!bt_io_accept(session->io, avdtp_connect_cb, session, NULL,
+ &err)) {
+ error("bt_io_accept: %s", err->message);
+ connection_lost(session, EACCES);
g_error_free(err);
+ return;
}

- return io;
-}
-
-static struct avdtp_server *avdtp_server_init(struct btd_adapter *adapter)
-{
- struct avdtp_server *server;
-
- server = g_new0(struct avdtp_server, 1);
-
- server->io = avdtp_server_socket(btd_adapter_get_address(adapter),
- TRUE);
- if (!server->io) {
- g_free(server);
- return NULL;
- }
-
- server->adapter = btd_adapter_ref(adapter);
-
- servers = g_slist_append(servers, server);
-
- return server;
+ /* This is so that avdtp_connect_cb will know to do the right thing
+ * with respect to the disconnect timer */
+ session->stream_setup = TRUE;
}

-struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
+struct avdtp_local_sep *avdtp_register_sep(struct avdtp_server *server,
uint8_t type,
uint8_t media_type,
uint8_t codec_type,
@@ -3703,20 +3654,12 @@ struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
struct avdtp_sep_cfm *cfm,
void *user_data)
{
- struct avdtp_server *server;
struct avdtp_local_sep *sep;
uint8_t seid = util_get_uid(&seids, MAX_SEID);

if (!seid)
return NULL;

- server = find_server(servers, adapter);
- if (!server) {
- server = avdtp_server_init(adapter);
- if (!server)
- return NULL;
- }
-
sep = g_new0(struct avdtp_local_sep, 1);

sep->state = AVDTP_STATE_IDLE;
@@ -3737,28 +3680,11 @@ struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
return sep;
}

-static void avdtp_server_destroy(struct avdtp_server *server)
-{
- g_slist_free_full(server->sessions, avdtp_free);
-
- servers = g_slist_remove(servers, server);
-
- g_io_channel_shutdown(server->io, TRUE, NULL);
- g_io_channel_unref(server->io);
- btd_adapter_unref(server->adapter);
- g_free(server);
-}
-
int avdtp_unregister_sep(struct avdtp_local_sep *sep)
{
- struct avdtp_server *server;
-
if (!sep)
return -EINVAL;

- server = sep->server;
- server->seps = g_slist_remove(server->seps, sep);
-
if (sep->stream)
release_stream(sep->stream, sep->stream->session);

@@ -3768,11 +3694,6 @@ int avdtp_unregister_sep(struct avdtp_local_sep *sep)
util_clear_uid(&seids, sep->info.seid);
g_free(sep);

- if (server->seps)
- return 0;
-
- avdtp_server_destroy(server);
-
return 0;
}

@@ -3836,6 +3757,11 @@ struct btd_device *avdtp_get_device(struct avdtp *session)
return session->device;
}

+struct avdtp_server *avdtp_get_server(struct avdtp_local_sep *lsep)
+{
+ return lsep->server;
+}
+
gboolean avdtp_has_stream(struct avdtp *session, struct avdtp_stream *stream)
{
return g_slist_find(session->streams, stream) ? TRUE : FALSE;
diff --git a/profiles/audio/avdtp.h b/profiles/audio/avdtp.h
index 390c154..8bf6611 100644
--- a/profiles/audio/avdtp.h
+++ b/profiles/audio/avdtp.h
@@ -29,6 +29,7 @@ typedef enum {
} avdtp_session_state_t;

struct avdtp;
+struct avdtp_server;
struct avdtp_stream;
struct avdtp_local_sep;
struct avdtp_remote_sep;
@@ -268,7 +269,7 @@ int avdtp_abort(struct avdtp *session, struct avdtp_stream *stream);
int avdtp_delay_report(struct avdtp *session, struct avdtp_stream *stream,
uint16_t delay);

-struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
+struct avdtp_local_sep *avdtp_register_sep(struct avdtp_server *server,
uint8_t type,
uint8_t media_type,
uint8_t codec_type,
@@ -293,3 +294,14 @@ int avdtp_error_posix_errno(struct avdtp_error *err);

struct btd_adapter *avdtp_get_adapter(struct avdtp *session);
struct btd_device *avdtp_get_device(struct avdtp *session);
+struct avdtp_server *avdtp_get_server(struct avdtp_local_sep *lsep);
+
+struct avdtp *avdtp_new(struct avdtp_server *server, GSList *sessions,
+ GIOChannel *chan,
+ struct btd_device *device);
+void avdtp_free(void *data);
+void connection_lost(struct avdtp *session, int err);
+void avdtp_accept(struct avdtp *session);
+bool avdtp_request_authorization(struct avdtp *session, const bdaddr_t *src,
+ const bdaddr_t *dst,
+ service_auth_cb cb);
--
2.1.0


2015-02-19 15:46:44

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 2/5] unit/avdtp: Refactor context destroy

From: Andrei Emeltchenko <[email protected]>

destroy_context() function will be used for dummy_tests without
context_execute()
---
unit/test-avdtp.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/unit/test-avdtp.c b/unit/test-avdtp.c
index a293a1d..5018350 100644
--- a/unit/test-avdtp.c
+++ b/unit/test-avdtp.c
@@ -264,10 +264,8 @@ static struct context *create_context(uint16_t version, gconstpointer data)
return context_new(version, 672, 672, data);
}

-static void execute_context(struct context *context)
+static void destroy_context(struct context *context)
{
- g_main_loop_run(context->main_loop);
-
if (context->source > 0)
g_source_remove(context->source);
avdtp_unref(context->session);
@@ -278,6 +276,13 @@ static void execute_context(struct context *context)
g_free(context);
}

+static void execute_context(struct context *context)
+{
+ g_main_loop_run(context->main_loop);
+
+ destroy_context(context);
+}
+
static gboolean sep_getcap_ind(struct avdtp *session,
struct avdtp_local_sep *sep,
GSList **caps, uint8_t *err,
@@ -596,6 +601,7 @@ static void test_server_seid(gconstpointer data)

/* Remove all SEPs */
queue_destroy(context->lseps, unregister_sep);
+ destroy_context(context);
}

static void test_server_seid_duplicate(gconstpointer data)
--
2.1.0


2015-02-19 15:46:45

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 3/5] android/avdtp: Remove extra check

From: Andrei Emeltchenko <[email protected]>

---
android/avdtp.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/android/avdtp.c b/android/avdtp.c
index 35bc548..b8a2147 100644
--- a/android/avdtp.c
+++ b/android/avdtp.c
@@ -3372,9 +3372,6 @@ struct avdtp_local_sep *avdtp_register_sep(struct queue *lseps, uint8_t type,
if (!seid)
return NULL;

- if (queue_length(lseps) > MAX_SEID)
- return NULL;
-
sep = g_new0(struct avdtp_local_sep, 1);

sep->state = AVDTP_STATE_IDLE;
--
2.1.0


2015-02-19 15:46:46

by Andrei Emeltchenko

[permalink] [raw]
Subject: [RFCv2 4/5] audio/avdtp: Use bitfield id generation

From: Andrei Emeltchenko <[email protected]>

Use bitfield utils for assigning SEP id. This fixes duplicated id
generation shown in unit/avdtp tests.
---
profiles/audio/avdtp.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index ba5f0e5..825fb79 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -41,6 +41,7 @@
#include <glib.h>

#include "src/log.h"
+#include "src/shared/util.h"

#include "btio/btio.h"
#include "lib/uuid.h"
@@ -54,6 +55,7 @@
#define AVDTP_PSM 25

#define MAX_SEID 0x3E
+static unsigned int seids;

#ifndef MAX
# define MAX(x, y) ((x) > (y) ? (x) : (y))
@@ -3703,6 +3705,10 @@ struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
{
struct avdtp_server *server;
struct avdtp_local_sep *sep;
+ uint8_t seid = util_get_uid(&seids, MAX_SEID);
+
+ if (!seid)
+ return NULL;

server = find_server(servers, adapter);
if (!server) {
@@ -3711,13 +3717,10 @@ struct avdtp_local_sep *avdtp_register_sep(struct btd_adapter *adapter,
return NULL;
}

- if (g_slist_length(server->seps) > MAX_SEID)
- return NULL;
-
sep = g_new0(struct avdtp_local_sep, 1);

sep->state = AVDTP_STATE_IDLE;
- sep->info.seid = g_slist_length(server->seps) + 1;
+ sep->info.seid = seid;
sep->info.type = type;
sep->info.media_type = media_type;
sep->codec = codec_type;
@@ -3762,6 +3765,7 @@ int avdtp_unregister_sep(struct avdtp_local_sep *sep)
DBG("SEP %p unregistered: type:%d codec:%d seid:%d", sep,
sep->info.type, sep->codec, sep->info.seid);

+ util_clear_uid(&seids, sep->info.seid);
g_free(sep);

if (server->seps)
--
2.1.0