Re: [PATCH BlueZ V4 5/5] AVRCP: Add handler for browsing pdu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Vani,

On Wed, Aug 8, 2012 at 12:27 PM, Vani-dineshbhai PATEL
<vani.patel@xxxxxxxxxxxxxx> wrote:
> From: Vani Patel <vani.patel@xxxxxxxxxxxxxx>
>
> Implement generic handling of browsing
> PDU IDs
> ---
>  audio/avctp.c |   27 ++++++++++++++++++--
>  audio/avrcp.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+), 3 deletions(-)
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index b5f84aa..7bbcecc 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -458,9 +458,9 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
>                                 gpointer data)
>  {
>         struct avctp *session = data;
> -       uint8_t  *buf;
> +       uint8_t *operands, *buf;
>         struct avctp_header *avctp;
> -       int ret, sock;
> +       int ret, packet_size, operand_count, sock;
>
>         buf = (uint8_t *)malloc(session->browsing_mtu);

Need a space after typecasting.

>         if (!(cond & G_IO_IN))
> @@ -472,6 +472,8 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
>         if (ret <= 0)
>                 goto failed;
>
> +       DBG("Got %d bytes of data for AVCTP Browsing session %p", ret, session);
> +
>         if ((unsigned int) ret < sizeof(struct avctp_header)) {
>                 error("Too small AVRCP packet on browsing channel");
>                 goto failed;
> @@ -479,9 +481,28 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
>
>         avctp = (struct avctp_header *) buf;
>
> +       DBG("AVCTP transaction %u, packet type %u, C/R %u, IPID %u, "
> +                       "PID 0x%04X",
> +                       avctp->transaction, avctp->packet_type,
> +                       avctp->cr, avctp->ipid, ntohs(avctp->pid));
> +
>         if (avctp->packet_type != AVCTP_PACKET_SINGLE)
>                 goto failed;

It's better to have a  blank line here.

> +       operands = buf + AVCTP_HEADER_LENGTH;
> +       ret -= AVCTP_HEADER_LENGTH;
> +       operand_count = ret;
> +
> +       packet_size = AVCTP_HEADER_LENGTH;
> +       avctp->cr = AVCTP_RESPONSE;
> +       if (browsing_handler)
> +               packet_size += browsing_handler->cb(session, avctp->transaction,
> +                       operands, operand_count, browsing_handler->user_data);
>
> +       if (packet_size !=0 ) {

Space required after '!=' and space prohibited before ')'

> +               ret = write(sock, buf, packet_size);
> +               if (ret != packet_size)
> +                       goto failed;
> +       }
>         free(buf);
>         return TRUE;
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 9d29073..deba4a6 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -94,6 +94,9 @@
>  #define AVRCP_ABORT_CONTINUING         0x41
>  #define AVRCP_SET_ABSOLUTE_VOLUME      0x50
>
> +#define AVRCP_INVALID_BROWSING_PDU     0x00
> +
> +#define AVRCP_GENERAL_REJECT           0xA0
>  /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
>  #define CAP_COMPANY_ID         0x02
>  #define CAP_EVENTS_SUPPORTED   0x03
> @@ -140,6 +143,12 @@ struct avrcp_header {
>  #error "Unknown byte order"
>  #endif
>
> +struct avrcp_browsing_header {
> +       uint8_t browsing_pdu;
> +       uint16_t param_len;
> +} __attribute__ ((packed));
> +#define AVRCP_BROWSING_HEADER_LENGTH 3
> +
>  #define AVRCP_MTU      (AVC_MTU - AVC_HEADER_LENGTH)
>  #define AVRCP_PDU_MTU  (AVRCP_MTU - AVRCP_HEADER_LENGTH)
>
> @@ -163,7 +172,9 @@ struct avrcp_player {
>         struct audio_device *dev;
>
>         unsigned int control_handler;
> +       unsigned int browsing_handler;
>         uint16_t registered_events;
> +       uint8_t transaction;
>         uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
>         struct pending_pdu *pending_pdu;
>
> @@ -1075,6 +1086,15 @@ static struct control_pdu_handler {
>                 { },
>  };
>
> +static struct pdu_browsing_handler {
> +       uint8_t browsing_pdu;
> +       void (*func) (struct avrcp_player *player,
> +                                       struct avrcp_browsing_header *pdu);
> +       } browsing_handlers[] = {
> +               { AVRCP_INVALID_BROWSING_PDU,
> +                                       NULL },
> +};
> +
>  /* handle vendordep pdu inside an avctp packet */
>  static size_t handle_vendordep_pdu(struct avctp *session, uint8_t transaction,
>                                         uint8_t *code, uint8_t *subunit,
> @@ -1134,6 +1154,49 @@ err_metadata:
>         return AVRCP_HEADER_LENGTH + 1;
>  }
>
> +static size_t handle_browsing_pdu(struct avctp *session,
> +                                       uint8_t transaction, uint8_t *operands,
> +                                       size_t operand_count, void *user_data)
> +{
> +       struct avrcp_player *player = user_data;
> +       struct pdu_browsing_handler *b_handler;
> +       struct avrcp_browsing_header *avrcp_browsing = (void *) operands;
> +       uint8_t status;
> +
> +       operand_count += AVRCP_BROWSING_HEADER_LENGTH;
> +
> +       for (b_handler = browsing_handlers; b_handler; b_handler++) {
> +               if (b_handler->browsing_pdu == AVRCP_INVALID_BROWSING_PDU) {
> +                       b_handler = NULL;
> +                       break;
> +               }
> +               if (b_handler->browsing_pdu == avrcp_browsing->browsing_pdu)
> +                       break;
> +       }
> +
> +       if (!b_handler) {
> +               avrcp_browsing->browsing_pdu = AVRCP_GENERAL_REJECT;
> +               status = AVRCP_STATUS_INVALID_COMMAND;
> +               goto err;
> +       }
> +
> +       if (!b_handler->func) {
> +               status = AVRCP_STATUS_INVALID_PARAM;
> +               avrcp_browsing->param_len = htons(sizeof(status));
> +               goto err;
> +       }
> +       player->transaction = transaction;
> +       b_handler->func(player, avrcp_browsing);
> +       return AVRCP_BROWSING_HEADER_LENGTH + ntohs(avrcp_browsing->param_len);
> +
> +err:
> +       avrcp_browsing->param_len = htons(sizeof(status));
> +       memcpy(&operands[AVRCP_BROWSING_HEADER_LENGTH], &status,
> +                                                       (sizeof(status)));
> +       return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
> +}
> +
> +
>  size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands)
>  {
>         struct avrcp_header *pdu = (void *) operands;
> @@ -1233,6 +1296,10 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
>                         avctp_unregister_pdu_handler(player->control_handler);
>                         player->control_handler = 0;
>                 }
> +               if (player->browsing_handler) {
> +                       avctp_unregister_browsing_pdu_handler();
> +                       player->browsing_handler = 0;
> +               }
>
>                 break;
>         case AVCTP_STATE_CONNECTING:
> @@ -1244,6 +1311,12 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
>                                                         AVC_OP_VENDORDEP,
>                                                         handle_vendordep_pdu,
>                                                         player);
> +               if (!player->browsing_handler)
> +                       player->browsing_handler =
> +                                       avctp_register_browsing_pdu_handler(
> +                                                       handle_browsing_pdu,
> +                                                       player);
> +
>                 break;
>         case AVCTP_STATE_CONNECTED:
>                 rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
> --
> 1.7.5.4
>

It's always better to run checkpatch.pl script on the patches before
sending to the ML. It can capture the coding style issues.

Thanks,
Syam.
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux