Request ctype of "AbortContinuingResponse" is CONTROL, so response should be
ACCEPTED instead of STABLE. This affect PTS Test Case for TP/RCR/BV-04-C.
---
audio/avrcp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index c9ec314..4573133 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -986,7 +986,7 @@ static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
player_abort_pending_pdu(player);
pdu->params_len = 0;
- return AVC_CTYPE_STABLE;
+ return AVC_CTYPE_ACCEPTED;
err:
pdu->params_len = htons(1);
--
on behalf of ST-Ericsson
Yes, you are right. I was thinging about radio cases, but for now I do not need this change.
Abandoned.
---
--
on behalf of ST-Ericsson
Hi Michal
On Thu, Mar 1, 2012 at 11:45 AM, Michal Labedzki
<[email protected]> wrote:
> According to the specification when duration and position are not
> supported then 0xFFFFFFFF should be returned. This patch avoid random
> position on stream where duration is not available.
> ---
> ?audio/avrcp.c | ? 10 ++++++----
> ?audio/media.c | ? ?3 ++-
> ?2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 2d29d56..e141030 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -859,16 +859,18 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
> ? ? ? ? ? ? ? ?return AVC_CTYPE_REJECTED;
> ? ? ? ?}
>
> - ? ? ? position = player->cb->get_position(player->user_data);
> ? ? ? ?pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?player->user_data);
>
> - ? ? ? if (pduration != NULL)
> + ? ? ? if (pduration != NULL) {
> ? ? ? ? ? ? ? ?duration = htonl(GPOINTER_TO_UINT(pduration));
> - ? ? ? else
> + ? ? ? ? ? ? ? position = htonl(player->cb->get_position(player->user_data));
Duration and position are independent. Only duration accepts
UINT32_MAX when it's not available. Position is always available
because we handle it inside bluetoothd.
Regards,
Lucas De Marchi
On Thu, Mar 1, 2012 at 11:45 AM, Michal Labedzki
<[email protected]> wrote:
> Request ctype of "AbortContinuingResponse" is CONTROL, so response should be
> ACCEPTED instead of STABLE. This affect PTS Test Case for TP/RCR/BV-04-C.
> ---
> ?audio/avrcp.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index c9ec314..4573133 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -986,7 +986,7 @@ static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
> ? ? ? ?player_abort_pending_pdu(player);
> ? ? ? ?pdu->params_len = 0;
>
> - ? ? ? return AVC_CTYPE_STABLE;
> + ? ? ? return AVC_CTYPE_ACCEPTED;
What version of PTS you are running? It seems like PTS was accepting
it before. Last time I checked it was passing exactly this test, but
searching PTS bugs I found this:
https://www.bluetooth.org/pts/issues/view_issue.cfm?id=8043 (Created:
10-Jun-11 by: Bradford Ayres ID: 38385)
So you have my ACK here.
Lucas De Marchi
Hi Michal,
On Thu, Mar 1, 2012 at 4:45 PM, Michal Labedzki
<[email protected]> wrote:
> Request ctype of "AbortContinuingResponse" is CONTROL, so response should be
> ACCEPTED instead of STABLE. This affect PTS Test Case for TP/RCR/BV-04-C.
> ---
> ?audio/avrcp.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index c9ec314..4573133 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -986,7 +986,7 @@ static uint8_t avrcp_handle_abort_continuing(struct avrcp_player *player,
> ? ? ? ?player_abort_pending_pdu(player);
> ? ? ? ?pdu->params_len = 0;
>
> - ? ? ? return AVC_CTYPE_STABLE;
> + ? ? ? return AVC_CTYPE_ACCEPTED;
>
> ?err:
> ? ? ? ?pdu->params_len = htons(1);
> --
> on behalf of ST-Ericsson
>
> --
Lucas can you take a look and ack this, it should be very straight forward.
--
Luiz Augusto von Dentz
Hi Michal,
On Thu, Mar 1, 2012 at 4:45 PM, Michal Labedzki
<[email protected]> wrote:
> According to the specification when duration and position are not
> supported then 0xFFFFFFFF should be returned. This patch avoid random
> position on stream where duration is not available.
Again you should quote the spec. Btw is really random or 0?
> ---
> ?audio/avrcp.c | ? 10 ++++++----
> ?audio/media.c | ? ?3 ++-
> ?2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 2d29d56..e141030 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -859,16 +859,18 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
> ? ? ? ? ? ? ? ?return AVC_CTYPE_REJECTED;
> ? ? ? ?}
>
> - ? ? ? position = player->cb->get_position(player->user_data);
> ? ? ? ?pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?player->user_data);
>
> - ? ? ? if (pduration != NULL)
> + ? ? ? if (pduration != NULL) {
> ? ? ? ? ? ? ? ?duration = htonl(GPOINTER_TO_UINT(pduration));
> - ? ? ? else
> + ? ? ? ? ? ? ? position = htonl(player->cb->get_position(player->user_data));
> + ? ? ? } else {
> ? ? ? ? ? ? ? ?duration = htonl(UINT32_MAX);
> + ? ? ? ? ? ? ? position = htonl(UINT32_MAX);
> + ? ? ? }
So the position cannot be known without the duration? What about live streams?
--
Luiz Augusto von Dentz
According to the specification when duration and position are not
supported then 0xFFFFFFFF should be returned. This patch avoid random
position on stream where duration is not available.
---
audio/avrcp.c | 10 ++++++----
audio/media.c | 3 ++-
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 2d29d56..e141030 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -859,16 +859,18 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
return AVC_CTYPE_REJECTED;
}
- position = player->cb->get_position(player->user_data);
pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
player->user_data);
- if (pduration != NULL)
+ if (pduration != NULL) {
duration = htonl(GPOINTER_TO_UINT(pduration));
- else
+ position = htonl(player->cb->get_position(player->user_data));
+ } else {
duration = htonl(UINT32_MAX);
+ position = htonl(UINT32_MAX);
+ }
- position = htonl(position);
+ DBG("position=%u duration=%u", ntohl(position), ntohl(duration));
memcpy(&pdu->params[0], &duration, 4);
memcpy(&pdu->params[4], &position, 4);
diff --git a/audio/media.c b/audio/media.c
index c0fd0c3..4887753 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -1569,7 +1569,8 @@ static struct media_player *media_player_create(struct media_adapter *adapter,
mp->sender = g_strdup(sender);
mp->path = g_strdup(path);
mp->timer = g_timer_new();
-
+ mp->position = UINT32_MAX;
+ mp->status = AVRCP_PLAY_STATUS_STOPPED;
mp->watch = g_dbus_add_disconnect_watch(adapter->conn, sender,
media_player_exit, mp,
NULL);
--
on behalf of ST-Ericsson
Make correct response for any vendor commands when there are not any
registered player. According to the specification responses should be
REJECTED with error code.
---
audio/avctp.c | 3 +++
audio/avctp.h | 5 +++++
audio/avrcp.c | 19 +++++++++++++++++++
3 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index 5bd5db1..38a2d20 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -465,6 +465,9 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
if (!handler) {
DBG("handler not found for 0x%02x", avc->opcode);
avc->code = AVC_CTYPE_REJECTED;
+
+ packet_size += handle_vendor_reject(session, avctp->transaction, &code,
+ &subunit, operands, operand_count, NULL);
goto done;
}
diff --git a/audio/avctp.h b/audio/avctp.h
index 9727485..a1109a9 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -97,3 +97,8 @@ int avctp_send_passthrough(struct avctp *session, uint8_t op);
int avctp_send_vendordep(struct avctp *session, uint8_t transaction,
uint8_t code, uint8_t subunit,
uint8_t *operands, size_t operand_count);
+
+size_t handle_vendor_reject(struct avctp *session, uint8_t transaction,
+ uint8_t *code, uint8_t *subunit,
+ uint8_t *operands, size_t operand_count,
+ void *user_data);
\ No newline at end of file
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 4573133..2d29d56 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -862,6 +862,7 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp_player *player,
position = player->cb->get_position(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
@@ -1092,6 +1093,24 @@ err_metadata:
return AVRCP_HEADER_LENGTH + 1;
}
+size_t handle_vendor_reject(struct avctp *session, uint8_t transaction,
+ uint8_t *code, uint8_t *subunit,
+ uint8_t *operands, size_t operand_count,
+ void *user_data)
+{
+ struct avrcp_header *pdu = (void *) operands;
+ uint32_t company_id = get_company_id(pdu->company_id);
+
+ *code = AVC_CTYPE_REJECTED;
+ pdu->params_len = htons(1);
+ pdu->params[0] = E_INTERNAL;
+
+ DBG("rejecting AVRCP PDU 0x%02X, company 0x%06X len 0x%04X",
+ pdu->pdu_id, company_id, pdu->params_len);
+
+ return AVRCP_HEADER_LENGTH + 1;
+}
+
static struct avrcp_server *find_server(GSList *list, const bdaddr_t *src)
{
for (; list; list = list->next) {
--
on behalf of ST-Ericsson