[PATCH v2 0/5] Request/Abort Continuing and some minor things

[PATCH v2 0/5] Request/Abort Continuing and some minor things

[Lucas De Marchi]

Changes from previous version:

 - 2/5: fix minor coding style issue
 - 3/5: allow 0 to be a valid duration value
 - 5/5: fill attribute header in player_fill_media_attribute() in order to
   simplify implementation


Lucas De Marchi (5):
  AVRCP: Use track's UID in event notification
  AVRCP: implement TRACK-REACHED-START event
  AVRCP: respond with UINT32_MAX if duration is not available
  AVRCP: Implement AbortContinuingResponse PDU
  AVRCP: Implement RequestContinuingResponse PDU

 audio/avrcp.c |  307 +++++++++++++++++++++++++++++++++++++++++----------------
 audio/avrcp.h |    3 +
 audio/media.c |   22 ++++-
 3 files changed, 246 insertions(+), 86 deletions(-)

-- 
1.7.7

[PATCH v2 1/5] AVRCP: Use track's UID in event notification

[Lucas De Marchi]

When registering for TRACK_CHANGED event, CT expects the track's UID in
the INTERIM and CHANGED responses. According to AVRCP 1.3 spec, there
are only 2 possible values:
	1) 0: there's a track selected
	2) UINT32_MAX: there's no track selected

AVRCP 1.4 reserves the value UINT64_MAX for the second case. Since this
later value is the one used in certification process for best IOP
we return UINT64_MAX instead of the former.

This implementation allows to pass PTS test TP/NFY/BV-05-C.
---
 audio/avrcp.c |   16 +++++-----------
 audio/avrcp.h |    1 +
 audio/media.c |   17 ++++++++++++++++-
 3 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index ac7c8d4..076fdf4 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -363,18 +363,11 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 		pdu->params[1] = *((uint8_t *)data);
 
 		break;
-	case AVRCP_EVENT_TRACK_CHANGED: {
+	case AVRCP_EVENT_TRACK_CHANGED:
 		size = 9;
-
-		/*
-		 * AVRCP 1.3 supports only one track identifier: PLAYING
-		 * (0x0). When 1.4 version is added, this shall be changed to
-		 * contain the identifier of the track.
-		 */
-		memset(&pdu->params[1], 0, 8);
+		memcpy(&pdu->params[1], data, sizeof(uint64_t));
 
 		break;
-	}
 	default:
 		error("Unknown event %u", id);
 		return -EINVAL;
@@ -828,6 +821,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
 						uint8_t transaction)
 {
 	uint16_t len = ntohs(pdu->params_len);
+	uint64_t uid;
 
 	/*
 	 * 1 byte for EventID, 4 bytes for Playback interval but the latest
@@ -845,8 +839,8 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
 		break;
 	case AVRCP_EVENT_TRACK_CHANGED:
 		len = 9;
-
-		memset(&pdu->params[1], 0, 8);
+		uid = player->cb->get_uid(player->user_data);
+		memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
 
 		break;
 	default:
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 360a80a..8cf95a4 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -75,6 +75,7 @@
 struct avrcp_player_cb {
 	int (*get_setting) (uint8_t attr, void *user_data);
 	int (*set_setting) (uint8_t attr, uint8_t value, void *user_data);
+	uint64_t (*get_uid) (void *user_data);
 	void *(*get_metadata) (uint32_t id, void *user_data);
 	GList *(*list_metadata) (void *user_data);
 	uint8_t (*get_status) (void *user_data);
diff --git a/audio/media.c b/audio/media.c
index 519cafe..9ef393b 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1168,6 +1168,18 @@ static GList *list_metadata(void *user_data)
 	return g_hash_table_get_keys(mp->track);
 }
 
+static uint64_t get_uid(void *user_data)
+{
+	struct media_player *mp = user_data;
+
+	DBG("%p", mp->track);
+
+	if (mp->track == NULL)
+		return UINT64_MAX;
+
+	return 0;
+}
+
 static void *get_metadata(uint32_t id, void *user_data)
 {
 	struct media_player *mp = user_data;
@@ -1220,6 +1232,7 @@ static struct avrcp_player_cb player_cb = {
 	.get_setting = get_setting,
 	.set_setting = set_setting,
 	.list_metadata = list_metadata,
+	.get_uid = get_uid,
 	.get_metadata = get_metadata,
 	.get_position = get_position,
 	.get_status = get_status
@@ -1369,6 +1382,7 @@ static gboolean parse_player_metadata(struct media_player *mp,
 	GHashTable *track;
 	int ctype;
 	gboolean title = FALSE;
+	uint64_t uid;
 
 	ctype = dbus_message_iter_get_arg_type(iter);
 	if (ctype != DBUS_TYPE_ARRAY)
@@ -1466,8 +1480,9 @@ static gboolean parse_player_metadata(struct media_player *mp,
 	mp->track = track;
 	mp->position = 0;
 	g_timer_start(mp->timer);
+	uid = get_uid(mp);
 
-	avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, NULL);
+	avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, &uid);
 
 	return TRUE;
 
-- 
1.7.7

[PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event

[Lucas De Marchi]

---
 audio/avrcp.c |   13 ++++++++++---
 audio/avrcp.h |    2 ++
 audio/media.c |    5 +++++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 076fdf4..b1c3d54 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -136,7 +136,7 @@ struct avrcp_player {
 
 	unsigned int handler;
 	uint16_t registered_events;
-	uint8_t transaction_events[AVRCP_EVENT_TRACK_CHANGED + 1];
+	uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
 
 	struct avrcp_player_cb *cb;
 	void *user_data;
@@ -368,6 +368,9 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 		memcpy(&pdu->params[1], data, sizeof(uint64_t));
 
 		break;
+	case AVRCP_EVENT_TRACK_REACHED_START:
+		size = 1;
+		break;
 	default:
 		error("Unknown event %u", id);
 		return -EINVAL;
@@ -504,10 +507,11 @@ static uint8_t avrcp_handle_get_capabilities(struct avrcp_player *player,
 
 		return AVC_CTYPE_STABLE;
 	case CAP_EVENTS_SUPPORTED:
-		pdu->params_len = htons(4);
-		pdu->params[1] = 2;
+		pdu->params_len = htons(5);
+		pdu->params[1] = 3;
 		pdu->params[2] = AVRCP_EVENT_STATUS_CHANGED;
 		pdu->params[3] = AVRCP_EVENT_TRACK_CHANGED;
+		pdu->params[4] = AVRCP_EVENT_TRACK_REACHED_START;
 
 		return AVC_CTYPE_STABLE;
 	}
@@ -843,6 +847,9 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
 		memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
 
 		break;
+	case AVRCP_EVENT_TRACK_REACHED_START:
+		len = 1;
+		break;
 	default:
 		/* All other events are not supported yet */
 		goto err;
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 8cf95a4..c798658 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -71,6 +71,8 @@
 /* Notification events */
 #define AVRCP_EVENT_STATUS_CHANGED	0x01
 #define AVRCP_EVENT_TRACK_CHANGED	0x02
+#define AVRCP_EVENT_TRACK_REACHED_START	0x04
+#define AVRCP_EVENT_LAST		AVRCP_EVENT_TRACK_REACHED_START
 
 struct avrcp_player_cb {
 	int (*get_setting) (uint8_t attr, void *user_data);
diff --git a/audio/media.c b/audio/media.c
index 9ef393b..a7647ea 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1289,6 +1289,11 @@ static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
 	mp->position = value;
 	g_timer_start(mp->timer);
 
+	if (!mp->position) {
+		avrcp_player_event(mp->player,
+					AVRCP_EVENT_TRACK_REACHED_START, NULL);
+	}
+
 	return TRUE;
 }
 
-- 
1.7.7

[PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available

[Lucas De Marchi]

Section 5.4.1 of AVRCP 1.3 spec says:

	If TG does not support SongLength And SongPosition on TG, then TG shall
	return 0xFFFFFFFF.

SongPosition is always available, but song length depends on user to
provied it.
---
 audio/avrcp.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index b1c3d54..5f8277c 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -796,6 +796,7 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
 	uint16_t len = ntohs(pdu->params_len);
 	uint32_t position;
 	uint32_t duration;
+	void *pduration;
 
 	if (len != 0) {
 		pdu->params_len = htons(1);
@@ -804,11 +805,13 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
 	}
 
 	position = player->cb->get_position(player->user_data);
-	duration = GPOINTER_TO_UINT(player->cb->get_metadata(
-						AVRCP_MEDIA_ATTRIBUTE_DURATION,
-						player->user_data));
+	pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
+							player->user_data);
+	if (pduration != NULL)
+		duration = htonl(GPOINTER_TO_UINT(pduration));
+	else
+		duration = htonl(UINT32_MAX);
 
-	duration = htonl(duration);
 	position = htonl(position);
 
 	memcpy(&pdu->params[0], &duration, 4);
-- 
1.7.7

[PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU

[Lucas De Marchi]

---
 audio/avrcp.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 5f8277c..585b095 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -77,6 +77,7 @@
 #define AVRCP_GET_ELEMENT_ATTRIBUTES	0x20
 #define AVRCP_GET_PLAY_STATUS		0x30
 #define AVRCP_REGISTER_NOTIFICATION	0x31
+#define AVRCP_ABORT_CONTINUING		0x41
 
 /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
 #define CAP_COMPANY_ID		0x02
@@ -129,6 +130,12 @@ struct avrcp_server {
 	struct avrcp_player *active_player;
 };
 
+struct pending_pdu {
+	uint8_t pdu_id;
+	GList *attr_ids;
+	uint16_t offset;
+};
+
 struct avrcp_player {
 	struct avrcp_server *server;
 	struct avctp *session;
@@ -137,6 +144,7 @@ struct avrcp_player {
 	unsigned int handler;
 	uint16_t registered_events;
 	uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
+	struct pending_pdu *pending_pdu;
 
 	struct avrcp_player_cb *cb;
 	void *user_data;
@@ -462,6 +470,18 @@ done:
 	return sizeof(struct media_info_elem) + len;
 }
 
+static gboolean player_abort_pending_pdu(struct avrcp_player *player)
+{
+	if (player->pending_pdu == NULL)
+		return FALSE;
+
+	g_list_free(player->pending_pdu->attr_ids);
+	g_free(player->pending_pdu);
+	player->pending_pdu = NULL;
+
+	return TRUE;
+}
+
 static int player_set_attribute(struct avrcp_player *player,
 						uint8_t attr, uint8_t val)
 {
@@ -872,6 +892,33 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
+						struct avrcp_header *pdu,
+						uint8_t transaction)
+{
+	uint16_t len = ntohs(pdu->params_len);
+	struct pending_pdu *pending;
+
+	if (len != 1 || player->pending_pdu == NULL)
+		goto err;
+
+	pending = player->pending_pdu;
+
+	if (pending->pdu_id != pdu->params[0])
+		goto err;
+
+	player_abort_pending_pdu(player);
+	pdu->params_len = 0;
+
+	return AVC_CTYPE_STABLE;
+
+err:
+	pdu->params_len = htons(1);
+	pdu->params[0] = E_INVALID_PARAM;
+	return AVC_CTYPE_REJECTED;
+}
+
+
 static struct pdu_handler {
 	uint8_t pdu_id;
 	uint8_t code;
@@ -903,6 +950,8 @@ static struct pdu_handler {
 					avrcp_handle_get_play_status },
 		{ AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
 					avrcp_handle_register_notification },
+		{ AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
+					avrcp_handle_abort_continuing },
 		{ },
 };
 
@@ -950,6 +999,10 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
 
 	*code = handler->func(player, pdu, transaction);
 
+	if (*code != AVC_CTYPE_REJECTED &&
+				pdu->pdu_id != AVRCP_ABORT_CONTINUING)
+		player_abort_pending_pdu(player);
+
 	return AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
 
 err_metadata:
@@ -1103,6 +1156,8 @@ static void player_destroy(gpointer data)
 	if (player->destroy)
 		player->destroy(player->user_data);
 
+	player_abort_pending_pdu(player);
+
 	if (player->handler)
 		avctp_unregister_pdu_handler(player->handler);
 
-- 
1.7.7

[PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU

[Lucas De Marchi]

---
 audio/avrcp.c |  212 +++++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 145 insertions(+), 67 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 585b095..75a8384 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -64,6 +64,12 @@
 #define E_PARAM_NOT_FOUND	0x02
 #define E_INTERNAL		0x03
 
+/* Packet types */
+#define AVRCP_PACKET_TYPE_SINGLE	0x00
+#define AVRCP_PACKET_TYPE_START		0x01
+#define AVRCP_PACKET_TYPE_CONTINUING	0x02
+#define AVRCP_PACKET_TYPE_END		0x03
+
 /* PDU types for metadata transfer */
 #define AVRCP_GET_CAPABILITIES		0x10
 #define AVRCP_LIST_PLAYER_ATTRIBUTES	0X11
@@ -77,6 +83,7 @@
 #define AVRCP_GET_ELEMENT_ATTRIBUTES	0x20
 #define AVRCP_GET_PLAY_STATUS		0x30
 #define AVRCP_REGISTER_NOTIFICATION	0x31
+#define AVRCP_REQUEST_CONTINUING	0x40
 #define AVRCP_ABORT_CONTINUING		0x41
 
 /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
@@ -398,76 +405,98 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
 	return 0;
 }
 
-/*
- * Copy media_info field to a buffer, intended to be used in a response to
- * GetElementAttributes message.
- *
- * It assumes there's enough space in the buffer and on success it returns the
- * size written.
- *
- * If @param id is not valid, -EINVAL is returned. If there's no space left on
- * the buffer -ENOBUFS is returned.
- */
-static int player_get_media_attribute(struct avrcp_player *player,
+static uint16_t player_write_media_attribute(struct avrcp_player *player,
 						uint32_t id, uint8_t *buf,
-						uint16_t maxlen)
+						uint16_t *pos,
+						uint16_t *offset)
 {
-	struct media_info_elem {
-		uint32_t id;
-		uint16_t charset;
-		uint16_t len;
-		uint8_t val[];
-	};
-	struct media_info_elem *elem = (void *)buf;
 	uint16_t len;
+	uint16_t attr_len;
 	char valstr[20];
 	void *value;
 
-	if (maxlen < sizeof(struct media_info_elem))
-		return -ENOBUFS;
-
-	/* Subtract the size of elem header from the available space */
-	maxlen -= sizeof(struct media_info_elem);
-
-	DBG("Get media attribute: %u", id);
-
-	if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
-			id > AVRCP_MEDIA_ATTRIBUTE_LAST)
-		return -ENOENT;
+	DBG("%u", id);
 
 	value = player->cb->get_metadata(id, player->user_data);
 	if (value == NULL) {
-		len = 0;
-		goto done;
+		*offset = 0;
+		return 0;
 	}
 
 	switch (id) {
-	case AVRCP_MEDIA_ATTRIBUTE_TITLE:
-	case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
-	case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
-	case AVRCP_MEDIA_ATTRIBUTE_GENRE:
-		len = strlen((char *) value);
-		if (len > maxlen)
-			return -ENOBUFS;
-		memcpy(elem->val, value, len);
-		break;
 	case AVRCP_MEDIA_ATTRIBUTE_TRACK:
 	case AVRCP_MEDIA_ATTRIBUTE_N_TRACKS:
 	case AVRCP_MEDIA_ATTRIBUTE_DURATION:
 		snprintf(valstr, 20, "%u", GPOINTER_TO_UINT(value));
-		len = strlen(valstr);
-		if (len > maxlen)
-			return -ENOBUFS;
-		memcpy(elem->val, valstr, len);
+		value = valstr;
 		break;
 	}
 
-done:
-	elem->id = htonl(id);
-	elem->charset = htons(0x6A); /* Always use UTF-8 */
-	elem->len = htons(len);
+	attr_len = strlen(value);
+	value = ((char *) value) + *offset;
+	len = attr_len - *offset;
 
-	return sizeof(struct media_info_elem) + len;
+	if (len > AVRCP_PDU_MTU - *pos) {
+		len = AVRCP_PDU_MTU - *pos;
+		*offset += len;
+	} else {
+		*offset = 0;
+	}
+
+	memcpy(&buf[*pos], value, len);
+	*pos += len;
+
+	return attr_len;
+}
+
+static GList *player_fill_media_attribute(struct avrcp_player *player,
+					GList *attr_ids, uint8_t *buf,
+					uint16_t *pos, uint16_t *offset)
+{
+	struct media_attribute_header {
+		uint32_t id;
+		uint16_t charset;
+		uint16_t len;
+	} *hdr = NULL;
+	GList *l;
+
+	for (l = attr_ids; l != NULL; l = g_list_delete_link(l, l)) {
+		uint32_t attr = GPOINTER_TO_UINT(l->data);
+		uint16_t attr_len;
+
+		if (*offset == 0) {
+			if (*pos + sizeof(*hdr) >= AVRCP_PDU_MTU)
+				break;
+
+			hdr = (void *) &buf[*pos];
+			hdr->id = htonl(attr);
+			hdr->charset = htons(0x6A); /* Always use UTF-8 */
+			*pos += sizeof(*hdr);
+		}
+
+		attr_len = player_write_media_attribute(player, attr, buf,
+								pos, offset);
+
+		if (hdr != NULL)
+			hdr->len = htons(attr_len);
+
+		if (*offset > 0)
+			break;
+	}
+
+	return l;
+}
+
+static struct pending_pdu *pending_pdu_new(uint8_t pdu_id, GList *attr_ids,
+							unsigned int offset)
+{
+	struct pending_pdu *pending = g_new(struct pending_pdu, 1);
+
+	pending->pdu_id = pdu_id;
+	pending->attr_ids = attr_ids;
+	pending->offset = offset;
+
+	return pending;
 }
 
 static gboolean player_abort_pending_pdu(struct avrcp_player *player)
@@ -611,8 +640,8 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
 	uint64_t *identifier = (uint64_t *) &pdu->params[0];
 	uint16_t pos;
 	uint8_t nattr;
-	int size;
-	GList *l, *attr_ids;
+	GList *attr_ids;
+	uint16_t offset;
 
 	if (len < 9 || *identifier != 0)
 		goto err;
@@ -628,12 +657,20 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
 		 * title must be returned if there's a track selected.
 		 */
 		attr_ids = player->cb->list_metadata(player->user_data);
+		len = g_list_length(attr_ids);
 	} else {
 		unsigned int i;
 		uint32_t *attr = (uint32_t *) &pdu->params[9];
 
-		for (i = 0, attr_ids = NULL; i < nattr; i++, attr++) {
+		for (i = 0, len = 0, attr_ids = NULL; i < nattr; i++, attr++) {
 			uint32_t id = ntohl(bt_get_unaligned(attr));
+
+			/* Don't add invalid attributes */
+			if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
+					id > AVRCP_MEDIA_ATTRIBUTE_LAST)
+				continue;
+
+			len++;
 			attr_ids = g_list_prepend(attr_ids,
 							GUINT_TO_POINTER(id));
 		}
@@ -641,24 +678,21 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
 		attr_ids = g_list_reverse(attr_ids);
 	}
 
-	for (l = attr_ids, len = 0, pos = 1; l != NULL; l = l->next) {
-		uint32_t attr = GPOINTER_TO_UINT(l->data);
+	if (!len)
+		goto err;
 
-		size = player_get_media_attribute(player, attr,
-							&pdu->params[pos],
-							AVRCP_PDU_MTU - pos);
+	player_abort_pending_pdu(player);
+	pos = 1;
+	offset = 0;
+	attr_ids = player_fill_media_attribute(player, attr_ids, pdu->params,
+								&pos, &offset);
 
-		if (size >= 0) {
-			len++;
-			pos += size;
-		}
+	if (attr_ids != NULL) {
+		player->pending_pdu = pending_pdu_new(pdu->pdu_id, attr_ids,
+								offset);
+		pdu->packet_type = AVRCP_PACKET_TYPE_START;
 	}
 
-	g_list_free(attr_ids);
-
-	if (!len)
-		goto err;
-
 	pdu->params[0] = len;
 	pdu->params_len = htons(pos);
 
@@ -892,6 +926,46 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static uint8_t avrcp_handle_request_continuing(struct avrcp_player *player,
+						struct avrcp_header *pdu,
+						uint8_t transaction)
+{
+	uint16_t len = ntohs(pdu->params_len);
+	struct pending_pdu *pending;
+
+	if (len != 1 || player->pending_pdu == NULL)
+		goto err;
+
+	pending = player->pending_pdu;
+
+	if (pending->pdu_id != pdu->params[0])
+		goto err;
+
+
+	len = 0;
+	pending->attr_ids = player_fill_media_attribute(player,
+							pending->attr_ids,
+							pdu->params, &len,
+							&pending->offset);
+	pdu->pdu_id = pending->pdu_id;
+
+	if (pending->attr_ids == NULL) {
+		g_free(player->pending_pdu);
+		player->pending_pdu = NULL;
+		pdu->packet_type = AVRCP_PACKET_TYPE_END;
+	} else {
+		pdu->packet_type = AVRCP_PACKET_TYPE_CONTINUING;
+	}
+
+	pdu->params_len = htons(len);
+
+	return AVC_CTYPE_STABLE;
+err:
+	pdu->params_len = htons(1);
+	pdu->params[0] = E_INVALID_PARAM;
+	return AVC_CTYPE_REJECTED;
+}
+
 static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
 						struct avrcp_header *pdu,
 						uint8_t transaction)
@@ -950,6 +1024,8 @@ static struct pdu_handler {
 					avrcp_handle_get_play_status },
 		{ AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
 					avrcp_handle_register_notification },
+		{ AVRCP_REQUEST_CONTINUING, AVC_CTYPE_CONTROL,
+					avrcp_handle_request_continuing },
 		{ AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
 					avrcp_handle_abort_continuing },
 		{ },
@@ -1000,6 +1076,8 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
 	*code = handler->func(player, pdu, transaction);
 
 	if (*code != AVC_CTYPE_REJECTED &&
+				pdu->pdu_id != AVRCP_GET_ELEMENT_ATTRIBUTES &&
+				pdu->pdu_id != AVRCP_REQUEST_CONTINUING &&
 				pdu->pdu_id != AVRCP_ABORT_CONTINUING)
 		player_abort_pending_pdu(player);
 
-- 
1.7.7

Re: [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU

[Luiz Augusto von Dentz]

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> ---
>  audio/avrcp.c |  212 +++++++++++++++++++++++++++++++++++++++------------------
>  1 files changed, 145 insertions(+), 67 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 585b095..75a8384 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -64,6 +64,12 @@
>  #define E_PARAM_NOT_FOUND      0x02
>  #define E_INTERNAL             0x03
>
> +/* Packet types */
> +#define AVRCP_PACKET_TYPE_SINGLE       0x00
> +#define AVRCP_PACKET_TYPE_START                0x01
> +#define AVRCP_PACKET_TYPE_CONTINUING   0x02
> +#define AVRCP_PACKET_TYPE_END          0x03
> +
>  /* PDU types for metadata transfer */
>  #define AVRCP_GET_CAPABILITIES         0x10
>  #define AVRCP_LIST_PLAYER_ATTRIBUTES   0X11
> @@ -77,6 +83,7 @@
>  #define AVRCP_GET_ELEMENT_ATTRIBUTES   0x20
>  #define AVRCP_GET_PLAY_STATUS          0x30
>  #define AVRCP_REGISTER_NOTIFICATION    0x31
> +#define AVRCP_REQUEST_CONTINUING       0x40
>  #define AVRCP_ABORT_CONTINUING         0x41
>
>  /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
> @@ -398,76 +405,98 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
>        return 0;
>  }
>
> -/*
> - * Copy media_info field to a buffer, intended to be used in a response to
> - * GetElementAttributes message.
> - *
> - * It assumes there's enough space in the buffer and on success it returns the
> - * size written.
> - *
> - * If @param id is not valid, -EINVAL is returned. If there's no space left on
> - * the buffer -ENOBUFS is returned.
> - */
> -static int player_get_media_attribute(struct avrcp_player *player,
> +static uint16_t player_write_media_attribute(struct avrcp_player *player,
>                                                uint32_t id, uint8_t *buf,
> -                                               uint16_t maxlen)
> +                                               uint16_t *pos,
> +                                               uint16_t *offset)
>  {
> -       struct media_info_elem {
> -               uint32_t id;
> -               uint16_t charset;
> -               uint16_t len;
> -               uint8_t val[];
> -       };
> -       struct media_info_elem *elem = (void *)buf;
>        uint16_t len;
> +       uint16_t attr_len;
>        char valstr[20];
>        void *value;
>
> -       if (maxlen < sizeof(struct media_info_elem))
> -               return -ENOBUFS;
> -
> -       /* Subtract the size of elem header from the available space */
> -       maxlen -= sizeof(struct media_info_elem);
> -
> -       DBG("Get media attribute: %u", id);
> -
> -       if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
> -                       id > AVRCP_MEDIA_ATTRIBUTE_LAST)
> -               return -ENOENT;
> +       DBG("%u", id);
>
>        value = player->cb->get_metadata(id, player->user_data);
>        if (value == NULL) {
> -               len = 0;
> -               goto done;
> +               *offset = 0;
> +               return 0;
>        }
>
>        switch (id) {
> -       case AVRCP_MEDIA_ATTRIBUTE_TITLE:
> -       case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
> -       case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
> -       case AVRCP_MEDIA_ATTRIBUTE_GENRE:
> -               len = strlen((char *) value);
> -               if (len > maxlen)
> -                       return -ENOBUFS;
> -               memcpy(elem->val, value, len);
> -               break;
>        case AVRCP_MEDIA_ATTRIBUTE_TRACK:
>        case AVRCP_MEDIA_ATTRIBUTE_N_TRACKS:
>        case AVRCP_MEDIA_ATTRIBUTE_DURATION:
>                snprintf(valstr, 20, "%u", GPOINTER_TO_UINT(value));
> -               len = strlen(valstr);
> -               if (len > maxlen)
> -                       return -ENOBUFS;
> -               memcpy(elem->val, valstr, len);
> +               value = valstr;
>                break;
>        }
>
> -done:
> -       elem->id = htonl(id);
> -       elem->charset = htons(0x6A); /* Always use UTF-8 */
> -       elem->len = htons(len);
> +       attr_len = strlen(value);
> +       value = ((char *) value) + *offset;
> +       len = attr_len - *offset;
>
> -       return sizeof(struct media_info_elem) + len;
> +       if (len > AVRCP_PDU_MTU - *pos) {
> +               len = AVRCP_PDU_MTU - *pos;
> +               *offset += len;
> +       } else {
> +               *offset = 0;
> +       }
> +
> +       memcpy(&buf[*pos], value, len);
> +       *pos += len;
> +
> +       return attr_len;
> +}
> +
> +static GList *player_fill_media_attribute(struct avrcp_player *player,
> +                                       GList *attr_ids, uint8_t *buf,
> +                                       uint16_t *pos, uint16_t *offset)
> +{
> +       struct media_attribute_header {
> +               uint32_t id;
> +               uint16_t charset;
> +               uint16_t len;
> +       } *hdr = NULL;
> +       GList *l;
> +
> +       for (l = attr_ids; l != NULL; l = g_list_delete_link(l, l)) {
> +               uint32_t attr = GPOINTER_TO_UINT(l->data);
> +               uint16_t attr_len;
> +
> +               if (*offset == 0) {
> +                       if (*pos + sizeof(*hdr) >= AVRCP_PDU_MTU)
> +                               break;
> +
> +                       hdr = (void *) &buf[*pos];
> +                       hdr->id = htonl(attr);
> +                       hdr->charset = htons(0x6A); /* Always use UTF-8 */
> +                       *pos += sizeof(*hdr);
> +               }
> +
> +               attr_len = player_write_media_attribute(player, attr, buf,
> +                                                               pos, offset);
> +
> +               if (hdr != NULL)
> +                       hdr->len = htons(attr_len);
> +
> +               if (*offset > 0)
> +                       break;
> +       }
> +
> +       return l;
> +}
> +
> +static struct pending_pdu *pending_pdu_new(uint8_t pdu_id, GList *attr_ids,
> +                                                       unsigned int offset)
> +{
> +       struct pending_pdu *pending = g_new(struct pending_pdu, 1);
> +
> +       pending->pdu_id = pdu_id;
> +       pending->attr_ids = attr_ids;
> +       pending->offset = offset;
> +
> +       return pending;
>  }
>
>  static gboolean player_abort_pending_pdu(struct avrcp_player *player)
> @@ -611,8 +640,8 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
>        uint64_t *identifier = (uint64_t *) &pdu->params[0];
>        uint16_t pos;
>        uint8_t nattr;
> -       int size;
> -       GList *l, *attr_ids;
> +       GList *attr_ids;
> +       uint16_t offset;
>
>        if (len < 9 || *identifier != 0)
>                goto err;
> @@ -628,12 +657,20 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
>                 * title must be returned if there's a track selected.
>                 */
>                attr_ids = player->cb->list_metadata(player->user_data);
> +               len = g_list_length(attr_ids);
>        } else {
>                unsigned int i;
>                uint32_t *attr = (uint32_t *) &pdu->params[9];
>
> -               for (i = 0, attr_ids = NULL; i < nattr; i++, attr++) {
> +               for (i = 0, len = 0, attr_ids = NULL; i < nattr; i++, attr++) {
>                        uint32_t id = ntohl(bt_get_unaligned(attr));
> +
> +                       /* Don't add invalid attributes */
> +                       if (id == AVRCP_MEDIA_ATTRIBUTE_ILLEGAL ||
> +                                       id > AVRCP_MEDIA_ATTRIBUTE_LAST)
> +                               continue;
> +
> +                       len++;
>                        attr_ids = g_list_prepend(attr_ids,
>                                                        GUINT_TO_POINTER(id));
>                }
> @@ -641,24 +678,21 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp_player *player,
>                attr_ids = g_list_reverse(attr_ids);
>        }
>
> -       for (l = attr_ids, len = 0, pos = 1; l != NULL; l = l->next) {
> -               uint32_t attr = GPOINTER_TO_UINT(l->data);
> +       if (!len)
> +               goto err;
>
> -               size = player_get_media_attribute(player, attr,
> -                                                       &pdu->params[pos],
> -                                                       AVRCP_PDU_MTU - pos);
> +       player_abort_pending_pdu(player);
> +       pos = 1;
> +       offset = 0;
> +       attr_ids = player_fill_media_attribute(player, attr_ids, pdu->params,
> +                                                               &pos, &offset);
>
> -               if (size >= 0) {
> -                       len++;
> -                       pos += size;
> -               }
> +       if (attr_ids != NULL) {
> +               player->pending_pdu = pending_pdu_new(pdu->pdu_id, attr_ids,
> +                                                               offset);
> +               pdu->packet_type = AVRCP_PACKET_TYPE_START;
>        }
>
> -       g_list_free(attr_ids);
> -
> -       if (!len)
> -               goto err;
> -
>        pdu->params[0] = len;
>        pdu->params_len = htons(pos);
>
> @@ -892,6 +926,46 @@ err:
>        return AVC_CTYPE_REJECTED;
>  }
>
> +static uint8_t avrcp_handle_request_continuing(struct avrcp_player *player,
> +                                               struct avrcp_header *pdu,
> +                                               uint8_t transaction)
> +{
> +       uint16_t len = ntohs(pdu->params_len);
> +       struct pending_pdu *pending;
> +
> +       if (len != 1 || player->pending_pdu == NULL)
> +               goto err;
> +
> +       pending = player->pending_pdu;
> +
> +       if (pending->pdu_id != pdu->params[0])
> +               goto err;
> +
> +
> +       len = 0;
> +       pending->attr_ids = player_fill_media_attribute(player,
> +                                                       pending->attr_ids,
> +                                                       pdu->params, &len,
> +                                                       &pending->offset);
> +       pdu->pdu_id = pending->pdu_id;
> +
> +       if (pending->attr_ids == NULL) {
> +               g_free(player->pending_pdu);
> +               player->pending_pdu = NULL;
> +               pdu->packet_type = AVRCP_PACKET_TYPE_END;
> +       } else {
> +               pdu->packet_type = AVRCP_PACKET_TYPE_CONTINUING;
> +       }
> +
> +       pdu->params_len = htons(len);
> +
> +       return AVC_CTYPE_STABLE;
> +err:
> +       pdu->params_len = htons(1);
> +       pdu->params[0] = E_INVALID_PARAM;
> +       return AVC_CTYPE_REJECTED;
> +}
> +
>  static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
>                                                struct avrcp_header *pdu,
>                                                uint8_t transaction)
> @@ -950,6 +1024,8 @@ static struct pdu_handler {
>                                        avrcp_handle_get_play_status },
>                { AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
>                                        avrcp_handle_register_notification },
> +               { AVRCP_REQUEST_CONTINUING, AVC_CTYPE_CONTROL,
> +                                       avrcp_handle_request_continuing },
>                { AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
>                                        avrcp_handle_abort_continuing },
>                { },
> @@ -1000,6 +1076,8 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
>        *code = handler->func(player, pdu, transaction);
>
>        if (*code != AVC_CTYPE_REJECTED &&
> +                               pdu->pdu_id != AVRCP_GET_ELEMENT_ATTRIBUTES &&
> +                               pdu->pdu_id != AVRCP_REQUEST_CONTINUING &&
>                                pdu->pdu_id != AVRCP_ABORT_CONTINUING)
>                player_abort_pending_pdu(player);
>
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU

[Luiz Augusto von Dentz]

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> ---
>  audio/avrcp.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 55 insertions(+), 0 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 5f8277c..585b095 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -77,6 +77,7 @@
>  #define AVRCP_GET_ELEMENT_ATTRIBUTES   0x20
>  #define AVRCP_GET_PLAY_STATUS          0x30
>  #define AVRCP_REGISTER_NOTIFICATION    0x31
> +#define AVRCP_ABORT_CONTINUING         0x41
>
>  /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
>  #define CAP_COMPANY_ID         0x02
> @@ -129,6 +130,12 @@ struct avrcp_server {
>        struct avrcp_player *active_player;
>  };
>
> +struct pending_pdu {
> +       uint8_t pdu_id;
> +       GList *attr_ids;
> +       uint16_t offset;
> +};
> +
>  struct avrcp_player {
>        struct avrcp_server *server;
>        struct avctp *session;
> @@ -137,6 +144,7 @@ struct avrcp_player {
>        unsigned int handler;
>        uint16_t registered_events;
>        uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
> +       struct pending_pdu *pending_pdu;
>
>        struct avrcp_player_cb *cb;
>        void *user_data;
> @@ -462,6 +470,18 @@ done:
>        return sizeof(struct media_info_elem) + len;
>  }
>
> +static gboolean player_abort_pending_pdu(struct avrcp_player *player)
> +{
> +       if (player->pending_pdu == NULL)
> +               return FALSE;
> +
> +       g_list_free(player->pending_pdu->attr_ids);
> +       g_free(player->pending_pdu);
> +       player->pending_pdu = NULL;
> +
> +       return TRUE;
> +}
> +
>  static int player_set_attribute(struct avrcp_player *player,
>                                                uint8_t attr, uint8_t val)
>  {
> @@ -872,6 +892,33 @@ err:
>        return AVC_CTYPE_REJECTED;
>  }
>
> +static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
> +                                               struct avrcp_header *pdu,
> +                                               uint8_t transaction)
> +{
> +       uint16_t len = ntohs(pdu->params_len);
> +       struct pending_pdu *pending;
> +
> +       if (len != 1 || player->pending_pdu == NULL)
> +               goto err;
> +
> +       pending = player->pending_pdu;
> +
> +       if (pending->pdu_id != pdu->params[0])
> +               goto err;
> +
> +       player_abort_pending_pdu(player);
> +       pdu->params_len = 0;
> +
> +       return AVC_CTYPE_STABLE;
> +
> +err:
> +       pdu->params_len = htons(1);
> +       pdu->params[0] = E_INVALID_PARAM;
> +       return AVC_CTYPE_REJECTED;
> +}
> +
> +
>  static struct pdu_handler {
>        uint8_t pdu_id;
>        uint8_t code;
> @@ -903,6 +950,8 @@ static struct pdu_handler {
>                                        avrcp_handle_get_play_status },
>                { AVRCP_REGISTER_NOTIFICATION, AVC_CTYPE_NOTIFY,
>                                        avrcp_handle_register_notification },
> +               { AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
> +                                       avrcp_handle_abort_continuing },
>                { },
>  };
>
> @@ -950,6 +999,10 @@ static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
>
>        *code = handler->func(player, pdu, transaction);
>
> +       if (*code != AVC_CTYPE_REJECTED &&
> +                               pdu->pdu_id != AVRCP_ABORT_CONTINUING)
> +               player_abort_pending_pdu(player);
> +
>        return AVRCP_HEADER_LENGTH + ntohs(pdu->params_len);
>
>  err_metadata:
> @@ -1103,6 +1156,8 @@ static void player_destroy(gpointer data)
>        if (player->destroy)
>                player->destroy(player->user_data);
>
> +       player_abort_pending_pdu(player);
> +
>        if (player->handler)
>                avctp_unregister_pdu_handler(player->handler);
>
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available

[Luiz Augusto von Dentz]

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> Section 5.4.1 of AVRCP 1.3 spec says:
>
>        If TG does not support SongLength And SongPosition on TG, then TG shall
>        return 0xFFFFFFFF.
>
> SongPosition is always available, but song length depends on user to
> provied it.
> ---
>  audio/avrcp.c |   11 +++++++----
>  1 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index b1c3d54..5f8277c 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -796,6 +796,7 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
>        uint16_t len = ntohs(pdu->params_len);
>        uint32_t position;
>        uint32_t duration;
> +       void *pduration;
>
>        if (len != 0) {
>                pdu->params_len = htons(1);
> @@ -804,11 +805,13 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
>        }
>
>        position = player->cb->get_position(player->user_data);
> -       duration = GPOINTER_TO_UINT(player->cb->get_metadata(
> -                                               AVRCP_MEDIA_ATTRIBUTE_DURATION,
> -                                               player->user_data));
> +       pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
> +                                                       player->user_data);
> +       if (pduration != NULL)
> +               duration = htonl(GPOINTER_TO_UINT(pduration));
> +       else
> +               duration = htonl(UINT32_MAX);
>
> -       duration = htonl(duration);
>        position = htonl(position);
>
>        memcpy(&pdu->params[0], &duration, 4);
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event

[Luiz Augusto von Dentz]

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> ---
>  audio/avrcp.c |   13 ++++++++++---
>  audio/avrcp.h |    2 ++
>  audio/media.c |    5 +++++
>  3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 076fdf4..b1c3d54 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -136,7 +136,7 @@ struct avrcp_player {
>
>        unsigned int handler;
>        uint16_t registered_events;
> -       uint8_t transaction_events[AVRCP_EVENT_TRACK_CHANGED + 1];
> +       uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
>
>        struct avrcp_player_cb *cb;
>        void *user_data;
> @@ -368,6 +368,9 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
>                memcpy(&pdu->params[1], data, sizeof(uint64_t));
>
>                break;
> +       case AVRCP_EVENT_TRACK_REACHED_START:
> +               size = 1;
> +               break;
>        default:
>                error("Unknown event %u", id);
>                return -EINVAL;
> @@ -504,10 +507,11 @@ static uint8_t avrcp_handle_get_capabilities(struct avrcp_player *player,
>
>                return AVC_CTYPE_STABLE;
>        case CAP_EVENTS_SUPPORTED:
> -               pdu->params_len = htons(4);
> -               pdu->params[1] = 2;
> +               pdu->params_len = htons(5);
> +               pdu->params[1] = 3;
>                pdu->params[2] = AVRCP_EVENT_STATUS_CHANGED;
>                pdu->params[3] = AVRCP_EVENT_TRACK_CHANGED;
> +               pdu->params[4] = AVRCP_EVENT_TRACK_REACHED_START;
>
>                return AVC_CTYPE_STABLE;
>        }
> @@ -843,6 +847,9 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
>                memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
>
>                break;
> +       case AVRCP_EVENT_TRACK_REACHED_START:
> +               len = 1;
> +               break;
>        default:
>                /* All other events are not supported yet */
>                goto err;
> diff --git a/audio/avrcp.h b/audio/avrcp.h
> index 8cf95a4..c798658 100644
> --- a/audio/avrcp.h
> +++ b/audio/avrcp.h
> @@ -71,6 +71,8 @@
>  /* Notification events */
>  #define AVRCP_EVENT_STATUS_CHANGED     0x01
>  #define AVRCP_EVENT_TRACK_CHANGED      0x02
> +#define AVRCP_EVENT_TRACK_REACHED_START        0x04
> +#define AVRCP_EVENT_LAST               AVRCP_EVENT_TRACK_REACHED_START
>
>  struct avrcp_player_cb {
>        int (*get_setting) (uint8_t attr, void *user_data);
> diff --git a/audio/media.c b/audio/media.c
> index 9ef393b..a7647ea 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1289,6 +1289,11 @@ static gboolean set_position(struct media_player *mp, DBusMessageIter *iter)
>        mp->position = value;
>        g_timer_start(mp->timer);
>
> +       if (!mp->position) {
> +               avrcp_player_event(mp->player,
> +                                       AVRCP_EVENT_TRACK_REACHED_START, NULL);
> +       }
> +
>        return TRUE;
>  }
>
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 1/5] AVRCP: Use track's UID in event notification

[Luiz Augusto von Dentz]

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> When registering for TRACK_CHANGED event, CT expects the track's UID in
> the INTERIM and CHANGED responses. According to AVRCP 1.3 spec, there
> are only 2 possible values:
>        1) 0: there's a track selected
>        2) UINT32_MAX: there's no track selected
>
> AVRCP 1.4 reserves the value UINT64_MAX for the second case. Since this
> later value is the one used in certification process for best IOP
> we return UINT64_MAX instead of the former.
>
> This implementation allows to pass PTS test TP/NFY/BV-05-C.
> ---
>  audio/avrcp.c |   16 +++++-----------
>  audio/avrcp.h |    1 +
>  audio/media.c |   17 ++++++++++++++++-
>  3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index ac7c8d4..076fdf4 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -363,18 +363,11 @@ int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
>                pdu->params[1] = *((uint8_t *)data);
>
>                break;
> -       case AVRCP_EVENT_TRACK_CHANGED: {
> +       case AVRCP_EVENT_TRACK_CHANGED:
>                size = 9;
> -
> -               /*
> -                * AVRCP 1.3 supports only one track identifier: PLAYING
> -                * (0x0). When 1.4 version is added, this shall be changed to
> -                * contain the identifier of the track.
> -                */
> -               memset(&pdu->params[1], 0, 8);
> +               memcpy(&pdu->params[1], data, sizeof(uint64_t));
>
>                break;
> -       }
>        default:
>                error("Unknown event %u", id);
>                return -EINVAL;
> @@ -828,6 +821,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
>                                                uint8_t transaction)
>  {
>        uint16_t len = ntohs(pdu->params_len);
> +       uint64_t uid;
>
>        /*
>         * 1 byte for EventID, 4 bytes for Playback interval but the latest
> @@ -845,8 +839,8 @@ static uint8_t avrcp_handle_register_notification(struct avrcp_player *player,
>                break;
>        case AVRCP_EVENT_TRACK_CHANGED:
>                len = 9;
> -
> -               memset(&pdu->params[1], 0, 8);
> +               uid = player->cb->get_uid(player->user_data);
> +               memcpy(&pdu->params[1], &uid, sizeof(uint64_t));
>
>                break;
>        default:
> diff --git a/audio/avrcp.h b/audio/avrcp.h
> index 360a80a..8cf95a4 100644
> --- a/audio/avrcp.h
> +++ b/audio/avrcp.h
> @@ -75,6 +75,7 @@
>  struct avrcp_player_cb {
>        int (*get_setting) (uint8_t attr, void *user_data);
>        int (*set_setting) (uint8_t attr, uint8_t value, void *user_data);
> +       uint64_t (*get_uid) (void *user_data);
>        void *(*get_metadata) (uint32_t id, void *user_data);
>        GList *(*list_metadata) (void *user_data);
>        uint8_t (*get_status) (void *user_data);
> diff --git a/audio/media.c b/audio/media.c
> index 519cafe..9ef393b 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -1168,6 +1168,18 @@ static GList *list_metadata(void *user_data)
>        return g_hash_table_get_keys(mp->track);
>  }
>
> +static uint64_t get_uid(void *user_data)
> +{
> +       struct media_player *mp = user_data;
> +
> +       DBG("%p", mp->track);
> +
> +       if (mp->track == NULL)
> +               return UINT64_MAX;
> +
> +       return 0;
> +}
> +
>  static void *get_metadata(uint32_t id, void *user_data)
>  {
>        struct media_player *mp = user_data;
> @@ -1220,6 +1232,7 @@ static struct avrcp_player_cb player_cb = {
>        .get_setting = get_setting,
>        .set_setting = set_setting,
>        .list_metadata = list_metadata,
> +       .get_uid = get_uid,
>        .get_metadata = get_metadata,
>        .get_position = get_position,
>        .get_status = get_status
> @@ -1369,6 +1382,7 @@ static gboolean parse_player_metadata(struct media_player *mp,
>        GHashTable *track;
>        int ctype;
>        gboolean title = FALSE;
> +       uint64_t uid;
>
>        ctype = dbus_message_iter_get_arg_type(iter);
>        if (ctype != DBUS_TYPE_ARRAY)
> @@ -1466,8 +1480,9 @@ static gboolean parse_player_metadata(struct media_player *mp,
>        mp->track = track;
>        mp->position = 0;
>        g_timer_start(mp->timer);
> +       uid = get_uid(mp);
>
> -       avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, NULL);
> +       avrcp_player_event(mp->player, AVRCP_EVENT_TRACK_CHANGED, &uid);
>
>        return TRUE;
>
> --
> 1.7.7
>
> --

Ack.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 0/5] Request/Abort Continuing and some minor things

[Johan Hedberg]

Hi Lucas,

On Mon, Oct 17, 2011, Lucas De Marchi wrote:
> Changes from previous version:
> 
>  - 2/5: fix minor coding style issue
>  - 3/5: allow 0 to be a valid duration value
>  - 5/5: fill attribute header in player_fill_media_attribute() in order to
>    simplify implementation
> 
> 
> Lucas De Marchi (5):
>   AVRCP: Use track's UID in event notification
>   AVRCP: implement TRACK-REACHED-START event
>   AVRCP: respond with UINT32_MAX if duration is not available
>   AVRCP: Implement AbortContinuingResponse PDU
>   AVRCP: Implement RequestContinuingResponse PDU
> 
>  audio/avrcp.c |  307 +++++++++++++++++++++++++++++++++++++++++----------------
>  audio/avrcp.h |    3 +
>  audio/media.c |   22 ++++-
>  3 files changed, 246 insertions(+), 86 deletions(-)

All five patches applied. Thanks.

Johan