* [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit
@ 2020-07-23 7:23 Archie Pusaka
2020-07-23 7:23 ` [Bluez PATCH v5 2/2] audio/transport: supply volume on transport init Archie Pusaka
2020-07-23 16:40 ` [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Luiz Augusto von Dentz
0 siblings, 2 replies; 3+ messages in thread
From: Archie Pusaka @ 2020-07-23 7:23 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz; +Cc: Archie Pusaka, Michael Sun
From: Archie Pusaka <apusaka@chromium.org>
The valid range of volume is 0 - 127, yet it is stored in 16bit
data type. This patch modifies it so we use 8bit data type to
store volume instead. Furthermore we also use signed type, so
negative values can be used to indicate invalid volume.
Reviewed-by: Michael Sun <michaelfsun@google.com>
---
Changes in v5:
-Fix coding style and conversion error
Changes in v4:
-Use int8_t instead of uint8_t
Changes in v3: None
Changes in v2: None
profiles/audio/avrcp.c | 17 +++++++++-----
profiles/audio/avrcp.h | 35 ++++++++++++++---------------
profiles/audio/media.c | 4 ++--
profiles/audio/transport.c | 45 +++++++++++++++++++-------------------
profiles/audio/transport.h | 8 +++----
5 files changed, 56 insertions(+), 53 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 1bf85041e..7a06c8353 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1595,6 +1595,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
struct btd_device *dev = session->dev;
uint16_t len = ntohs(pdu->params_len);
uint64_t uid;
+ int8_t volume;
GList *settings;
/*
@@ -1659,10 +1660,11 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
len = 1;
break;
case AVRCP_EVENT_VOLUME_CHANGED:
- pdu->params[1] = media_transport_get_device_volume(dev);
- if (pdu->params[1] > 127)
+ volume = media_transport_get_device_volume(dev);
+ if (volume < 0)
goto err;
+ pdu->params[1] = volume;
len = 2;
break;
@@ -1757,7 +1759,7 @@ static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
uint8_t transaction)
{
uint16_t len = ntohs(pdu->params_len);
- uint8_t volume;
+ int8_t volume;
if (len != 1)
goto err;
@@ -3623,7 +3625,7 @@ static void avrcp_volume_changed(struct avrcp *session,
struct avrcp_header *pdu)
{
struct avrcp_player *player = target_get_player(session);
- uint8_t volume;
+ int8_t volume;
volume = pdu->params[1] & 0x7F;
@@ -4371,7 +4373,7 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
struct avrcp *session = user_data;
struct avrcp_player *player = target_get_player(session);
struct avrcp_header *pdu = (void *) operands;
- uint8_t volume;
+ int8_t volume;
if (code == AVC_CTYPE_REJECTED || code == AVC_CTYPE_NOT_IMPLEMENTED ||
pdu == NULL)
@@ -4434,13 +4436,16 @@ static int avrcp_event(struct avrcp *session, uint8_t id, const void *data)
return err;
}
-int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify)
+int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
{
struct avrcp_server *server;
struct avrcp *session;
uint8_t buf[AVRCP_HEADER_LENGTH + 1];
struct avrcp_header *pdu = (void *) buf;
+ if (volume < 0)
+ return -EINVAL;
+
server = find_server(servers, device_get_adapter(dev));
if (server == NULL)
return -EINVAL;
diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index 86d310c73..a08e7325e 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -83,27 +83,27 @@
#define AVRCP_EVENT_LAST AVRCP_EVENT_VOLUME_CHANGED
struct avrcp_player_cb {
- GList *(*list_settings) (void *user_data);
- const char *(*get_setting) (const char *key, void *user_data);
- int (*set_setting) (const char *key, const char *value,
+ GList *(*list_settings)(void *user_data);
+ const char *(*get_setting)(const char *key, void *user_data);
+ int (*set_setting)(const char *key, const char *value,
void *user_data);
- uint64_t (*get_uid) (void *user_data);
- const char *(*get_metadata) (const char *key, void *user_data);
- GList *(*list_metadata) (void *user_data);
- const char *(*get_status) (void *user_data);
- uint32_t (*get_position) (void *user_data);
- uint32_t (*get_duration) (void *user_data);
- const char *(*get_name) (void *user_data);
- void (*set_volume) (uint8_t volume, struct btd_device *dev,
+ uint64_t (*get_uid)(void *user_data);
+ const char *(*get_metadata)(const char *key, void *user_data);
+ GList *(*list_metadata)(void *user_data);
+ const char *(*get_status)(void *user_data);
+ uint32_t (*get_position)(void *user_data);
+ uint32_t (*get_duration)(void *user_data);
+ const char *(*get_name)(void *user_data);
+ void (*set_volume)(int8_t volume, struct btd_device *dev,
void *user_data);
- bool (*play) (void *user_data);
- bool (*stop) (void *user_data);
- bool (*pause) (void *user_data);
- bool (*next) (void *user_data);
- bool (*previous) (void *user_data);
+ bool (*play)(void *user_data);
+ bool (*stop)(void *user_data);
+ bool (*pause)(void *user_data);
+ bool (*next)(void *user_data);
+ bool (*previous)(void *user_data);
};
-int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify);
+int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify);
struct avrcp_player *avrcp_register_player(struct btd_adapter *adapter,
struct avrcp_player_cb *cb,
@@ -114,6 +114,5 @@ void avrcp_unregister_player(struct avrcp_player *player);
void avrcp_player_event(struct avrcp_player *player, uint8_t id,
const void *data);
-
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
size_t avrcp_browsing_general_reject(uint8_t *operands);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index a0173fdd4..d4d58ec86 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -120,7 +120,7 @@ struct media_player {
char *status;
uint32_t position;
uint32_t duration;
- uint8_t volume;
+ int8_t volume;
GTimer *timer;
bool play;
bool pause;
@@ -1199,7 +1199,7 @@ static uint32_t get_duration(void *user_data)
return mp->duration;
}
-static void set_volume(uint8_t volume, struct btd_device *dev, void *user_data)
+static void set_volume(int8_t volume, struct btd_device *dev, void *user_data)
{
struct media_player *mp = user_data;
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 48fabba9b..2ae5118c4 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -86,7 +86,7 @@ struct media_owner {
struct a2dp_transport {
struct avdtp *session;
uint16_t delay;
- uint16_t volume;
+ int8_t volume;
};
struct media_transport {
@@ -634,7 +634,7 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
struct media_transport *transport = data;
struct a2dp_transport *a2dp = transport->data;
- return a2dp->volume <= 127;
+ return a2dp->volume >= 0;
}
static gboolean get_volume(const GDBusPropertyTable *property,
@@ -642,8 +642,9 @@ static gboolean get_volume(const GDBusPropertyTable *property,
{
struct media_transport *transport = data;
struct a2dp_transport *a2dp = transport->data;
+ uint16_t volume = (uint16_t)a2dp->volume;
- dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &a2dp->volume);
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
return TRUE;
}
@@ -654,27 +655,20 @@ static void set_volume(const GDBusPropertyTable *property,
{
struct media_transport *transport = data;
struct a2dp_transport *a2dp = transport->data;
- uint16_t volume;
+ uint16_t arg;
+ int8_t volume;
bool notify;
- if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16) {
- g_dbus_pending_property_error(id,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
+ goto error;
- dbus_message_iter_get_basic(iter, &volume);
-
- if (volume > 127) {
- g_dbus_pending_property_error(id,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- return;
- }
+ dbus_message_iter_get_basic(iter, &arg);
+ if (arg > INT8_MAX)
+ goto error;
g_dbus_pending_property_success(id);
+ volume = (int8_t)arg;
if (a2dp->volume == volume)
return;
@@ -688,6 +682,11 @@ static void set_volume(const GDBusPropertyTable *property,
"Volume");
avrcp_set_volume(transport->device, volume, notify);
+ return;
+
+error:
+ g_dbus_pending_property_error(id, ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
}
static gboolean endpoint_exists(const GDBusPropertyTable *property, void *data)
@@ -931,14 +930,14 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
return transport->device;
}
-uint16_t media_transport_get_volume(struct media_transport *transport)
+int8_t media_transport_get_volume(struct media_transport *transport)
{
struct a2dp_transport *a2dp = transport->data;
return a2dp->volume;
}
void media_transport_update_volume(struct media_transport *transport,
- uint8_t volume)
+ int8_t volume)
{
struct a2dp_transport *a2dp = transport->data;
@@ -953,12 +952,12 @@ void media_transport_update_volume(struct media_transport *transport,
MEDIA_TRANSPORT_INTERFACE, "Volume");
}
-uint8_t media_transport_get_device_volume(struct btd_device *dev)
+int8_t media_transport_get_device_volume(struct btd_device *dev)
{
GSList *l;
if (dev == NULL)
- return 128;
+ return -1;
for (l = transports; l; l = l->next) {
struct media_transport *transport = l->data;
@@ -974,7 +973,7 @@ uint8_t media_transport_get_device_volume(struct btd_device *dev)
}
void media_transport_update_device_volume(struct btd_device *dev,
- uint8_t volume)
+ int8_t volume)
{
GSList *l;
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index ac542bf6c..78024372f 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -32,14 +32,14 @@ struct media_transport *media_transport_create(struct btd_device *device,
void media_transport_destroy(struct media_transport *transport);
const char *media_transport_get_path(struct media_transport *transport);
struct btd_device *media_transport_get_dev(struct media_transport *transport);
-uint16_t media_transport_get_volume(struct media_transport *transport);
+int8_t media_transport_get_volume(struct media_transport *transport);
void media_transport_update_delay(struct media_transport *transport,
uint16_t delay);
void media_transport_update_volume(struct media_transport *transport,
- uint8_t volume);
+ int8_t volume);
void transport_get_properties(struct media_transport *transport,
DBusMessageIter *iter);
-uint8_t media_transport_get_device_volume(struct btd_device *dev);
+int8_t media_transport_get_device_volume(struct btd_device *dev);
void media_transport_update_device_volume(struct btd_device *dev,
- uint8_t volume);
+ int8_t volume);
--
2.28.0.rc0.105.gf9edc3c819-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Bluez PATCH v5 2/2] audio/transport: supply volume on transport init
2020-07-23 7:23 [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Archie Pusaka
@ 2020-07-23 7:23 ` Archie Pusaka
2020-07-23 16:40 ` [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Archie Pusaka @ 2020-07-23 7:23 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz; +Cc: Archie Pusaka, Yu Liu
From: Archie Pusaka <apusaka@chromium.org>
Sometimes the response of RegisterNotification for volume change
event came before we create the transport for the corresponding
device. If that happens, the volume will be stuck to an
uninitialized invalid value. The property Volume of
MediaTransport1 will also be left unaccessible.
This patch supplies the initial volume when creating a new
transport. The value is obtained from the media_player object.
However, since the avrcp session might not be created by the time
the transport is created, we also try to initialize the volume
when creating avrcp session.
Reviewed-by: Yu Liu <yudiliu@google.com>
---
Changes in v5: None
Changes in v4: None
Changes in v3:
-Add missing library
Changes in v2:
-Get the volume from media_player instead of from separate list
profiles/audio/avrcp.c | 27 ++++++++++++++++-
profiles/audio/avrcp.h | 2 ++
profiles/audio/media.c | 60 +++++++++++++++++++++++++++++---------
profiles/audio/media.h | 2 ++
profiles/audio/transport.c | 2 +-
5 files changed, 77 insertions(+), 16 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 7a06c8353..4e7ff75c0 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -64,6 +64,7 @@
#include "avctp.h"
#include "avrcp.h"
#include "control.h"
+#include "media.h"
#include "player.h"
#include "transport.h"
@@ -1703,7 +1704,6 @@ static uint8_t avrcp_handle_request_continuing(struct avrcp *session,
if (pending->pdu_id != pdu->params[0])
goto err;
-
len = 0;
pending->attr_ids = player_fill_media_attribute(player,
pending->attr_ids,
@@ -4039,8 +4039,12 @@ static void target_init(struct avrcp *session)
player = g_slist_nth_data(server->players, 0);
if (player != NULL) {
+ int8_t init_volume;
target->player = player;
player->sessions = g_slist_prepend(player->sessions, session);
+
+ init_volume = media_player_get_device_volume(session->dev);
+ media_transport_update_device_volume(session->dev, init_volume);
}
session->supported_events |= (1 << AVRCP_EVENT_STATUS_CHANGED) |
@@ -4478,6 +4482,27 @@ int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
avrcp_handle_set_volume, session);
}
+struct avrcp_player *avrcp_get_target_player_by_device(struct btd_device *dev)
+{
+ struct avrcp_server *server;
+ struct avrcp *session;
+ struct avrcp_data *target;
+
+ server = find_server(servers, device_get_adapter(dev));
+ if (server == NULL)
+ return NULL;
+
+ session = find_session(server->sessions, dev);
+ if (session == NULL)
+ return NULL;
+
+ target = session->target;
+ if (target == NULL)
+ return NULL;
+
+ return target->player;
+}
+
static int avrcp_connect(struct btd_service *service)
{
struct btd_device *dev = btd_service_get_device(service);
diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
index a08e7325e..159ccf846 100644
--- a/profiles/audio/avrcp.h
+++ b/profiles/audio/avrcp.h
@@ -116,3 +116,5 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id,
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
size_t avrcp_browsing_general_reject(uint8_t *operands);
+
+struct avrcp_player *avrcp_get_target_player_by_device(struct btd_device *dev);
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index d4d58ec86..02bf82a49 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -239,6 +239,20 @@ static void media_endpoint_exit(DBusConnection *connection, void *user_data)
media_endpoint_remove(endpoint);
}
+static struct media_adapter *find_adapter(struct btd_device *device)
+{
+ GSList *l;
+
+ for (l = adapters; l; l = l->next) {
+ struct media_adapter *adapter = l->data;
+
+ if (adapter->btd_adapter == device_get_adapter(device))
+ return adapter;
+ }
+
+ return NULL;
+}
+
static void clear_configuration(struct media_endpoint *endpoint,
struct media_transport *transport)
{
@@ -426,6 +440,33 @@ struct a2dp_config_data {
a2dp_endpoint_config_t cb;
};
+int8_t media_player_get_device_volume(struct btd_device *device)
+{
+ struct avrcp_player *target_player;
+ struct media_adapter *adapter;
+ GSList *l;
+
+ if (!device)
+ return -1;
+
+ target_player = avrcp_get_target_player_by_device(device);
+ if (!target_player)
+ return -1;
+
+ adapter = find_adapter(device);
+ if (!adapter)
+ return -1;
+
+ for (l = adapter->players; l; l = l->next) {
+ struct media_player *mp = l->data;
+
+ if (mp->player == target_player)
+ return mp->volume;
+ }
+
+ return -1;
+}
+
static gboolean set_configuration(struct media_endpoint *endpoint,
uint8_t *configuration, size_t size,
media_endpoint_cb_t cb,
@@ -439,6 +480,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
const char *path;
DBusMessageIter iter;
struct media_transport *transport;
+ int8_t init_volume;
transport = find_device_transport(endpoint, device);
@@ -451,6 +493,9 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
if (transport == NULL)
return FALSE;
+ init_volume = media_player_get_device_volume(device);
+ media_transport_update_volume(transport, init_volume);
+
msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
MEDIA_ENDPOINT_INTERFACE,
"SetConfiguration");
@@ -646,20 +691,6 @@ static gboolean endpoint_init_a2dp_sink(struct media_endpoint *endpoint,
return TRUE;
}
-static struct media_adapter *find_adapter(struct btd_device *device)
-{
- GSList *l;
-
- for (l = adapters; l; l = l->next) {
- struct media_adapter *adapter = l->data;
-
- if (adapter->btd_adapter == device_get_adapter(device))
- return adapter;
- }
-
- return NULL;
-}
-
static bool endpoint_properties_exists(const char *uuid,
struct btd_device *dev,
void *user_data)
@@ -1779,6 +1810,7 @@ static struct media_player *media_player_create(struct media_adapter *adapter,
mp->sender = g_strdup(sender);
mp->path = g_strdup(path);
mp->timer = g_timer_new();
+ mp->volume = -1;
mp->watch = g_dbus_add_disconnect_watch(conn, sender,
media_player_exit, mp,
diff --git a/profiles/audio/media.h b/profiles/audio/media.h
index dd630d432..53694f4c6 100644
--- a/profiles/audio/media.h
+++ b/profiles/audio/media.h
@@ -33,3 +33,5 @@ void media_unregister(struct btd_adapter *btd_adapter);
struct a2dp_sep *media_endpoint_get_sep(struct media_endpoint *endpoint);
const char *media_endpoint_get_uuid(struct media_endpoint *endpoint);
uint8_t media_endpoint_get_codec(struct media_endpoint *endpoint);
+
+int8_t media_player_get_device_volume(struct btd_device *device);
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 2ae5118c4..a2c4f7dfb 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -977,7 +977,7 @@ void media_transport_update_device_volume(struct btd_device *dev,
{
GSList *l;
- if (dev == NULL)
+ if (dev == NULL || volume < 0)
return;
for (l = transports; l; l = l->next) {
--
2.28.0.rc0.105.gf9edc3c819-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit
2020-07-23 7:23 [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Archie Pusaka
2020-07-23 7:23 ` [Bluez PATCH v5 2/2] audio/transport: supply volume on transport init Archie Pusaka
@ 2020-07-23 16:40 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2020-07-23 16:40 UTC (permalink / raw)
To: Archie Pusaka; +Cc: linux-bluetooth, Archie Pusaka, Michael Sun
Hi Archie,
On Thu, Jul 23, 2020 at 12:23 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> The valid range of volume is 0 - 127, yet it is stored in 16bit
> data type. This patch modifies it so we use 8bit data type to
> store volume instead. Furthermore we also use signed type, so
> negative values can be used to indicate invalid volume.
>
> Reviewed-by: Michael Sun <michaelfsun@google.com>
> ---
>
> Changes in v5:
> -Fix coding style and conversion error
>
> Changes in v4:
> -Use int8_t instead of uint8_t
>
> Changes in v3: None
> Changes in v2: None
>
> profiles/audio/avrcp.c | 17 +++++++++-----
> profiles/audio/avrcp.h | 35 ++++++++++++++---------------
> profiles/audio/media.c | 4 ++--
> profiles/audio/transport.c | 45 +++++++++++++++++++-------------------
> profiles/audio/transport.h | 8 +++----
> 5 files changed, 56 insertions(+), 53 deletions(-)
>
> diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
> index 1bf85041e..7a06c8353 100644
> --- a/profiles/audio/avrcp.c
> +++ b/profiles/audio/avrcp.c
> @@ -1595,6 +1595,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
> struct btd_device *dev = session->dev;
> uint16_t len = ntohs(pdu->params_len);
> uint64_t uid;
> + int8_t volume;
> GList *settings;
>
> /*
> @@ -1659,10 +1660,11 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
> len = 1;
> break;
> case AVRCP_EVENT_VOLUME_CHANGED:
> - pdu->params[1] = media_transport_get_device_volume(dev);
> - if (pdu->params[1] > 127)
> + volume = media_transport_get_device_volume(dev);
> + if (volume < 0)
> goto err;
>
> + pdu->params[1] = volume;
> len = 2;
>
> break;
> @@ -1757,7 +1759,7 @@ static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
> uint8_t transaction)
> {
> uint16_t len = ntohs(pdu->params_len);
> - uint8_t volume;
> + int8_t volume;
>
> if (len != 1)
> goto err;
> @@ -3623,7 +3625,7 @@ static void avrcp_volume_changed(struct avrcp *session,
> struct avrcp_header *pdu)
> {
> struct avrcp_player *player = target_get_player(session);
> - uint8_t volume;
> + int8_t volume;
>
> volume = pdu->params[1] & 0x7F;
>
> @@ -4371,7 +4373,7 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
> struct avrcp *session = user_data;
> struct avrcp_player *player = target_get_player(session);
> struct avrcp_header *pdu = (void *) operands;
> - uint8_t volume;
> + int8_t volume;
>
> if (code == AVC_CTYPE_REJECTED || code == AVC_CTYPE_NOT_IMPLEMENTED ||
> pdu == NULL)
> @@ -4434,13 +4436,16 @@ static int avrcp_event(struct avrcp *session, uint8_t id, const void *data)
> return err;
> }
>
> -int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify)
> +int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify)
> {
> struct avrcp_server *server;
> struct avrcp *session;
> uint8_t buf[AVRCP_HEADER_LENGTH + 1];
> struct avrcp_header *pdu = (void *) buf;
>
> + if (volume < 0)
> + return -EINVAL;
> +
> server = find_server(servers, device_get_adapter(dev));
> if (server == NULL)
> return -EINVAL;
> diff --git a/profiles/audio/avrcp.h b/profiles/audio/avrcp.h
> index 86d310c73..a08e7325e 100644
> --- a/profiles/audio/avrcp.h
> +++ b/profiles/audio/avrcp.h
> @@ -83,27 +83,27 @@
> #define AVRCP_EVENT_LAST AVRCP_EVENT_VOLUME_CHANGED
>
> struct avrcp_player_cb {
> - GList *(*list_settings) (void *user_data);
> - const char *(*get_setting) (const char *key, void *user_data);
> - int (*set_setting) (const char *key, const char *value,
> + GList *(*list_settings)(void *user_data);
> + const char *(*get_setting)(const char *key, void *user_data);
> + int (*set_setting)(const char *key, const char *value,
> void *user_data);
> - uint64_t (*get_uid) (void *user_data);
> - const char *(*get_metadata) (const char *key, void *user_data);
> - GList *(*list_metadata) (void *user_data);
> - const char *(*get_status) (void *user_data);
> - uint32_t (*get_position) (void *user_data);
> - uint32_t (*get_duration) (void *user_data);
> - const char *(*get_name) (void *user_data);
> - void (*set_volume) (uint8_t volume, struct btd_device *dev,
> + uint64_t (*get_uid)(void *user_data);
> + const char *(*get_metadata)(const char *key, void *user_data);
> + GList *(*list_metadata)(void *user_data);
> + const char *(*get_status)(void *user_data);
> + uint32_t (*get_position)(void *user_data);
> + uint32_t (*get_duration)(void *user_data);
> + const char *(*get_name)(void *user_data);
> + void (*set_volume)(int8_t volume, struct btd_device *dev,
> void *user_data);
> - bool (*play) (void *user_data);
> - bool (*stop) (void *user_data);
> - bool (*pause) (void *user_data);
> - bool (*next) (void *user_data);
> - bool (*previous) (void *user_data);
> + bool (*play)(void *user_data);
> + bool (*stop)(void *user_data);
> + bool (*pause)(void *user_data);
> + bool (*next)(void *user_data);
> + bool (*previous)(void *user_data);
> };
>
> -int avrcp_set_volume(struct btd_device *dev, uint8_t volume, bool notify);
> +int avrcp_set_volume(struct btd_device *dev, int8_t volume, bool notify);
>
> struct avrcp_player *avrcp_register_player(struct btd_adapter *adapter,
> struct avrcp_player_cb *cb,
> @@ -114,6 +114,5 @@ void avrcp_unregister_player(struct avrcp_player *player);
> void avrcp_player_event(struct avrcp_player *player, uint8_t id,
> const void *data);
>
> -
> size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
> size_t avrcp_browsing_general_reject(uint8_t *operands);
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index a0173fdd4..d4d58ec86 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -120,7 +120,7 @@ struct media_player {
> char *status;
> uint32_t position;
> uint32_t duration;
> - uint8_t volume;
> + int8_t volume;
> GTimer *timer;
> bool play;
> bool pause;
> @@ -1199,7 +1199,7 @@ static uint32_t get_duration(void *user_data)
> return mp->duration;
> }
>
> -static void set_volume(uint8_t volume, struct btd_device *dev, void *user_data)
> +static void set_volume(int8_t volume, struct btd_device *dev, void *user_data)
> {
> struct media_player *mp = user_data;
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 48fabba9b..2ae5118c4 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -86,7 +86,7 @@ struct media_owner {
> struct a2dp_transport {
> struct avdtp *session;
> uint16_t delay;
> - uint16_t volume;
> + int8_t volume;
> };
>
> struct media_transport {
> @@ -634,7 +634,7 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
> struct media_transport *transport = data;
> struct a2dp_transport *a2dp = transport->data;
>
> - return a2dp->volume <= 127;
> + return a2dp->volume >= 0;
> }
>
> static gboolean get_volume(const GDBusPropertyTable *property,
> @@ -642,8 +642,9 @@ static gboolean get_volume(const GDBusPropertyTable *property,
> {
> struct media_transport *transport = data;
> struct a2dp_transport *a2dp = transport->data;
> + uint16_t volume = (uint16_t)a2dp->volume;
>
> - dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &a2dp->volume);
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
>
> return TRUE;
> }
> @@ -654,27 +655,20 @@ static void set_volume(const GDBusPropertyTable *property,
> {
> struct media_transport *transport = data;
> struct a2dp_transport *a2dp = transport->data;
> - uint16_t volume;
> + uint16_t arg;
> + int8_t volume;
> bool notify;
>
> - if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16) {
> - g_dbus_pending_property_error(id,
> - ERROR_INTERFACE ".InvalidArguments",
> - "Invalid arguments in method call");
> - return;
> - }
> + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
> + goto error;
>
> - dbus_message_iter_get_basic(iter, &volume);
> -
> - if (volume > 127) {
> - g_dbus_pending_property_error(id,
> - ERROR_INTERFACE ".InvalidArguments",
> - "Invalid arguments in method call");
> - return;
> - }
> + dbus_message_iter_get_basic(iter, &arg);
> + if (arg > INT8_MAX)
> + goto error;
>
> g_dbus_pending_property_success(id);
>
> + volume = (int8_t)arg;
> if (a2dp->volume == volume)
> return;
>
> @@ -688,6 +682,11 @@ static void set_volume(const GDBusPropertyTable *property,
> "Volume");
>
> avrcp_set_volume(transport->device, volume, notify);
> + return;
> +
> +error:
> + g_dbus_pending_property_error(id, ERROR_INTERFACE ".InvalidArguments",
> + "Invalid arguments in method call");
> }
>
> static gboolean endpoint_exists(const GDBusPropertyTable *property, void *data)
> @@ -931,14 +930,14 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
> return transport->device;
> }
>
> -uint16_t media_transport_get_volume(struct media_transport *transport)
> +int8_t media_transport_get_volume(struct media_transport *transport)
> {
> struct a2dp_transport *a2dp = transport->data;
> return a2dp->volume;
> }
>
> void media_transport_update_volume(struct media_transport *transport,
> - uint8_t volume)
> + int8_t volume)
> {
> struct a2dp_transport *a2dp = transport->data;
>
> @@ -953,12 +952,12 @@ void media_transport_update_volume(struct media_transport *transport,
> MEDIA_TRANSPORT_INTERFACE, "Volume");
> }
>
> -uint8_t media_transport_get_device_volume(struct btd_device *dev)
> +int8_t media_transport_get_device_volume(struct btd_device *dev)
> {
> GSList *l;
>
> if (dev == NULL)
> - return 128;
> + return -1;
>
> for (l = transports; l; l = l->next) {
> struct media_transport *transport = l->data;
> @@ -974,7 +973,7 @@ uint8_t media_transport_get_device_volume(struct btd_device *dev)
> }
>
> void media_transport_update_device_volume(struct btd_device *dev,
> - uint8_t volume)
> + int8_t volume)
> {
> GSList *l;
>
> diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
> index ac542bf6c..78024372f 100644
> --- a/profiles/audio/transport.h
> +++ b/profiles/audio/transport.h
> @@ -32,14 +32,14 @@ struct media_transport *media_transport_create(struct btd_device *device,
> void media_transport_destroy(struct media_transport *transport);
> const char *media_transport_get_path(struct media_transport *transport);
> struct btd_device *media_transport_get_dev(struct media_transport *transport);
> -uint16_t media_transport_get_volume(struct media_transport *transport);
> +int8_t media_transport_get_volume(struct media_transport *transport);
> void media_transport_update_delay(struct media_transport *transport,
> uint16_t delay);
> void media_transport_update_volume(struct media_transport *transport,
> - uint8_t volume);
> + int8_t volume);
> void transport_get_properties(struct media_transport *transport,
> DBusMessageIter *iter);
>
> -uint8_t media_transport_get_device_volume(struct btd_device *dev);
> +int8_t media_transport_get_device_volume(struct btd_device *dev);
> void media_transport_update_device_volume(struct btd_device *dev,
> - uint8_t volume);
> + int8_t volume);
> --
> 2.28.0.rc0.105.gf9edc3c819-goog
Applied, thanks.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-23 16:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 7:23 [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Archie Pusaka
2020-07-23 7:23 ` [Bluez PATCH v5 2/2] audio/transport: supply volume on transport init Archie Pusaka
2020-07-23 16:40 ` [Bluez PATCH v5 1/2] audio/transport: change volume to 8bit Luiz Augusto von Dentz
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).