diff mbox series

[BlueZ,v3,2/5] audio: connect VCP profile client to MediaTransport

Message ID 20250121144404.4087658-3-michal.dzik@streamunlimited.com (mailing list archive)
State New
Headers show
Series connect VCP profile to MediaTransport volume | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success

Commit Message

Michal Dzik Jan. 21, 2025, 2:44 p.m. UTC 
It is now possible to control absolute volume of remote volume renderer
device via dbus and also get notifications if the volume changes.
---
 Makefile.plugins           |  2 +-
 profiles/audio/transport.c | 37 +++++++++++++++++++------
 profiles/audio/vcp.c       | 57 ++++++++++++++++++++++++++++++++++++--
 profiles/audio/vcp.h       | 12 ++++++++
 4 files changed, 95 insertions(+), 13 deletions(-)
 create mode 100644 profiles/audio/vcp.h

Comments

Luiz Augusto von Dentz Jan. 22, 2025, 8:14 p.m. UTC | #1 
Hi Michal,

On Tue, Jan 21, 2025 at 9:44 AM Michal Dzik
<michal.dzik@streamunlimited.com> wrote:
>
> It is now possible to control absolute volume of remote volume renderer
> device via dbus and also get notifications if the volume changes.
> ---
>  Makefile.plugins           |  2 +-
>  profiles/audio/transport.c | 37 +++++++++++++++++++------
>  profiles/audio/vcp.c       | 57 ++++++++++++++++++++++++++++++++++++--
>  profiles/audio/vcp.h       | 12 ++++++++
>  4 files changed, 95 insertions(+), 13 deletions(-)
>  create mode 100644 profiles/audio/vcp.h
>
> diff --git a/Makefile.plugins b/Makefile.plugins
> index 97335d643..81cf3155a 100644
> --- a/Makefile.plugins
> +++ b/Makefile.plugins
> @@ -128,7 +128,7 @@ endif
>
>  if VCP
>  builtin_modules += vcp
> -builtin_sources += profiles/audio/vcp.c
> +builtin_sources += profiles/audio/vcp.h profiles/audio/vcp.c
>  endif
>
>  if MICP
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index a4198d23a..eff95a7c2 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -55,6 +55,7 @@
>  #include "media.h"
>  #include "transport.h"
>  #include "bass.h"
> +#include "vcp.h"
>
>  #define MEDIA_TRANSPORT_INTERFACE "org.bluez.MediaTransport1"
>
> @@ -1420,6 +1421,7 @@ static const GDBusPropertyTable transport_bap_uc_properties[] = {
>         { "Location", "u", get_location },
>         { "Metadata", "ay", get_metadata },
>         { "Links", "ao", get_links, NULL, links_exists },
> +       { "Volume", "q", get_volume, set_volume, volume_exists },
>         { }
>  };
>
> @@ -2188,6 +2190,17 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
>         bap_update_links(transport);
>  }
>
> +static int8_t transport_bap_get_volume(struct media_transport *transport)
> +{
> +       return bt_audio_vcp_get_volume(transport->device);
> +}
> +
> +static int transport_bap_set_volume(struct media_transport *transport,
> +                                                               int8_t volume)
> +{
> +       return bt_audio_vcp_set_volume(transport->device, volume) ? 0 : -EIO;
> +}
> +
>  static void transport_bap_destroy(void *data)
>  {
>         struct bap_transport *bap = data;
> @@ -2411,7 +2424,8 @@ static void *transport_asha_init(struct media_transport *transport, void *data)
>                         transport_bap_init, \
>                         transport_bap_resume, transport_bap_suspend, \
>                         transport_bap_cancel, _set_state, \
> -                       transport_bap_get_stream, NULL, NULL, NULL, \
> +                       transport_bap_get_stream, transport_bap_get_volume, \
> +                       transport_bap_set_volume, NULL, \
>                         _update_links, transport_bap_destroy)
>
>  #define BAP_UC_OPS(_uuid) \
> @@ -2573,17 +2587,18 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
>  void media_transport_update_volume(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;
> +       if (media_endpoint_get_sep(transport->endpoint)) {
> +               struct a2dp_transport *a2dp = transport->data;
>
> -       a2dp->volume = volume;
> +               /* 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");
> @@ -2628,11 +2643,15 @@ 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;
> +               const char *uuid = media_endpoint_get_uuid(transport->endpoint);
>                 if (transport->device != dev)
>                         continue;
>
> -               /* Volume is A2DP only */
> -               if (media_endpoint_get_sep(transport->endpoint)) {
> +               /* Volume is A2DP and BAP only */
> +               if (media_endpoint_get_sep(transport->endpoint) ||
> +                               strcasecmp(uuid, PAC_SINK_UUID) ||
> +                               strcasecmp(uuid, PAC_SOURCE_UUID) ||
> +                               strcasecmp(uuid, BAA_SERVICE_UUID)) {
>                         media_transport_update_volume(transport, volume);
>                         break;
>                 }
> diff --git a/profiles/audio/vcp.c b/profiles/audio/vcp.c
> index 175275f2e..cc6f352c1 100644
> --- a/profiles/audio/vcp.c
> +++ b/profiles/audio/vcp.c
> @@ -51,6 +51,9 @@
>  #include "src/log.h"
>  #include "src/error.h"
>
> +#include "vcp.h"
> +#include "transport.h"
> +
>  #define VCS_UUID_STR "00001844-0000-1000-8000-00805f9b34fb"
>  #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
>
> @@ -83,6 +86,33 @@ static struct vcp_data *vcp_data_new(struct btd_device *device)
>         return data;
>  }
>
> +static bool match_data(const void *data, const void *match_data)
> +{
> +       const struct vcp_data *vdata = data;
> +       const struct bt_vcp *vcp = match_data;
> +
> +       return vdata->vcp == vcp;
> +}
> +
> +static int8_t scale_volume_vcp2transport(uint8_t volume)
> +{
> +       /* Transport has volume range 0-127, VCP has range 0-255 */
> +       return volume / 2;
> +}
> +
> +static uint8_t scale_volume_transport2vcp(int8_t volume)
> +{
> +       return volume * 2;
> +}

Now I understand why you are doing this, it seems there is an
assumption that we should stick to 0-127 range from A2DP, but perhaps
we document that Transport.Volume range is dependent on the
Transport.UUID so for VCP we can use 0-255, in case you were looking
into how ASHA was implemented that has a different problem because
Volume is returning uint16 we can't really use negative numbers thus
why that uses scaling. Anyway for now let go this way but if you have
time please just update the document so we can remove this scaling and
leave it for the clients to interpret based on the Transport.UUID.

> +static void vcp_volume_changed(struct bt_vcp *vcp, uint8_t volume)
> +{
> +       struct vcp_data *data = queue_find(sessions, match_data, vcp);
> +
> +       if (data)
> +               media_transport_update_device_volume(data->device, scale_volume_vcp2transport(volume));
> +}
> +
>  static void vcp_data_add(struct vcp_data *data)
>  {
>         DBG("data %p", data);
> @@ -93,6 +123,7 @@ static void vcp_data_add(struct vcp_data *data)
>         }
>
>         bt_vcp_set_debug(data->vcp, vcp_debug, NULL, NULL);
> +       bt_vcp_set_volume_callback(data->vcp, vcp_volume_changed);
>
>         if (!sessions)
>                 sessions = queue_new();
> @@ -103,12 +134,12 @@ static void vcp_data_add(struct vcp_data *data)
>                 btd_service_set_user_data(data->service, data);
>  }
>
> -static bool match_data(const void *data, const void *match_data)
> +static bool match_device(const void *data, const void *match_data)
>  {
>         const struct vcp_data *vdata = data;
> -       const struct bt_vcp *vcp = match_data;
> +       const struct btd_device *device = match_data;
>
> -       return vdata->vcp == vcp;
> +       return vdata->device == device;
>  }
>
>  static void vcp_data_free(struct vcp_data *data)
> @@ -137,6 +168,26 @@ static void vcp_data_remove(struct vcp_data *data)
>         }
>  }
>
> +int8_t bt_audio_vcp_get_volume(struct btd_device *device)
> +{
> +       struct vcp_data *data = queue_find(sessions, match_device, device);
> +
> +       if (data)
> +               return scale_volume_vcp2transport(bt_vcp_get_volume(data->vcp));
> +
> +       return 0;
> +}
> +
> +bool bt_audio_vcp_set_volume(struct btd_device *device, int8_t volume)
> +{
> +       struct vcp_data *data = queue_find(sessions, match_device, device);
> +
> +       if (data)
> +               return bt_vcp_set_volume(data->vcp, scale_volume_transport2vcp(volume));
> +
> +       return FALSE;
> +}
> +
>  static void vcp_detached(struct bt_vcp *vcp, void *user_data)
>  {
>         struct vcp_data *data;
> diff --git a/profiles/audio/vcp.h b/profiles/audio/vcp.h
> new file mode 100644
> index 000000000..f313cd96a
> --- /dev/null
> +++ b/profiles/audio/vcp.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + *
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2024 StreamUnlimited Engineering GmbH
> + *
> + *
> + */
> +
> +int8_t bt_audio_vcp_get_volume(struct btd_device *device);
> +bool bt_audio_vcp_set_volume(struct btd_device *device, int8_t volume);
> --
> 2.34.1
>
>
diff mbox series

Patch

diff --git a/Makefile.plugins b/Makefile.plugins
index 97335d643..81cf3155a 100644
--- a/Makefile.plugins
+++ b/Makefile.plugins
@@ -128,7 +128,7 @@  endif
 
 if VCP
 builtin_modules += vcp
-builtin_sources += profiles/audio/vcp.c
+builtin_sources += profiles/audio/vcp.h profiles/audio/vcp.c
 endif
 
 if MICP
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index a4198d23a..eff95a7c2 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -55,6 +55,7 @@ 
 #include "media.h"
 #include "transport.h"
 #include "bass.h"
+#include "vcp.h"
 
 #define MEDIA_TRANSPORT_INTERFACE "org.bluez.MediaTransport1"
 
@@ -1420,6 +1421,7 @@  static const GDBusPropertyTable transport_bap_uc_properties[] = {
 	{ "Location", "u", get_location },
 	{ "Metadata", "ay", get_metadata },
 	{ "Links", "ao", get_links, NULL, links_exists },
+	{ "Volume", "q", get_volume, set_volume, volume_exists },
 	{ }
 };
 
@@ -2188,6 +2190,17 @@  static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
 	bap_update_links(transport);
 }
 
+static int8_t transport_bap_get_volume(struct media_transport *transport)
+{
+	return bt_audio_vcp_get_volume(transport->device);
+}
+
+static int transport_bap_set_volume(struct media_transport *transport,
+								int8_t volume)
+{
+	return bt_audio_vcp_set_volume(transport->device, volume) ? 0 : -EIO;
+}
+
 static void transport_bap_destroy(void *data)
 {
 	struct bap_transport *bap = data;
@@ -2411,7 +2424,8 @@  static void *transport_asha_init(struct media_transport *transport, void *data)
 			transport_bap_init, \
 			transport_bap_resume, transport_bap_suspend, \
 			transport_bap_cancel, _set_state, \
-			transport_bap_get_stream, NULL, NULL, NULL, \
+			transport_bap_get_stream, transport_bap_get_volume, \
+			transport_bap_set_volume, NULL, \
 			_update_links, transport_bap_destroy)
 
 #define BAP_UC_OPS(_uuid) \
@@ -2573,17 +2587,18 @@  struct btd_device *media_transport_get_dev(struct media_transport *transport)
 void media_transport_update_volume(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;
+	if (media_endpoint_get_sep(transport->endpoint)) {
+		struct a2dp_transport *a2dp = transport->data;
 
-	a2dp->volume = volume;
+		/* 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");
@@ -2628,11 +2643,15 @@  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;
+		const char *uuid = media_endpoint_get_uuid(transport->endpoint);
 		if (transport->device != dev)
 			continue;
 
-		/* Volume is A2DP only */
-		if (media_endpoint_get_sep(transport->endpoint)) {
+		/* Volume is A2DP and BAP only */
+		if (media_endpoint_get_sep(transport->endpoint) ||
+				strcasecmp(uuid, PAC_SINK_UUID) ||
+				strcasecmp(uuid, PAC_SOURCE_UUID) ||
+				strcasecmp(uuid, BAA_SERVICE_UUID)) {
 			media_transport_update_volume(transport, volume);
 			break;
 		}
diff --git a/profiles/audio/vcp.c b/profiles/audio/vcp.c
index 175275f2e..cc6f352c1 100644
--- a/profiles/audio/vcp.c
+++ b/profiles/audio/vcp.c
@@ -51,6 +51,9 @@ 
 #include "src/log.h"
 #include "src/error.h"
 
+#include "vcp.h"
+#include "transport.h"
+
 #define VCS_UUID_STR "00001844-0000-1000-8000-00805f9b34fb"
 #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
 
@@ -83,6 +86,33 @@  static struct vcp_data *vcp_data_new(struct btd_device *device)
 	return data;
 }
 
+static bool match_data(const void *data, const void *match_data)
+{
+	const struct vcp_data *vdata = data;
+	const struct bt_vcp *vcp = match_data;
+
+	return vdata->vcp == vcp;
+}
+
+static int8_t scale_volume_vcp2transport(uint8_t volume)
+{
+	/* Transport has volume range 0-127, VCP has range 0-255 */
+	return volume / 2;
+}
+
+static uint8_t scale_volume_transport2vcp(int8_t volume)
+{
+	return volume * 2;
+}
+
+static void vcp_volume_changed(struct bt_vcp *vcp, uint8_t volume)
+{
+	struct vcp_data *data = queue_find(sessions, match_data, vcp);
+
+	if (data)
+		media_transport_update_device_volume(data->device, scale_volume_vcp2transport(volume));
+}
+
 static void vcp_data_add(struct vcp_data *data)
 {
 	DBG("data %p", data);
@@ -93,6 +123,7 @@  static void vcp_data_add(struct vcp_data *data)
 	}
 
 	bt_vcp_set_debug(data->vcp, vcp_debug, NULL, NULL);
+	bt_vcp_set_volume_callback(data->vcp, vcp_volume_changed);
 
 	if (!sessions)
 		sessions = queue_new();
@@ -103,12 +134,12 @@  static void vcp_data_add(struct vcp_data *data)
 		btd_service_set_user_data(data->service, data);
 }
 
-static bool match_data(const void *data, const void *match_data)
+static bool match_device(const void *data, const void *match_data)
 {
 	const struct vcp_data *vdata = data;
-	const struct bt_vcp *vcp = match_data;
+	const struct btd_device *device = match_data;
 
-	return vdata->vcp == vcp;
+	return vdata->device == device;
 }
 
 static void vcp_data_free(struct vcp_data *data)
@@ -137,6 +168,26 @@  static void vcp_data_remove(struct vcp_data *data)
 	}
 }
 
+int8_t bt_audio_vcp_get_volume(struct btd_device *device)
+{
+	struct vcp_data *data = queue_find(sessions, match_device, device);
+
+	if (data)
+		return scale_volume_vcp2transport(bt_vcp_get_volume(data->vcp));
+
+	return 0;
+}
+
+bool bt_audio_vcp_set_volume(struct btd_device *device, int8_t volume)
+{
+	struct vcp_data *data = queue_find(sessions, match_device, device);
+
+	if (data)
+		return bt_vcp_set_volume(data->vcp, scale_volume_transport2vcp(volume));
+
+	return FALSE;
+}
+
 static void vcp_detached(struct bt_vcp *vcp, void *user_data)
 {
 	struct vcp_data *data;
diff --git a/profiles/audio/vcp.h b/profiles/audio/vcp.h
new file mode 100644
index 000000000..f313cd96a
--- /dev/null
+++ b/profiles/audio/vcp.h
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2024 StreamUnlimited Engineering GmbH
+ *
+ *
+ */
+
+int8_t bt_audio_vcp_get_volume(struct btd_device *device);
+bool bt_audio_vcp_set_volume(struct btd_device *device, int8_t volume);