These are simple fixes for AVRCP 1.0 and a preparation for the upcoming 1.3.
I'm working on the task of implementing AVRCP 1.3 now, based when possible on
previous patches sent to this list.
Lucas De Marchi (3):
avrcp: Check that AVCTP packet is of type COMMAND
avrcp: Factor out function to handle vendordep pdu
avrcp: return NOT_IMPLEMENTED on request for vendor dep command
audio/control.c | 31 +++++++++++++++----------------
1 files changed, 15 insertions(+), 16 deletions(-)
--
1.7.6
Hi Lucas,
On Thu, Jun 30, 2011, Lucas De Marchi wrote:
> These are simple fixes for AVRCP 1.0 and a preparation for the upcoming 1.3.
> I'm working on the task of implementing AVRCP 1.3 now, based when possible on
> previous patches sent to this list.
>
> Lucas De Marchi (3):
> avrcp: Check that AVCTP packet is of type COMMAND
> avrcp: Factor out function to handle vendordep pdu
> avrcp: return NOT_IMPLEMENTED on request for vendor dep command
>
> audio/control.c | 31 +++++++++++++++----------------
> 1 files changed, 15 insertions(+), 16 deletions(-)
All three patches have been applied. Thanks.
Johan
When a vendor dependent command is requested but target does not
implement it, the correct return value is CTYPE_NOT_IMPLEMENTED instead
of CTYPE_REJECTED.
AVRCP 1.3 spec clearly says so on section 4.5.1:
[ It is assumed that devices that do not support this metadata
transfer related features shall return a response of NOT
IMPLEMENTED as per AV/C protocol specification ]
And AV/C General Specification, section 8.3.2 talks about legacy
behavior and mandates that NOT_IMPLEMENTED is returned.
Finally, in section 11.6.1 we see that VENDOR-DEPENDENT command frame
depends on the company_ID. Therefore we can't assume it has the same
format as the one specified for metadata transfer (in case the company
ID is 0x001958)
---
audio/control.c | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/audio/control.c b/audio/control.c
index 9202966..defbbc8 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -424,19 +424,8 @@ static int handle_vendordep_pdu(struct control *control,
struct avrcp_header *avrcp,
int operand_count)
{
- struct avrcp_spec_avc_pdu *pdu = (void *)(avrcp) +
- AVRCP_SPECAVCPDU_HEADER_LENGTH;
-
- /* Reply with REJECT msg with error code 0x0
- * (Invalid Command) as defined in AVRCP spec (6.15.1) */
- avrcp->code = CTYPE_REJECTED;
-
- pdu->packet_type = 0;
- pdu->rsvd = 0;
- pdu->params[0] = 0; /* invalid command */
- pdu->params_len = htons(1);
-
- return AVRCP_HEADER_LENGTH + AVRCP_SPECAVCPDU_HEADER_LENGTH + 1;
+ avrcp->code = CTYPE_NOT_IMPLEMENTED;
+ return AVRCP_HEADER_LENGTH;
}
static void avctp_disconnected(struct audio_device *dev)
--
1.7.6
In order to support vendordep pdu as required by AVRCP 1.3 this part
will get very large. So, separate it to a new function like is done for
panel_passthrough.
---
audio/control.c | 39 ++++++++++++++++++++++++---------------
1 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/audio/control.c b/audio/control.c
index 57a4edc..9202966 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -419,6 +419,26 @@ static void handle_panel_passthrough(struct control *control,
operands[0] & 0x7F, status);
}
+/* handle vendordep pdu inside an avctp packet */
+static int handle_vendordep_pdu(struct control *control,
+ struct avrcp_header *avrcp,
+ int operand_count)
+{
+ struct avrcp_spec_avc_pdu *pdu = (void *)(avrcp) +
+ AVRCP_SPECAVCPDU_HEADER_LENGTH;
+
+ /* Reply with REJECT msg with error code 0x0
+ * (Invalid Command) as defined in AVRCP spec (6.15.1) */
+ avrcp->code = CTYPE_REJECTED;
+
+ pdu->packet_type = 0;
+ pdu->rsvd = 0;
+ pdu->params[0] = 0; /* invalid command */
+ pdu->params_len = htons(1);
+
+ return AVRCP_HEADER_LENGTH + AVRCP_SPECAVCPDU_HEADER_LENGTH + 1;
+}
+
static void avctp_disconnected(struct audio_device *dev)
{
struct control *control = dev->control;
@@ -592,22 +612,11 @@ static gboolean control_cb(GIOChannel *chan, GIOCondition cond,
"OP_UNITINFO" : "OP_SUBUNITINFO");
} else if (avctp->cr == AVCTP_COMMAND &&
avrcp->opcode == OP_VENDORDEP) {
- /* Reply with REJECT msg with error code 0x0
- * (Invalid Command) as defined in AVRCP spec (6.15.1) */
- struct avrcp_spec_avc_pdu *pdu = (void *) operands;
-
+ int r_size;
+ operand_count -= 3;
avctp->cr = AVCTP_RESPONSE;
- avrcp->code = CTYPE_REJECTED;
-
- pdu->packet_type = 0;
- pdu->rsvd = 0;
- pdu->params[0] = 0; /* invalid command */
- pdu->params_len = htons(1);
-
- packet_size = sizeof(struct avctp_header)
- + sizeof(struct avrcp_header)
- + sizeof(struct avrcp_spec_avc_pdu)
- + 1;
+ r_size = handle_vendordep_pdu(control, avrcp, operand_count);
+ packet_size = AVCTP_HEADER_LENGTH + r_size;
} else {
avctp->cr = AVCTP_RESPONSE;
avrcp->code = CTYPE_REJECTED;
--
1.7.6
---
audio/control.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/audio/control.c b/audio/control.c
index 3db7d8e..57a4edc 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -590,7 +590,8 @@ static gboolean control_cb(GIOChannel *chan, GIOCondition cond,
operands[1] = SUBUNIT_PANEL << 3;
DBG("reply to %s", avrcp->opcode == OP_UNITINFO ?
"OP_UNITINFO" : "OP_SUBUNITINFO");
- } else if (avrcp->opcode == OP_VENDORDEP) {
+ } else if (avctp->cr == AVCTP_COMMAND &&
+ avrcp->opcode == OP_VENDORDEP) {
/* Reply with REJECT msg with error code 0x0
* (Invalid Command) as defined in AVRCP spec (6.15.1) */
struct avrcp_spec_avc_pdu *pdu = (void *) operands;
--
1.7.6