Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ] AVRCP: Fix not handling commands while browsing is connecting
@ 2013-01-27 21:03 Luiz Augusto von Dentz
  0 siblings, 0 replies; 2+ messages in thread
From: Luiz Augusto von Dentz @ 2013-01-27 21:03 UTC (permalink / raw)
  To: linux-bluetooth

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

With introdution of browsing channel the .init callback is called when
browsing channel connection completes, but in the meantime the remote
device can send commands over control channel.

To fix this a new callback .connect is introduced to register the PDU
handlers as soon as the control connection completes leaving .init with
only command initialization.
---
 profiles/audio/avrcp.c | 59 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4965b0c..8c92968 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -192,6 +192,7 @@ struct avrcp {
 	int features;
 	bool initialized;
 
+	void (*connect) (struct avrcp *session);
 	void (*init) (struct avrcp *session);
 	void (*destroy) (struct avrcp *sesion);
 
@@ -2165,6 +2166,28 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
+static void session_tg_connect(struct avrcp *session)
+{
+	if (session->control_id > 0)
+		return;
+
+	session->control_handlers = tg_control_handlers;
+	session->supported_events = (1 << AVRCP_EVENT_STATUS_CHANGED) |
+				(1 << AVRCP_EVENT_TRACK_CHANGED) |
+				(1 << AVRCP_EVENT_TRACK_REACHED_START) |
+				(1 << AVRCP_EVENT_TRACK_REACHED_END) |
+				(1 << AVRCP_EVENT_SETTINGS_CHANGED);
+
+	session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
+	session->browsing_id = avctp_register_browsing_pdu_handler(
+							session->conn,
+							handle_browsing_pdu,
+							session);
+}
+
 static void session_tg_init(struct avrcp *session)
 {
 	struct avrcp_server *server = session->server;
@@ -2180,25 +2203,25 @@ static void session_tg_init(struct avrcp *session)
 		player->sessions = g_slist_prepend(player->sessions, session);
 	}
 
-	session->control_handlers = tg_control_handlers;
-	session->supported_events = (1 << AVRCP_EVENT_STATUS_CHANGED) |
-				(1 << AVRCP_EVENT_TRACK_CHANGED) |
-				(1 << AVRCP_EVENT_TRACK_REACHED_START) |
-				(1 << AVRCP_EVENT_TRACK_REACHED_END) |
-				(1 << AVRCP_EVENT_SETTINGS_CHANGED);
-
 	if (session->version >= 0x0104)
 		avrcp_register_notification(session,
 						AVRCP_EVENT_VOLUME_CHANGED);
+}
+
+static void session_ct_connect(struct avrcp *session)
+{
+	if (session->control_id > 0)
+		return;
 
+	session->control_handlers = ct_control_handlers;
 	session->control_id = avctp_register_pdu_handler(session->conn,
 							AVC_OP_VENDORDEP,
 							handle_vendordep_pdu,
 							session);
-	session->browsing_id = avctp_register_browsing_pdu_handler(
-							session->conn,
-							handle_browsing_pdu,
-							session);
+	if (session->version < 0x0103)
+		return;
+
+	session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
 }
 
 static void session_ct_init(struct avrcp *session)
@@ -2207,20 +2230,10 @@ static void session_ct_init(struct avrcp *session)
 	struct media_player *mp;
 	const char *path;
 
-	session->control_handlers = ct_control_handlers;
-
-	if (session->version >= 0x0104)
-		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
-
 	DBG("%p version 0x%04x", session, session->version);
 
 	session->initialized = true;
 
-	session->control_id = avctp_register_pdu_handler(session->conn,
-							AVC_OP_VENDORDEP,
-							handle_vendordep_pdu,
-							session);
-
 	player = g_new0(struct avrcp_player, 1);
 	player->sessions = g_slist_prepend(player->sessions, session);
 	session->player = player;
@@ -2316,10 +2329,12 @@ static struct avrcp *session_create(struct avrcp_server *server,
 		session->target = FALSE;
 
 	if (session->target) {
+		session->connect = session_tg_connect;
 		session->init = session_tg_init;
 		session->destroy = session_tg_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_REMOTE_UUID);
 	} else {
+		session->connect = session_ct_connect;
 		session->init = session_ct_init;
 		session->destroy = session_ct_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
@@ -2371,6 +2386,8 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		if (session == NULL || session->initialized)
 			break;
 
+		session->connect(session);
+
 		/* Initialize session if browsing cannot be used */
 		if (session->version <= 0x0103 ||
 				old_state == AVCTP_STATE_BROWSING_CONNECTING ||
-- 
1.8.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
* [PATCH BlueZ] AVRCP: Fix not handling commands while browsing is connecting
@ 2013-01-27 22:37 Luiz Augusto von Dentz
  0 siblings, 0 replies; 2+ messages in thread
From: Luiz Augusto von Dentz @ 2013-01-27 22:37 UTC (permalink / raw)
  To: linux-bluetooth

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

With introdution of browsing channel the .init callback is called when
browsing channel connection completes, but in the meantime the remote
device can send commands over control channel.

To fix this the init callaback of control and browsing channel are now
separated into .init_control and .init_browsing so the handler can be
register as soon the respective channel connection completes.
---
v2: Separate control and browsing inits and use handler id to track if channel
has been initialized or not.

 profiles/audio/avrcp.c | 73 +++++++++++++++++++++++++++++---------------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4965b0c..6b61664 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -190,9 +190,9 @@ struct avrcp {
 	gboolean target;
 	uint16_t version;
 	int features;
-	bool initialized;
 
-	void (*init) (struct avrcp *session);
+	void (*init_control) (struct avrcp *session);
+	void (*init_browsing) (struct avrcp *session);
 	void (*destroy) (struct avrcp *sesion);
 
 	const struct control_pdu_handler *control_handlers;
@@ -2165,14 +2165,23 @@ static struct avrcp *find_session(GSList *list, struct audio_device *dev)
 	return NULL;
 }
 
-static void session_tg_init(struct avrcp *session)
+static void session_tg_init_browsing(struct avrcp *session)
+{
+	session->browsing_id = avctp_register_browsing_pdu_handler(
+							session->conn,
+							handle_browsing_pdu,
+							session);
+}
+
+static void session_tg_init_control(struct avrcp *session)
 {
 	struct avrcp_server *server = session->server;
 	struct avrcp_player *player;
 
-	DBG("%p version 0x%04x", session, session->version);
+	if (session->version < 0x0103)
+		return;
 
-	session->initialized = true;
+	DBG("%p version 0x%04x", session, session->version);
 
 	player = g_slist_nth_data(server->players, 0);
 	if (player != NULL) {
@@ -2180,6 +2189,10 @@ static void session_tg_init(struct avrcp *session)
 		player->sessions = g_slist_prepend(player->sessions, session);
 	}
 
+	session->control_id = avctp_register_pdu_handler(session->conn,
+							AVC_OP_VENDORDEP,
+							handle_vendordep_pdu,
+							session);
 	session->control_handlers = tg_control_handlers;
 	session->supported_events = (1 << AVRCP_EVENT_STATUS_CHANGED) |
 				(1 << AVRCP_EVENT_TRACK_CHANGED) |
@@ -2190,36 +2203,31 @@ static void session_tg_init(struct avrcp *session)
 	if (session->version >= 0x0104)
 		avrcp_register_notification(session,
 						AVRCP_EVENT_VOLUME_CHANGED);
+}
 
-	session->control_id = avctp_register_pdu_handler(session->conn,
-							AVC_OP_VENDORDEP,
-							handle_vendordep_pdu,
-							session);
+static void session_ct_init_browsing(struct avrcp *session)
+{
 	session->browsing_id = avctp_register_browsing_pdu_handler(
 							session->conn,
 							handle_browsing_pdu,
 							session);
 }
 
-static void session_ct_init(struct avrcp *session)
+static void session_ct_init_control(struct avrcp *session)
 {
 	struct avrcp_player *player;
 	struct media_player *mp;
 	const char *path;
 
-	session->control_handlers = ct_control_handlers;
-
-	if (session->version >= 0x0104)
-		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
-
 	DBG("%p version 0x%04x", session, session->version);
 
-	session->initialized = true;
-
 	session->control_id = avctp_register_pdu_handler(session->conn,
 							AVC_OP_VENDORDEP,
 							handle_vendordep_pdu,
 							session);
+	session->control_handlers = ct_control_handlers;
+	if (session->version >= 0x0104)
+		session->supported_events = (1 << AVRCP_EVENT_VOLUME_CHANGED);
 
 	player = g_new0(struct avrcp_player, 1);
 	player->sessions = g_slist_prepend(player->sessions, session);
@@ -2316,11 +2324,13 @@ static struct avrcp *session_create(struct avrcp_server *server,
 		session->target = FALSE;
 
 	if (session->target) {
-		session->init = session_tg_init;
+		session->init_control = session_tg_init_control;
+		session->init_browsing = session_tg_init_browsing;
 		session->destroy = session_tg_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_REMOTE_UUID);
 	} else {
-		session->init = session_ct_init;
+		session->init_control = session_ct_init_control;
+		session->init_browsing = session_ct_init_browsing;
 		session->destroy = session_ct_destroy;
 		rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
 	}
@@ -2368,26 +2378,27 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 
 		break;
 	case AVCTP_STATE_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL)
 			break;
 
-		/* Initialize session if browsing cannot be used */
-		if (session->version <= 0x0103 ||
-				old_state == AVCTP_STATE_BROWSING_CONNECTING ||
-				!(session->features & AVRCP_FEATURE_BROWSING)) {
-			session->init(session);
-			break;
-		}
+		if (session->browsing_id > 0)
+			session->browsing_id = 0;
+
+		if (session->control_id > 0)
+			return;
+
+		session->init_control(session);
 
-		if (avctp_connect_browsing(session->conn) != 0)
-			session->init(session);
+		if (session->version >= 0x0104 &&
+				session->features & AVRCP_FEATURE_BROWSING)
+			avctp_connect_browsing(session->conn);
 
 		break;
 	case AVCTP_STATE_BROWSING_CONNECTED:
-		if (session == NULL || session->initialized)
+		if (session == NULL || session->browsing_id > 0)
 			break;
 
-		session->init(session);
+		session->init_browsing(session);
 		break;
 	default:
 		return;
-- 
1.8.1


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

end of thread, other threads:[~2013-01-27 22:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-27 21:03 [PATCH BlueZ] AVRCP: Fix not handling commands while browsing is connecting Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2013-01-27 22:37 Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox