* [PATCH] Fix parser of AVRCP continuing messages
@ 2011-09-13 14:21 Lucas De Marchi
2011-09-13 14:40 ` Luiz Augusto von Dentz
2011-09-27 9:58 ` Johan Hedberg
0 siblings, 2 replies; 5+ messages in thread
From: Lucas De Marchi @ 2011-09-13 14:21 UTC (permalink / raw)
To: linux-bluetooth; +Cc: luiz.dentz, Lucas De Marchi
If packet_type is not START or SINGLE, we have to continue where we
stopped from previous packet. Therefore we must store where we left on
previous packet due to packet size limit. We store both the number of
attributes missing and the lenght of the last attribute that is missing.
An example interaction for this implementation, obtained with PTS test
TC_TG_MDI_BV_04_C (I reduced the MTU in order to reproduce it here and
values between brackets I added now):
> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
AV/C: Status: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt Single len 0x0009
Identifier: 0x0 (PLAYING)
AttributeCount: 0x00
< AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
AV/C: Stable: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt Start len 0x0118
AttributeCount: 0x04
Attribute: 0x00000001 (Title)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x001b
AttributeValue: isso eh um titulo mei longo
Attribute: 0x00000003 (Album)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x00fe
AttributeValue: super-long-album-name super-long-album-name
super-long-album-name super-long-album-name super-long-album
super-long-album-name [... snip... ] super-long-album-name-1234
> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
AV/C: Control: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: RequestContinuingResponse: pt Single len 0x0001
< AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
AV/C: Stable: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt End len 0x002a
ContinuingAttributeValue: 678900000000000000
Attribute: 0x00000005 (Track Total)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0002
AttributeValue: 30
Attribute: 0x00000006 (Genre)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0006
AttributeValue: Gospel
---
parser/avrcp.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 79 insertions(+), 15 deletions(-)
diff --git a/parser/avrcp.c b/parser/avrcp.c
index 0b98a3e..65d3bda 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -87,6 +87,12 @@
#define AVC_PANEL_FORWARD 0x4b
#define AVC_PANEL_BACKWARD 0x4c
+/* 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 ids */
#define AVRCP_GET_CAPABILITIES 0x10
#define AVRCP_LIST_PLAYER_ATTRIBUTES 0x11
@@ -176,6 +182,11 @@
#define AVRCP_PLAY_STATUS_REV_SEEK 0x04
#define AVRCP_PLAY_STATUS_ERROR 0xFF
+static struct avrcp_continuing {
+ uint16_t num;
+ uint16_t size;
+} avrcp_continuing;
+
static const char *ctype2str(uint8_t ctype)
{
switch (ctype & 0x0f) {
@@ -224,6 +235,22 @@ static const char *opcode2str(uint8_t opcode)
}
}
+static const char *pt2str(uint8_t pt)
+{
+ switch (pt) {
+ case AVRCP_PACKET_TYPE_SINGLE:
+ return "Single";
+ case AVRCP_PACKET_TYPE_START:
+ return "Start";
+ case AVRCP_PACKET_TYPE_CONTINUING:
+ return "Continuing";
+ case AVRCP_PACKET_TYPE_END:
+ return "End";
+ default:
+ return "Unknown";
+ }
+}
+
static const char *pdu2str(uint8_t pduid)
{
switch (pduid) {
@@ -917,7 +944,8 @@ static const char *mediattr2str(uint32_t attr)
}
static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
- uint8_t ctype, uint16_t len)
+ uint8_t ctype, uint16_t len,
+ uint8_t pt)
{
uint64_t id;
uint8_t num;
@@ -953,18 +981,45 @@ static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
return;
response:
- if (len < 1) {
- printf("PDU Malformed\n");
- raw_dump(level, frm);
- return;
- }
+ if (pt == AVRCP_PACKET_TYPE_SINGLE || pt == AVRCP_PACKET_TYPE_START) {
+ if (len < 1) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
- num = get_u8(frm);
- printf("AttributeCount: 0x%02x\n", num);
+ num = get_u8(frm);
+ avrcp_continuing.num = num;
+ printf("AttributeCount: 0x%02x\n", num);
+ len--;
+ } else {
+ num = avrcp_continuing.num;
+
+ if (avrcp_continuing.size > 0) {
+ uint16_t size;
+
+ if (avrcp_continuing.size > len) {
+ size = len;
+ avrcp_continuing.size -= len;
+ } else {
+ size = avrcp_continuing.size;
+ avrcp_continuing.size = 0;
+ }
+
+ printf("ContinuingAttributeValue: ");
+ for (; size > 0; size--) {
+ uint8_t c = get_u8(frm);
+ printf("%1c", isprint(c) ? c : '.');
+ }
+ printf("\n");
- for (; num > 0; num--) {
+ len -= size;
+ }
+ }
+
+ while (num > 0 && len > 0) {
uint32_t attr;
- uint16_t charset, len;
+ uint16_t charset, attrlen;
p_indent(level, frm);
@@ -978,19 +1033,26 @@ response:
charset2str(charset));
p_indent(level, frm);
+ attrlen = get_u16(frm);
+ printf("AttributeValueLength: 0x%04x\n", attrlen);
- len = get_u16(frm);
- printf("AttributeValueLength: 0x%04x\n", len);
+ len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
+ num--;
p_indent(level, frm);
printf("AttributeValue: ");
- for (; len > 0; len--) {
+ for (; attrlen > 0 && len > 0; attrlen--, len--) {
uint8_t c = get_u8(frm);
printf("%1c", isprint(c) ? c : '.');
}
printf("\n");
+
+ if (attrlen > 0)
+ avrcp_continuing.size = attrlen;
}
+
+ avrcp_continuing.num = num;
}
static const char *playstatus2str(uint8_t status)
@@ -1153,7 +1215,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
pt = get_u8(frm);
len = get_u16(frm);
- printf("AVRCP: %s: pt 0x%02x len 0x%04x\n", pdu2str(pduid), pt, len);
+ printf("AVRCP: %s: pt %s len 0x%04x\n", pdu2str(pduid),
+ pt2str(pt), len);
if (len != frm->len) {
p_indent(level, frm);
@@ -1198,7 +1261,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
avrcp_ct_battery_status_dump(level + 1, frm, ctype, len);
break;
case AVRCP_GET_ELEMENT_ATTRIBUTES:
- avrcp_get_element_attributes_dump(level + 1, frm, ctype, len);
+ avrcp_get_element_attributes_dump(level + 1, frm, ctype, len,
+ pt);
break;
case AVRCP_GET_PLAY_STATUS:
avrcp_get_play_status_dump(level + 1, frm, ctype, len);
--
1.7.6.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix parser of AVRCP continuing messages
2011-09-13 14:21 [PATCH] Fix parser of AVRCP continuing messages Lucas De Marchi
@ 2011-09-13 14:40 ` Luiz Augusto von Dentz
2011-09-13 16:33 ` Lucas De Marchi
2011-09-27 9:58 ` Johan Hedberg
1 sibling, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2011-09-13 14:40 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth
Hi Lucas,
On Tue, Sep 13, 2011 at 5:21 PM, Lucas De Marchi
<lucas.demarchi@profusion.mobi> wrote:
> If packet_type is not START or SINGLE, we have to continue where we
> stopped from previous packet. Therefore we must store where we left on
> previous packet due to packet size limit. We store both the number of
> attributes missing and the lenght of the last attribute that is missing.
>
> An example interaction for this implementation, obtained with PTS test
> TC_TG_MDI_BV_04_C (I reduced the MTU in order to reproduce it here and
> values between brackets I added now):
>
>> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
> AV/C: Status: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt Single len 0x0009
> Identifier: 0x0 (PLAYING)
> AttributeCount: 0x00
> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
> AV/C: Stable: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt Start len 0x0118
> AttributeCount: 0x04
> Attribute: 0x00000001 (Title)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x001b
> AttributeValue: isso eh um titulo mei longo
> Attribute: 0x00000003 (Album)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x00fe
> AttributeValue: super-long-album-name super-long-album-name
> super-long-album-name super-long-album-name super-long-album
> super-long-album-name [... snip... ] super-long-album-name-1234
>> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
> AV/C: Control: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: RequestContinuingResponse: pt Single len 0x0001
> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
> AV/C: Stable: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt End len 0x002a
> ContinuingAttributeValue: 678900000000000000
> Attribute: 0x00000005 (Track Total)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0002
> AttributeValue: 30
> Attribute: 0x00000006 (Genre)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0006
> AttributeValue: Gospel
> ---
> parser/avrcp.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 79 insertions(+), 15 deletions(-)
>
> diff --git a/parser/avrcp.c b/parser/avrcp.c
> index 0b98a3e..65d3bda 100644
> --- a/parser/avrcp.c
> +++ b/parser/avrcp.c
> @@ -87,6 +87,12 @@
> #define AVC_PANEL_FORWARD 0x4b
> #define AVC_PANEL_BACKWARD 0x4c
>
> +/* 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 ids */
> #define AVRCP_GET_CAPABILITIES 0x10
> #define AVRCP_LIST_PLAYER_ATTRIBUTES 0x11
> @@ -176,6 +182,11 @@
> #define AVRCP_PLAY_STATUS_REV_SEEK 0x04
> #define AVRCP_PLAY_STATUS_ERROR 0xFF
>
> +static struct avrcp_continuing {
> + uint16_t num;
> + uint16_t size;
> +} avrcp_continuing;
> +
> static const char *ctype2str(uint8_t ctype)
> {
> switch (ctype & 0x0f) {
> @@ -224,6 +235,22 @@ static const char *opcode2str(uint8_t opcode)
> }
> }
>
> +static const char *pt2str(uint8_t pt)
> +{
> + switch (pt) {
> + case AVRCP_PACKET_TYPE_SINGLE:
> + return "Single";
> + case AVRCP_PACKET_TYPE_START:
> + return "Start";
> + case AVRCP_PACKET_TYPE_CONTINUING:
> + return "Continuing";
> + case AVRCP_PACKET_TYPE_END:
> + return "End";
> + default:
> + return "Unknown";
> + }
> +}
> +
> static const char *pdu2str(uint8_t pduid)
> {
> switch (pduid) {
> @@ -917,7 +944,8 @@ static const char *mediattr2str(uint32_t attr)
> }
>
> static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
> - uint8_t ctype, uint16_t len)
> + uint8_t ctype, uint16_t len,
> + uint8_t pt)
> {
> uint64_t id;
> uint8_t num;
> @@ -953,18 +981,45 @@ static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
> return;
>
> response:
> - if (len < 1) {
> - printf("PDU Malformed\n");
> - raw_dump(level, frm);
> - return;
> - }
> + if (pt == AVRCP_PACKET_TYPE_SINGLE || pt == AVRCP_PACKET_TYPE_START) {
> + if (len < 1) {
> + printf("PDU Malformed\n");
> + raw_dump(level, frm);
> + return;
> + }
>
> - num = get_u8(frm);
> - printf("AttributeCount: 0x%02x\n", num);
> + num = get_u8(frm);
> + avrcp_continuing.num = num;
> + printf("AttributeCount: 0x%02x\n", num);
> + len--;
> + } else {
> + num = avrcp_continuing.num;
> +
> + if (avrcp_continuing.size > 0) {
> + uint16_t size;
> +
> + if (avrcp_continuing.size > len) {
> + size = len;
> + avrcp_continuing.size -= len;
> + } else {
> + size = avrcp_continuing.size;
> + avrcp_continuing.size = 0;
> + }
> +
> + printf("ContinuingAttributeValue: ");
> + for (; size > 0; size--) {
> + uint8_t c = get_u8(frm);
> + printf("%1c", isprint(c) ? c : '.');
> + }
> + printf("\n");
>
> - for (; num > 0; num--) {
> + len -= size;
> + }
> + }
> +
> + while (num > 0 && len > 0) {
> uint32_t attr;
> - uint16_t charset, len;
> + uint16_t charset, attrlen;
>
> p_indent(level, frm);
>
> @@ -978,19 +1033,26 @@ response:
> charset2str(charset));
>
> p_indent(level, frm);
> + attrlen = get_u16(frm);
> + printf("AttributeValueLength: 0x%04x\n", attrlen);
>
> - len = get_u16(frm);
> - printf("AttributeValueLength: 0x%04x\n", len);
> + len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> + num--;
>
> p_indent(level, frm);
>
> printf("AttributeValue: ");
> - for (; len > 0; len--) {
> + for (; attrlen > 0 && len > 0; attrlen--, len--) {
> uint8_t c = get_u8(frm);
> printf("%1c", isprint(c) ? c : '.');
> }
> printf("\n");
> +
> + if (attrlen > 0)
> + avrcp_continuing.size = attrlen;
> }
> +
> + avrcp_continuing.num = num;
> }
>
> static const char *playstatus2str(uint8_t status)
> @@ -1153,7 +1215,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> pt = get_u8(frm);
> len = get_u16(frm);
>
> - printf("AVRCP: %s: pt 0x%02x len 0x%04x\n", pdu2str(pduid), pt, len);
> + printf("AVRCP: %s: pt %s len 0x%04x\n", pdu2str(pduid),
> + pt2str(pt), len);
>
> if (len != frm->len) {
> p_indent(level, frm);
> @@ -1198,7 +1261,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
> avrcp_ct_battery_status_dump(level + 1, frm, ctype, len);
> break;
> case AVRCP_GET_ELEMENT_ATTRIBUTES:
> - avrcp_get_element_attributes_dump(level + 1, frm, ctype, len);
> + avrcp_get_element_attributes_dump(level + 1, frm, ctype, len,
> + pt);
> break;
> case AVRCP_GET_PLAY_STATUS:
> avrcp_get_play_status_dump(level + 1, frm, ctype, len);
> --
> 1.7.6.1
Ack, please remember to add hcidump to prefix next time.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix parser of AVRCP continuing messages
2011-09-13 14:40 ` Luiz Augusto von Dentz
@ 2011-09-13 16:33 ` Lucas De Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Lucas De Marchi @ 2011-09-13 16:33 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On Tue, Sep 13, 2011 at 11:40 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Lucas,
>
> On Tue, Sep 13, 2011 at 5:21 PM, Lucas De Marchi
> <lucas.demarchi@profusion.mobi> wrote:
>> If packet_type is not START or SINGLE, we have to continue where we
>> stopped from previous packet. Therefore we must store where we left on
>> previous packet due to packet size limit. We store both the number of
>> attributes missing and the lenght of the last attribute that is missing.
>>
>> An example interaction for this implementation, obtained with PTS test
>> TC_TG_MDI_BV_04_C (I reduced the MTU in order to reproduce it here and
>> values between brackets I added now):
>>
>>> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
>> AV/C: Status: address 0x48 opcode 0x00
>> Subunit: Panel
>> Opcode: Vendor Dependent
>> Company ID: 0x001958
>> AVRCP: GetElementAttributes: pt Single len 0x0009
>> Identifier: 0x0 (PLAYING)
>> AttributeCount: 0x00
>> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
>> AV/C: Stable: address 0x48 opcode 0x00
>> Subunit: Panel
>> Opcode: Vendor Dependent
>> Company ID: 0x001958
>> AVRCP: GetElementAttributes: pt Start len 0x0118
>> AttributeCount: 0x04
>> Attribute: 0x00000001 (Title)
>> CharsetID: 0x006a (UTF-8)
>> AttributeValueLength: 0x001b
>> AttributeValue: isso eh um titulo mei longo
>> Attribute: 0x00000003 (Album)
>> CharsetID: 0x006a (UTF-8)
>> AttributeValueLength: 0x00fe
>> AttributeValue: super-long-album-name super-long-album-name
>> super-long-album-name super-long-album-name super-long-album
>> super-long-album-name [... snip... ] super-long-album-name-1234
>>> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
>> AV/C: Control: address 0x48 opcode 0x00
>> Subunit: Panel
>> Opcode: Vendor Dependent
>> Company ID: 0x001958
>> AVRCP: RequestContinuingResponse: pt Single len 0x0001
>> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
>> AV/C: Stable: address 0x48 opcode 0x00
>> Subunit: Panel
>> Opcode: Vendor Dependent
>> Company ID: 0x001958
>> AVRCP: GetElementAttributes: pt End len 0x002a
>> ContinuingAttributeValue: 678900000000000000
>> Attribute: 0x00000005 (Track Total)
>> CharsetID: 0x006a (UTF-8)
>> AttributeValueLength: 0x0002
>> AttributeValue: 30
>> Attribute: 0x00000006 (Genre)
>> CharsetID: 0x006a (UTF-8)
>> AttributeValueLength: 0x0006
>> AttributeValue: Gospel
>> ---
>> parser/avrcp.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++---------
>> 1 files changed, 79 insertions(+), 15 deletions(-)
>>
>> diff --git a/parser/avrcp.c b/parser/avrcp.c
>> index 0b98a3e..65d3bda 100644
>> --- a/parser/avrcp.c
>> +++ b/parser/avrcp.c
>> @@ -87,6 +87,12 @@
>> #define AVC_PANEL_FORWARD 0x4b
>> #define AVC_PANEL_BACKWARD 0x4c
>>
>> +/* 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 ids */
>> #define AVRCP_GET_CAPABILITIES 0x10
>> #define AVRCP_LIST_PLAYER_ATTRIBUTES 0x11
>> @@ -176,6 +182,11 @@
>> #define AVRCP_PLAY_STATUS_REV_SEEK 0x04
>> #define AVRCP_PLAY_STATUS_ERROR 0xFF
>>
>> +static struct avrcp_continuing {
>> + uint16_t num;
>> + uint16_t size;
>> +} avrcp_continuing;
>> +
>> static const char *ctype2str(uint8_t ctype)
>> {
>> switch (ctype & 0x0f) {
>> @@ -224,6 +235,22 @@ static const char *opcode2str(uint8_t opcode)
>> }
>> }
>>
>> +static const char *pt2str(uint8_t pt)
>> +{
>> + switch (pt) {
>> + case AVRCP_PACKET_TYPE_SINGLE:
>> + return "Single";
>> + case AVRCP_PACKET_TYPE_START:
>> + return "Start";
>> + case AVRCP_PACKET_TYPE_CONTINUING:
>> + return "Continuing";
>> + case AVRCP_PACKET_TYPE_END:
>> + return "End";
>> + default:
>> + return "Unknown";
>> + }
>> +}
>> +
>> static const char *pdu2str(uint8_t pduid)
>> {
>> switch (pduid) {
>> @@ -917,7 +944,8 @@ static const char *mediattr2str(uint32_t attr)
>> }
>>
>> static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
>> - uint8_t ctype, uint16_t len)
>> + uint8_t ctype, uint16_t len,
>> + uint8_t pt)
>> {
>> uint64_t id;
>> uint8_t num;
>> @@ -953,18 +981,45 @@ static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
>> return;
>>
>> response:
>> - if (len < 1) {
>> - printf("PDU Malformed\n");
>> - raw_dump(level, frm);
>> - return;
>> - }
>> + if (pt == AVRCP_PACKET_TYPE_SINGLE || pt == AVRCP_PACKET_TYPE_START) {
>> + if (len < 1) {
>> + printf("PDU Malformed\n");
>> + raw_dump(level, frm);
>> + return;
>> + }
>>
>> - num = get_u8(frm);
>> - printf("AttributeCount: 0x%02x\n", num);
>> + num = get_u8(frm);
>> + avrcp_continuing.num = num;
>> + printf("AttributeCount: 0x%02x\n", num);
>> + len--;
>> + } else {
>> + num = avrcp_continuing.num;
>> +
>> + if (avrcp_continuing.size > 0) {
>> + uint16_t size;
>> +
>> + if (avrcp_continuing.size > len) {
>> + size = len;
>> + avrcp_continuing.size -= len;
>> + } else {
>> + size = avrcp_continuing.size;
>> + avrcp_continuing.size = 0;
>> + }
>> +
>> + printf("ContinuingAttributeValue: ");
>> + for (; size > 0; size--) {
>> + uint8_t c = get_u8(frm);
>> + printf("%1c", isprint(c) ? c : '.');
>> + }
>> + printf("\n");
>>
>> - for (; num > 0; num--) {
>> + len -= size;
>> + }
>> + }
>> +
>> + while (num > 0 && len > 0) {
>> uint32_t attr;
>> - uint16_t charset, len;
>> + uint16_t charset, attrlen;
>>
>> p_indent(level, frm);
>>
>> @@ -978,19 +1033,26 @@ response:
>> charset2str(charset));
>>
>> p_indent(level, frm);
>> + attrlen = get_u16(frm);
>> + printf("AttributeValueLength: 0x%04x\n", attrlen);
>>
>> - len = get_u16(frm);
>> - printf("AttributeValueLength: 0x%04x\n", len);
>> + len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
>> + num--;
>>
>> p_indent(level, frm);
>>
>> printf("AttributeValue: ");
>> - for (; len > 0; len--) {
>> + for (; attrlen > 0 && len > 0; attrlen--, len--) {
>> uint8_t c = get_u8(frm);
>> printf("%1c", isprint(c) ? c : '.');
>> }
>> printf("\n");
>> +
>> + if (attrlen > 0)
>> + avrcp_continuing.size = attrlen;
>> }
>> +
>> + avrcp_continuing.num = num;
>> }
>>
>> static const char *playstatus2str(uint8_t status)
>> @@ -1153,7 +1215,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
>> pt = get_u8(frm);
>> len = get_u16(frm);
>>
>> - printf("AVRCP: %s: pt 0x%02x len 0x%04x\n", pdu2str(pduid), pt, len);
>> + printf("AVRCP: %s: pt %s len 0x%04x\n", pdu2str(pduid),
>> + pt2str(pt), len);
>>
>> if (len != frm->len) {
>> p_indent(level, frm);
>> @@ -1198,7 +1261,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
>> avrcp_ct_battery_status_dump(level + 1, frm, ctype, len);
>> break;
>> case AVRCP_GET_ELEMENT_ATTRIBUTES:
>> - avrcp_get_element_attributes_dump(level + 1, frm, ctype, len);
>> + avrcp_get_element_attributes_dump(level + 1, frm, ctype, len,
>> + pt);
>> break;
>> case AVRCP_GET_PLAY_STATUS:
>> avrcp_get_play_status_dump(level + 1, frm, ctype, len);
>> --
>> 1.7.6.1
>
> Ack, please remember to add hcidump to prefix next time.
sorry, my git config was not configured right.
Lucas De Marchi
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix parser of AVRCP continuing messages
2011-09-13 14:21 [PATCH] Fix parser of AVRCP continuing messages Lucas De Marchi
2011-09-13 14:40 ` Luiz Augusto von Dentz
@ 2011-09-27 9:58 ` Johan Hedberg
1 sibling, 0 replies; 5+ messages in thread
From: Johan Hedberg @ 2011-09-27 9:58 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: linux-bluetooth, luiz.dentz
Hi Lucas,
On Tue, Sep 13, 2011, Lucas De Marchi wrote:
> If packet_type is not START or SINGLE, we have to continue where we
> stopped from previous packet. Therefore we must store where we left on
> previous packet due to packet size limit. We store both the number of
> attributes missing and the lenght of the last attribute that is missing.
>
> An example interaction for this implementation, obtained with PTS test
> TC_TG_MDI_BV_04_C (I reduced the MTU in order to reproduce it here and
> values between brackets I added now):
>
> > AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
> AV/C: Status: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt Single len 0x0009
> Identifier: 0x0 (PLAYING)
> AttributeCount: 0x00
> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
> AV/C: Stable: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt Start len 0x0118
> AttributeCount: 0x04
> Attribute: 0x00000001 (Title)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x001b
> AttributeValue: isso eh um titulo mei longo
> Attribute: 0x00000003 (Album)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x00fe
> AttributeValue: super-long-album-name super-long-album-name
> super-long-album-name super-long-album-name super-long-album
> super-long-album-name [... snip... ] super-long-album-name-1234
> > AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
> AV/C: Control: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: RequestContinuingResponse: pt Single len 0x0001
> < AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
> AV/C: Stable: address 0x48 opcode 0x00
> Subunit: Panel
> Opcode: Vendor Dependent
> Company ID: 0x001958
> AVRCP: GetElementAttributes: pt End len 0x002a
> ContinuingAttributeValue: 678900000000000000
> Attribute: 0x00000005 (Track Total)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0002
> AttributeValue: 30
> Attribute: 0x00000006 (Genre)
> CharsetID: 0x006a (UTF-8)
> AttributeValueLength: 0x0006
> AttributeValue: Gospel
> ---
> parser/avrcp.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 79 insertions(+), 15 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Fix parser of AVRCP continuing messages
@ 2011-09-09 17:40 Lucas De Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Lucas De Marchi @ 2011-09-09 17:40 UTC (permalink / raw)
To: linux-bluetooth; +Cc: luiz.dentz, Lucas De Marchi
If packet_type is not START or SINGLE, we have to continue where we
stopped from previous packet. Therefore we must store where we left on
previous packet due to packet size limit. We store both the number of
attributes missing and the lenght of the last attribute that is missing.
An example interaction for this implementation, obtained with PTS test
TC_TG_MDI_BV_04_C (I reduced the MTU in order to reproduce it here and
values between brackets I added now):
> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
AV/C: Status: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt Single len 0x0009
Identifier: 0x0 (PLAYING)
AttributeCount: 0x00
< AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
AV/C: Stable: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt Start len 0x0118
AttributeCount: 0x04
Attribute: 0x00000001 (Title)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x001b
AttributeValue: isso eh um titulo mei longo
Attribute: 0x00000003 (Album)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x00fe
AttributeValue: super-long-album-name super-long-album-name
super-long-album-name super-long-album-name super-long-album
super-long-album-name [... snip... ] super-long-album-name-1234
> AVCTP: Command : pt 0x00 transaction 2 pid 0x110e
AV/C: Control: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: RequestContinuingResponse: pt Single len 0x0001
< AVCTP: Response : pt 0x00 transaction 2 pid 0x110e
AV/C: Stable: address 0x48 opcode 0x00
Subunit: Panel
Opcode: Vendor Dependent
Company ID: 0x001958
AVRCP: GetElementAttributes: pt End len 0x002a
ContinuingAttributeValue: 678900000000000000
Attribute: 0x00000005 (Track Total)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0002
AttributeValue: 30
Attribute: 0x00000006 (Genre)
CharsetID: 0x006a (UTF-8)
AttributeValueLength: 0x0006
AttributeValue: Gospel
---
parser/avrcp.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 79 insertions(+), 15 deletions(-)
diff --git a/parser/avrcp.c b/parser/avrcp.c
index 0b98a3e..65d3bda 100644
--- a/parser/avrcp.c
+++ b/parser/avrcp.c
@@ -87,6 +87,12 @@
#define AVC_PANEL_FORWARD 0x4b
#define AVC_PANEL_BACKWARD 0x4c
+/* 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 ids */
#define AVRCP_GET_CAPABILITIES 0x10
#define AVRCP_LIST_PLAYER_ATTRIBUTES 0x11
@@ -176,6 +182,11 @@
#define AVRCP_PLAY_STATUS_REV_SEEK 0x04
#define AVRCP_PLAY_STATUS_ERROR 0xFF
+static struct avrcp_continuing {
+ uint16_t num;
+ uint16_t size;
+} avrcp_continuing;
+
static const char *ctype2str(uint8_t ctype)
{
switch (ctype & 0x0f) {
@@ -224,6 +235,22 @@ static const char *opcode2str(uint8_t opcode)
}
}
+static const char *pt2str(uint8_t pt)
+{
+ switch (pt) {
+ case AVRCP_PACKET_TYPE_SINGLE:
+ return "Single";
+ case AVRCP_PACKET_TYPE_START:
+ return "Start";
+ case AVRCP_PACKET_TYPE_CONTINUING:
+ return "Continuing";
+ case AVRCP_PACKET_TYPE_END:
+ return "End";
+ default:
+ return "Unknown";
+ }
+}
+
static const char *pdu2str(uint8_t pduid)
{
switch (pduid) {
@@ -917,7 +944,8 @@ static const char *mediattr2str(uint32_t attr)
}
static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
- uint8_t ctype, uint16_t len)
+ uint8_t ctype, uint16_t len,
+ uint8_t pt)
{
uint64_t id;
uint8_t num;
@@ -953,18 +981,45 @@ static void avrcp_get_element_attributes_dump(int level, struct frame *frm,
return;
response:
- if (len < 1) {
- printf("PDU Malformed\n");
- raw_dump(level, frm);
- return;
- }
+ if (pt == AVRCP_PACKET_TYPE_SINGLE || pt == AVRCP_PACKET_TYPE_START) {
+ if (len < 1) {
+ printf("PDU Malformed\n");
+ raw_dump(level, frm);
+ return;
+ }
- num = get_u8(frm);
- printf("AttributeCount: 0x%02x\n", num);
+ num = get_u8(frm);
+ avrcp_continuing.num = num;
+ printf("AttributeCount: 0x%02x\n", num);
+ len--;
+ } else {
+ num = avrcp_continuing.num;
+
+ if (avrcp_continuing.size > 0) {
+ uint16_t size;
+
+ if (avrcp_continuing.size > len) {
+ size = len;
+ avrcp_continuing.size -= len;
+ } else {
+ size = avrcp_continuing.size;
+ avrcp_continuing.size = 0;
+ }
+
+ printf("ContinuingAttributeValue: ");
+ for (; size > 0; size--) {
+ uint8_t c = get_u8(frm);
+ printf("%1c", isprint(c) ? c : '.');
+ }
+ printf("\n");
- for (; num > 0; num--) {
+ len -= size;
+ }
+ }
+
+ while (num > 0 && len > 0) {
uint32_t attr;
- uint16_t charset, len;
+ uint16_t charset, attrlen;
p_indent(level, frm);
@@ -978,19 +1033,26 @@ response:
charset2str(charset));
p_indent(level, frm);
+ attrlen = get_u16(frm);
+ printf("AttributeValueLength: 0x%04x\n", attrlen);
- len = get_u16(frm);
- printf("AttributeValueLength: 0x%04x\n", len);
+ len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
+ num--;
p_indent(level, frm);
printf("AttributeValue: ");
- for (; len > 0; len--) {
+ for (; attrlen > 0 && len > 0; attrlen--, len--) {
uint8_t c = get_u8(frm);
printf("%1c", isprint(c) ? c : '.');
}
printf("\n");
+
+ if (attrlen > 0)
+ avrcp_continuing.size = attrlen;
}
+
+ avrcp_continuing.num = num;
}
static const char *playstatus2str(uint8_t status)
@@ -1153,7 +1215,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
pt = get_u8(frm);
len = get_u16(frm);
- printf("AVRCP: %s: pt 0x%02x len 0x%04x\n", pdu2str(pduid), pt, len);
+ printf("AVRCP: %s: pt %s len 0x%04x\n", pdu2str(pduid),
+ pt2str(pt), len);
if (len != frm->len) {
p_indent(level, frm);
@@ -1198,7 +1261,8 @@ static void avrcp_pdu_dump(int level, struct frame *frm, uint8_t ctype)
avrcp_ct_battery_status_dump(level + 1, frm, ctype, len);
break;
case AVRCP_GET_ELEMENT_ATTRIBUTES:
- avrcp_get_element_attributes_dump(level + 1, frm, ctype, len);
+ avrcp_get_element_attributes_dump(level + 1, frm, ctype, len,
+ pt);
break;
case AVRCP_GET_PLAY_STATUS:
avrcp_get_play_status_dump(level + 1, frm, ctype, len);
--
1.7.6.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-09-27 9:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-13 14:21 [PATCH] Fix parser of AVRCP continuing messages Lucas De Marchi
2011-09-13 14:40 ` Luiz Augusto von Dentz
2011-09-13 16:33 ` Lucas De Marchi
2011-09-27 9:58 ` Johan Hedberg
-- strict thread matches above, loose matches on Subject: below --
2011-09-09 17:40 Lucas De Marchi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.