linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision
@ 2012-06-15 14:41 Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 2/9 v3] audio: Fix handling of A2DP suspend indication Luiz Augusto von Dentz
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Check collision for AVDTP OPEN, CLOSE, START, SUSPEND and ABORT
commands and if they collided remove the pending request if sep
has accepted the indication.
---
v2: Fix capitalizing some acronyms
v3: Rebase and fix not using avdtp_start return in case of error

 audio/avdtp.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index eb3f213..1287698 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -314,6 +314,7 @@ struct pending_req {
 	size_t data_size;
 	struct avdtp_stream *stream; /* Set if the request targeted a stream */
 	guint timeout;
+	gboolean collided;
 };
 
 struct avdtp_remote_sep {
@@ -1643,6 +1644,69 @@ static gboolean avdtp_reconf_cmd(struct avdtp *session, uint8_t transaction,
 					AVDTP_RECONFIGURE, &rej, sizeof(rej));
 }
 
+static void check_seid_collision(struct pending_req *req, uint8_t id)
+{
+	struct seid_req *seid = req->data;
+
+	if (seid->acp_seid == id)
+		req->collided = TRUE;
+}
+
+static void check_start_collision(struct pending_req *req, uint8_t id)
+{
+	struct start_req *start = req->data;
+	struct seid *seid = &start->first_seid;
+	int count = 1 + req->data_size - sizeof(struct start_req);
+	int i;
+
+	for (i = 0; i < count; i++, seid++) {
+		if (seid->seid == id) {
+			req->collided = TRUE;
+			return;
+		}
+	}
+}
+
+static void check_suspend_collision(struct pending_req *req, uint8_t id)
+{
+	struct suspend_req *suspend = req->data;
+	struct seid *seid = &suspend->first_seid;
+	int count = 1 + req->data_size - sizeof(struct suspend_req);
+	int i;
+
+	for (i = 0; i < count; i++, seid++) {
+		if (seid->seid == id) {
+			req->collided = TRUE;
+			return;
+		}
+	}
+}
+
+static void avdtp_check_collision(struct avdtp *session, uint8_t cmd,
+					struct avdtp_stream *stream)
+{
+	struct pending_req *req = session->req;
+
+	if (req == NULL || (req->signal_id != cmd && cmd != AVDTP_ABORT))
+		return;
+
+	if (cmd == AVDTP_ABORT)
+		cmd = req->signal_id;
+
+	switch (cmd) {
+	case AVDTP_OPEN:
+	case AVDTP_CLOSE:
+		check_seid_collision(req, stream->rseid);
+		break;
+	case AVDTP_START:
+		check_start_collision(req, stream->rseid);
+		break;
+	case AVDTP_SUSPEND:
+		check_suspend_collision(req, stream->rseid);
+		break;
+	}
+}
+
 static gboolean avdtp_open_cmd(struct avdtp *session, uint8_t transaction,
 				struct seid_req *req, unsigned int size)
 {
@@ -1674,6 +1738,8 @@ static gboolean avdtp_open_cmd(struct avdtp *session, uint8_t transaction,
 			goto failed;
 	}
 
+	avdtp_check_collision(session, AVDTP_OPEN, stream);
+
 	if (!avdtp_send(session, transaction, AVDTP_MSG_TYPE_ACCEPT,
 						AVDTP_OPEN, NULL, 0))
 		return FALSE;
@@ -1736,6 +1802,8 @@ static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
 				goto failed;
 		}
 
+		avdtp_check_collision(session, AVDTP_START, stream);
+
 		avdtp_sep_set_state(session, sep, AVDTP_STATE_STREAMING);
 	}
 
@@ -1783,6 +1851,8 @@ static gboolean avdtp_close_cmd(struct avdtp *session, uint8_t transaction,
 			goto failed;
 	}
 
+	avdtp_check_collision(session, AVDTP_CLOSE, stream);
+
 	avdtp_sep_set_state(session, sep, AVDTP_STATE_CLOSING);
 
 	if (!avdtp_send(session, transaction, AVDTP_MSG_TYPE_ACCEPT,
@@ -1842,6 +1912,8 @@ static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
 				goto failed;
 		}
 
+		avdtp_check_collision(session, AVDTP_SUSPEND, stream);
+
 		avdtp_sep_set_state(session, sep, AVDTP_STATE_OPEN);
 	}
 
@@ -1880,6 +1952,8 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
 			goto failed;
 	}
 
+	avdtp_check_collision(session, AVDTP_ABORT, sep->stream);
+
 	ret = avdtp_send(session, transaction, AVDTP_MSG_TYPE_ACCEPT,
 						AVDTP_ABORT, NULL, 0);
 	if (ret)
@@ -2172,6 +2246,11 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 		if (session->streams && session->dc_timer)
 			remove_disconnect_timer(session);
 
+		if (session->req && session->req->collided) {
+			DBG("Collision detected");
+			goto next;
+		}
+
 		return TRUE;
 	}
 
@@ -2222,6 +2301,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
 		break;
 	}
 
+next:
 	pending_req_free(session->req);
 	session->req = NULL;
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 2/9 v3] audio: Fix handling of A2DP suspend indication
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 3/9 v3] audio: Fix handling of A2DP open indication Luiz Augusto von Dentz
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

When accepting the suspend indication all callbacks should be notified
that suspend completed.

In addition to this fix not using avdtp_start return in indication
callback as well as in the confirmation.
---
 audio/a2dp.c |   38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index cc8f139..fa97645 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -986,6 +986,9 @@ static gboolean suspend_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 				void *user_data)
 {
 	struct a2dp_sep *a2dp_sep = user_data;
+	struct a2dp_setup *setup;
+	gboolean start;
+	int start_err;
 
 	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
 		DBG("Sink %p: Suspend_Ind", sep);
@@ -999,6 +1002,30 @@ static gboolean suspend_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 		a2dp_sep->session = NULL;
 	}
 
+	if (!a2dp_sep->suspending)
+		return TRUE;
+
+	a2dp_sep->suspending = FALSE;
+
+	setup = find_setup_by_session(session);
+	if (!setup)
+		return TRUE;
+
+	start = setup->start;
+	setup->start = FALSE;
+
+	finalize_suspend(setup);
+
+	if (!start)
+		return TRUE;
+
+	start_err = avdtp_start(session, a2dp_sep->stream);
+	if (start_err < 0) {
+		error("avdtp_start: %s (%d)", strerror(-start_err),
+								-start_err);
+		finalize_setup_errno(setup, start_err, finalize_resume);
+	}
+
 	return TRUE;
 }
 
@@ -1009,7 +1036,7 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	struct a2dp_sep *a2dp_sep = user_data;
 	struct a2dp_setup *setup;
 	gboolean start;
-	int perr;
+	int start_err;
 
 	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
 		DBG("Sink %p: Suspend_Cfm", sep);
@@ -1040,10 +1067,11 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 		return;
 	}
 
-	perr = avdtp_start(session, a2dp_sep->stream);
-	if (perr < 0) {
-		error("Error on avdtp_start %s (%d)", strerror(-perr), -perr);
-		finalize_setup_errno(setup, -EIO, finalize_suspend, NULL);
+	start_err = avdtp_start(session, a2dp_sep->stream);
+	if (start_err < 0) {
+		error("avdtp_start: %s (%d)", strerror(-start_err),
+								-start_err);
+		finalize_setup_errno(setup, start_err, finalize_suspend, NULL);
 	}
 }
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 3/9 v3] audio: Fix handling of A2DP open indication
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 2/9 v3] audio: Fix handling of A2DP suspend indication Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 4/9 v3] audio: Fix handling of A2DP start indication Luiz Augusto von Dentz
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

When accepting the open indication all config callbacks should be
notified that open completed.
---
 audio/a2dp.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index fa97645..64de24a 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -883,11 +883,22 @@ static gboolean open_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 				void *user_data)
 {
 	struct a2dp_sep *a2dp_sep = user_data;
+	struct a2dp_setup *setup;
 
 	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
 		DBG("Sink %p: Open_Ind", sep);
 	else
 		DBG("Source %p: Open_Ind", sep);
+
+	setup = find_setup_by_session(session);
+	if (!setup)
+		return TRUE;
+
+	if (setup->reconfigure)
+		setup->reconfigure = FALSE;
+
+	finalize_config(setup);
+
 	return TRUE;
 }
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 4/9 v3] audio: Fix handling of A2DP start indication
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 2/9 v3] audio: Fix handling of A2DP suspend indication Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 3/9 v3] audio: Fix handling of A2DP open indication Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication Luiz Augusto von Dentz
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Only process callbacks if avdtp_start was sent, otherwise it may cancel
setup callbacks that were registere via g_idle_add.
---
 audio/a2dp.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 64de24a..5139f61 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -954,10 +954,6 @@ static gboolean start_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 	else
 		DBG("Source %p: Start_Ind", sep);
 
-	setup = find_setup_by_session(session);
-	if (setup)
-		finalize_resume(setup);
-
 	if (!a2dp_sep->locked) {
 		a2dp_sep->session = avdtp_ref(session);
 		a2dp_sep->suspend_timer = g_timeout_add_seconds(SUSPEND_TIMEOUT,
@@ -965,6 +961,15 @@ static gboolean start_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 						a2dp_sep);
 	}
 
+	if (!a2dp_sep->starting)
+		return TRUE;
+
+	a2dp_sep->starting = FALSE;
+
+	setup = find_setup_by_session(session);
+	if (setup)
+		finalize_resume(setup);
+
 	return TRUE;
 }
 
@@ -980,6 +985,8 @@ static void start_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	else
 		DBG("Source %p: Start_Cfm", sep);
 
+	a2dp_sep->starting = FALSE;
+
 	setup = find_setup_by_session(session);
 	if (!setup)
 		return;
@@ -2217,6 +2224,7 @@ unsigned int a2dp_resume(struct avdtp *session, struct a2dp_sep *sep,
 			error("avdtp_start failed");
 			goto failed;
 		}
+		sep->starting = TRUE;
 		break;
 	case AVDTP_STATE_STREAMING:
 		if (!sep->suspending && sep->suspend_timer) {
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 4/9 v3] audio: Fix handling of A2DP start indication Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2013-01-21 14:53   ` Chan-yeol Park
  2012-06-15 14:41 ` [PATCH BlueZ 6/9 v3] audio: Wait remote side to send AVDTP_START when acting as acceptor Luiz Augusto von Dentz
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

When an abort is received all setup callbacks should return an error.
---
 audio/a2dp.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index 5139f61..d9dcead 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1182,6 +1182,7 @@ static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 				void *user_data)
 {
 	struct a2dp_sep *a2dp_sep = user_data;
+	struct a2dp_setup *setup;
 
 	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
 		DBG("Sink %p: Abort_Ind", sep);
@@ -1190,6 +1191,14 @@ static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 
 	a2dp_sep->stream = NULL;
 
+	setup = find_setup_by_session(session);
+	if (!setup)
+		return TRUE;
+
+	finalize_setup_errno(setup, -ECONNRESET, finalize_suspend,
+							finalize_resume,
+							finalize_config);
+
 	return TRUE;
 }
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 6/9 v3] audio: Wait remote side to send AVDTP_START when acting as acceptor
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (3 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 7/9 v3] AVDTP: Do not respond ABORT command with invalid id Luiz Augusto von Dentz
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Some devices like Sony Ericsson MW600 reject AVDTP_START if it was the
initiator of the connection, apparently it follows recommendation 12 of
simultaneous use of HFP, A2DP and AVRCP profiles white paper which says:

  "If the RD has configured and opened a stream it is also responsible to
  start the streaming via GAVDP_START."

If the client is fast enough and try to acquire the transport this cause
an error, so instead of sending AVDTP_START the code now checks if it is
the acceptor of the stream and wait the remote side to send the command.
---
 audio/avdtp.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 1287698..736eed7 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1068,12 +1068,19 @@ static void avdtp_sep_set_state(struct avdtp *session,
 		break;
 	case AVDTP_STATE_OPEN:
 		stream->starting = FALSE;
-		if (old_state > AVDTP_STATE_OPEN && session->auto_dc)
+		if ((old_state > AVDTP_STATE_OPEN && session->auto_dc) ||
+							stream->open_acp)
 			stream->idle_timer = g_timeout_add_seconds(STREAM_TIMEOUT,
 								stream_timeout,
 								stream);
 		break;
 	case AVDTP_STATE_STREAMING:
+		if (stream->idle_timer) {
+			g_source_remove(stream->idle_timer);
+			stream->idle_timer = 0;
+		}
+		stream->open_acp = FALSE;
+		break;
 	case AVDTP_STATE_CLOSING:
 	case AVDTP_STATE_ABORTING:
 		if (stream->idle_timer) {
@@ -3660,6 +3667,15 @@ int avdtp_start(struct avdtp *session, struct avdtp_stream *stream)
 	if (stream->lsep->state != AVDTP_STATE_OPEN)
 		return -EINVAL;
 
+	/* Recommendation 12:
+	 *  If the RD has configured and opened a stream it is also responsible
+	 *  to start the streaming via GAVDP_START.
+	 */
+	if (stream->open_acp) {
+		stream->starting = TRUE;
+		return 0;
+	}
+
 	if (stream->close_int == TRUE) {
 		error("avdtp_start: rejecting start since close is initiated");
 		return -EINVAL;
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 7/9 v3] AVDTP: Do not respond ABORT command with invalid id
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (4 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 6/9 v3] audio: Wait remote side to send AVDTP_START when acting as acceptor Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 8/9 v3] AVDTP: Fix responding to ABORT with reject Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

AVDTP spec, 8.15.2 Abort Response:

  "If an AVDTP_ABORT_CMD contains an invalid SEID, no response shall be
  sent."
---
 audio/avdtp.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/audio/avdtp.c b/audio/avdtp.c
index 736eed7..e9dea6c 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1948,10 +1948,8 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
 	}
 
 	sep = find_local_sep_by_seid(session->server, req->acp_seid);
-	if (!sep || !sep->stream) {
-		err = AVDTP_BAD_ACP_SEID;
-		goto failed;
-	}
+	if (!sep || !sep->stream)
+		return TRUE;
 
 	if (sep->ind && sep->ind->abort) {
 		if (!sep->ind->abort(session, sep, sep->stream, &err,
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 8/9 v3] AVDTP: Fix responding to ABORT with reject
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (5 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 7/9 v3] AVDTP: Do not respond ABORT command with invalid id Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:41 ` [PATCH BlueZ 9/9 v3] audio: Fix aborting A2DP setup while AVDTP_START is in progress Luiz Augusto von Dentz
  2012-06-15 14:52 ` [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Johan Hedberg
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

ABORT command cannot be rejected
---
 audio/a2dp.c  |    6 +++---
 audio/avdtp.c |   12 +++---------
 audio/avdtp.h |    2 +-
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index d9dcead..fafff87 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1177,7 +1177,7 @@ static void close_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 		g_timeout_add(RECONFIGURE_TIMEOUT, a2dp_reconfigure, setup);
 }
 
-static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
+static void abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 				struct avdtp_stream *stream, uint8_t *err,
 				void *user_data)
 {
@@ -1193,13 +1193,13 @@ static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 
 	setup = find_setup_by_session(session);
 	if (!setup)
-		return TRUE;
+		return;
 
 	finalize_setup_errno(setup, -ECONNRESET, finalize_suspend,
 							finalize_resume,
 							finalize_config);
 
-	return TRUE;
+	return;
 }
 
 static void abort_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
diff --git a/audio/avdtp.c b/audio/avdtp.c
index e9dea6c..3ba2366 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -1951,11 +1951,9 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
 	if (!sep || !sep->stream)
 		return TRUE;
 
-	if (sep->ind && sep->ind->abort) {
-		if (!sep->ind->abort(session, sep, sep->stream, &err,
-					sep->user_data))
-			goto failed;
-	}
+	if (sep->ind && sep->ind->abort)
+		sep->ind->abort(session, sep, sep->stream, &err,
+							sep->user_data);
 
 	avdtp_check_collision(session, AVDTP_ABORT, sep->stream);
 
@@ -1965,10 +1963,6 @@ static gboolean avdtp_abort_cmd(struct avdtp *session, uint8_t transaction,
 		avdtp_sep_set_state(session, sep, AVDTP_STATE_ABORTING);
 
 	return ret;
-
-failed:
-	return avdtp_send(session, transaction, AVDTP_MSG_TYPE_REJECT,
-					AVDTP_ABORT, &err, sizeof(err));
 }
 
 static gboolean avdtp_secctl_cmd(struct avdtp *session, uint8_t transaction,
diff --git a/audio/avdtp.h b/audio/avdtp.h
index 5f37dc3..dac093b 100644
--- a/audio/avdtp.h
+++ b/audio/avdtp.h
@@ -198,7 +198,7 @@ struct avdtp_sep_ind {
 	gboolean (*close) (struct avdtp *session, struct avdtp_local_sep *sep,
 				struct avdtp_stream *stream, uint8_t *err,
 				void *user_data);
-	gboolean (*abort) (struct avdtp *session, struct avdtp_local_sep *sep,
+	void (*abort) (struct avdtp *session, struct avdtp_local_sep *sep,
 				struct avdtp_stream *stream, uint8_t *err,
 				void *user_data);
 	gboolean (*reconfigure) (struct avdtp *session,
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 9/9 v3] audio: Fix aborting A2DP setup while AVDTP_START is in progress
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (6 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 8/9 v3] AVDTP: Fix responding to ABORT with reject Luiz Augusto von Dentz
@ 2012-06-15 14:41 ` Luiz Augusto von Dentz
  2012-06-15 14:52 ` [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Johan Hedberg
  8 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2012-06-15 14:41 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Change return of avdtp_start to -EINPROGRESS so the caller can check if
the operation is in progress and don't abort because of that.
---
 audio/a2dp.c  |    4 ++--
 audio/avdtp.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/a2dp.c b/audio/a2dp.c
index fafff87..404be53 100644
--- a/audio/a2dp.c
+++ b/audio/a2dp.c
@@ -1038,7 +1038,7 @@ static gboolean suspend_ind(struct avdtp *session, struct avdtp_local_sep *sep,
 		return TRUE;
 
 	start_err = avdtp_start(session, a2dp_sep->stream);
-	if (start_err < 0) {
+	if (start_err < 0 && start_err != -EINPROGRESS) {
 		error("avdtp_start: %s (%d)", strerror(-start_err),
 								-start_err);
 		finalize_setup_errno(setup, start_err, finalize_resume);
@@ -1086,7 +1086,7 @@ static void suspend_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	}
 
 	start_err = avdtp_start(session, a2dp_sep->stream);
-	if (start_err < 0) {
+	if (start_err < 0 && start_err != -EINPROGRESS) {
 		error("avdtp_start: %s (%d)", strerror(-start_err),
 								-start_err);
 		finalize_setup_errno(setup, start_err, finalize_suspend, NULL);
diff --git a/audio/avdtp.c b/audio/avdtp.c
index 3ba2366..eb56c7c 100644
--- a/audio/avdtp.c
+++ b/audio/avdtp.c
@@ -3675,7 +3675,7 @@ int avdtp_start(struct avdtp *session, struct avdtp_stream *stream)
 
 	if (stream->starting == TRUE) {
 		DBG("stream already started");
-		return -EINVAL;
+		return -EINPROGRESS;
 	}
 
 	memset(&req, 0, sizeof(req));
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision
  2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
                   ` (7 preceding siblings ...)
  2012-06-15 14:41 ` [PATCH BlueZ 9/9 v3] audio: Fix aborting A2DP setup while AVDTP_START is in progress Luiz Augusto von Dentz
@ 2012-06-15 14:52 ` Johan Hedberg
  8 siblings, 0 replies; 11+ messages in thread
From: Johan Hedberg @ 2012-06-15 14:52 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Fri, Jun 15, 2012, Luiz Augusto von Dentz wrote:
> Check collision for AVDTP OPEN, CLOSE, START, SUSPEND and ABORT
> commands and if they collided remove the pending request if sep
> has accepted the indication.
> ---
> v2: Fix capitalizing some acronyms
> v3: Rebase and fix not using avdtp_start return in case of error
> 
>  audio/avdtp.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication
  2012-06-15 14:41 ` [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication Luiz Augusto von Dentz
@ 2013-01-21 14:53   ` Chan-yeol Park
  0 siblings, 0 replies; 11+ messages in thread
From: Chan-yeol Park @ 2013-01-21 14:53 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, SYAM SIDHARDHAN

Hi Luiz.

Regarding your patch that was applied already, I think
finalize_setup_errno() function missed NULL argument.

This is reported by Syam Sidhardhan <s.syam@samsung.com>

On 06/15/2012 11:41 PM, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> When an abort is received all setup callbacks should return an error.
> ---
>   audio/a2dp.c |    9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/audio/a2dp.c b/audio/a2dp.c
> index 5139f61..d9dcead 100644
> --- a/audio/a2dp.c
> +++ b/audio/a2dp.c
> @@ -1182,6 +1182,7 @@ static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
>   				void *user_data)
>   {
>   	struct a2dp_sep *a2dp_sep = user_data;
> +	struct a2dp_setup *setup;
>   
>   	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
>   		DBG("Sink %p: Abort_Ind", sep);
> @@ -1190,6 +1191,14 @@ static gboolean abort_ind(struct avdtp *session, struct avdtp_local_sep *sep,
>   
>   	a2dp_sep->stream = NULL;
>   
> +	setup = find_setup_by_session(session);
> +	if (!setup)
> +		return TRUE;
> +
> +	finalize_setup_errno(setup, -ECONNRESET, finalize_suspend,
> +							finalize_resume,
> +							finalize_config);
> +
Here.

Could you give us your opinion?

Regards
Chanyeol


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-01-21 14:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-15 14:41 [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 2/9 v3] audio: Fix handling of A2DP suspend indication Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 3/9 v3] audio: Fix handling of A2DP open indication Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 4/9 v3] audio: Fix handling of A2DP start indication Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 5/9 v3] audio: Fix handling of A2DP abort indication Luiz Augusto von Dentz
2013-01-21 14:53   ` Chan-yeol Park
2012-06-15 14:41 ` [PATCH BlueZ 6/9 v3] audio: Wait remote side to send AVDTP_START when acting as acceptor Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 7/9 v3] AVDTP: Do not respond ABORT command with invalid id Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 8/9 v3] AVDTP: Fix responding to ABORT with reject Luiz Augusto von Dentz
2012-06-15 14:41 ` [PATCH BlueZ 9/9 v3] audio: Fix aborting A2DP setup while AVDTP_START is in progress Luiz Augusto von Dentz
2012-06-15 14:52 ` [PATCH BlueZ 1/9 v3] audio: Add handling of AVDTP command collision Johan Hedberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).