linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] audio/avrcp: Add Set Addressed Player support
@ 2015-08-13 15:20 Bharat Panda
  2015-08-14 12:30 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Bharat Panda @ 2015-08-13 15:20 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: cpgs, Bharat Panda

Support added to handle Set Addressed Player PDU in TG role.
Send EVENT_ADDRESSED_PLAYER_CHANGED on SetAddressedPlayer
SUCCESS and follow procedure to reject all player specific events
currently registered with the player.

      Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
      AVCTP Control: Command: type 0x00 label 0 PID 0x110e
        AV/C: Control: address 0x48 opcode 0x00
          Subunit: Panel
          Opcode: Vendor Dependent
          Company ID: 0x001958
          AVRCP: SetAddressedPlayer pt Single len 0x0002
            PlayerID: 0x0000 (0)

      Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
      AVCTP Control: Response: type 0x00 label 0 PID 0x110e
        AV/C: Accepted: address 0x48 opcode 0x00
          Subunit: Panel
          Opcode: Vendor Dependent
          Company ID: 0x001958
          AVRCP: SetAddressedPlayer pt Single len 0x0002
            Status: 0x04 (Success)

      Channel: 64 len 18 [PSM 23 mode 0] {chan 0}
      AVCTP Control: Response: type 0x00 label 0 PID 0x110e
        AV/C: Changed: address 0x48 opcode 0x00
          Subunit: Panel
          Opcode: Vendor Dependent
          Company ID: 0x001958
          AVRCP: RegisterNotification pt Single len 0x0005
            EventID: 0x0b (EVENT_ADDRESSED_PLAYER_CHANGED)
            PlayerID: 0x0000 (0)
            UIDCounter: 0x0000 (0)
---
 profiles/audio/avrcp.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index d66f670..61d91ea 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -103,6 +103,7 @@
 #define AVRCP_REQUEST_CONTINUING	0x40
 #define AVRCP_ABORT_CONTINUING		0x41
 #define AVRCP_SET_ABSOLUTE_VOLUME	0x50
+#define AVRCP_SET_ADDRESSED_PLAYER	0x60
 #define AVRCP_SET_BROWSED_PLAYER	0x70
 #define AVRCP_GET_FOLDER_ITEMS		0x71
 #define AVRCP_CHANGE_PATH		0x72
@@ -204,8 +205,10 @@ struct avrcp_player {
 	uint64_t uid;
 	uint16_t uid_counter;
 	bool browsed;
+	bool addressed;
 	uint8_t *features;
 	char *path;
+	guint notify_id;
 
 	struct pending_list_items *p;
 	char *change_path;
@@ -631,6 +634,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 	GSList *l;
 	int attr;
 	int val;
+	bool player_changed = false;
 
 	if (player->sessions == NULL)
 		return;
@@ -674,6 +678,12 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 		pdu->params[size++] = attr;
 		pdu->params[size++] = val;
 		break;
+	case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
+		size = 5;
+		memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
+		memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
+		player_changed = true;
+		break;
 	case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
 		size = 1;
 		break;
@@ -695,6 +705,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 					session->transaction_events[id],
 					AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
 					buf, size + AVRCP_HEADER_LENGTH);
+
 		if (err < 0)
 			continue;
 
@@ -702,6 +713,52 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
 		session->registered_events ^= 1 << id;
 	}
 
+	if (player_changed) {
+		uint8_t player_events[6] = {
+					AVRCP_EVENT_STATUS_CHANGED,
+					AVRCP_EVENT_TRACK_CHANGED,
+					AVRCP_EVENT_TRACK_REACHED_START,
+					AVRCP_EVENT_TRACK_REACHED_END,
+					AVRCP_EVENT_SETTINGS_CHANGED,
+					AVRCP_EVENT_PLAYBACK_POS_CHANGED
+					};
+
+		for (l = player->sessions; l; l = l->next) {
+			struct avrcp *session = l->data;
+			int err;
+			int i;
+
+			for (i = 0; i < 6; i++) {
+				if (!(session->registered_events &
+						(1 << player_events[i])))
+					continue;
+
+				if (session->registered_events &
+						(1 << player_events[i])) {
+					session->registered_events &=
+						~(1 << player_events[i]);
+				/*
+				 * TG shall complete all player specific
+				 * notifications with AV/C C-Type REJECTED
+				 * with error code as Addressed Player Changed.
+				 */
+
+				pdu->params[0] =
+					AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
+				pdu->params_len = htons(1);
+				err = avctp_send_vendordep(session->conn,
+					session->transaction_events[player_events[i]],
+					AVC_CTYPE_REJECTED, AVC_SUBUNIT_PANEL,
+					buf, 1 + AVRCP_HEADER_LENGTH);
+				}
+			}
+
+			if (err < 0)
+				continue;
+
+		}
+	}
+
 	return;
 }
 
@@ -1494,6 +1551,11 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 		}
 
 		break;
+	case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
+		len = 5;
+		memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
+		memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
+		break;
 	case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
 		len = 1;
 		break;
@@ -1617,6 +1679,72 @@ err:
 	return AVC_CTYPE_REJECTED;
 }
 
+static struct avrcp_player *find_tg_player(struct avrcp *session, uint16_t id)
+{
+	struct avrcp_server *server = session->server;
+	GSList *l;
+
+	for (l = server->players; l; l = l->next) {
+		struct avrcp_player *player = l->data;
+
+		if (player->id == id)
+			return player;
+	}
+
+	return NULL;
+}
+
+static gboolean notify_addressed_player_changed(gpointer user_data)
+{
+	struct avrcp_player *player = user_data;
+
+	avrcp_player_event(player,
+				AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
+	g_source_remove(player->notify_id);
+
+	return TRUE;
+}
+
+static uint8_t avrcp_handle_set_addressed_player(struct avrcp *session,
+						struct avrcp_header *pdu,
+						uint8_t transaction)
+{
+	struct avrcp_player *player;
+	uint16_t len = ntohs(pdu->params_len);
+	uint16_t player_id = 0;
+	uint8_t status;
+
+	if (len < 1) {
+		status = AVRCP_STATUS_INVALID_PARAM;
+		goto err;
+	}
+
+	player_id = bt_get_be16(&pdu->params[0]);
+	player = find_tg_player(session, player_id);
+	pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
+
+	if (player) {
+		player->addressed = true;
+		status = AVRCP_STATUS_SUCCESS;
+		pdu->params_len = htons(len);
+		pdu->params[0] = status;
+	} else {
+		status = AVRCP_STATUS_INVALID_PLAYER_ID;
+		goto err;
+	}
+
+	player->notify_id =
+			g_idle_add((GSourceFunc)notify_addressed_player_changed,
+			player);
+
+	return AVC_CTYPE_ACCEPTED;
+
+err:
+	pdu->params_len = htons(sizeof(status));
+	pdu->params[0] = status;
+	return AVC_CTYPE_REJECTED;
+}
+
 static const struct control_pdu_handler control_handlers[] = {
 		{ AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
 					avrcp_handle_get_capabilities },
@@ -1648,6 +1776,8 @@ static const struct control_pdu_handler control_handlers[] = {
 					avrcp_handle_request_continuing },
 		{ AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
 					avrcp_handle_abort_continuing },
+		{ AVRCP_SET_ADDRESSED_PLAYER, AVC_CTYPE_CONTROL,
+					avrcp_handle_set_addressed_player },
 		{ },
 };
 
@@ -3521,6 +3651,7 @@ static void target_init(struct avrcp *session)
 		return;
 
 	session->supported_events |=
+				(1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
 				(1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
 
 	/* Only check capabilities if controller is not supported */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] audio/avrcp: Add Set Addressed Player support
  2015-08-13 15:20 [PATCH v1] audio/avrcp: Add Set Addressed Player support Bharat Panda
@ 2015-08-14 12:30 ` Luiz Augusto von Dentz
  2015-08-14 12:44   ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2015-08-14 12:30 UTC (permalink / raw)
  To: Bharat Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs

Hi Bharat,

On Thu, Aug 13, 2015 at 6:20 PM, Bharat Panda <bharat.panda@samsung.com> wrote:
> Support added to handle Set Addressed Player PDU in TG role.
> Send EVENT_ADDRESSED_PLAYER_CHANGED on SetAddressedPlayer
> SUCCESS and follow procedure to reject all player specific events
> currently registered with the player.
>
>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
>       AVCTP Control: Command: type 0x00 label 0 PID 0x110e
>         AV/C: Control: address 0x48 opcode 0x00
>           Subunit: Panel
>           Opcode: Vendor Dependent
>           Company ID: 0x001958
>           AVRCP: SetAddressedPlayer pt Single len 0x0002
>             PlayerID: 0x0000 (0)
>
>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
>         AV/C: Accepted: address 0x48 opcode 0x00
>           Subunit: Panel
>           Opcode: Vendor Dependent
>           Company ID: 0x001958
>           AVRCP: SetAddressedPlayer pt Single len 0x0002
>             Status: 0x04 (Success)
>
>       Channel: 64 len 18 [PSM 23 mode 0] {chan 0}
>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
>         AV/C: Changed: address 0x48 opcode 0x00
>           Subunit: Panel
>           Opcode: Vendor Dependent
>           Company ID: 0x001958
>           AVRCP: RegisterNotification pt Single len 0x0005
>             EventID: 0x0b (EVENT_ADDRESSED_PLAYER_CHANGED)
>             PlayerID: 0x0000 (0)
>             UIDCounter: 0x0000 (0)
> ---
>  profiles/audio/avrcp.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 131 insertions(+)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index d66f670..61d91ea 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -103,6 +103,7 @@
>  #define AVRCP_REQUEST_CONTINUING       0x40
>  #define AVRCP_ABORT_CONTINUING         0x41
>  #define AVRCP_SET_ABSOLUTE_VOLUME      0x50
> +#define AVRCP_SET_ADDRESSED_PLAYER     0x60
>  #define AVRCP_SET_BROWSED_PLAYER       0x70
>  #define AVRCP_GET_FOLDER_ITEMS         0x71
>  #define AVRCP_CHANGE_PATH              0x72
> @@ -204,8 +205,10 @@ struct avrcp_player {
>         uint64_t uid;
>         uint16_t uid_counter;
>         bool browsed;
> +       bool addressed;
>         uint8_t *features;
>         char *path;
> +       guint notify_id;
>
>         struct pending_list_items *p;
>         char *change_path;
> @@ -631,6 +634,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>         GSList *l;
>         int attr;
>         int val;
> +       bool player_changed = false;
>
>         if (player->sessions == NULL)
>                 return;
> @@ -674,6 +678,12 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>                 pdu->params[size++] = attr;
>                 pdu->params[size++] = val;
>                 break;
> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> +               size = 5;
> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
> +               memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
> +               player_changed = true;
> +               break;
>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
>                 size = 1;
>                 break;
> @@ -695,6 +705,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>                                         session->transaction_events[id],
>                                         AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
>                                         buf, size + AVRCP_HEADER_LENGTH);
> +
>                 if (err < 0)
>                         continue;
>
> @@ -702,6 +713,52 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>                 session->registered_events ^= 1 << id;
>         }
>
> +       if (player_changed) {
> +               uint8_t player_events[6] = {
> +                                       AVRCP_EVENT_STATUS_CHANGED,
> +                                       AVRCP_EVENT_TRACK_CHANGED,
> +                                       AVRCP_EVENT_TRACK_REACHED_START,
> +                                       AVRCP_EVENT_TRACK_REACHED_END,
> +                                       AVRCP_EVENT_SETTINGS_CHANGED,
> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED
> +                                       };
> +
> +               for (l = player->sessions; l; l = l->next) {
> +                       struct avrcp *session = l->data;
> +                       int err;
> +                       int i;
> +
> +                       for (i = 0; i < 6; i++) {

Check with i < sizeof(player_events) so in case we need to add some
new event we don't need to always keep changing this.

> +                               if (!(session->registered_events &
> +                                               (1 << player_events[i])))
> +                                       continue;
> +
> +                               if (session->registered_events &
> +                                               (1 << player_events[i])) {
> +                                       session->registered_events &=
> +                                               ~(1 << player_events[i]);
> +                               /*
> +                                * TG shall complete all player specific
> +                                * notifications with AV/C C-Type REJECTED
> +                                * with error code as Addressed Player Changed.
> +                                */
> +
> +                               pdu->params[0] =
> +                                       AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
> +                               pdu->params_len = htons(1);
> +                               err = avctp_send_vendordep(session->conn,
> +                                       session->transaction_events[player_events[i]],
> +                                       AVC_CTYPE_REJECTED, AVC_SUBUNIT_PANEL,
> +                                       buf, 1 + AVRCP_HEADER_LENGTH);
> +                               }
> +                       }
> +
> +                       if (err < 0)
> +                               continue;
> +
> +               }
> +       }
> +
>         return;
>  }
>
> @@ -1494,6 +1551,11 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>                 }
>
>                 break;
> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> +               len = 5;
> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
> +               memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
> +               break;
>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
>                 len = 1;
>                 break;
> @@ -1617,6 +1679,72 @@ err:
>         return AVC_CTYPE_REJECTED;
>  }
>
> +static struct avrcp_player *find_tg_player(struct avrcp *session, uint16_t id)
> +{
> +       struct avrcp_server *server = session->server;
> +       GSList *l;
> +
> +       for (l = server->players; l; l = l->next) {
> +               struct avrcp_player *player = l->data;
> +
> +               if (player->id == id)
> +                       return player;
> +       }
> +
> +       return NULL;
> +}
> +
> +static gboolean notify_addressed_player_changed(gpointer user_data)
> +{
> +       struct avrcp_player *player = user_data;
> +
> +       avrcp_player_event(player,
> +                               AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
> +       g_source_remove(player->notify_id);

Removing the source wouldn't be necessary if you return FALSE or
G_SOURCE_REMOVE if we want to more gnomish, furthermore notify_id
shall be set to 0 either way and the call to g_source_remove shall be
move to where the player freed so we stop the callback to be called if
the player is unregistered.

> +
> +       return TRUE;
> +}
> +
> +static uint8_t avrcp_handle_set_addressed_player(struct avrcp *session,
> +                                               struct avrcp_header *pdu,
> +                                               uint8_t transaction)
> +{
> +       struct avrcp_player *player;
> +       uint16_t len = ntohs(pdu->params_len);
> +       uint16_t player_id = 0;
> +       uint8_t status;
> +
> +       if (len < 1) {
> +               status = AVRCP_STATUS_INVALID_PARAM;
> +               goto err;
> +       }
> +
> +       player_id = bt_get_be16(&pdu->params[0]);
> +       player = find_tg_player(session, player_id);
> +       pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
> +
> +       if (player) {
> +               player->addressed = true;
> +               status = AVRCP_STATUS_SUCCESS;
> +               pdu->params_len = htons(len);
> +               pdu->params[0] = status;
> +       } else {
> +               status = AVRCP_STATUS_INVALID_PLAYER_ID;
> +               goto err;
> +       }
> +
> +       player->notify_id =
> +                       g_idle_add((GSourceFunc)notify_addressed_player_changed,
> +                       player);
> +
> +       return AVC_CTYPE_ACCEPTED;
> +
> +err:
> +       pdu->params_len = htons(sizeof(status));
> +       pdu->params[0] = status;
> +       return AVC_CTYPE_REJECTED;
> +}
> +
>  static const struct control_pdu_handler control_handlers[] = {
>                 { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
>                                         avrcp_handle_get_capabilities },
> @@ -1648,6 +1776,8 @@ static const struct control_pdu_handler control_handlers[] = {
>                                         avrcp_handle_request_continuing },
>                 { AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
>                                         avrcp_handle_abort_continuing },
> +               { AVRCP_SET_ADDRESSED_PLAYER, AVC_CTYPE_CONTROL,
> +                                       avrcp_handle_set_addressed_player },
>                 { },
>  };
>
> @@ -3521,6 +3651,7 @@ static void target_init(struct avrcp *session)
>                 return;
>
>         session->supported_events |=
> +                               (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
>                                 (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
>
>         /* Only check capabilities if controller is not supported */
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v1] audio/avrcp: Add Set Addressed Player support
  2015-08-14 12:30 ` Luiz Augusto von Dentz
@ 2015-08-14 12:44   ` Luiz Augusto von Dentz
  2015-08-14 12:51     ` Bharat Bhusan Panda
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2015-08-14 12:44 UTC (permalink / raw)
  To: Bharat Panda; +Cc: linux-bluetooth@vger.kernel.org, cpgs

Hi Bharat,

On Fri, Aug 14, 2015 at 3:30 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Bharat,
>
> On Thu, Aug 13, 2015 at 6:20 PM, Bharat Panda <bharat.panda@samsung.com> wrote:
>> Support added to handle Set Addressed Player PDU in TG role.
>> Send EVENT_ADDRESSED_PLAYER_CHANGED on SetAddressedPlayer
>> SUCCESS and follow procedure to reject all player specific events
>> currently registered with the player.
>>
>>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
>>       AVCTP Control: Command: type 0x00 label 0 PID 0x110e
>>         AV/C: Control: address 0x48 opcode 0x00
>>           Subunit: Panel
>>           Opcode: Vendor Dependent
>>           Company ID: 0x001958
>>           AVRCP: SetAddressedPlayer pt Single len 0x0002
>>             PlayerID: 0x0000 (0)
>>
>>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
>>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
>>         AV/C: Accepted: address 0x48 opcode 0x00
>>           Subunit: Panel
>>           Opcode: Vendor Dependent
>>           Company ID: 0x001958
>>           AVRCP: SetAddressedPlayer pt Single len 0x0002
>>             Status: 0x04 (Success)
>>
>>       Channel: 64 len 18 [PSM 23 mode 0] {chan 0}
>>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
>>         AV/C: Changed: address 0x48 opcode 0x00
>>           Subunit: Panel
>>           Opcode: Vendor Dependent
>>           Company ID: 0x001958
>>           AVRCP: RegisterNotification pt Single len 0x0005
>>             EventID: 0x0b (EVENT_ADDRESSED_PLAYER_CHANGED)
>>             PlayerID: 0x0000 (0)
>>             UIDCounter: 0x0000 (0)
>> ---
>>  profiles/audio/avrcp.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 131 insertions(+)
>>
>> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
>> index d66f670..61d91ea 100644
>> --- a/profiles/audio/avrcp.c
>> +++ b/profiles/audio/avrcp.c
>> @@ -103,6 +103,7 @@
>>  #define AVRCP_REQUEST_CONTINUING       0x40
>>  #define AVRCP_ABORT_CONTINUING         0x41
>>  #define AVRCP_SET_ABSOLUTE_VOLUME      0x50
>> +#define AVRCP_SET_ADDRESSED_PLAYER     0x60
>>  #define AVRCP_SET_BROWSED_PLAYER       0x70
>>  #define AVRCP_GET_FOLDER_ITEMS         0x71
>>  #define AVRCP_CHANGE_PATH              0x72
>> @@ -204,8 +205,10 @@ struct avrcp_player {
>>         uint64_t uid;
>>         uint16_t uid_counter;
>>         bool browsed;
>> +       bool addressed;
>>         uint8_t *features;
>>         char *path;
>> +       guint notify_id;
>>
>>         struct pending_list_items *p;
>>         char *change_path;
>> @@ -631,6 +634,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>>         GSList *l;
>>         int attr;
>>         int val;
>> +       bool player_changed = false;
>>
>>         if (player->sessions == NULL)
>>                 return;
>> @@ -674,6 +678,12 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>>                 pdu->params[size++] = attr;
>>                 pdu->params[size++] = val;
>>                 break;
>> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
>> +               size = 5;
>> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
>> +               memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
>> +               player_changed = true;
>> +               break;
>>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
>>                 size = 1;
>>                 break;
>> @@ -695,6 +705,7 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>>                                         session->transaction_events[id],
>>                                         AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
>>                                         buf, size + AVRCP_HEADER_LENGTH);
>> +
>>                 if (err < 0)
>>                         continue;
>>
>> @@ -702,6 +713,52 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
>>                 session->registered_events ^= 1 << id;
>>         }
>>
>> +       if (player_changed) {
>> +               uint8_t player_events[6] = {
>> +                                       AVRCP_EVENT_STATUS_CHANGED,
>> +                                       AVRCP_EVENT_TRACK_CHANGED,
>> +                                       AVRCP_EVENT_TRACK_REACHED_START,
>> +                                       AVRCP_EVENT_TRACK_REACHED_END,
>> +                                       AVRCP_EVENT_SETTINGS_CHANGED,
>> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED
>> +                                       };
>> +
>> +               for (l = player->sessions; l; l = l->next) {
>> +                       struct avrcp *session = l->data;
>> +                       int err;
>> +                       int i;
>> +
>> +                       for (i = 0; i < 6; i++) {
>
> Check with i < sizeof(player_events) so in case we need to add some
> new event we don't need to always keep changing this.
>
>> +                               if (!(session->registered_events &
>> +                                               (1 << player_events[i])))
>> +                                       continue;
>> +
>> +                               if (session->registered_events &
>> +                                               (1 << player_events[i])) {
>> +                                       session->registered_events &=
>> +                                               ~(1 << player_events[i]);
>> +                               /*
>> +                                * TG shall complete all player specific
>> +                                * notifications with AV/C C-Type REJECTED
>> +                                * with error code as Addressed Player Changed.
>> +                                */
>> +
>> +                               pdu->params[0] =
>> +                                       AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
>> +                               pdu->params_len = htons(1);
>> +                               err = avctp_send_vendordep(session->conn,
>> +                                       session->transaction_events[player_events[i]],
>> +                                       AVC_CTYPE_REJECTED, AVC_SUBUNIT_PANEL,
>> +                                       buf, 1 + AVRCP_HEADER_LENGTH);
>> +                               }
>> +                       }
>> +
>> +                       if (err < 0)
>> +                               continue;
>> +
>> +               }
>> +       }
>> +
>>         return;
>>  }
>>
>> @@ -1494,6 +1551,11 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
>>                 }
>>
>>                 break;
>> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
>> +               len = 5;
>> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
>> +               memcpy(&pdu->params[3], &player->uid_counter, sizeof(uint16_t));
>> +               break;
>>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
>>                 len = 1;
>>                 break;
>> @@ -1617,6 +1679,72 @@ err:
>>         return AVC_CTYPE_REJECTED;
>>  }
>>
>> +static struct avrcp_player *find_tg_player(struct avrcp *session, uint16_t id)
>> +{
>> +       struct avrcp_server *server = session->server;
>> +       GSList *l;
>> +
>> +       for (l = server->players; l; l = l->next) {
>> +               struct avrcp_player *player = l->data;
>> +
>> +               if (player->id == id)
>> +                       return player;
>> +       }
>> +
>> +       return NULL;
>> +}
>> +
>> +static gboolean notify_addressed_player_changed(gpointer user_data)
>> +{
>> +       struct avrcp_player *player = user_data;
>> +
>> +       avrcp_player_event(player,
>> +                               AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
>> +       g_source_remove(player->notify_id);
>
> Removing the source wouldn't be necessary if you return FALSE or
> G_SOURCE_REMOVE if we want to more gnomish, furthermore notify_id
> shall be set to 0 either way and the call to g_source_remove shall be
> move to where the player freed so we stop the callback to be called if
> the player is unregistered.

Actually don't use G_SOURCE_REMOVE since it was introduced in 2.32 and
we depend on 2.28 or later.

>
>> +
>> +       return TRUE;
>> +}
>> +
>> +static uint8_t avrcp_handle_set_addressed_player(struct avrcp *session,
>> +                                               struct avrcp_header *pdu,
>> +                                               uint8_t transaction)
>> +{
>> +       struct avrcp_player *player;
>> +       uint16_t len = ntohs(pdu->params_len);
>> +       uint16_t player_id = 0;
>> +       uint8_t status;
>> +
>> +       if (len < 1) {
>> +               status = AVRCP_STATUS_INVALID_PARAM;
>> +               goto err;
>> +       }
>> +
>> +       player_id = bt_get_be16(&pdu->params[0]);
>> +       player = find_tg_player(session, player_id);
>> +       pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
>> +
>> +       if (player) {
>> +               player->addressed = true;
>> +               status = AVRCP_STATUS_SUCCESS;
>> +               pdu->params_len = htons(len);
>> +               pdu->params[0] = status;
>> +       } else {
>> +               status = AVRCP_STATUS_INVALID_PLAYER_ID;
>> +               goto err;
>> +       }
>> +
>> +       player->notify_id =
>> +                       g_idle_add((GSourceFunc)notify_addressed_player_changed,
>> +                       player);
>> +
>> +       return AVC_CTYPE_ACCEPTED;
>> +
>> +err:
>> +       pdu->params_len = htons(sizeof(status));
>> +       pdu->params[0] = status;
>> +       return AVC_CTYPE_REJECTED;
>> +}
>> +
>>  static const struct control_pdu_handler control_handlers[] = {
>>                 { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
>>                                         avrcp_handle_get_capabilities },
>> @@ -1648,6 +1776,8 @@ static const struct control_pdu_handler control_handlers[] = {
>>                                         avrcp_handle_request_continuing },
>>                 { AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
>>                                         avrcp_handle_abort_continuing },
>> +               { AVRCP_SET_ADDRESSED_PLAYER, AVC_CTYPE_CONTROL,
>> +                                       avrcp_handle_set_addressed_player },
>>                 { },
>>  };
>>
>> @@ -3521,6 +3651,7 @@ static void target_init(struct avrcp *session)
>>                 return;
>>
>>         session->supported_events |=
>> +                               (1 << AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
>>                                 (1 << AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
>>
>>         /* Only check capabilities if controller is not supported */
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v1] audio/avrcp: Add Set Addressed Player support
  2015-08-14 12:44   ` Luiz Augusto von Dentz
@ 2015-08-14 12:51     ` Bharat Bhusan Panda
  0 siblings, 0 replies; 4+ messages in thread
From: Bharat Bhusan Panda @ 2015-08-14 12:51 UTC (permalink / raw)
  To: 'Luiz Augusto von Dentz'; +Cc: linux-bluetooth, cpgs

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@gmail.com]
> Sent: Friday, August 14, 2015 6:15 PM
> To: Bharat Panda
> Cc: linux-bluetooth@vger.kernel.org; cpgs@samsung.com
> Subject: Re: [PATCH v1] audio/avrcp: Add Set Addressed Player support
> 
> Hi Bharat,
> 
> On Fri, Aug 14, 2015 at 3:30 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Hi Bharat,
> >
> > On Thu, Aug 13, 2015 at 6:20 PM, Bharat Panda
> <bharat.panda@samsung.com> wrote:
> >> Support added to handle Set Addressed Player PDU in TG role.
> >> Send EVENT_ADDRESSED_PLAYER_CHANGED on SetAddressedPlayer
> SUCCESS and
> >> follow procedure to reject all player specific events currently
> >> registered with the player.
> >>
> >>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
> >>       AVCTP Control: Command: type 0x00 label 0 PID 0x110e
> >>         AV/C: Control: address 0x48 opcode 0x00
> >>           Subunit: Panel
> >>           Opcode: Vendor Dependent
> >>           Company ID: 0x001958
> >>           AVRCP: SetAddressedPlayer pt Single len 0x0002
> >>             PlayerID: 0x0000 (0)
> >>
> >>       Channel: 64 len 15 [PSM 23 mode 0] {chan 0}
> >>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
> >>         AV/C: Accepted: address 0x48 opcode 0x00
> >>           Subunit: Panel
> >>           Opcode: Vendor Dependent
> >>           Company ID: 0x001958
> >>           AVRCP: SetAddressedPlayer pt Single len 0x0002
> >>             Status: 0x04 (Success)
> >>
> >>       Channel: 64 len 18 [PSM 23 mode 0] {chan 0}
> >>       AVCTP Control: Response: type 0x00 label 0 PID 0x110e
> >>         AV/C: Changed: address 0x48 opcode 0x00
> >>           Subunit: Panel
> >>           Opcode: Vendor Dependent
> >>           Company ID: 0x001958
> >>           AVRCP: RegisterNotification pt Single len 0x0005
> >>             EventID: 0x0b (EVENT_ADDRESSED_PLAYER_CHANGED)
> >>             PlayerID: 0x0000 (0)
> >>             UIDCounter: 0x0000 (0)
> >> ---
> >>  profiles/audio/avrcp.c | 131
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 131 insertions(+)
> >>
> >> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c index
> >> d66f670..61d91ea 100644
> >> --- a/profiles/audio/avrcp.c
> >> +++ b/profiles/audio/avrcp.c
> >> @@ -103,6 +103,7 @@
> >>  #define AVRCP_REQUEST_CONTINUING       0x40
> >>  #define AVRCP_ABORT_CONTINUING         0x41
> >>  #define AVRCP_SET_ABSOLUTE_VOLUME      0x50
> >> +#define AVRCP_SET_ADDRESSED_PLAYER     0x60
> >>  #define AVRCP_SET_BROWSED_PLAYER       0x70
> >>  #define AVRCP_GET_FOLDER_ITEMS         0x71
> >>  #define AVRCP_CHANGE_PATH              0x72
> >> @@ -204,8 +205,10 @@ struct avrcp_player {
> >>         uint64_t uid;
> >>         uint16_t uid_counter;
> >>         bool browsed;
> >> +       bool addressed;
> >>         uint8_t *features;
> >>         char *path;
> >> +       guint notify_id;
> >>
> >>         struct pending_list_items *p;
> >>         char *change_path;
> >> @@ -631,6 +634,7 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >>         GSList *l;
> >>         int attr;
> >>         int val;
> >> +       bool player_changed = false;
> >>
> >>         if (player->sessions == NULL)
> >>                 return;
> >> @@ -674,6 +678,12 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >>                 pdu->params[size++] = attr;
> >>                 pdu->params[size++] = val;
> >>                 break;
> >> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> >> +               size = 5;
> >> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
> >> +               memcpy(&pdu->params[3], &player->uid_counter,
> sizeof(uint16_t));
> >> +               player_changed = true;
> >> +               break;
> >>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
> >>                 size = 1;
> >>                 break;
> >> @@ -695,6 +705,7 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >>                                         session->transaction_events[id],
> >>                                         AVC_CTYPE_CHANGED, AVC_SUBUNIT_PANEL,
> >>                                         buf, size +
> >> AVRCP_HEADER_LENGTH);
> >> +
> >>                 if (err < 0)
> >>                         continue;
> >>
> >> @@ -702,6 +713,52 @@ void avrcp_player_event(struct avrcp_player
> *player, uint8_t id,
> >>                 session->registered_events ^= 1 << id;
> >>         }
> >>
> >> +       if (player_changed) {
> >> +               uint8_t player_events[6] = {
> >> +                                       AVRCP_EVENT_STATUS_CHANGED,
> >> +                                       AVRCP_EVENT_TRACK_CHANGED,
> >> +                                       AVRCP_EVENT_TRACK_REACHED_START,
> >> +                                       AVRCP_EVENT_TRACK_REACHED_END,
> >> +                                       AVRCP_EVENT_SETTINGS_CHANGED,
> >> +                                       AVRCP_EVENT_PLAYBACK_POS_CHANGED
> >> +                                       };
> >> +
> >> +               for (l = player->sessions; l; l = l->next) {
> >> +                       struct avrcp *session = l->data;
> >> +                       int err;
> >> +                       int i;
> >> +
> >> +                       for (i = 0; i < 6; i++) {
> >
> > Check with i < sizeof(player_events) so in case we need to add some
> > new event we don't need to always keep changing this.
> >
> >> +                               if (!(session->registered_events &
> >> +                                               (1 << player_events[i])))
> >> +                                       continue;
> >> +
> >> +                               if (session->registered_events &
> >> +                                               (1 << player_events[i])) {
> >> +                                       session->registered_events &=
> >> +                                               ~(1 << player_events[i]);
> >> +                               /*
> >> +                                * TG shall complete all player specific
> >> +                                * notifications with AV/C C-Type REJECTED
> >> +                                * with error code as Addressed Player Changed.
> >> +                                */
> >> +
> >> +                               pdu->params[0] =
> >> +                                       AVRCP_STATUS_ADDRESSED_PLAYER_CHANGED;
> >> +                               pdu->params_len = htons(1);
> >> +                               err = avctp_send_vendordep(session->conn,
> >> +                                       session->transaction_events[player_events[i]],
> >> +                                       AVC_CTYPE_REJECTED, AVC_SUBUNIT_PANEL,
> >> +                                       buf, 1 + AVRCP_HEADER_LENGTH);
> >> +                               }
> >> +                       }
> >> +
> >> +                       if (err < 0)
> >> +                               continue;
> >> +
> >> +               }
> >> +       }
> >> +
> >>         return;
> >>  }
> >>
> >> @@ -1494,6 +1551,11 @@ static uint8_t
> avrcp_handle_register_notification(struct avrcp *session,
> >>                 }
> >>
> >>                 break;
> >> +       case AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED:
> >> +               len = 5;
> >> +               memcpy(&pdu->params[1], &player->id, sizeof(uint16_t));
> >> +               memcpy(&pdu->params[3], &player->uid_counter,
> sizeof(uint16_t));
> >> +               break;
> >>         case AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED:
> >>                 len = 1;
> >>                 break;
> >> @@ -1617,6 +1679,72 @@ err:
> >>         return AVC_CTYPE_REJECTED;
> >>  }
> >>
> >> +static struct avrcp_player *find_tg_player(struct avrcp *session,
> >> +uint16_t id) {
> >> +       struct avrcp_server *server = session->server;
> >> +       GSList *l;
> >> +
> >> +       for (l = server->players; l; l = l->next) {
> >> +               struct avrcp_player *player = l->data;
> >> +
> >> +               if (player->id == id)
> >> +                       return player;
> >> +       }
> >> +
> >> +       return NULL;
> >> +}
> >> +
> >> +static gboolean notify_addressed_player_changed(gpointer user_data)
> >> +{
> >> +       struct avrcp_player *player = user_data;
> >> +
> >> +       avrcp_player_event(player,
> >> +                               AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED, NULL);
> >> +       g_source_remove(player->notify_id);
> >
> > Removing the source wouldn't be necessary if you return FALSE or
> > G_SOURCE_REMOVE if we want to more gnomish, furthermore notify_id
> > shall be set to 0 either way and the call to g_source_remove shall be
> > move to where the player freed so we stop the callback to be called if
> > the player is unregistered.
> 
> Actually don't use G_SOURCE_REMOVE since it was introduced in 2.32 and
> we depend on 2.28 or later.
Ok, so here we should return only FALSE and upon unregister_player, will remove source using g_source_remove(player->notify_id) will be enough then.
> 
> >
> >> +
> >> +       return TRUE;
> >> +}
> >> +
> >> +static uint8_t avrcp_handle_set_addressed_player(struct avrcp
> *session,
> >> +                                               struct avrcp_header *pdu,
> >> +                                               uint8_t transaction)
> >> +{
> >> +       struct avrcp_player *player;
> >> +       uint16_t len = ntohs(pdu->params_len);
> >> +       uint16_t player_id = 0;
> >> +       uint8_t status;
> >> +
> >> +       if (len < 1) {
> >> +               status = AVRCP_STATUS_INVALID_PARAM;
> >> +               goto err;
> >> +       }
> >> +
> >> +       player_id = bt_get_be16(&pdu->params[0]);
> >> +       player = find_tg_player(session, player_id);
> >> +       pdu->packet_type = AVRCP_PACKET_TYPE_SINGLE;
> >> +
> >> +       if (player) {
> >> +               player->addressed = true;
> >> +               status = AVRCP_STATUS_SUCCESS;
> >> +               pdu->params_len = htons(len);
> >> +               pdu->params[0] = status;
> >> +       } else {
> >> +               status = AVRCP_STATUS_INVALID_PLAYER_ID;
> >> +               goto err;
> >> +       }
> >> +
> >> +       player->notify_id =
> >> +
> g_idle_add((GSourceFunc)notify_addressed_player_changed,
> >> +                       player);
> >> +
> >> +       return AVC_CTYPE_ACCEPTED;
> >> +
> >> +err:
> >> +       pdu->params_len = htons(sizeof(status));
> >> +       pdu->params[0] = status;
> >> +       return AVC_CTYPE_REJECTED;
> >> +}
> >> +
> >>  static const struct control_pdu_handler control_handlers[] = {
> >>                 { AVRCP_GET_CAPABILITIES, AVC_CTYPE_STATUS,
> >>                                         avrcp_handle_get_capabilities
> >> }, @@ -1648,6 +1776,8 @@ static const struct control_pdu_handler
> control_handlers[] = {
> >>                                         avrcp_handle_request_continuing },
> >>                 { AVRCP_ABORT_CONTINUING, AVC_CTYPE_CONTROL,
> >>                                         avrcp_handle_abort_continuing
> >> },
> >> +               { AVRCP_SET_ADDRESSED_PLAYER, AVC_CTYPE_CONTROL,
> >> +
> >> + avrcp_handle_set_addressed_player },
> >>                 { },
> >>  };
> >>
> >> @@ -3521,6 +3651,7 @@ static void target_init(struct avrcp *session)
> >>                 return;
> >>
> >>         session->supported_events |=
> >> +                               (1 <<
> >> + AVRCP_EVENT_ADDRESSED_PLAYER_CHANGED) |
> >>                                 (1 <<
> >> AVRCP_EVENT_AVAILABLE_PLAYERS_CHANGED);
> >>
> >>         /* Only check capabilities if controller is not supported */
> >> --
> >> 1.9.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe
> >> linux-bluetooth" in the body of a message to
> >> majordomo@vger.kernel.org More majordomo info at
> >> http://vger.kernel.org/majordomo-info.html
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
> 
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Bharat


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-08-14 12:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-13 15:20 [PATCH v1] audio/avrcp: Add Set Addressed Player support Bharat Panda
2015-08-14 12:30 ` Luiz Augusto von Dentz
2015-08-14 12:44   ` Luiz Augusto von Dentz
2015-08-14 12:51     ` Bharat Bhusan Panda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).