2011-10-17 20:34:26

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH v2 0/5] Request/Abort Continuing and some minor things

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



2011-10-18 07:40:03

by Johan Hedberg

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

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

2011-10-18 07:24:17

by Luiz Augusto von Dentz

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

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<[email protected]> 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

2011-10-18 07:23:47

by Luiz Augusto von Dentz

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

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<[email protected]> 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

2011-10-18 07:23:22

by Luiz Augusto von Dentz

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

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<[email protected]> 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

2011-10-18 07:22:55

by Luiz Augusto von Dentz

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

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<[email protected]> 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

2011-10-18 07:22:32

by Luiz Augusto von Dentz

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

Hi Lucas,

On Mon, Oct 17, 2011 at 11:34 PM, Lucas De Marchi
<[email protected]> 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

2011-10-17 20:34:31

by Lucas De Marchi

[permalink] [raw]
Subject: [PATCH v2 5/5] AVRCP: Implement RequestContinuingResponse PDU

---
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


2011-10-17 20:34:30

by Lucas De Marchi

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

---
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


2011-10-17 20:34:29

by Lucas De Marchi

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

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


2011-10-17 20:34:28

by Lucas De Marchi

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

---
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


2011-10-17 20:34:27

by Lucas De Marchi

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

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