* [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport
@ 2022-10-11 11:31 Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 2/3] shared/vcp: Add callback to update media volume Sathish Narasimman
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Sathish Narasimman @ 2022-10-11 11:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
Initialize set_volume and get_volume to media transport and update the
volume when media_transport_update_device_volume is called.
---
profiles/audio/transport.c | 126 +++++++++++++++++++++++++++++++++++--
1 file changed, 120 insertions(+), 6 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 41339da51e17..a1445cba7993 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -91,6 +91,7 @@ struct bap_transport {
uint8_t rtn;
uint16_t latency;
uint32_t delay;
+ int8_t volume;
};
struct media_transport {
@@ -116,6 +117,9 @@ struct media_transport {
guint id);
void (*set_state) (struct media_transport *transport,
transport_state_t state);
+ void (*set_volume) (struct media_transport *transp,
+ int8_t volume);
+ int8_t (*get_volume) (struct media_transport *transp);
GDestroyNotify destroy;
void *data;
};
@@ -769,6 +773,58 @@ error:
"Invalid arguments in method call");
}
+static gboolean volume_bap_exists(const GDBusPropertyTable *property,
+ void *data)
+{
+ struct media_transport *transport = data;
+ struct bap_transport *bap = transport->data;
+
+ return bap->volume >= 0;
+}
+
+static gboolean get_bap_volume(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct media_transport *transport = data;
+ struct bap_transport *bap = transport->data;
+ uint16_t volume = (uint16_t)bap->volume;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
+
+ return TRUE;
+}
+
+static void set_bap_volume(const GDBusPropertyTable *property,
+ DBusMessageIter *iter, GDBusPendingPropertySet id,
+ void *data)
+{
+ struct media_transport *transport = data;
+ struct bap_transport *bap = transport->data;
+ uint16_t arg;
+ int8_t volume;
+
+ if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
+ goto error;
+
+ dbus_message_iter_get_basic(iter, &arg);
+ if (arg > INT8_MAX)
+ goto error;
+
+ g_dbus_pending_property_success(id);
+
+ volume = (int8_t)arg;
+ if (bap->volume == volume)
+ return;
+
+ /*TODO vcp_send_volume */
+ 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)
{
struct media_transport *transport = data;
@@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
{ "Retransmissions", "y", get_retransmissions },
{ "Latency", "q", get_latency },
{ "Delay", "u", get_delay },
+ { "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
{ "Location", "u", get_location },
{ "Metadata", "ay", get_metadata },
@@ -1048,6 +1105,31 @@ static void source_state_changed(struct btd_service *service,
transport_update_playing(transport, FALSE);
}
+static int8_t get_volume_a2dp(struct media_transport *transport)
+{
+ struct a2dp_transport *a2dp = transport->data;
+
+ return a2dp->volume;
+}
+
+static void set_volume_a2dp(struct media_transport *transport, int8_t volume)
+{
+ struct a2dp_transport *a2dp = transport->data;
+
+ if (volume < 0)
+ return;
+
+ /* Check if volume really changed */
+ if (a2dp->volume == volume)
+ return;
+
+ a2dp->volume = volume;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ transport->path,
+ MEDIA_TRANSPORT_INTERFACE, "Volume");
+}
+
static int media_transport_init_source(struct media_transport *transport)
{
struct btd_service *service;
@@ -1061,6 +1143,8 @@ static int media_transport_init_source(struct media_transport *transport)
transport->resume = resume_a2dp;
transport->suspend = suspend_a2dp;
+ transport->set_volume = set_volume_a2dp;
+ transport->get_volume = get_volume_a2dp;
transport->cancel = cancel_a2dp;
transport->data = a2dp;
transport->destroy = destroy_a2dp;
@@ -1387,6 +1471,31 @@ static void free_bap(void *data)
free(bap);
}
+static void set_volume_bap(struct media_transport *transport, int8_t volume)
+{
+ struct bap_transport *bap = transport->data;
+
+ if (volume < 0)
+ return;
+
+ /* Check if volume really changed */
+ if (bap->volume == volume)
+ return;
+
+ bap->volume = volume;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ transport->path,
+ MEDIA_TRANSPORT_INTERFACE, "Volume");
+}
+
+static int8_t get_volume_bap(struct media_transport *transport)
+{
+ struct bap_transport *bap = transport->data;
+
+ return bap->volume;
+}
+
static int media_transport_init_bap(struct media_transport *transport,
void *stream)
{
@@ -1403,6 +1512,7 @@ static int media_transport_init_bap(struct media_transport *transport,
bap->rtn = qos->rtn;
bap->latency = qos->latency;
bap->delay = qos->delay;
+ bap->volume = 127;
bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
bap_state_changed,
bap_connecting,
@@ -1413,6 +1523,8 @@ static int media_transport_init_bap(struct media_transport *transport,
transport->suspend = suspend_bap;
transport->cancel = cancel_bap;
transport->set_state = set_state_bap;
+ transport->set_volume = set_volume_bap;
+ transport->get_volume = get_volume_bap;
transport->destroy = free_bap;
return 0;
@@ -1537,12 +1649,13 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
/* Attempt to locate the transport to get its volume */
for (l = transports; l; l = l->next) {
struct media_transport *transport = l->data;
+
if (transport->device != dev)
continue;
- /* Volume is A2DP only */
- if (media_endpoint_get_sep(transport->endpoint))
- return media_transport_get_volume(transport);
+ /* Get transport volume */
+ if (transport->get_volume)
+ return transport->get_volume(transport);
}
/* If transport volume doesn't exists use device_volume */
@@ -1560,12 +1673,13 @@ void media_transport_update_device_volume(struct btd_device *dev,
/* Attempt to locate the transport to set its volume */
for (l = transports; l; l = l->next) {
struct media_transport *transport = l->data;
+
if (transport->device != dev)
continue;
- /* Volume is A2DP only */
- if (media_endpoint_get_sep(transport->endpoint)) {
- media_transport_update_volume(transport, volume);
+ /* Set transport volume */
+ if (transport->set_volume) {
+ transport->set_volume(transport, volume);
return;
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH BlueZ v3 2/3] shared/vcp: Add callback to update media volume
2022-10-11 11:31 [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport Sathish Narasimman
@ 2022-10-11 11:31 ` Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 3/3] profiles: Register callback function to update volume Sathish Narasimman
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Sathish Narasimman @ 2022-10-11 11:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
Add support for callback functions to update media transport volume.
Fix to register debug functions. Also invoke vcp_attached function the
right place. Fix check for existing session if available and attach.
---
src/shared/vcp.c | 139 +++++++++++++++++++++++++++++++++++++++++------
src/shared/vcp.h | 7 +++
2 files changed, 130 insertions(+), 16 deletions(-)
diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index 5459cf892a7d..8e1964234338 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -41,6 +41,11 @@ struct bt_vcp_db {
struct bt_vcs *vcs;
};
+struct ev_cb {
+ const struct bt_vcp_vr_ops *ops;
+ void *data;
+};
+
typedef void (*vcp_func_t)(struct bt_vcp *vcp, bool success, uint8_t att_ecode,
const uint8_t *value, uint16_t length,
void *user_data);
@@ -89,11 +94,16 @@ struct bt_vcp {
unsigned int vstate_id;
unsigned int vflag_id;
+ unsigned int disconn_id;
struct queue *notify;
struct queue *pending;
bt_vcp_debug_func_t debug_func;
bt_vcp_destroy_func_t debug_destroy;
+
+ struct bt_vcp_vr_ops *ops;
+ struct ev_cb *cb;
+
void *debug_data;
void *user_data;
};
@@ -124,6 +134,18 @@ static struct queue *vcp_db;
static struct queue *vcp_cbs;
static struct queue *sessions;
+static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
+{
+ va_list ap;
+
+ if (!vcp || !format || !vcp->debug_func)
+ return;
+
+ va_start(ap, format);
+ util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
+ va_end(ap);
+}
+
static void *iov_pull_mem(struct iovec *iov, size_t len)
{
void *data = iov->iov_base;
@@ -183,12 +205,17 @@ static void vcp_detached(void *data, void *user_data)
void bt_vcp_detach(struct bt_vcp *vcp)
{
+ DBG(vcp, "%p", vcp);
+
if (!queue_remove(sessions, vcp))
return;
bt_gatt_client_unref(vcp->client);
vcp->client = NULL;
+ bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+ vcp->att = NULL;
+
queue_foreach(vcp_cbs, vcp_detached, vcp);
}
@@ -267,24 +294,14 @@ void bt_vcp_unref(struct bt_vcp *vcp)
vcp_free(vcp);
}
-static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
-{
- va_list ap;
-
- if (!vcp || !format || !vcp->debug_func)
- return;
-
- va_start(ap, format);
- util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
- va_end(ap);
-}
-
static void vcp_disconnected(int err, void *user_data)
{
struct bt_vcp *vcp = user_data;
DBG(vcp, "vcp %p disconnected err %d", vcp, err);
+ vcp->disconn_id = 0;
+
bt_vcp_detach(vcp);
}
@@ -303,12 +320,9 @@ static struct bt_vcp *vcp_get_session(struct bt_att *att, struct gatt_db *db)
vcp = bt_vcp_new(db, NULL);
vcp->att = att;
- bt_att_register_disconnect(att, vcp_disconnected, vcp, NULL);
-
bt_vcp_attach(vcp, NULL);
return vcp;
-
}
static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
@@ -317,6 +331,7 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "Volume Down");
@@ -344,6 +359,9 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -356,6 +374,7 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "Volume Up");
@@ -383,6 +402,9 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -395,6 +417,7 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "Un Mute and Volume Down");
@@ -423,6 +446,9 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -435,6 +461,7 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "UN Mute and Volume Up");
@@ -463,6 +490,9 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -475,6 +505,7 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
struct bt_vcs_ab_vol *req;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "Set Absolute Volume");
@@ -502,6 +533,9 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->vol_set = req->vol_set;
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -514,6 +548,7 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "Un Mute");
@@ -541,6 +576,9 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->mute = 0x00;
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
sizeof(struct vol_state),
bt_vcp_get_att(vcp));
@@ -553,6 +591,7 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
struct bt_vcp_db *vdb;
struct vol_state *vstate;
uint8_t *change_counter;
+ struct ev_cb *cb = vcp->cb;
DBG(vcp, "MUTE");
@@ -580,6 +619,13 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
vstate->mute = 0x01;
vstate->counter = -~vstate->counter; /*Increment Change Counter*/
+ if (cb && cb->ops && cb->ops->set_volume)
+ cb->ops->set_volume(vcp, 0, cb->data);
+
+ gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
+ sizeof(struct vol_state),
+ bt_vcp_get_att(vcp));
+
return 0;
}
@@ -689,8 +735,10 @@ static void vcs_state_read(struct gatt_db_attribute *attrib,
void *user_data)
{
struct bt_vcs *vcs = user_data;
+ struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
struct iovec iov;
+ DBG(vcp, "VCP State Read");
iov.iov_base = vcs->vstate;
iov.iov_len = sizeof(*vcs->vstate);
@@ -704,8 +752,10 @@ static void vcs_flag_read(struct gatt_db_attribute *attrib,
void *user_data)
{
struct bt_vcs *vcs = user_data;
+ struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
struct iovec iov;
+ DBG(vcp, "VCP Flag Read");
iov.iov_base = &vcs->vol_flag;
iov.iov_len = sizeof(vcs->vol_flag);
@@ -868,6 +918,14 @@ bool bt_vcp_unregister(unsigned int id)
return true;
}
+static void vcp_attached(void *data, void *user_data)
+{
+ struct bt_vcp_cb *cb = data;
+ struct bt_vcp *vcp = user_data;
+
+ cb->attached(vcp, cb->user_data);
+}
+
struct bt_vcp *bt_vcp_new(struct gatt_db *ldb, struct gatt_db *rdb)
{
struct bt_vcp *vcp;
@@ -1068,6 +1126,26 @@ static unsigned int vcp_register_notify(struct bt_vcp *vcp,
return notify->id;
}
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+ void *data)
+{
+ struct ev_cb *cb;
+
+ if (!vcp)
+ return false;
+
+ if (vcp->cb)
+ free(vcp->cb);
+
+ cb = new0(struct ev_cb, 1);
+ cb->ops = ops;
+ cb->data = data;
+
+ vcp->cb = cb;
+
+ return true;
+}
+
static void foreach_vcs_char(struct gatt_db_attribute *attr, void *user_data)
{
struct bt_vcp *vcp = user_data;
@@ -1141,25 +1219,54 @@ static void foreach_vcs_service(struct gatt_db_attribute *attr,
gatt_db_service_foreach_char(attr, foreach_vcs_char, vcp);
}
+static void vcp_attach_att(struct bt_vcp *vcp, struct bt_att *att)
+{
+ if (vcp->disconn_id) {
+ if (att == bt_vcp_get_att(vcp))
+ return;
+ bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+ }
+
+ vcp->disconn_id = bt_att_register_disconnect(vcp->att,
+ vcp_disconnected,
+ vcp, NULL);
+}
+
bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client)
{
bt_uuid_t uuid;
+ if (queue_find(sessions, NULL, vcp)) {
+ /* If instance already been set but there is no client proceed
+ * to clone it otherwise considered it already attached.
+ */
+ if (client && !vcp->client)
+ goto clone;
+ return true;
+ }
+
if (!sessions)
sessions = queue_new();
queue_push_tail(sessions, vcp);
- if (!client)
+ queue_foreach(vcp_cbs, vcp_attached, vcp);
+
+ if (!client) {
+ vcp_attach_att(vcp, vcp->att);
return true;
+ }
if (vcp->client)
return false;
+clone:
vcp->client = bt_gatt_client_clone(client);
if (!vcp->client)
return false;
+ vcp_attach_att(vcp, bt_gatt_client_get_att(client));
+
bt_uuid16_create(&uuid, VCS_UUID);
gatt_db_foreach_service(vcp->ldb->db, &uuid, foreach_vcs_service, vcp);
diff --git a/src/shared/vcp.h b/src/shared/vcp.h
index 26db5732d19b..7d70aefe0089 100644
--- a/src/shared/vcp.h
+++ b/src/shared/vcp.h
@@ -33,6 +33,13 @@
struct bt_vcp;
+struct bt_vcp_vr_ops {
+ void (*set_volume)(struct bt_vcp *vcp, int8_t volume, void *data);
+};
+
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+ void *data);
+
typedef void (*bt_vcp_destroy_func_t)(void *user_data);
typedef void (*bt_vcp_debug_func_t)(const char *str, void *user_data);
typedef void (*bt_vcp_func_t)(struct bt_vcp *vcp, void *user_data);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH BlueZ v3 3/3] profiles: Register callback function to update volume
2022-10-11 11:31 [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 2/3] shared/vcp: Add callback to update media volume Sathish Narasimman
@ 2022-10-11 11:31 ` Sathish Narasimman
2022-10-11 12:36 ` [BlueZ,v3,1/3] audio/transport: Adding volume callback to media transport bluez.test.bot
2022-10-11 19:28 ` [PATCH BlueZ v3 1/3] " Luiz Augusto von Dentz
3 siblings, 0 replies; 7+ messages in thread
From: Sathish Narasimman @ 2022-10-11 11:31 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sathish Narasimman
Callback function has to be registered to call
media_transport_update_device_volume to change transport volume.
---
profiles/audio/vcp.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/profiles/audio/vcp.c b/profiles/audio/vcp.c
index b42b0a4f79dd..4b790b03c032 100644
--- a/profiles/audio/vcp.c
+++ b/profiles/audio/vcp.c
@@ -50,6 +50,7 @@
#include "src/service.h"
#include "src/log.h"
#include "src/error.h"
+#include "transport.h"
#define VCS_UUID_STR "00001844-0000-1000-8000-00805f9b34fb"
#define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
@@ -83,6 +84,20 @@ static struct vcp_data *vcp_data_new(struct btd_device *device)
return data;
}
+static void vr_set_volume(struct bt_vcp *vcp, int8_t volume, void *data)
+{
+ struct vcp_data *user_data = data;
+ struct btd_device *device = user_data->device;
+
+ DBG("set volume");
+
+ media_transport_update_device_volume(device, volume);
+}
+
+static struct bt_vcp_vr_ops vcp_vr_ops = {
+ .set_volume = vr_set_volume,
+};
+
static void vcp_data_add(struct vcp_data *data)
{
DBG("data %p", data);
@@ -94,6 +109,7 @@ static void vcp_data_add(struct vcp_data *data)
bt_vcp_set_debug(data->vcp, vcp_debug, NULL, NULL);
+ bt_vcp_vr_set_ops(data->vcp, &vcp_vr_ops, data);
if (!sessions)
sessions = queue_new();
@@ -178,6 +194,7 @@ static void vcp_attached(struct bt_vcp *vcp, void *user_data)
data->vcp = vcp;
vcp_data_add(data);
+
}
static int vcp_probe(struct btd_service *service)
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [BlueZ,v3,1/3] audio/transport: Adding volume callback to media transport
2022-10-11 11:31 [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 2/3] shared/vcp: Add callback to update media volume Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 3/3] profiles: Register callback function to update volume Sathish Narasimman
@ 2022-10-11 12:36 ` bluez.test.bot
2022-10-11 19:28 ` [PATCH BlueZ v3 1/3] " Luiz Augusto von Dentz
3 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2022-10-11 12:36 UTC (permalink / raw)
To: linux-bluetooth, sathish.narasimman
[-- Attachment #1: Type: text/plain, Size: 2193 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=684476
---Test result---
Test Summary:
CheckPatch FAIL 2.38 seconds
GitLint PASS 1.31 seconds
Prep - Setup ELL PASS 30.54 seconds
Build - Prep PASS 0.78 seconds
Build - Configure PASS 9.45 seconds
Build - Make PASS 1039.19 seconds
Make Check PASS 12.27 seconds
Make Check w/Valgrind PASS 331.94 seconds
Make Distcheck PASS 273.49 seconds
Build w/ext ELL - Configure PASS 9.51 seconds
Build w/ext ELL - Make PASS 99.86 seconds
Incremental Build w/ patches PASS 350.20 seconds
Scan Build PASS 642.82 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,v3,1/3] audio/transport: Adding volume callback to media transport
WARNING:SPACING: Unnecessary space before function pointer arguments
#87: FILE: profiles/audio/transport.c:120:
+ void (*set_volume) (struct media_transport *transp,
WARNING:SPACING: Unnecessary space before function pointer arguments
#89: FILE: profiles/audio/transport.c:122:
+ int8_t (*get_volume) (struct media_transport *transp);
/github/workspace/src/13003868.patch total: 0 errors, 2 warnings, 198 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/13003868.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport
2022-10-11 11:31 [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport Sathish Narasimman
` (2 preceding siblings ...)
2022-10-11 12:36 ` [BlueZ,v3,1/3] audio/transport: Adding volume callback to media transport bluez.test.bot
@ 2022-10-11 19:28 ` Luiz Augusto von Dentz
2022-10-12 5:42 ` Sathish Narasimman
3 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2022-10-11 19:28 UTC (permalink / raw)
To: Sathish Narasimman; +Cc: linux-bluetooth
Hi Sathish,
On Tue, Oct 11, 2022 at 4:38 AM Sathish Narasimman
<sathish.narasimman@intel.com> wrote:
>
> Initialize set_volume and get_volume to media transport and update the
> volume when media_transport_update_device_volume is called.
> ---
> profiles/audio/transport.c | 126 +++++++++++++++++++++++++++++++++++--
> 1 file changed, 120 insertions(+), 6 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 41339da51e17..a1445cba7993 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -91,6 +91,7 @@ struct bap_transport {
> uint8_t rtn;
> uint16_t latency;
> uint32_t delay;
> + int8_t volume;
> };
>
> struct media_transport {
> @@ -116,6 +117,9 @@ struct media_transport {
> guint id);
> void (*set_state) (struct media_transport *transport,
> transport_state_t state);
> + void (*set_volume) (struct media_transport *transp,
> + int8_t volume);
> + int8_t (*get_volume) (struct media_transport *transp);
> GDestroyNotify destroy;
> void *data;
> };
> @@ -769,6 +773,58 @@ error:
> "Invalid arguments in method call");
> }
>
> +static gboolean volume_bap_exists(const GDBusPropertyTable *property,
> + void *data)
> +{
> + struct media_transport *transport = data;
> + struct bap_transport *bap = transport->data;
> +
> + return bap->volume >= 0;
> +}
> +
> +static gboolean get_bap_volume(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, void *data)
> +{
> + struct media_transport *transport = data;
> + struct bap_transport *bap = transport->data;
> + uint16_t volume = (uint16_t)bap->volume;
> +
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
> +
> + return TRUE;
> +}
> +
> +static void set_bap_volume(const GDBusPropertyTable *property,
> + DBusMessageIter *iter, GDBusPendingPropertySet id,
> + void *data)
> +{
> + struct media_transport *transport = data;
> + struct bap_transport *bap = transport->data;
> + uint16_t arg;
> + int8_t volume;
> +
> + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
> + goto error;
> +
> + dbus_message_iter_get_basic(iter, &arg);
> + if (arg > INT8_MAX)
> + goto error;
> +
> + g_dbus_pending_property_success(id);
> +
> + volume = (int8_t)arg;
> + if (bap->volume == volume)
> + return;
> +
> + /*TODO vcp_send_volume */
What is this TODO item for? Can we complete it right now? Afaik we
should be able to handle local changes and notify it to remote peers
or that is not how VCP works?
> + 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)
> {
> struct media_transport *transport = data;
> @@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
> { "Retransmissions", "y", get_retransmissions },
> { "Latency", "q", get_latency },
> { "Delay", "u", get_delay },
> + { "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
Now that we have set_volume/get_volume as callbacks these could be
changed to use them instead of having dedicated callback like abouve,
something like:
https://gist.github.com/Vudentz/19edcf96735567c1f7437a5e1dee7e04
> { "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
> { "Location", "u", get_location },
> { "Metadata", "ay", get_metadata },
> @@ -1048,6 +1105,31 @@ static void source_state_changed(struct btd_service *service,
> transport_update_playing(transport, FALSE);
> }
>
> +static int8_t get_volume_a2dp(struct media_transport *transport)
> +{
> + struct a2dp_transport *a2dp = transport->data;
> +
> + return a2dp->volume;
> +}
> +
> +static void set_volume_a2dp(struct media_transport *transport, int8_t volume)
> +{
> + struct a2dp_transport *a2dp = transport->data;
> +
> + if (volume < 0)
> + return;
> +
> + /* Check if volume really changed */
> + if (a2dp->volume == volume)
> + return;
> +
> + a2dp->volume = volume;
> +
> + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> + transport->path,
> + MEDIA_TRANSPORT_INTERFACE, "Volume");
> +}
> +
> static int media_transport_init_source(struct media_transport *transport)
> {
> struct btd_service *service;
> @@ -1061,6 +1143,8 @@ static int media_transport_init_source(struct media_transport *transport)
>
> transport->resume = resume_a2dp;
> transport->suspend = suspend_a2dp;
> + transport->set_volume = set_volume_a2dp;
> + transport->get_volume = get_volume_a2dp;
> transport->cancel = cancel_a2dp;
> transport->data = a2dp;
> transport->destroy = destroy_a2dp;
> @@ -1387,6 +1471,31 @@ static void free_bap(void *data)
> free(bap);
> }
>
> +static void set_volume_bap(struct media_transport *transport, int8_t volume)
> +{
> + struct bap_transport *bap = transport->data;
> +
> + if (volume < 0)
> + return;
> +
> + /* Check if volume really changed */
> + if (bap->volume == volume)
> + return;
> +
> + bap->volume = volume;
> +
> + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> + transport->path,
> + MEDIA_TRANSPORT_INTERFACE, "Volume");
> +}
> +
> +static int8_t get_volume_bap(struct media_transport *transport)
> +{
> + struct bap_transport *bap = transport->data;
> +
> + return bap->volume;
> +}
> +
> static int media_transport_init_bap(struct media_transport *transport,
> void *stream)
> {
> @@ -1403,6 +1512,7 @@ static int media_transport_init_bap(struct media_transport *transport,
> bap->rtn = qos->rtn;
> bap->latency = qos->latency;
> bap->delay = qos->delay;
> + bap->volume = 127;
> bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
> bap_state_changed,
> bap_connecting,
> @@ -1413,6 +1523,8 @@ static int media_transport_init_bap(struct media_transport *transport,
> transport->suspend = suspend_bap;
> transport->cancel = cancel_bap;
> transport->set_state = set_state_bap;
> + transport->set_volume = set_volume_bap;
> + transport->get_volume = get_volume_bap;
> transport->destroy = free_bap;
>
> return 0;
> @@ -1537,12 +1649,13 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
> /* Attempt to locate the transport to get its volume */
> for (l = transports; l; l = l->next) {
> struct media_transport *transport = l->data;
> +
> if (transport->device != dev)
> continue;
>
> - /* Volume is A2DP only */
> - if (media_endpoint_get_sep(transport->endpoint))
> - return media_transport_get_volume(transport);
> + /* Get transport volume */
> + if (transport->get_volume)
> + return transport->get_volume(transport);
> }
>
> /* If transport volume doesn't exists use device_volume */
> @@ -1560,12 +1673,13 @@ void media_transport_update_device_volume(struct btd_device *dev,
> /* Attempt to locate the transport to set its volume */
> for (l = transports; l; l = l->next) {
> struct media_transport *transport = l->data;
> +
> if (transport->device != dev)
> continue;
>
> - /* Volume is A2DP only */
> - if (media_endpoint_get_sep(transport->endpoint)) {
> - media_transport_update_volume(transport, volume);
> + /* Set transport volume */
> + if (transport->set_volume) {
> + transport->set_volume(transport, volume);
> return;
> }
> }
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport
2022-10-11 19:28 ` [PATCH BlueZ v3 1/3] " Luiz Augusto von Dentz
@ 2022-10-12 5:42 ` Sathish Narasimman
2022-10-12 19:22 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Sathish Narasimman @ 2022-10-12 5:42 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Sathish Narasimman, linux-bluetooth
Hi Luiz
On Wed, Oct 12, 2022 at 1:28 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sathish,
>
> On Tue, Oct 11, 2022 at 4:38 AM Sathish Narasimman
> <sathish.narasimman@intel.com> wrote:
> >
> > Initialize set_volume and get_volume to media transport and update the
> > volume when media_transport_update_device_volume is called.
> > ---
> > profiles/audio/transport.c | 126 +++++++++++++++++++++++++++++++++++--
> > 1 file changed, 120 insertions(+), 6 deletions(-)
> >
> > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > index 41339da51e17..a1445cba7993 100644
> > --- a/profiles/audio/transport.c
> > +++ b/profiles/audio/transport.c
> > @@ -91,6 +91,7 @@ struct bap_transport {
> > uint8_t rtn;
> > uint16_t latency;
> > uint32_t delay;
> > + int8_t volume;
> > };
> >
> > struct media_transport {
> > @@ -116,6 +117,9 @@ struct media_transport {
> > guint id);
> > void (*set_state) (struct media_transport *transport,
> > transport_state_t state);
> > + void (*set_volume) (struct media_transport *transp,
> > + int8_t volume);
> > + int8_t (*get_volume) (struct media_transport *transp);
> > GDestroyNotify destroy;
> > void *data;
> > };
> > @@ -769,6 +773,58 @@ error:
> > "Invalid arguments in method call");
> > }
> >
> > +static gboolean volume_bap_exists(const GDBusPropertyTable *property,
> > + void *data)
> > +{
> > + struct media_transport *transport = data;
> > + struct bap_transport *bap = transport->data;
> > +
> > + return bap->volume >= 0;
> > +}
> > +
> > +static gboolean get_bap_volume(const GDBusPropertyTable *property,
> > + DBusMessageIter *iter, void *data)
> > +{
> > + struct media_transport *transport = data;
> > + struct bap_transport *bap = transport->data;
> > + uint16_t volume = (uint16_t)bap->volume;
> > +
> > + dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
> > +
> > + return TRUE;
> > +}
> > +
> > +static void set_bap_volume(const GDBusPropertyTable *property,
> > + DBusMessageIter *iter, GDBusPendingPropertySet id,
> > + void *data)
> > +{
> > + struct media_transport *transport = data;
> > + struct bap_transport *bap = transport->data;
> > + uint16_t arg;
> > + int8_t volume;
> > +
> > + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
> > + goto error;
> > +
> > + dbus_message_iter_get_basic(iter, &arg);
> > + if (arg > INT8_MAX)
> > + goto error;
> > +
> > + g_dbus_pending_property_success(id);
> > +
> > + volume = (int8_t)arg;
> > + if (bap->volume == volume)
> > + return;
> > +
> > + /*TODO vcp_send_volume */
>
> What is this TODO item for? Can we complete it right now? Afaik we
> should be able to handle local changes and notify it to remote peers
> or that is not how VCP works?
The TODO part is for VCP Client. At present only VCP Server is implemented.
Whenever there is a change in volume locally we have to notify to the remote
peer in case of server Role(Volume renderer).
>
> > + 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)
> > {
> > struct media_transport *transport = data;
> > @@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
> > { "Retransmissions", "y", get_retransmissions },
> > { "Latency", "q", get_latency },
> > { "Delay", "u", get_delay },
> > + { "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
>
> Now that we have set_volume/get_volume as callbacks these could be
> changed to use them instead of having dedicated callback like abouve,
> something like:
>
> https://gist.github.com/Vudentz/19edcf96735567c1f7437a5e1dee7e04
>
will check and update
> > { "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
> > { "Location", "u", get_location },
> > { "Metadata", "ay", get_metadata },
> > @@ -1048,6 +1105,31 @@ static void source_state_changed(struct btd_service *service,
> > transport_update_playing(transport, FALSE);
> > }
> >
> > +static int8_t get_volume_a2dp(struct media_transport *transport)
> > +{
> > + struct a2dp_transport *a2dp = transport->data;
> > +
> > + return a2dp->volume;
> > +}
> > +
> > +static void set_volume_a2dp(struct media_transport *transport, int8_t volume)
> > +{
> > + struct a2dp_transport *a2dp = transport->data;
> > +
> > + if (volume < 0)
> > + return;
> > +
> > + /* Check if volume really changed */
> > + if (a2dp->volume == volume)
> > + return;
> > +
> > + a2dp->volume = volume;
> > +
> > + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> > + transport->path,
> > + MEDIA_TRANSPORT_INTERFACE, "Volume");
> > +}
> > +
> > static int media_transport_init_source(struct media_transport *transport)
> > {
> > struct btd_service *service;
> > @@ -1061,6 +1143,8 @@ static int media_transport_init_source(struct media_transport *transport)
> >
> > transport->resume = resume_a2dp;
> > transport->suspend = suspend_a2dp;
> > + transport->set_volume = set_volume_a2dp;
> > + transport->get_volume = get_volume_a2dp;
> > transport->cancel = cancel_a2dp;
> > transport->data = a2dp;
> > transport->destroy = destroy_a2dp;
> > @@ -1387,6 +1471,31 @@ static void free_bap(void *data)
> > free(bap);
> > }
> >
> > +static void set_volume_bap(struct media_transport *transport, int8_t volume)
> > +{
> > + struct bap_transport *bap = transport->data;
> > +
> > + if (volume < 0)
> > + return;
> > +
> > + /* Check if volume really changed */
> > + if (bap->volume == volume)
> > + return;
> > +
> > + bap->volume = volume;
> > +
> > + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> > + transport->path,
> > + MEDIA_TRANSPORT_INTERFACE, "Volume");
> > +}
> > +
> > +static int8_t get_volume_bap(struct media_transport *transport)
> > +{
> > + struct bap_transport *bap = transport->data;
> > +
> > + return bap->volume;
> > +}
> > +
> > static int media_transport_init_bap(struct media_transport *transport,
> > void *stream)
> > {
> > @@ -1403,6 +1512,7 @@ static int media_transport_init_bap(struct media_transport *transport,
> > bap->rtn = qos->rtn;
> > bap->latency = qos->latency;
> > bap->delay = qos->delay;
> > + bap->volume = 127;
> > bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
> > bap_state_changed,
> > bap_connecting,
> > @@ -1413,6 +1523,8 @@ static int media_transport_init_bap(struct media_transport *transport,
> > transport->suspend = suspend_bap;
> > transport->cancel = cancel_bap;
> > transport->set_state = set_state_bap;
> > + transport->set_volume = set_volume_bap;
> > + transport->get_volume = get_volume_bap;
> > transport->destroy = free_bap;
> >
> > return 0;
> > @@ -1537,12 +1649,13 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
> > /* Attempt to locate the transport to get its volume */
> > for (l = transports; l; l = l->next) {
> > struct media_transport *transport = l->data;
> > +
> > if (transport->device != dev)
> > continue;
> >
> > - /* Volume is A2DP only */
> > - if (media_endpoint_get_sep(transport->endpoint))
> > - return media_transport_get_volume(transport);
> > + /* Get transport volume */
> > + if (transport->get_volume)
> > + return transport->get_volume(transport);
> > }
> >
> > /* If transport volume doesn't exists use device_volume */
> > @@ -1560,12 +1673,13 @@ void media_transport_update_device_volume(struct btd_device *dev,
> > /* Attempt to locate the transport to set its volume */
> > for (l = transports; l; l = l->next) {
> > struct media_transport *transport = l->data;
> > +
> > if (transport->device != dev)
> > continue;
> >
> > - /* Volume is A2DP only */
> > - if (media_endpoint_get_sep(transport->endpoint)) {
> > - media_transport_update_volume(transport, volume);
> > + /* Set transport volume */
> > + if (transport->set_volume) {
> > + transport->set_volume(transport, volume);
> > return;
> > }
> > }
> > --
> > 2.25.1
> >
>
>
> --
> Luiz Augusto von Dentz
Sathish N
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport
2022-10-12 5:42 ` Sathish Narasimman
@ 2022-10-12 19:22 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2022-10-12 19:22 UTC (permalink / raw)
To: Sathish Narasimman; +Cc: Sathish Narasimman, linux-bluetooth
Hi Sathish,
On Tue, Oct 11, 2022 at 10:42 PM Sathish Narasimman
<nsathish41@gmail.com> wrote:
>
> Hi Luiz
>
> On Wed, Oct 12, 2022 at 1:28 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Sathish,
> >
> > On Tue, Oct 11, 2022 at 4:38 AM Sathish Narasimman
> > <sathish.narasimman@intel.com> wrote:
> > >
> > > Initialize set_volume and get_volume to media transport and update the
> > > volume when media_transport_update_device_volume is called.
> > > ---
> > > profiles/audio/transport.c | 126 +++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 120 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > > index 41339da51e17..a1445cba7993 100644
> > > --- a/profiles/audio/transport.c
> > > +++ b/profiles/audio/transport.c
> > > @@ -91,6 +91,7 @@ struct bap_transport {
> > > uint8_t rtn;
> > > uint16_t latency;
> > > uint32_t delay;
> > > + int8_t volume;
> > > };
> > >
> > > struct media_transport {
> > > @@ -116,6 +117,9 @@ struct media_transport {
> > > guint id);
> > > void (*set_state) (struct media_transport *transport,
> > > transport_state_t state);
> > > + void (*set_volume) (struct media_transport *transp,
> > > + int8_t volume);
> > > + int8_t (*get_volume) (struct media_transport *transp);
> > > GDestroyNotify destroy;
> > > void *data;
> > > };
> > > @@ -769,6 +773,58 @@ error:
> > > "Invalid arguments in method call");
> > > }
> > >
> > > +static gboolean volume_bap_exists(const GDBusPropertyTable *property,
> > > + void *data)
> > > +{
> > > + struct media_transport *transport = data;
> > > + struct bap_transport *bap = transport->data;
> > > +
> > > + return bap->volume >= 0;
> > > +}
> > > +
> > > +static gboolean get_bap_volume(const GDBusPropertyTable *property,
> > > + DBusMessageIter *iter, void *data)
> > > +{
> > > + struct media_transport *transport = data;
> > > + struct bap_transport *bap = transport->data;
> > > + uint16_t volume = (uint16_t)bap->volume;
> > > +
> > > + dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
> > > +
> > > + return TRUE;
> > > +}
> > > +
> > > +static void set_bap_volume(const GDBusPropertyTable *property,
> > > + DBusMessageIter *iter, GDBusPendingPropertySet id,
> > > + void *data)
> > > +{
> > > + struct media_transport *transport = data;
> > > + struct bap_transport *bap = transport->data;
> > > + uint16_t arg;
> > > + int8_t volume;
> > > +
> > > + if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
> > > + goto error;
> > > +
> > > + dbus_message_iter_get_basic(iter, &arg);
> > > + if (arg > INT8_MAX)
> > > + goto error;
> > > +
> > > + g_dbus_pending_property_success(id);
> > > +
> > > + volume = (int8_t)arg;
> > > + if (bap->volume == volume)
> > > + return;
> > > +
> > > + /*TODO vcp_send_volume */
> >
> > What is this TODO item for? Can we complete it right now? Afaik we
> > should be able to handle local changes and notify it to remote peers
> > or that is not how VCP works?
>
> The TODO part is for VCP Client. At present only VCP Server is implemented.
> Whenever there is a change in volume locally we have to notify to the remote
> peer in case of server Role(Volume renderer).
Well the callback to Volume property shall in case of the server
notify then, otherwise the client has no idea anything has happened,
so I think we need to implement procedure anyway handling the
distinction of client/server internally.
> >
> > > + 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)
> > > {
> > > struct media_transport *transport = data;
> > > @@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
> > > { "Retransmissions", "y", get_retransmissions },
> > > { "Latency", "q", get_latency },
> > > { "Delay", "u", get_delay },
> > > + { "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
> >
> > Now that we have set_volume/get_volume as callbacks these could be
> > changed to use them instead of having dedicated callback like abouve,
> > something like:
> >
> > https://gist.github.com/Vudentz/19edcf96735567c1f7437a5e1dee7e04
> >
> will check and update
> > > { "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
> > > { "Location", "u", get_location },
> > > { "Metadata", "ay", get_metadata },
> > > @@ -1048,6 +1105,31 @@ static void source_state_changed(struct btd_service *service,
> > > transport_update_playing(transport, FALSE);
> > > }
> > >
> > > +static int8_t get_volume_a2dp(struct media_transport *transport)
> > > +{
> > > + struct a2dp_transport *a2dp = transport->data;
> > > +
> > > + return a2dp->volume;
> > > +}
> > > +
> > > +static void set_volume_a2dp(struct media_transport *transport, int8_t volume)
> > > +{
> > > + struct a2dp_transport *a2dp = transport->data;
> > > +
> > > + if (volume < 0)
> > > + return;
> > > +
> > > + /* Check if volume really changed */
> > > + if (a2dp->volume == volume)
> > > + return;
> > > +
> > > + a2dp->volume = volume;
> > > +
> > > + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> > > + transport->path,
> > > + MEDIA_TRANSPORT_INTERFACE, "Volume");
> > > +}
> > > +
> > > static int media_transport_init_source(struct media_transport *transport)
> > > {
> > > struct btd_service *service;
> > > @@ -1061,6 +1143,8 @@ static int media_transport_init_source(struct media_transport *transport)
> > >
> > > transport->resume = resume_a2dp;
> > > transport->suspend = suspend_a2dp;
> > > + transport->set_volume = set_volume_a2dp;
> > > + transport->get_volume = get_volume_a2dp;
> > > transport->cancel = cancel_a2dp;
> > > transport->data = a2dp;
> > > transport->destroy = destroy_a2dp;
> > > @@ -1387,6 +1471,31 @@ static void free_bap(void *data)
> > > free(bap);
> > > }
> > >
> > > +static void set_volume_bap(struct media_transport *transport, int8_t volume)
> > > +{
> > > + struct bap_transport *bap = transport->data;
> > > +
> > > + if (volume < 0)
> > > + return;
> > > +
> > > + /* Check if volume really changed */
> > > + if (bap->volume == volume)
> > > + return;
> > > +
> > > + bap->volume = volume;
> > > +
> > > + g_dbus_emit_property_changed(btd_get_dbus_connection(),
> > > + transport->path,
> > > + MEDIA_TRANSPORT_INTERFACE, "Volume");
> > > +}
> > > +
> > > +static int8_t get_volume_bap(struct media_transport *transport)
> > > +{
> > > + struct bap_transport *bap = transport->data;
> > > +
> > > + return bap->volume;
> > > +}
> > > +
> > > static int media_transport_init_bap(struct media_transport *transport,
> > > void *stream)
> > > {
> > > @@ -1403,6 +1512,7 @@ static int media_transport_init_bap(struct media_transport *transport,
> > > bap->rtn = qos->rtn;
> > > bap->latency = qos->latency;
> > > bap->delay = qos->delay;
> > > + bap->volume = 127;
> > > bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
> > > bap_state_changed,
> > > bap_connecting,
> > > @@ -1413,6 +1523,8 @@ static int media_transport_init_bap(struct media_transport *transport,
> > > transport->suspend = suspend_bap;
> > > transport->cancel = cancel_bap;
> > > transport->set_state = set_state_bap;
> > > + transport->set_volume = set_volume_bap;
> > > + transport->get_volume = get_volume_bap;
> > > transport->destroy = free_bap;
> > >
> > > return 0;
> > > @@ -1537,12 +1649,13 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
> > > /* Attempt to locate the transport to get its volume */
> > > for (l = transports; l; l = l->next) {
> > > struct media_transport *transport = l->data;
> > > +
> > > if (transport->device != dev)
> > > continue;
> > >
> > > - /* Volume is A2DP only */
> > > - if (media_endpoint_get_sep(transport->endpoint))
> > > - return media_transport_get_volume(transport);
> > > + /* Get transport volume */
> > > + if (transport->get_volume)
> > > + return transport->get_volume(transport);
> > > }
> > >
> > > /* If transport volume doesn't exists use device_volume */
> > > @@ -1560,12 +1673,13 @@ void media_transport_update_device_volume(struct btd_device *dev,
> > > /* Attempt to locate the transport to set its volume */
> > > for (l = transports; l; l = l->next) {
> > > struct media_transport *transport = l->data;
> > > +
> > > if (transport->device != dev)
> > > continue;
> > >
> > > - /* Volume is A2DP only */
> > > - if (media_endpoint_get_sep(transport->endpoint)) {
> > > - media_transport_update_volume(transport, volume);
> > > + /* Set transport volume */
> > > + if (transport->set_volume) {
> > > + transport->set_volume(transport, volume);
> > > return;
> > > }
> > > }
> > > --
> > > 2.25.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
>
> Sathish N
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-10-12 19:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-11 11:31 [PATCH BlueZ v3 1/3] audio/transport: Adding volume callback to media transport Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 2/3] shared/vcp: Add callback to update media volume Sathish Narasimman
2022-10-11 11:31 ` [PATCH BlueZ v3 3/3] profiles: Register callback function to update volume Sathish Narasimman
2022-10-11 12:36 ` [BlueZ,v3,1/3] audio/transport: Adding volume callback to media transport bluez.test.bot
2022-10-11 19:28 ` [PATCH BlueZ v3 1/3] " Luiz Augusto von Dentz
2022-10-12 5:42 ` Sathish Narasimman
2022-10-12 19:22 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.