2014-09-12 13:16:56

by Vikrampal Yadav

[permalink] [raw]
Subject: [PATCH v2 1/2] Monitor: Modify design of AVRCP callback functions

Modified the design of AVRCP callback functions.
---
monitor/avctp.c | 103 ++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 66 insertions(+), 37 deletions(-)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index c7e242b..f366b87 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -158,6 +158,13 @@
#define AVRCP_ATTRIBUTE_SHUFFLE 0x03
#define AVRCP_ATTRIBUTE_SCAN 0x04

+struct avctp_frame {
+ uint8_t hdr;
+ uint8_t pt;
+ uint16_t pid;
+ struct l2cap_frame l2cap_frame;
+};
+
static const char *ctype2str(uint8_t ctype)
{
switch (ctype & 0x0f) {
@@ -517,15 +524,19 @@ static const char *charset2str(uint16_t charset)
}
}

-static bool avrcp_passthrough_packet(struct l2cap_frame *frame)
+static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+
packet_hexdump(frame->data, frame->size);
return true;
}

-static bool avrcp_get_capabilities(struct l2cap_frame *frame, uint8_t ctype,
- uint8_t len, uint8_t indent)
+static bool avrcp_get_capabilities(struct avctp_frame *avctp_frame,
+ uint8_t ctype, uint8_t len,
+ uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t cap, count;
int i;

@@ -576,10 +587,11 @@ static bool avrcp_get_capabilities(struct l2cap_frame *frame, uint8_t ctype,
return true;
}

-static bool avrcp_list_player_attributes(struct l2cap_frame *frame,
+static bool avrcp_list_player_attributes(struct avctp_frame *avctp_frame,
uint8_t ctype, uint8_t len,
uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t num;
int i;

@@ -604,9 +616,11 @@ static bool avrcp_list_player_attributes(struct l2cap_frame *frame,
return true;
}

-static bool avrcp_list_player_values(struct l2cap_frame *frame, uint8_t ctype,
- uint8_t len, uint8_t indent)
+static bool avrcp_list_player_values(struct avctp_frame *avctp_frame,
+ uint8_t ctype, uint8_t len,
+ uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
static uint8_t attr = 0;
uint8_t num;

@@ -640,10 +654,11 @@ response:
return true;
}

-static bool avrcp_get_current_player_value(struct l2cap_frame *frame,
+static bool avrcp_get_current_player_value(struct avctp_frame *avctp_frame,
uint8_t ctype, uint8_t len,
uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t num;

if (!l2cap_frame_get_u8(frame, &num))
@@ -688,9 +703,11 @@ response:
return true;
}

-static bool avrcp_set_player_value(struct l2cap_frame *frame, uint8_t ctype,
- uint8_t len, uint8_t indent)
+static bool avrcp_set_player_value(struct avctp_frame *avctp_frame,
+ uint8_t ctype, uint8_t len,
+ uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t num;

if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
@@ -720,10 +737,11 @@ static bool avrcp_set_player_value(struct l2cap_frame *frame, uint8_t ctype,
return true;
}

-static bool avrcp_get_player_attribute_text(struct l2cap_frame *frame,
+static bool avrcp_get_player_attribute_text(struct avctp_frame *avctp_frame,
uint8_t ctype, uint8_t len,
uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t num;

if (!l2cap_frame_get_u8(frame, &num))
@@ -783,10 +801,11 @@ response:
return true;
}

-static bool avrcp_get_player_value_text(struct l2cap_frame *frame,
+static bool avrcp_get_player_value_text(struct avctp_frame *avctp_frame,
uint8_t ctype, uint8_t len,
uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
static uint8_t attr = 0;
uint8_t num;

@@ -858,9 +877,11 @@ response:
return true;
}

-static bool avrcp_displayable_charset(struct l2cap_frame *frame, uint8_t ctype,
- uint8_t len, uint8_t indent)
+static bool avrcp_displayable_charset(struct avctp_frame *avctp_frame,
+ uint8_t ctype, uint8_t len,
+ uint8_t indent)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint8_t num;

if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
@@ -886,8 +907,8 @@ static bool avrcp_displayable_charset(struct l2cap_frame *frame, uint8_t ctype,

struct avrcp_ctrl_pdu_data {
uint8_t pduid;
- bool (*func) (struct l2cap_frame *frame, uint8_t ctype, uint8_t len,
- uint8_t indent);
+ bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
+ uint8_t len, uint8_t indent);
};

static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
@@ -915,10 +936,11 @@ static bool avrcp_rejected_packet(struct l2cap_frame *frame, uint8_t indent)
return true;
}

-static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
+static bool avrcp_pdu_packet(struct avctp_frame *avctp_frame, uint8_t ctype,
uint8_t indent)
{
- uint8_t pduid, pt;
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+ uint8_t pduid;
uint16_t len;
int i;
const struct avrcp_ctrl_pdu_data *ctrl_pdu_data = NULL;
@@ -926,14 +948,14 @@ static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
if (!l2cap_frame_get_u8(frame, &pduid))
return false;

- if (!l2cap_frame_get_u8(frame, &pt))
+ if (!l2cap_frame_get_u8(frame, &avctp_frame->pt))
return false;

if (!l2cap_frame_get_be16(frame, &len))
return false;

print_indent(indent, COLOR_OFF, "AVRCP: ", pdu2str(pduid), COLOR_OFF,
- " pt %s len 0x%04x", pt2str(pt), len);
+ " pt %s len 0x%04x", pt2str(avctp_frame->pt), len);

if (frame->size != len)
return false;
@@ -953,11 +975,13 @@ static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
return true;
}

- return ctrl_pdu_data->func(frame, ctype, len, indent + 2);
+ return ctrl_pdu_data->func(avctp_frame, ctype, len, indent + 2);
}

-static bool avrcp_control_packet(struct l2cap_frame *frame)
+static bool avrcp_control_packet(struct avctp_frame *avctp_frame)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+
uint8_t ctype, address, subunit, opcode, company[3], indent = 2;

if (!l2cap_frame_get_u8(frame, &ctype) ||
@@ -988,7 +1012,7 @@ static bool avrcp_control_packet(struct l2cap_frame *frame)

switch (opcode) {
case 0x7c:
- return avrcp_passthrough_packet(frame);
+ return avrcp_passthrough_packet(avctp_frame);
case 0x00:
if (!l2cap_frame_get_u8(frame, &company[0]) ||
!l2cap_frame_get_u8(frame, &company[1]) ||
@@ -998,29 +1022,32 @@ static bool avrcp_control_packet(struct l2cap_frame *frame)
print_field("%*cCompany ID: 0x%02x%02x%02x", indent, ' ',
company[0], company[1], company[2]);

- return avrcp_pdu_packet(frame, ctype, 10);
+ return avrcp_pdu_packet(avctp_frame, ctype, 10);
default:
packet_hexdump(frame->data, frame->size);
return true;
}
}

-static bool avrcp_browsing_packet(struct l2cap_frame *frame, uint8_t hdr)
+static bool avrcp_browsing_packet(struct avctp_frame *avctp_frame)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+
packet_hexdump(frame->data, frame->size);
return true;
}

-static void avrcp_packet(struct l2cap_frame *frame, uint8_t hdr)
+static void avrcp_packet(struct avctp_frame *avctp_frame)
{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
bool ret;

switch (frame->psm) {
case 0x17:
- ret = avrcp_control_packet(frame);
+ ret = avrcp_control_packet(avctp_frame);
break;
case 0x1B:
- ret = avrcp_browsing_packet(frame, hdr);
+ ret = avrcp_browsing_packet(avctp_frame);
break;
default:
packet_hexdump(frame->data, frame->size);
@@ -1035,15 +1062,16 @@ static void avrcp_packet(struct l2cap_frame *frame, uint8_t hdr)

void avctp_packet(const struct l2cap_frame *frame)
{
- uint8_t hdr;
- uint16_t pid;
- struct l2cap_frame avctp_frame;
+ struct l2cap_frame *l2cap_frame;
+ struct avctp_frame avctp_frame;
const char *pdu_color;

- l2cap_frame_pull(&avctp_frame, frame, 0);
+ l2cap_frame_pull(&avctp_frame.l2cap_frame, frame, 0);
+
+ l2cap_frame = &avctp_frame.l2cap_frame;

- if (!l2cap_frame_get_u8(&avctp_frame, &hdr) ||
- !l2cap_frame_get_be16(&avctp_frame, &pid)) {
+ if (!l2cap_frame_get_u8(l2cap_frame, &avctp_frame.hdr) ||
+ !l2cap_frame_get_be16(l2cap_frame, &avctp_frame.pid)) {
print_text(COLOR_ERROR, "frame too short");
packet_hexdump(frame->data, frame->size);
return;
@@ -1057,11 +1085,12 @@ void avctp_packet(const struct l2cap_frame *frame)
print_indent(6, pdu_color, "AVCTP", "", COLOR_OFF,
" %s: %s: type 0x%02x label %d PID 0x%04x",
frame->psm == 23 ? "Control" : "Browsing",
- hdr & 0x02 ? "Response" : "Command",
- hdr & 0x0c, hdr >> 4, pid);
+ avctp_frame.hdr & 0x02 ? "Response" : "Command",
+ avctp_frame.hdr & 0x0c, avctp_frame.hdr >> 4,
+ avctp_frame.pid);

- if (pid == 0x110e || pid == 0x110c)
- avrcp_packet(&avctp_frame, hdr);
+ if (avctp_frame.pid == 0x110e || avctp_frame.pid == 0x110c)
+ avrcp_packet(&avctp_frame);
else
packet_hexdump(frame->data, frame->size);
}
--
1.9.1



2014-09-15 10:32:53

by Vikrampal Yadav

[permalink] [raw]
Subject: RE: [PATCH v2 2/2] Monitor: Add AVRCP GetElementAttributes support

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:[email protected]]
> Sent: Monday, September 15, 2014 2:21 PM
> To: Vikrampal Yadav
> Cc: [email protected]; Dmitry Kasatkin; [email protected]
> Subject: Re: [PATCH v2 2/2] Monitor: Add AVRCP GetElementAttributes
> support
>
> Hi Vikram,
>
> On Mon, Sep 15, 2014 at 11:30 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > Hi Vikram,
> >
> > On Fri, Sep 12, 2014 at 4:16 PM, Vikrampal Yadav
> <[email protected]> wrote:
> >> Support for decoding AVRCP GetElementAttributes added in Bluetooth
> >> monitor.
> >> ---
> >> monitor/avctp.c | 157
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 157 insertions(+)
> >>
> >> diff --git a/monitor/avctp.c b/monitor/avctp.c index f366b87..89997d0
> >> 100644
> >> --- a/monitor/avctp.c
> >> +++ b/monitor/avctp.c
> >> @@ -158,6 +158,16 @@
> >> #define AVRCP_ATTRIBUTE_SHUFFLE 0x03
> >> #define AVRCP_ATTRIBUTE_SCAN 0x04
> >>
> >> +/* media attributes */
> >> +#define AVRCP_MEDIA_ATTRIBUTE_ILLEGAL 0x00
> >> +#define AVRCP_MEDIA_ATTRIBUTE_TITLE 0x01
> >> +#define AVRCP_MEDIA_ATTRIBUTE_ARTIST 0x02
> >> +#define AVRCP_MEDIA_ATTRIBUTE_ALBUM 0x03
> >> +#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x04
> >> +#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x05
> >> +#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x06
> >> +#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x07
> >> +
> >> struct avctp_frame {
> >> uint8_t hdr;
> >> uint8_t pt;
> >> @@ -165,6 +175,11 @@ struct avctp_frame {
> >> struct l2cap_frame l2cap_frame; };
> >>
> >> +static struct avrcp_continuing {
> >> + uint16_t num;
> >> + uint16_t size;
> >> +} avrcp_continuing;
> >> +
> >> static const char *ctype2str(uint8_t ctype) {
> >> switch (ctype & 0x0f) {
> >> @@ -524,6 +539,30 @@ static const char *charset2str(uint16_t charset)
> >> }
> >> }
> >>
> >> +static const char *mediattr2str(uint32_t attr) {
> >> + switch (attr) {
> >> + case AVRCP_MEDIA_ATTRIBUTE_ILLEGAL:
> >> + return "Illegal";
> >> + case AVRCP_MEDIA_ATTRIBUTE_TITLE:
> >> + return "Title";
> >> + case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
> >> + return "Artist";
> >> + case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
> >> + return "Album";
> >> + case AVRCP_MEDIA_ATTRIBUTE_TRACK:
> >> + return "Track";
> >> + case AVRCP_MEDIA_ATTRIBUTE_TOTAL:
> >> + return "Track Total";
> >> + case AVRCP_MEDIA_ATTRIBUTE_GENRE:
> >> + return "Genre";
> >> + case AVRCP_MEDIA_ATTRIBUTE_DURATION:
> >> + return "Track duration";
> >> + default:
> >> + return "Reserved";
> >> + }
> >> +}
> >> +
> >> static bool avrcp_passthrough_packet(struct avctp_frame
> >> *avctp_frame) {
> >> struct l2cap_frame *frame = &avctp_frame->l2cap_frame; @@
> >> -905,6 +944,123 @@ static bool avrcp_displayable_charset(struct
> avctp_frame *avctp_frame,
> >> return true;
> >> }
> >>
> >> +static bool avrcp_get_element_attributes(struct avctp_frame
> *avctp_frame,
> >> + uint8_t ctype, uint8_t len,
> >> + uint8_t indent) {
> >> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> >> + uint64_t id;
> >> + uint8_t num;
> >> +
> >> + if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
> >> + goto response;
> >> +
> >> + if (!l2cap_frame_get_be64(frame, &id))
> >> + return false;
> >> +
> >> + print_field("%*cIdentifier: 0x%jx (%s)", (indent - 8), ' ',
> >> + id, id ? "Reserved" :
> >> + "PLAYING");
> >> +
> >> + if (!l2cap_frame_get_u8(frame, &num))
> >> + return false;
> >> +
> >> + print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ',
> >> + num);
> >> +
> >> + for (; num > 0; num--) {
> >> + uint32_t attr;
> >> +
> >> + if (!l2cap_frame_get_be32(frame, &attr))
> >> + return false;
> >> +
> >> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
> >> + ' ', attr, mediattr2str(attr));
> >> + }
> >> +
> >> + return true;
> >> +
> >> +response:
> >> + if (avctp_frame->pt == AVRCP_PACKET_TYPE_SINGLE
> >> + || avctp_frame->pt == AVRCP_PACKET_TYPE_START) {
> >> + if (!l2cap_frame_get_u8(frame, &num))
> >> + return false;
> >> +
> >> + avrcp_continuing.num = num;
> >> + print_field("%*cAttributeCount: 0x%02x", (indent - 8),
> >> + ' ', 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;
> >> +
> >> + if (!l2cap_frame_get_u8(frame, &c))
> >> + return false;
> >> +
> >> + printf("%1c", isprint(c) ? c : '.');
> >> + }
> >> + printf("\n");
> >> +
> >> + len -= size;
> >> + }
> >> + }
> >
> > I would handle this with a switch and I need to double check but I
> > don't think it is valid to fragment on attribute level.
>
> Seems to be a valid according to spec.
>
> >
> >> + while (num > 0 && len > 0) {
> >> + uint32_t attr;
> >> + uint16_t charset, attrlen;
> >> +
> >> + if (!l2cap_frame_get_be32(frame, &attr))
> >> + return false;
> >> +
> >> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
> >> + ' ', attr,
> >> + mediattr2str(attr));
> >> +
> >> + if (!l2cap_frame_get_be16(frame, &charset))
> >> + return false;
> >> +
> >> + print_field("%*cCharsetID: 0x%04x (%s)", (indent - 8),
> >> + ' ', charset, charset2str(charset));
> >> +
> >> + if (!l2cap_frame_get_be16(frame, &attrlen))
> >> + return false;
> >> +
> >> + print_field("%*cAttributeValueLength: 0x%04x",
> >> + (indent - 8), ' ',
> >> + attrlen);
> >> +
> >> + len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> >> + num--;
> >> +
> >> + print_field("%*cAttributeValue: ", (indent - 8), ' ');
> >> + for (; attrlen > 0 && len > 0; attrlen--, len--) {
> >> + uint8_t c;
> >> +
> >> + if (!l2cap_frame_get_u8(frame, &c))
> >> + return false;
> >> +
> >> + printf("%1c", isprint(c) ? c : '.');
> >> + }
> >> +
> >> + if (attrlen > 0)
> >> + avrcp_continuing.size = attrlen;
> >> + }
> >> +
> >> + avrcp_continuing.num = num;
> >> +
> >> + return true;
> >> +}
> >> +
> >> struct avrcp_ctrl_pdu_data {
> >> uint8_t pduid;
> >> bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
> >> @@ -920,6 +1076,7 @@ static const struct avrcp_ctrl_pdu_data
> avrcp_ctrl_pdu_table[] = {
> >> { 0x15, avrcp_get_player_attribute_text },
> >> { 0x16, avrcp_get_player_value_text },
> >> { 0x17, avrcp_displayable_charset },
> >> + { 0x20, avrcp_get_element_attributes },
> >> { }
> >> };
> >>
> >> --
> >> 1.9.1
> >>
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentzv

The new function after your suggestion seems like:

static bool avrcp_get_element_attributes(struct avctp_frame *avctp_frame,
uint8_t ctype, uint8_t len,
uint8_t indent)
{
struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
uint64_t id;
uint8_t num;

if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
goto response;

if (!l2cap_frame_get_be64(frame, &id))
return false;

print_field("%*cIdentifier: 0x%jx (%s)", (indent - 8), ' ',
id, id ? "Reserved" : "PLAYING");

if (!l2cap_frame_get_u8(frame, &num))
return false;

print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);

for (; num > 0; num--) {
uint32_t attr;

if (!l2cap_frame_get_be32(frame, &attr))
return false;

print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
' ', attr, mediattr2str(attr));
}

return true;

response:
switch (avctp_frame->pt) {
case AVRCP_PACKET_TYPE_SINGLE:
case AVRCP_PACKET_TYPE_START:
if (!l2cap_frame_get_u8(frame, &num))
return false;

avrcp_continuing.num = num;
print_field("%*cAttributeCount: 0x%02x", (indent - 8),
' ', num);
len--;
break;
case AVRCP_PACKET_TYPE_CONTINUING:
case AVRCP_PACKET_TYPE_END:
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;

if (!l2cap_frame_get_u8(frame, &c))
return false;

printf("%1c", isprint(c) ? c : '.');
}
printf("\n");

len -= size;
}
break;
default:
return false;
}

while (num > 0 && len > 0) {
uint32_t attr;
uint16_t charset, attrlen;

if (!l2cap_frame_get_be32(frame, &attr))
return false;

print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
' ', attr, mediattr2str(attr));

if (!l2cap_frame_get_be16(frame, &charset))
return false;

print_field("%*cCharsetID: 0x%04x (%s)", (indent - 8),
' ', charset, charset2str(charset));

if (!l2cap_frame_get_be16(frame, &attrlen))
return false;

print_field("%*cAttributeValueLength: 0x%04x",
(indent - 8), ' ', attrlen);

len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
num--;

print_field("%*cAttributeValue: ", (indent - 8), ' ');
for (; attrlen > 0 && len > 0; attrlen--, len--) {
uint8_t c;

if (!l2cap_frame_get_u8(frame, &c))
return false;

printf("%1c", isprint(c) ? c : '.');
}

if (attrlen > 0)
avrcp_continuing.size = attrlen;
}

avrcp_continuing.num = num;

return true;
}

Regards,
Vikram


2014-09-15 08:51:23

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] Monitor: Modify design of AVRCP callback functions

Hi,

On Fri, Sep 12, 2014 at 4:16 PM, Vikrampal Yadav <[email protected]> wrote:
> Modified the design of AVRCP callback functions.
> ---
> monitor/avctp.c | 103 ++++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 66 insertions(+), 37 deletions(-)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index c7e242b..f366b87 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -158,6 +158,13 @@
> #define AVRCP_ATTRIBUTE_SHUFFLE 0x03
> #define AVRCP_ATTRIBUTE_SCAN 0x04
>
> +struct avctp_frame {
> + uint8_t hdr;
> + uint8_t pt;
> + uint16_t pid;
> + struct l2cap_frame l2cap_frame;
> +};
> +
> static const char *ctype2str(uint8_t ctype)
> {
> switch (ctype & 0x0f) {
> @@ -517,15 +524,19 @@ static const char *charset2str(uint16_t charset)
> }
> }
>
> -static bool avrcp_passthrough_packet(struct l2cap_frame *frame)
> +static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> +
> packet_hexdump(frame->data, frame->size);
> return true;
> }
>
> -static bool avrcp_get_capabilities(struct l2cap_frame *frame, uint8_t ctype,
> - uint8_t len, uint8_t indent)
> +static bool avrcp_get_capabilities(struct avctp_frame *avctp_frame,
> + uint8_t ctype, uint8_t len,
> + uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t cap, count;
> int i;
>
> @@ -576,10 +587,11 @@ static bool avrcp_get_capabilities(struct l2cap_frame *frame, uint8_t ctype,
> return true;
> }
>
> -static bool avrcp_list_player_attributes(struct l2cap_frame *frame,
> +static bool avrcp_list_player_attributes(struct avctp_frame *avctp_frame,
> uint8_t ctype, uint8_t len,
> uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t num;
> int i;
>
> @@ -604,9 +616,11 @@ static bool avrcp_list_player_attributes(struct l2cap_frame *frame,
> return true;
> }
>
> -static bool avrcp_list_player_values(struct l2cap_frame *frame, uint8_t ctype,
> - uint8_t len, uint8_t indent)
> +static bool avrcp_list_player_values(struct avctp_frame *avctp_frame,
> + uint8_t ctype, uint8_t len,
> + uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> static uint8_t attr = 0;
> uint8_t num;
>
> @@ -640,10 +654,11 @@ response:
> return true;
> }
>
> -static bool avrcp_get_current_player_value(struct l2cap_frame *frame,
> +static bool avrcp_get_current_player_value(struct avctp_frame *avctp_frame,
> uint8_t ctype, uint8_t len,
> uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t num;
>
> if (!l2cap_frame_get_u8(frame, &num))
> @@ -688,9 +703,11 @@ response:
> return true;
> }
>
> -static bool avrcp_set_player_value(struct l2cap_frame *frame, uint8_t ctype,
> - uint8_t len, uint8_t indent)
> +static bool avrcp_set_player_value(struct avctp_frame *avctp_frame,
> + uint8_t ctype, uint8_t len,
> + uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t num;
>
> if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
> @@ -720,10 +737,11 @@ static bool avrcp_set_player_value(struct l2cap_frame *frame, uint8_t ctype,
> return true;
> }
>
> -static bool avrcp_get_player_attribute_text(struct l2cap_frame *frame,
> +static bool avrcp_get_player_attribute_text(struct avctp_frame *avctp_frame,
> uint8_t ctype, uint8_t len,
> uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t num;
>
> if (!l2cap_frame_get_u8(frame, &num))
> @@ -783,10 +801,11 @@ response:
> return true;
> }
>
> -static bool avrcp_get_player_value_text(struct l2cap_frame *frame,
> +static bool avrcp_get_player_value_text(struct avctp_frame *avctp_frame,
> uint8_t ctype, uint8_t len,
> uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> static uint8_t attr = 0;
> uint8_t num;
>
> @@ -858,9 +877,11 @@ response:
> return true;
> }
>
> -static bool avrcp_displayable_charset(struct l2cap_frame *frame, uint8_t ctype,
> - uint8_t len, uint8_t indent)
> +static bool avrcp_displayable_charset(struct avctp_frame *avctp_frame,
> + uint8_t ctype, uint8_t len,
> + uint8_t indent)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> uint8_t num;
>
> if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
> @@ -886,8 +907,8 @@ static bool avrcp_displayable_charset(struct l2cap_frame *frame, uint8_t ctype,
>
> struct avrcp_ctrl_pdu_data {
> uint8_t pduid;
> - bool (*func) (struct l2cap_frame *frame, uint8_t ctype, uint8_t len,
> - uint8_t indent);
> + bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
> + uint8_t len, uint8_t indent);
> };
>
> static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
> @@ -915,10 +936,11 @@ static bool avrcp_rejected_packet(struct l2cap_frame *frame, uint8_t indent)
> return true;
> }
>
> -static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
> +static bool avrcp_pdu_packet(struct avctp_frame *avctp_frame, uint8_t ctype,
> uint8_t indent)
> {
> - uint8_t pduid, pt;
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> + uint8_t pduid;
> uint16_t len;
> int i;
> const struct avrcp_ctrl_pdu_data *ctrl_pdu_data = NULL;
> @@ -926,14 +948,14 @@ static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
> if (!l2cap_frame_get_u8(frame, &pduid))
> return false;
>
> - if (!l2cap_frame_get_u8(frame, &pt))
> + if (!l2cap_frame_get_u8(frame, &avctp_frame->pt))
> return false;
>
> if (!l2cap_frame_get_be16(frame, &len))
> return false;
>
> print_indent(indent, COLOR_OFF, "AVRCP: ", pdu2str(pduid), COLOR_OFF,
> - " pt %s len 0x%04x", pt2str(pt), len);
> + " pt %s len 0x%04x", pt2str(avctp_frame->pt), len);
>
> if (frame->size != len)
> return false;
> @@ -953,11 +975,13 @@ static bool avrcp_pdu_packet(struct l2cap_frame *frame, uint8_t ctype,
> return true;
> }
>
> - return ctrl_pdu_data->func(frame, ctype, len, indent + 2);
> + return ctrl_pdu_data->func(avctp_frame, ctype, len, indent + 2);
> }
>
> -static bool avrcp_control_packet(struct l2cap_frame *frame)
> +static bool avrcp_control_packet(struct avctp_frame *avctp_frame)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> +
> uint8_t ctype, address, subunit, opcode, company[3], indent = 2;
>
> if (!l2cap_frame_get_u8(frame, &ctype) ||
> @@ -988,7 +1012,7 @@ static bool avrcp_control_packet(struct l2cap_frame *frame)
>
> switch (opcode) {
> case 0x7c:
> - return avrcp_passthrough_packet(frame);
> + return avrcp_passthrough_packet(avctp_frame);
> case 0x00:
> if (!l2cap_frame_get_u8(frame, &company[0]) ||
> !l2cap_frame_get_u8(frame, &company[1]) ||
> @@ -998,29 +1022,32 @@ static bool avrcp_control_packet(struct l2cap_frame *frame)
> print_field("%*cCompany ID: 0x%02x%02x%02x", indent, ' ',
> company[0], company[1], company[2]);
>
> - return avrcp_pdu_packet(frame, ctype, 10);
> + return avrcp_pdu_packet(avctp_frame, ctype, 10);
> default:
> packet_hexdump(frame->data, frame->size);
> return true;
> }
> }
>
> -static bool avrcp_browsing_packet(struct l2cap_frame *frame, uint8_t hdr)
> +static bool avrcp_browsing_packet(struct avctp_frame *avctp_frame)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> +
> packet_hexdump(frame->data, frame->size);
> return true;
> }
>
> -static void avrcp_packet(struct l2cap_frame *frame, uint8_t hdr)
> +static void avrcp_packet(struct avctp_frame *avctp_frame)
> {
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> bool ret;
>
> switch (frame->psm) {
> case 0x17:
> - ret = avrcp_control_packet(frame);
> + ret = avrcp_control_packet(avctp_frame);
> break;
> case 0x1B:
> - ret = avrcp_browsing_packet(frame, hdr);
> + ret = avrcp_browsing_packet(avctp_frame);
> break;
> default:
> packet_hexdump(frame->data, frame->size);
> @@ -1035,15 +1062,16 @@ static void avrcp_packet(struct l2cap_frame *frame, uint8_t hdr)
>
> void avctp_packet(const struct l2cap_frame *frame)
> {
> - uint8_t hdr;
> - uint16_t pid;
> - struct l2cap_frame avctp_frame;
> + struct l2cap_frame *l2cap_frame;
> + struct avctp_frame avctp_frame;
> const char *pdu_color;
>
> - l2cap_frame_pull(&avctp_frame, frame, 0);
> + l2cap_frame_pull(&avctp_frame.l2cap_frame, frame, 0);
> +
> + l2cap_frame = &avctp_frame.l2cap_frame;
>
> - if (!l2cap_frame_get_u8(&avctp_frame, &hdr) ||
> - !l2cap_frame_get_be16(&avctp_frame, &pid)) {
> + if (!l2cap_frame_get_u8(l2cap_frame, &avctp_frame.hdr) ||
> + !l2cap_frame_get_be16(l2cap_frame, &avctp_frame.pid)) {
> print_text(COLOR_ERROR, "frame too short");
> packet_hexdump(frame->data, frame->size);
> return;
> @@ -1057,11 +1085,12 @@ void avctp_packet(const struct l2cap_frame *frame)
> print_indent(6, pdu_color, "AVCTP", "", COLOR_OFF,
> " %s: %s: type 0x%02x label %d PID 0x%04x",
> frame->psm == 23 ? "Control" : "Browsing",
> - hdr & 0x02 ? "Response" : "Command",
> - hdr & 0x0c, hdr >> 4, pid);
> + avctp_frame.hdr & 0x02 ? "Response" : "Command",
> + avctp_frame.hdr & 0x0c, avctp_frame.hdr >> 4,
> + avctp_frame.pid);
>
> - if (pid == 0x110e || pid == 0x110c)
> - avrcp_packet(&avctp_frame, hdr);
> + if (avctp_frame.pid == 0x110e || avctp_frame.pid == 0x110c)
> + avrcp_packet(&avctp_frame);
> else
> packet_hexdump(frame->data, frame->size);
> }
> --
> 1.9.1

Pushed, thanks.


--
Luiz Augusto von Dentz

2014-09-15 08:50:44

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Monitor: Add AVRCP GetElementAttributes support

Hi Vikram,

On Mon, Sep 15, 2014 at 11:30 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Vikram,
>
> On Fri, Sep 12, 2014 at 4:16 PM, Vikrampal Yadav <[email protected]> wrote:
>> Support for decoding AVRCP GetElementAttributes added in Bluetooth
>> monitor.
>> ---
>> monitor/avctp.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 157 insertions(+)
>>
>> diff --git a/monitor/avctp.c b/monitor/avctp.c
>> index f366b87..89997d0 100644
>> --- a/monitor/avctp.c
>> +++ b/monitor/avctp.c
>> @@ -158,6 +158,16 @@
>> #define AVRCP_ATTRIBUTE_SHUFFLE 0x03
>> #define AVRCP_ATTRIBUTE_SCAN 0x04
>>
>> +/* media attributes */
>> +#define AVRCP_MEDIA_ATTRIBUTE_ILLEGAL 0x00
>> +#define AVRCP_MEDIA_ATTRIBUTE_TITLE 0x01
>> +#define AVRCP_MEDIA_ATTRIBUTE_ARTIST 0x02
>> +#define AVRCP_MEDIA_ATTRIBUTE_ALBUM 0x03
>> +#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x04
>> +#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x05
>> +#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x06
>> +#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x07
>> +
>> struct avctp_frame {
>> uint8_t hdr;
>> uint8_t pt;
>> @@ -165,6 +175,11 @@ struct avctp_frame {
>> struct l2cap_frame l2cap_frame;
>> };
>>
>> +static struct avrcp_continuing {
>> + uint16_t num;
>> + uint16_t size;
>> +} avrcp_continuing;
>> +
>> static const char *ctype2str(uint8_t ctype)
>> {
>> switch (ctype & 0x0f) {
>> @@ -524,6 +539,30 @@ static const char *charset2str(uint16_t charset)
>> }
>> }
>>
>> +static const char *mediattr2str(uint32_t attr)
>> +{
>> + switch (attr) {
>> + case AVRCP_MEDIA_ATTRIBUTE_ILLEGAL:
>> + return "Illegal";
>> + case AVRCP_MEDIA_ATTRIBUTE_TITLE:
>> + return "Title";
>> + case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
>> + return "Artist";
>> + case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
>> + return "Album";
>> + case AVRCP_MEDIA_ATTRIBUTE_TRACK:
>> + return "Track";
>> + case AVRCP_MEDIA_ATTRIBUTE_TOTAL:
>> + return "Track Total";
>> + case AVRCP_MEDIA_ATTRIBUTE_GENRE:
>> + return "Genre";
>> + case AVRCP_MEDIA_ATTRIBUTE_DURATION:
>> + return "Track duration";
>> + default:
>> + return "Reserved";
>> + }
>> +}
>> +
>> static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame)
>> {
>> struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
>> @@ -905,6 +944,123 @@ static bool avrcp_displayable_charset(struct avctp_frame *avctp_frame,
>> return true;
>> }
>>
>> +static bool avrcp_get_element_attributes(struct avctp_frame *avctp_frame,
>> + uint8_t ctype, uint8_t len,
>> + uint8_t indent)
>> +{
>> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
>> + uint64_t id;
>> + uint8_t num;
>> +
>> + if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
>> + goto response;
>> +
>> + if (!l2cap_frame_get_be64(frame, &id))
>> + return false;
>> +
>> + print_field("%*cIdentifier: 0x%jx (%s)", (indent - 8), ' ',
>> + id, id ? "Reserved" : "PLAYING");
>> +
>> + if (!l2cap_frame_get_u8(frame, &num))
>> + return false;
>> +
>> + print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
>> +
>> + for (; num > 0; num--) {
>> + uint32_t attr;
>> +
>> + if (!l2cap_frame_get_be32(frame, &attr))
>> + return false;
>> +
>> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
>> + ' ', attr, mediattr2str(attr));
>> + }
>> +
>> + return true;
>> +
>> +response:
>> + if (avctp_frame->pt == AVRCP_PACKET_TYPE_SINGLE
>> + || avctp_frame->pt == AVRCP_PACKET_TYPE_START) {
>> + if (!l2cap_frame_get_u8(frame, &num))
>> + return false;
>> +
>> + avrcp_continuing.num = num;
>> + print_field("%*cAttributeCount: 0x%02x", (indent - 8),
>> + ' ', 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;
>> +
>> + if (!l2cap_frame_get_u8(frame, &c))
>> + return false;
>> +
>> + printf("%1c", isprint(c) ? c : '.');
>> + }
>> + printf("\n");
>> +
>> + len -= size;
>> + }
>> + }
>
> I would handle this with a switch and I need to double check but I
> don't think it is valid to fragment on attribute level.

Seems to be a valid according to spec.

>
>> + while (num > 0 && len > 0) {
>> + uint32_t attr;
>> + uint16_t charset, attrlen;
>> +
>> + if (!l2cap_frame_get_be32(frame, &attr))
>> + return false;
>> +
>> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
>> + ' ', attr, mediattr2str(attr));
>> +
>> + if (!l2cap_frame_get_be16(frame, &charset))
>> + return false;
>> +
>> + print_field("%*cCharsetID: 0x%04x (%s)", (indent - 8),
>> + ' ', charset, charset2str(charset));
>> +
>> + if (!l2cap_frame_get_be16(frame, &attrlen))
>> + return false;
>> +
>> + print_field("%*cAttributeValueLength: 0x%04x",
>> + (indent - 8), ' ', attrlen);
>> +
>> + len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
>> + num--;
>> +
>> + print_field("%*cAttributeValue: ", (indent - 8), ' ');
>> + for (; attrlen > 0 && len > 0; attrlen--, len--) {
>> + uint8_t c;
>> +
>> + if (!l2cap_frame_get_u8(frame, &c))
>> + return false;
>> +
>> + printf("%1c", isprint(c) ? c : '.');
>> + }
>> +
>> + if (attrlen > 0)
>> + avrcp_continuing.size = attrlen;
>> + }
>> +
>> + avrcp_continuing.num = num;
>> +
>> + return true;
>> +}
>> +
>> struct avrcp_ctrl_pdu_data {
>> uint8_t pduid;
>> bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
>> @@ -920,6 +1076,7 @@ static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
>> { 0x15, avrcp_get_player_attribute_text },
>> { 0x16, avrcp_get_player_value_text },
>> { 0x17, avrcp_displayable_charset },
>> + { 0x20, avrcp_get_element_attributes },
>> { }
>> };
>>
>> --
>> 1.9.1
>>
>
>
>
> --
> Luiz Augusto von Dentz



--
Luiz Augusto von Dentz

2014-09-15 08:30:32

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] Monitor: Add AVRCP GetElementAttributes support

Hi Vikram,

On Fri, Sep 12, 2014 at 4:16 PM, Vikrampal Yadav <[email protected]> wrote:
> Support for decoding AVRCP GetElementAttributes added in Bluetooth
> monitor.
> ---
> monitor/avctp.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 157 insertions(+)
>
> diff --git a/monitor/avctp.c b/monitor/avctp.c
> index f366b87..89997d0 100644
> --- a/monitor/avctp.c
> +++ b/monitor/avctp.c
> @@ -158,6 +158,16 @@
> #define AVRCP_ATTRIBUTE_SHUFFLE 0x03
> #define AVRCP_ATTRIBUTE_SCAN 0x04
>
> +/* media attributes */
> +#define AVRCP_MEDIA_ATTRIBUTE_ILLEGAL 0x00
> +#define AVRCP_MEDIA_ATTRIBUTE_TITLE 0x01
> +#define AVRCP_MEDIA_ATTRIBUTE_ARTIST 0x02
> +#define AVRCP_MEDIA_ATTRIBUTE_ALBUM 0x03
> +#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x04
> +#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x05
> +#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x06
> +#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x07
> +
> struct avctp_frame {
> uint8_t hdr;
> uint8_t pt;
> @@ -165,6 +175,11 @@ struct avctp_frame {
> struct l2cap_frame l2cap_frame;
> };
>
> +static struct avrcp_continuing {
> + uint16_t num;
> + uint16_t size;
> +} avrcp_continuing;
> +
> static const char *ctype2str(uint8_t ctype)
> {
> switch (ctype & 0x0f) {
> @@ -524,6 +539,30 @@ static const char *charset2str(uint16_t charset)
> }
> }
>
> +static const char *mediattr2str(uint32_t attr)
> +{
> + switch (attr) {
> + case AVRCP_MEDIA_ATTRIBUTE_ILLEGAL:
> + return "Illegal";
> + case AVRCP_MEDIA_ATTRIBUTE_TITLE:
> + return "Title";
> + case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
> + return "Artist";
> + case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
> + return "Album";
> + case AVRCP_MEDIA_ATTRIBUTE_TRACK:
> + return "Track";
> + case AVRCP_MEDIA_ATTRIBUTE_TOTAL:
> + return "Track Total";
> + case AVRCP_MEDIA_ATTRIBUTE_GENRE:
> + return "Genre";
> + case AVRCP_MEDIA_ATTRIBUTE_DURATION:
> + return "Track duration";
> + default:
> + return "Reserved";
> + }
> +}
> +
> static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame)
> {
> struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> @@ -905,6 +944,123 @@ static bool avrcp_displayable_charset(struct avctp_frame *avctp_frame,
> return true;
> }
>
> +static bool avrcp_get_element_attributes(struct avctp_frame *avctp_frame,
> + uint8_t ctype, uint8_t len,
> + uint8_t indent)
> +{
> + struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
> + uint64_t id;
> + uint8_t num;
> +
> + if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
> + goto response;
> +
> + if (!l2cap_frame_get_be64(frame, &id))
> + return false;
> +
> + print_field("%*cIdentifier: 0x%jx (%s)", (indent - 8), ' ',
> + id, id ? "Reserved" : "PLAYING");
> +
> + if (!l2cap_frame_get_u8(frame, &num))
> + return false;
> +
> + print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
> +
> + for (; num > 0; num--) {
> + uint32_t attr;
> +
> + if (!l2cap_frame_get_be32(frame, &attr))
> + return false;
> +
> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
> + ' ', attr, mediattr2str(attr));
> + }
> +
> + return true;
> +
> +response:
> + if (avctp_frame->pt == AVRCP_PACKET_TYPE_SINGLE
> + || avctp_frame->pt == AVRCP_PACKET_TYPE_START) {
> + if (!l2cap_frame_get_u8(frame, &num))
> + return false;
> +
> + avrcp_continuing.num = num;
> + print_field("%*cAttributeCount: 0x%02x", (indent - 8),
> + ' ', 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;
> +
> + if (!l2cap_frame_get_u8(frame, &c))
> + return false;
> +
> + printf("%1c", isprint(c) ? c : '.');
> + }
> + printf("\n");
> +
> + len -= size;
> + }
> + }

I would handle this with a switch and I need to double check but I
don't think it is valid to fragment on attribute level.


> + while (num > 0 && len > 0) {
> + uint32_t attr;
> + uint16_t charset, attrlen;
> +
> + if (!l2cap_frame_get_be32(frame, &attr))
> + return false;
> +
> + print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
> + ' ', attr, mediattr2str(attr));
> +
> + if (!l2cap_frame_get_be16(frame, &charset))
> + return false;
> +
> + print_field("%*cCharsetID: 0x%04x (%s)", (indent - 8),
> + ' ', charset, charset2str(charset));
> +
> + if (!l2cap_frame_get_be16(frame, &attrlen))
> + return false;
> +
> + print_field("%*cAttributeValueLength: 0x%04x",
> + (indent - 8), ' ', attrlen);
> +
> + len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
> + num--;
> +
> + print_field("%*cAttributeValue: ", (indent - 8), ' ');
> + for (; attrlen > 0 && len > 0; attrlen--, len--) {
> + uint8_t c;
> +
> + if (!l2cap_frame_get_u8(frame, &c))
> + return false;
> +
> + printf("%1c", isprint(c) ? c : '.');
> + }
> +
> + if (attrlen > 0)
> + avrcp_continuing.size = attrlen;
> + }
> +
> + avrcp_continuing.num = num;
> +
> + return true;
> +}
> +
> struct avrcp_ctrl_pdu_data {
> uint8_t pduid;
> bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
> @@ -920,6 +1076,7 @@ static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
> { 0x15, avrcp_get_player_attribute_text },
> { 0x16, avrcp_get_player_value_text },
> { 0x17, avrcp_displayable_charset },
> + { 0x20, avrcp_get_element_attributes },
> { }
> };
>
> --
> 1.9.1
>



--
Luiz Augusto von Dentz

2014-09-12 13:16:57

by Vikrampal Yadav

[permalink] [raw]
Subject: [PATCH v2 2/2] Monitor: Add AVRCP GetElementAttributes support

Support for decoding AVRCP GetElementAttributes added in Bluetooth
monitor.
---
monitor/avctp.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 157 insertions(+)

diff --git a/monitor/avctp.c b/monitor/avctp.c
index f366b87..89997d0 100644
--- a/monitor/avctp.c
+++ b/monitor/avctp.c
@@ -158,6 +158,16 @@
#define AVRCP_ATTRIBUTE_SHUFFLE 0x03
#define AVRCP_ATTRIBUTE_SCAN 0x04

+/* media attributes */
+#define AVRCP_MEDIA_ATTRIBUTE_ILLEGAL 0x00
+#define AVRCP_MEDIA_ATTRIBUTE_TITLE 0x01
+#define AVRCP_MEDIA_ATTRIBUTE_ARTIST 0x02
+#define AVRCP_MEDIA_ATTRIBUTE_ALBUM 0x03
+#define AVRCP_MEDIA_ATTRIBUTE_TRACK 0x04
+#define AVRCP_MEDIA_ATTRIBUTE_TOTAL 0x05
+#define AVRCP_MEDIA_ATTRIBUTE_GENRE 0x06
+#define AVRCP_MEDIA_ATTRIBUTE_DURATION 0x07
+
struct avctp_frame {
uint8_t hdr;
uint8_t pt;
@@ -165,6 +175,11 @@ struct avctp_frame {
struct l2cap_frame l2cap_frame;
};

+static struct avrcp_continuing {
+ uint16_t num;
+ uint16_t size;
+} avrcp_continuing;
+
static const char *ctype2str(uint8_t ctype)
{
switch (ctype & 0x0f) {
@@ -524,6 +539,30 @@ static const char *charset2str(uint16_t charset)
}
}

+static const char *mediattr2str(uint32_t attr)
+{
+ switch (attr) {
+ case AVRCP_MEDIA_ATTRIBUTE_ILLEGAL:
+ return "Illegal";
+ case AVRCP_MEDIA_ATTRIBUTE_TITLE:
+ return "Title";
+ case AVRCP_MEDIA_ATTRIBUTE_ARTIST:
+ return "Artist";
+ case AVRCP_MEDIA_ATTRIBUTE_ALBUM:
+ return "Album";
+ case AVRCP_MEDIA_ATTRIBUTE_TRACK:
+ return "Track";
+ case AVRCP_MEDIA_ATTRIBUTE_TOTAL:
+ return "Track Total";
+ case AVRCP_MEDIA_ATTRIBUTE_GENRE:
+ return "Genre";
+ case AVRCP_MEDIA_ATTRIBUTE_DURATION:
+ return "Track duration";
+ default:
+ return "Reserved";
+ }
+}
+
static bool avrcp_passthrough_packet(struct avctp_frame *avctp_frame)
{
struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
@@ -905,6 +944,123 @@ static bool avrcp_displayable_charset(struct avctp_frame *avctp_frame,
return true;
}

+static bool avrcp_get_element_attributes(struct avctp_frame *avctp_frame,
+ uint8_t ctype, uint8_t len,
+ uint8_t indent)
+{
+ struct l2cap_frame *frame = &avctp_frame->l2cap_frame;
+ uint64_t id;
+ uint8_t num;
+
+ if (ctype > AVC_CTYPE_GENERAL_INQUIRY)
+ goto response;
+
+ if (!l2cap_frame_get_be64(frame, &id))
+ return false;
+
+ print_field("%*cIdentifier: 0x%jx (%s)", (indent - 8), ' ',
+ id, id ? "Reserved" : "PLAYING");
+
+ if (!l2cap_frame_get_u8(frame, &num))
+ return false;
+
+ print_field("%*cAttributeCount: 0x%02x", (indent - 8), ' ', num);
+
+ for (; num > 0; num--) {
+ uint32_t attr;
+
+ if (!l2cap_frame_get_be32(frame, &attr))
+ return false;
+
+ print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
+ ' ', attr, mediattr2str(attr));
+ }
+
+ return true;
+
+response:
+ if (avctp_frame->pt == AVRCP_PACKET_TYPE_SINGLE
+ || avctp_frame->pt == AVRCP_PACKET_TYPE_START) {
+ if (!l2cap_frame_get_u8(frame, &num))
+ return false;
+
+ avrcp_continuing.num = num;
+ print_field("%*cAttributeCount: 0x%02x", (indent - 8),
+ ' ', 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;
+
+ if (!l2cap_frame_get_u8(frame, &c))
+ return false;
+
+ printf("%1c", isprint(c) ? c : '.');
+ }
+ printf("\n");
+
+ len -= size;
+ }
+ }
+
+ while (num > 0 && len > 0) {
+ uint32_t attr;
+ uint16_t charset, attrlen;
+
+ if (!l2cap_frame_get_be32(frame, &attr))
+ return false;
+
+ print_field("%*cAttribute: 0x%08x (%s)", (indent - 8),
+ ' ', attr, mediattr2str(attr));
+
+ if (!l2cap_frame_get_be16(frame, &charset))
+ return false;
+
+ print_field("%*cCharsetID: 0x%04x (%s)", (indent - 8),
+ ' ', charset, charset2str(charset));
+
+ if (!l2cap_frame_get_be16(frame, &attrlen))
+ return false;
+
+ print_field("%*cAttributeValueLength: 0x%04x",
+ (indent - 8), ' ', attrlen);
+
+ len -= sizeof(attr) + sizeof(charset) + sizeof(attrlen);
+ num--;
+
+ print_field("%*cAttributeValue: ", (indent - 8), ' ');
+ for (; attrlen > 0 && len > 0; attrlen--, len--) {
+ uint8_t c;
+
+ if (!l2cap_frame_get_u8(frame, &c))
+ return false;
+
+ printf("%1c", isprint(c) ? c : '.');
+ }
+
+ if (attrlen > 0)
+ avrcp_continuing.size = attrlen;
+ }
+
+ avrcp_continuing.num = num;
+
+ return true;
+}
+
struct avrcp_ctrl_pdu_data {
uint8_t pduid;
bool (*func) (struct avctp_frame *avctp_frame, uint8_t ctype,
@@ -920,6 +1076,7 @@ static const struct avrcp_ctrl_pdu_data avrcp_ctrl_pdu_table[] = {
{ 0x15, avrcp_get_player_attribute_text },
{ 0x16, avrcp_get_player_value_text },
{ 0x17, avrcp_displayable_charset },
+ { 0x20, avrcp_get_element_attributes },
{ }
};

--
1.9.1