Re: [PATCH v2 BlueZ 01/12] android/avrcp-lib: Add support for parsing GetCapabilities response

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

 



Hi,

On Sun, Mar 30, 2014 at 10:33 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> ---
> v2: Fix typo and add more comments to PDU parsing.
>
>  android/avrcp-lib.c | 128 ++++++++++++++++++++++++++++++++++++++++++++++------
>  android/avrcp-lib.h |   6 ++-
>  android/avrcp.c     |  40 ++++++++--------
>  unit/test-avrcp.c   |   3 +-
>  4 files changed, 138 insertions(+), 39 deletions(-)
>
> diff --git a/android/avrcp-lib.c b/android/avrcp-lib.c
> index c7b8b6a..90ada8a 100644
> --- a/android/avrcp-lib.c
> +++ b/android/avrcp-lib.c
> @@ -135,6 +135,27 @@ void avrcp_shutdown(struct avrcp *session)
>         g_free(session);
>  }
>
> +static struct avrcp_header *parse_pdu(uint8_t *operands, size_t operand_count)
> +{
> +       struct avrcp_header *pdu;
> +
> +       if (!operands || operand_count < sizeof(*pdu)) {
> +               error("AVRCP: packet too small (%zu bytes)", operand_count);
> +               return NULL;
> +       }
> +
> +       pdu = (void *) operands;
> +       pdu->params_len = ntohs(pdu->params_len);
> +
> +       if (operand_count != pdu->params_len + sizeof(*pdu)) {
> +               error("AVRCP: invalid parameter length (%u bytes)",
> +                                                       pdu->params_len);
> +               return NULL;
> +       }
> +
> +       return pdu;
> +}
> +
>  static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>                                         uint8_t *code, uint8_t *subunit,
>                                         uint8_t *operands, size_t operand_count,
> @@ -142,26 +163,27 @@ static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>  {
>         struct avrcp *session = user_data;
>         const struct avrcp_control_handler *handler;
> -       struct avrcp_header *pdu = (void *) operands;
> -       uint32_t company_id = ntoh24(pdu->company_id);
> -       uint16_t params_len = ntohs(pdu->params_len);
> +       struct avrcp_header *pdu;
> +       uint32_t company_id;
>         ssize_t ret;
>
> +       pdu = parse_pdu(operands, operand_count);
> +       if (!pdu) {
> +               pdu->params[0] = AVRCP_STATUS_INVALID_COMMAND;
> +               goto reject;
> +       }
> +
> +       company_id = ntoh24(pdu->company_id);
>         if (company_id != IEEEID_BTSIG) {
>                 *code = AVC_CTYPE_NOT_IMPLEMENTED;
>                 return 0;
>         }
>
> -       DBG("AVRCP PDU 0x%02X, len 0x%04X", pdu->pdu_id, params_len);
> +       DBG("AVRCP PDU 0x%02X, len 0x%04X", pdu->pdu_id, pdu->params_len);
>
>         pdu->packet_type = 0;
>         pdu->rsvd = 0;
>
> -       if (operand_count < AVRCP_HEADER_LENGTH) {
> -               pdu->params[0] = AVRCP_STATUS_INVALID_COMMAND;
> -               goto reject;
> -       }
> -
>         if (!session->control_handlers)
>                 goto reject;
>
> @@ -180,7 +202,7 @@ static ssize_t handle_vendordep_pdu(struct avctp *conn, uint8_t transaction,
>                 goto reject;
>         }
>
> -       ret = handler->func(session, transaction, params_len, pdu->params,
> +       ret = handler->func(session, transaction, pdu->params_len, pdu->params,
>                                                         session->control_data);
>         if (ret < 0) {
>                 switch (ret) {
> @@ -670,6 +692,31 @@ int avrcp_send(struct avrcp *session, uint8_t transaction, uint8_t code,
>                                                         session->tx_buf, len);
>  }
>
> +static int parse_status(struct avrcp_header *pdu)
> +{
> +       if (pdu->params_len < 1)
> +               return -EPROTO;
> +
> +       switch (pdu->params[0]) {
> +       case AVRCP_STATUS_INVALID_COMMAND:
> +               return -ENOSYS;
> +       case AVRCP_STATUS_INVALID_PARAM:
> +               return -EINVAL;
> +       case AVRCP_STATUS_SUCCESS:
> +               return 0;
> +       case AVRCP_STATUS_OUT_OF_BOUNDS:
> +               return -EOVERFLOW;
> +       case AVRCP_STATUS_INTERNAL_ERROR:
> +       case AVRCP_STATUS_INVALID_PLAYER_ID:
> +       case AVRCP_STATUS_PLAYER_NOT_BROWSABLE:
> +       case AVRCP_STATUS_NO_AVAILABLE_PLAYERS:
> +       case AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED:
> +               return -EPERM;
> +       default:
> +               return -EPROTO;
> +       }
> +}
> +
>  static int avrcp_send_req(struct avrcp *session, uint8_t code, uint8_t subunit,
>                                         uint8_t pdu_id, uint8_t *params,
>                                         size_t params_len, avctp_rsp_cb func,
> @@ -698,12 +745,67 @@ static int avrcp_send_req(struct avrcp *session, uint8_t code, uint8_t subunit,
>                                         session->tx_buf, len, func, user_data);
>  }
>
> -int avrcp_get_capabilities(struct avrcp *session, uint8_t param,
> -                                       avctp_rsp_cb func, void *user_data)
> +static gboolean get_capabilities_rsp(struct avctp *conn,
> +                                       uint8_t code, uint8_t subunit,
> +                                       uint8_t *operands, size_t operand_count,
> +                                       void *user_data)
> +{
> +       struct avrcp *session = user_data;
> +       struct avrcp_player *player = session->player;
> +       struct avrcp_header *pdu;
> +       uint8_t number = 0;
> +       uint8_t *params = NULL;
> +       int err;
> +
> +       DBG("");
> +
> +       if (!player || !player->cfm || !player->cfm->get_capabilities)
> +               return FALSE;
> +
> +       pdu = parse_pdu(operands, operand_count);
> +       if (!pdu) {
> +               err = -EPROTO;
> +               goto done;
> +       }
> +
> +       if (code == AVC_CTYPE_REJECTED) {
> +               err = parse_status(pdu);
> +               goto done;
> +       }
> +
> +       if (pdu->params_len < 2) {
> +               err = -EPROTO;
> +               goto done;
> +       }
> +
> +       switch (pdu->params[0]) {
> +       case CAP_COMPANY_ID:
> +       case CAP_EVENTS_SUPPORTED:
> +               break;
> +       default:
> +               err = -EPROTO;
> +               goto done;
> +       }
> +
> +       number = pdu->params[1];
> +
> +       if (number > 0)
> +               params = &pdu->params[2];
> +
> +       err = 0;
> +
> +done:
> +       player->cfm->get_capabilities(session, err, number, params,
> +                                                       player->user_data);
> +
> +       return FALSE;
> +}
> +
> +int avrcp_get_capabilities(struct avrcp *session, uint8_t param)
>  {
>         return avrcp_send_req(session, AVC_CTYPE_STATUS, AVC_SUBUNIT_PANEL,
>                                 AVRCP_GET_CAPABILITIES, &param, sizeof(param),
> -                               func, user_data);
> +                               get_capabilities_rsp, session);
>  }
>
>  int avrcp_register_notification(struct avrcp *session, uint8_t event,
> diff --git a/android/avrcp-lib.h b/android/avrcp-lib.h
> index d9acb7d..ab110cb 100644
> --- a/android/avrcp-lib.h
> +++ b/android/avrcp-lib.h
> @@ -164,6 +164,9 @@ struct avrcp_control_ind {
>  };
>
>  struct avrcp_control_cfm {
> +       void (*get_capabilities) (struct avrcp *session, int err,
> +                                       uint8_t number, uint8_t *params,
> +                                       void *user_data);
>  };
>
>  struct avrcp_passthrough_handler {
> @@ -190,8 +193,7 @@ int avrcp_init_uinput(struct avrcp *session, const char *name,
>  int avrcp_send(struct avrcp *session, uint8_t transaction, uint8_t code,
>                                         uint8_t subunit, uint8_t pdu_id,
>                                         uint8_t *params, size_t params_len);
> -int avrcp_get_capabilities(struct avrcp *session, uint8_t param,
> -                                       avctp_rsp_cb func, void *user_data);
> +int avrcp_get_capabilities(struct avrcp *session, uint8_t param);
>  int avrcp_register_notification(struct avrcp *session, uint8_t event,
>                                         uint32_t interval, avctp_rsp_cb func,
>                                         void *user_data);
> diff --git a/android/avrcp.c b/android/avrcp.c
> index ec98139..0995a06 100644
> --- a/android/avrcp.c
> +++ b/android/avrcp.c
> @@ -759,38 +759,35 @@ static gboolean register_notification_rsp(struct avctp *conn,
>         return FALSE;
>  }
>
> -static gboolean get_capabilities_rsp(struct avctp *conn,
> -                                       uint8_t code, uint8_t subunit,
> -                                       uint8_t *operands, size_t operand_count,
> +static void handle_get_capabilities_rsp(struct avrcp *session, int err,
> +                                       uint8_t number, uint8_t *events,
>                                         void *user_data)
>  {
>         struct avrcp_device *dev = user_data;
> -       uint8_t *params;
> -       uint8_t count;
> -
> -       if (operands == NULL || operand_count < 7)
> -               return FALSE;
> -
> -       params = &operands[7];
> -
> -       if (params == NULL || params[0] != CAP_EVENTS_SUPPORTED)
> -               return FALSE;
> +       int i;
>
> -       for (count = params[1]; count > 0; count--) {
> -               uint8_t event = params[1 + count];
> +       if (err < 0) {
> +               error("AVRCP: %s", strerror(-err));
> +               return;
> +       }
>
> -               if (event != AVRCP_EVENT_VOLUME_CHANGED)
> +       for (i = 0; i < number; i++) {
> +               if (events[i] != AVRCP_EVENT_VOLUME_CHANGED)
>                         continue;
>
> -               avrcp_register_notification(dev->session, event, 0,
> +               avrcp_register_notification(dev->session, events[i], 0,
>                                                 register_notification_rsp,
>                                                 dev);
> -               return FALSE;
> +               break;
>         }
>
> -       return FALSE;
> +       return;
>  }
>
> +static const struct avrcp_control_cfm control_cfm = {
> +       .get_capabilities = handle_get_capabilities_rsp,
> +};
> +
>  static int avrcp_device_add_session(struct avrcp_device *dev, int fd,
>                                                 uint16_t imtu, uint16_t omtu)
>  {
> @@ -804,7 +801,7 @@ static int avrcp_device_add_session(struct avrcp_device *dev, int fd,
>         avrcp_set_destroy_cb(dev->session, disconnect_cb, dev);
>         avrcp_set_passthrough_handlers(dev->session, passthrough_handlers,
>                                                                         dev);
> -       avrcp_register_player(dev->session, &control_ind, NULL, dev);
> +       avrcp_register_player(dev->session, &control_ind, &control_cfm, dev);
>
>         dev->queue = g_queue_new();
>
> @@ -828,8 +825,7 @@ static int avrcp_device_add_session(struct avrcp_device *dev, int fd,
>
>         ev.features |= HAL_AVRCP_FEATURE_ABSOLUTE_VOLUME;
>
> -       avrcp_get_capabilities(dev->session, CAP_EVENTS_SUPPORTED,
> -                                               get_capabilities_rsp, dev);
> +       avrcp_get_capabilities(dev->session, CAP_EVENTS_SUPPORTED);
>
>  done:
>         ipc_send_notif(hal_ipc, HAL_SERVICE_ID_AVRCP,
> diff --git a/unit/test-avrcp.c b/unit/test-avrcp.c
> index eb00238..d6be6a6 100644
> --- a/unit/test-avrcp.c
> +++ b/unit/test-avrcp.c
> @@ -491,8 +491,7 @@ static void test_client(gconstpointer data)
>                                                                         NULL);
>
>         if (g_str_equal(context->data->test_name, "/TP/CFG/BV-01-C"))
> -               avrcp_get_capabilities(context->session, CAP_EVENTS_SUPPORTED,
> -                                                               NULL, NULL);
> +               avrcp_get_capabilities(context->session, CAP_EVENTS_SUPPORTED);
>
>         if (g_str_equal(context->data->test_name, "/TP/PAS/BV-01-C"))
>                 avrcp_list_player_attributes(context->session, NULL, NULL);
> --
> 1.8.5.3

Pushed.


-- 
Luiz Augusto von Dentz
--
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