* [PATCH v2 1/5] AVRCP: Use track's UID in event notification
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
@ 2011-10-17 20:34 ` Lucas De Marchi
2011-10-18 7:24 ` Luiz Augusto von Dentz
2011-10-17 20:34 ` [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event Lucas De Marchi
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2011-10-17 20:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/5] AVRCP: Use track's UID in event notification
2011-10-17 20:34 ` [PATCH v2 1/5] AVRCP: Use track's UID in event notification Lucas De Marchi
@ 2011-10-18 7:24 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2011-10-18 7:24 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
2011-10-17 20:34 ` [PATCH v2 1/5] AVRCP: Use track's UID in event notification Lucas De Marchi
@ 2011-10-17 20:34 ` Lucas De Marchi
2011-10-18 7:23 ` Luiz Augusto von Dentz
2011-10-17 20:34 ` [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available Lucas De Marchi
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2011-10-17 20:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event
2011-10-17 20:34 ` [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event Lucas De Marchi
@ 2011-10-18 7:23 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2011-10-18 7:23 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
2011-10-17 20:34 ` [PATCH v2 1/5] AVRCP: Use track's UID in event notification Lucas De Marchi
2011-10-17 20:34 ` [PATCH v2 2/5] AVRCP: implement TRACK-REACHED-START event Lucas De Marchi
@ 2011-10-17 20:34 ` Lucas De Marchi
2011-10-18 7:23 ` Luiz Augusto von Dentz
2011-10-17 20:34 ` [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU Lucas De Marchi
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2011-10-17 20:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available
2011-10-17 20:34 ` [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available Lucas De Marchi
@ 2011-10-18 7:23 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2011-10-18 7:23 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
` (2 preceding siblings ...)
2011-10-17 20:34 ` [PATCH v2 3/5] AVRCP: respond with UINT32_MAX if duration is not available Lucas De Marchi
@ 2011-10-17 20:34 ` Lucas De Marchi
2011-10-18 7:22 ` Luiz Augusto von Dentz
2011-10-17 20:34 ` [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU Lucas De Marchi
2011-10-18 7:40 ` [PATCH v2 0/5] Request/Abort Continuing and some minor things Johan Hedberg
5 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2011-10-17 20:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU
2011-10-17 20:34 ` [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU Lucas De Marchi
@ 2011-10-18 7:22 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2011-10-18 7:22 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
` (3 preceding siblings ...)
2011-10-17 20:34 ` [PATCH v2 4/5] AVRCP: Implement AbortContinuingResponse PDU Lucas De Marchi
@ 2011-10-17 20:34 ` Lucas De Marchi
2011-10-18 7:22 ` Luiz Augusto von Dentz
2011-10-18 7:40 ` [PATCH v2 0/5] Request/Abort Continuing and some minor things Johan Hedberg
5 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2011-10-17 20:34 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU
2011-10-17 20:34 ` [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU Lucas De Marchi
@ 2011-10-18 7:22 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2011-10-18 7:22 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/5] Request/Abort Continuing and some minor things
2011-10-17 20:34 [PATCH v2 0/5] Request/Abort Continuing and some minor things Lucas De Marchi
` (4 preceding siblings ...)
2011-10-17 20:34 ` [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU Lucas De Marchi
@ 2011-10-18 7:40 ` Johan Hedberg
5 siblings, 0 replies; 12+ messages in thread
From: Johan Hedberg @ 2011-10-18 7:40 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
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
^ permalink raw reply [flat|nested] 12+ messages in thread