[PATCH BlueZ 1/5] AVCTP: Fix setting wrong transaction id expected for responses

[PATCH BlueZ 1/5] AVCTP: Fix setting wrong transaction id expected for responses

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

The id were incremented after being set to the request so it is always
+1 of the actual transaction.
---
 audio/avctp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index d98d097..f2d5954 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -981,7 +981,7 @@ int avctp_send_vendordep_req(struct avctp *session, uint8_t code,
 	struct avctp_rsp_handler *handler;
 	int err;
 
-	err = avctp_send(session, id++, AVCTP_COMMAND, code, subunit,
+	err = avctp_send(session, ++id, AVCTP_COMMAND, code, subunit,
 				AVC_OP_VENDORDEP, operands, operand_count);
 	if (err < 0)
 		return err;
-- 
1.7.7.6

[PATCH BlueZ 2/5] AVRCP: Fix not setting audio device connected to player

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

While connecting device should be set to match the AVCTP session and when
disconnected reset it back to NULL.
---
 audio/avrcp.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index cb6906d..b09fab0 100755
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1197,6 +1197,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 	switch (new_state) {
 	case AVCTP_STATE_DISCONNECTED:
 		player->session = NULL;
+		player->dev = NULL;
 		player->registered_events = 0;
 
 		if (player->handler) {
@@ -1207,6 +1208,7 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
 		break;
 	case AVCTP_STATE_CONNECTING:
 		player->session = avctp_connect(&dev->src, &dev->dst);
+		player->dev = dev;
 
 		if (!player->handler)
 			player->handler = avctp_register_pdu_handler(
-- 
1.7.7.6

[PATCH BlueZ 3/5] AVRCP: Add support for sending SetAbsoluteVolume

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Once the transport volume is changed update the remote volume by sending
SetAbsoluteVolume:

< AVCTP: Command : pt 0x00 transaction 9 pid 0x110e
    AV/C: Changed: address 0x48 opcode 0x00
      Subunit: Panel
      Opcode: Vendor Dependent
      Company ID: 0x001958
      AVRCP: SetAbsoluteVolume: pt Single len 0x0001
        Volume: 100.00% (127/127)
> AVCTP: Response : pt 0x00 transaction 9 pid 0x110e
    AV/C: Accepted: address 0x48 opcode 0x00
      Subunit: Panel
      Opcode: Vendor Dependent
      Company ID: 0x001958
      AVRCP: SetAbsoluteVolume: pt Single len 0x0001
        Volume: 100.00% (127/127)
---
 audio/avrcp.c     |   34 ++++++++++++++++++++++++++++++++++
 audio/avrcp.h     |    2 ++
 audio/transport.c |   15 +++++++++++++++
 3 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index b09fab0..cdec75c 100755
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -87,6 +87,7 @@
 #define AVRCP_REGISTER_NOTIFICATION	0x31
 #define AVRCP_REQUEST_CONTINUING	0x40
 #define AVRCP_ABORT_CONTINUING		0x41
+#define AVRCP_SET_ABSOLUTE_VOLUME	0x50
 
 /* Capabilities for AVRCP_GET_CAPABILITIES pdu */
 #define CAP_COMPANY_ID		0x02
@@ -1404,3 +1405,36 @@ void avrcp_unregister_player(struct avrcp_player *player)
 
 	player_destroy(player);
 }
+
+int avrcp_set_volume(struct audio_device *dev, uint8_t volume)
+{
+	struct avrcp_server *server;
+	struct avrcp_player *player;
+	uint8_t buf[AVRCP_HEADER_LENGTH + 1];
+	struct avrcp_header *pdu = (void *) buf;
+
+	server = find_server(servers, &dev->src);
+	if (server == NULL)
+		return -EINVAL;
+
+	player = server->active_player;
+	if (player == NULL)
+		return -ENOTSUP;
+
+	if (player->session == NULL)
+		return -ENOTCONN;
+
+	memset(buf, 0, sizeof(buf));
+
+	set_company_id(pdu->company_id, IEEEID_BTSIG);
+
+	pdu->pdu_id = AVRCP_SET_ABSOLUTE_VOLUME;
+	pdu->params[0] = volume;
+	pdu->params_len = htons(1);
+
+	DBG("volume=%u", volume);
+
+	return avctp_send_vendordep_req(player->session, AVC_CTYPE_CHANGED,
+					AVC_SUBUNIT_PANEL, buf, sizeof(buf),
+					NULL, NULL);
+}
diff --git a/audio/avrcp.h b/audio/avrcp.h
index b520ef6..bf11a6c 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -93,6 +93,7 @@ void avrcp_unregister(const bdaddr_t *src);
 
 gboolean avrcp_connect(struct audio_device *dev);
 void avrcp_disconnect(struct audio_device *dev);
+int avrcp_set_volume(struct audio_device *dev, uint8_t volume);
 
 struct avrcp_player *avrcp_register_player(const bdaddr_t *src,
 						struct avrcp_player_cb *cb,
@@ -102,4 +103,5 @@ void avrcp_unregister_player(struct avrcp_player *player);
 
 int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
 
+
 size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
diff --git a/audio/transport.c b/audio/transport.c
index 4ad8608..b4af0e7 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -43,6 +43,7 @@
 #include "a2dp.h"
 #include "headset.h"
 #include "gateway.h"
+#include "avrcp.h"
 
 #ifndef DBUS_TYPE_UNIX_FD
 #define DBUS_TYPE_UNIX_FD -1
@@ -753,6 +754,20 @@ static int set_property_a2dp(struct media_transport *transport,
 
 		/* FIXME: send new delay */
 		return 0;
+	} else if (g_strcmp0(property, "Volume") == 0) {
+		uint8_t volume;
+
+		if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_BYTE)
+			return -EINVAL;
+
+		dbus_message_iter_get_basic(value, &volume);
+
+		if (transport->volume == volume)
+			return 0;
+
+		transport->volume = volume;
+
+		return avrcp_set_volume(transport->device, volume);
 	}
 
 	return -EINVAL;
-- 
1.7.7.6

[PATCH BlueZ 4/5] audio: Add Volume property to A2DP transport GetProperties

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Now that volume is being handled by SetProperty it should also be
available in GetProperties.
---
 audio/transport.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/audio/transport.c b/audio/transport.c
index b4af0e7..3bc7a36 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -78,7 +78,7 @@ struct media_transport {
 	uint16_t		omtu;		/* Transport output mtu */
 	uint16_t		delay;		/* Transport delay (a2dp only) */
 	unsigned int		nrec_id;	/* Transport nrec watch (headset only) */
-	uint8_t			volume;		/* Transport volume */
+	int8_t			volume;		/* Transport volume */
 	gboolean		read_lock;
 	gboolean		write_lock;
 	gboolean		in_use;
@@ -857,6 +857,10 @@ static void get_properties_a2dp(struct media_transport *transport,
 						DBusMessageIter *dict)
 {
 	dict_append_entry(dict, "Delay", DBUS_TYPE_UINT16, &transport->delay);
+
+	if (transport->volume >= 0)
+		dict_append_entry(dict, "Volume", DBUS_TYPE_BYTE,
+							&transport->volume);
 }
 
 static void get_properties_headset(struct media_transport *transport,
@@ -1012,6 +1016,7 @@ struct media_transport *media_transport_create(DBusConnection *conn,
 	transport->size = size;
 	transport->path = g_strdup_printf("%s/fd%d", device->path, fd++);
 	transport->fd = -1;
+	transport->volume = -1;
 
 	uuid = media_endpoint_get_uuid(endpoint);
 	if (strcasecmp(uuid, A2DP_SOURCE_UUID) == 0 ||
-- 
1.7.7.6

[PATCH BlueZ 5/5] media-api: Update documentation to include Volume property

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 doc/media-api.txt |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/doc/media-api.txt b/doc/media-api.txt
index 7a593c3..20772f0 100644
--- a/doc/media-api.txt
+++ b/doc/media-api.txt
@@ -341,3 +341,11 @@ Properties	object Device [readonly]
 			Optional. Indicates where is the transport being routed
 
 			Possible Values: "HCI" or "PCM"
+
+		byte Volume [readwrite]
+
+			Optional. Indicates volume level of the transport,
+			this property is only writeable when the transport was
+			acquired by the sender.
+
+			Possible Values: 0-127
-- 
1.7.7.6

Re: [PATCH BlueZ 1/5] AVCTP: Fix setting wrong transaction id expected for responses

[Johan Hedberg]

Hi Luiz,

On Tue, May 22, 2012, Luiz Augusto von Dentz wrote:
> The id were incremented after being set to the request so it is always
> +1 of the actual transaction.
> ---
>  audio/avctp.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/audio/avctp.c b/audio/avctp.c
> index d98d097..f2d5954 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -981,7 +981,7 @@ int avctp_send_vendordep_req(struct avctp *session, uint8_t code,
>  	struct avctp_rsp_handler *handler;
>  	int err;
>  
> -	err = avctp_send(session, id++, AVCTP_COMMAND, code, subunit,
> +	err = avctp_send(session, ++id, AVCTP_COMMAND, code, subunit,
>  				AVC_OP_VENDORDEP, operands, operand_count);
>  	if (err < 0)
>  		return err;

By switching to pre-increment you're essentially jumping over one
possible value since the send_passthrough function uses post-increment.
Not a very big issue, but what would be a bit cleaner imo is to drop the
increment from the avctp_send call and instead do a post-increment
together with the assignment to handler->id.

Johan

Re: [PATCH BlueZ 5/5] media-api: Update documentation to include Volume property

[Johan Hedberg]

Hi Luiz,

On Tue, May 22, 2012, Luiz Augusto von Dentz wrote:
> ---
>  doc/media-api.txt |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/media-api.txt b/doc/media-api.txt
> index 7a593c3..20772f0 100644
> --- a/doc/media-api.txt
> +++ b/doc/media-api.txt
> @@ -341,3 +341,11 @@ Properties	object Device [readonly]
>  			Optional. Indicates where is the transport being routed
>  
>  			Possible Values: "HCI" or "PCM"
> +
> +		byte Volume [readwrite]
> +
> +			Optional. Indicates volume level of the transport,
> +			this property is only writeable when the transport was
> +			acquired by the sender.
> +
> +			Possible Values: 0-127

I'm not sure if byte is best here. I'd rather have this as an unsigned
value (the code itself looks strange too with you having to switch to
int8_t from uint8_t). Since there's no one byte unsigned D-Bus parameter
I suppose we should use uint16.

Johan

Re: [PATCH BlueZ 3/5] AVRCP: Add support for sending SetAbsoluteVolume

[Johan Hedberg]

Hi Luiz,

On Tue, May 22, 2012, Luiz Augusto von Dentz wrote:
>  int avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
>  
> +
>  size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);

Looks like you've got an extra empty line above.

Johan

Re: [PATCH BlueZ 1/5] AVCTP: Fix setting wrong transaction id expected for responses

[Luiz Augusto von Dentz]

Hi Johan,

On Wed, May 23, 2012 at 5:21 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Luiz,
>
> On Tue, May 22, 2012, Luiz Augusto von Dentz wrote:
>> The id were incremented after being set to the request so it is always
>> +1 of the actual transaction.
>> ---
>>  audio/avctp.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/audio/avctp.c b/audio/avctp.c
>> index d98d097..f2d5954 100644
>> --- a/audio/avctp.c
>> +++ b/audio/avctp.c
>> @@ -981,7 +981,7 @@ int avctp_send_vendordep_req(struct avctp *session, uint8_t code,
>>       struct avctp_rsp_handler *handler;
>>       int err;
>>
>> -     err = avctp_send(session, id++, AVCTP_COMMAND, code, subunit,
>> +     err = avctp_send(session, ++id, AVCTP_COMMAND, code, subunit,
>>                               AVC_OP_VENDORDEP, operands, operand_count);
>>       if (err < 0)
>>               return err;
>
> By switching to pre-increment you're essentially jumping over one
> possible value since the send_passthrough function uses post-increment.
> Not a very big issue, but what would be a bit cleaner imo is to drop the
> increment from the avctp_send call and instead do a post-increment
> together with the assignment to handler->id.

Yep, I will change as suggested.


-- 
Luiz Augusto von Dentz