* [PATCH BlueZ] transport: fix VCP volume updating and sink volumes
@ 2025-11-25 17:15 Pauli Virtanen
2025-11-25 18:32 ` [BlueZ] " bluez.test.bot
2025-11-26 15:48 ` [PATCH BlueZ] " Luiz Augusto von Dentz
0 siblings, 2 replies; 4+ messages in thread
From: Pauli Virtanen @ 2025-11-25 17:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
Signaling VCP volume value update was broken in
media_transport_update_device_volume() due to inverted strcasecmp(), it
also has to be done for all transports since they show the volume.
VCP output volume was incorrectly shown on input transports. Don't
expose Volume for BAP input transports, since AICS is only partly
implemented.
---
profiles/audio/transport.c | 36 +++++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index fc23cf33d..d466ec9b6 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -2308,16 +2308,32 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
static int transport_bap_get_volume(struct media_transport *transport)
{
#ifdef HAVE_VCP
- return bt_audio_vcp_get_volume(transport->device);
-#else
- return -ENODEV;
+ const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+
+ /* TODO: PAC_SINK_UUID needs AICS */
+ /* TODO: VOCS */
+
+ if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
+ strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
+ return bt_audio_vcp_get_volume(transport->device);
#endif /* HAVE_VCP */
+
+ return -ENODEV;
}
static int transport_bap_set_volume(struct media_transport *transport,
int volume)
{
#ifdef HAVE_VCP
+ const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+
+ /* TODO: PAC_SINK_UUID needs AICS */
+ /* TODO: VOCS */
+
+ if (strcasecmp(uuid, PAC_SOURCE_UUID) &&
+ strcasecmp(uuid, BAA_SERVICE_UUID))
+ return -ENODEV;
+
if (volume < 0 || volume > 255)
return -EINVAL;
@@ -2802,7 +2818,6 @@ void media_transport_update_device_volume(struct btd_device *dev,
if (dev == NULL || volume < 0)
return;
-#ifdef HAVE_A2DP
/* Attempt to locate the transport to set its volume */
for (l = transports; l; l = l->next) {
struct media_transport *transport = l->data;
@@ -2811,16 +2826,19 @@ void media_transport_update_device_volume(struct btd_device *dev,
continue;
/* 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)) {
+#ifdef HAVE_A2DP
+ if (media_endpoint_get_sep(transport->endpoint)) {
media_transport_update_volume(transport, volume);
break;
}
- }
#endif
+ /* This is sink volume */
+ if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
+ strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
+ media_transport_update_volume(transport, volume);
+ }
+
btd_device_set_volume(dev, volume);
}
--
2.51.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [BlueZ] transport: fix VCP volume updating and sink volumes
2025-11-25 17:15 [PATCH BlueZ] transport: fix VCP volume updating and sink volumes Pauli Virtanen
@ 2025-11-25 18:32 ` bluez.test.bot
2025-11-26 15:48 ` [PATCH BlueZ] " Luiz Augusto von Dentz
1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2025-11-25 18:32 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 1261 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=1027465
---Test result---
Test Summary:
CheckPatch PENDING 0.46 seconds
GitLint PENDING 0.25 seconds
BuildEll PASS 19.97 seconds
BluezMake PASS 643.87 seconds
MakeCheck PASS 21.82 seconds
MakeDistcheck PASS 236.25 seconds
CheckValgrind PASS 293.75 seconds
CheckSmatch PASS 341.22 seconds
bluezmakeextell PASS 178.24 seconds
IncrementalBuild PENDING 0.31 seconds
ScanBuild PASS 953.47 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH BlueZ] transport: fix VCP volume updating and sink volumes
2025-11-25 17:15 [PATCH BlueZ] transport: fix VCP volume updating and sink volumes Pauli Virtanen
2025-11-25 18:32 ` [BlueZ] " bluez.test.bot
@ 2025-11-26 15:48 ` Luiz Augusto von Dentz
2025-11-26 16:51 ` Pauli Virtanen
1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2025-11-26 15:48 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hi Pauli,
On Tue, Nov 25, 2025 at 12:16 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Signaling VCP volume value update was broken in
> media_transport_update_device_volume() due to inverted strcasecmp(), it
> also has to be done for all transports since they show the volume.
>
> VCP output volume was incorrectly shown on input transports. Don't
> expose Volume for BAP input transports, since AICS is only partly
> implemented.
> ---
> profiles/audio/transport.c | 36 +++++++++++++++++++++++++++---------
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index fc23cf33d..d466ec9b6 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -2308,16 +2308,32 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
> static int transport_bap_get_volume(struct media_transport *transport)
> {
> #ifdef HAVE_VCP
> - return bt_audio_vcp_get_volume(transport->device);
> -#else
> - return -ENODEV;
> + const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> +
> + /* TODO: PAC_SINK_UUID needs AICS */
> + /* TODO: VOCS */
> +
> + if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
> + strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
> + return bt_audio_vcp_get_volume(transport->device);
> #endif /* HAVE_VCP */
> +
> + return -ENODEV;
> }
>
> static int transport_bap_set_volume(struct media_transport *transport,
> int volume)
> {
> #ifdef HAVE_VCP
> + const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> +
> + /* TODO: PAC_SINK_UUID needs AICS */
> + /* TODO: VOCS */
> +
> + if (strcasecmp(uuid, PAC_SOURCE_UUID) &&
> + strcasecmp(uuid, BAA_SERVICE_UUID))
> + return -ENODEV;
> +
> if (volume < 0 || volume > 255)
> return -EINVAL;
>
> @@ -2802,7 +2818,6 @@ void media_transport_update_device_volume(struct btd_device *dev,
> if (dev == NULL || volume < 0)
> return;
>
> -#ifdef HAVE_A2DP
> /* Attempt to locate the transport to set its volume */
> for (l = transports; l; l = l->next) {
> struct media_transport *transport = l->data;
> @@ -2811,16 +2826,19 @@ void media_transport_update_device_volume(struct btd_device *dev,
> continue;
>
> /* 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)) {
> +#ifdef HAVE_A2DP
> + if (media_endpoint_get_sep(transport->endpoint)) {
> media_transport_update_volume(transport, volume);
> break;
> }
> - }
> #endif
>
> + /* This is sink volume */
> + if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
> + strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
> + media_transport_update_volume(transport, volume);
> + }
Current code sounds quite messy with respect to callbacks, we should
really only add the callback if the feature is supported per UUID,
perhaps we have to rethink how media.c interfaces with profiles to
avoid having to do #ifdef everywhere.
> btd_device_set_volume(dev, volume);
> }
>
> --
> 2.51.1
>
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH BlueZ] transport: fix VCP volume updating and sink volumes
2025-11-26 15:48 ` [PATCH BlueZ] " Luiz Augusto von Dentz
@ 2025-11-26 16:51 ` Pauli Virtanen
0 siblings, 0 replies; 4+ messages in thread
From: Pauli Virtanen @ 2025-11-26 16:51 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
ke, 2025-11-26 kello 10:48 -0500, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
>
> On Tue, Nov 25, 2025 at 12:16 PM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Signaling VCP volume value update was broken in
> > media_transport_update_device_volume() due to inverted strcasecmp(), it
> > also has to be done for all transports since they show the volume.
> >
> > VCP output volume was incorrectly shown on input transports. Don't
> > expose Volume for BAP input transports, since AICS is only partly
> > implemented.
> > ---
> > profiles/audio/transport.c | 36 +++++++++++++++++++++++++++---------
> > 1 file changed, 27 insertions(+), 9 deletions(-)
> >
> > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > index fc23cf33d..d466ec9b6 100644
> > --- a/profiles/audio/transport.c
> > +++ b/profiles/audio/transport.c
> > @@ -2308,16 +2308,32 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
> > static int transport_bap_get_volume(struct media_transport *transport)
> > {
> > #ifdef HAVE_VCP
> > - return bt_audio_vcp_get_volume(transport->device);
> > -#else
> > - return -ENODEV;
> > + const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> > +
> > + /* TODO: PAC_SINK_UUID needs AICS */
> > + /* TODO: VOCS */
> > +
> > + if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
> > + strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
> > + return bt_audio_vcp_get_volume(transport->device);
> > #endif /* HAVE_VCP */
> > +
> > + return -ENODEV;
> > }
> >
> > static int transport_bap_set_volume(struct media_transport *transport,
> > int volume)
> > {
> > #ifdef HAVE_VCP
> > + const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> > +
> > + /* TODO: PAC_SINK_UUID needs AICS */
> > + /* TODO: VOCS */
> > +
> > + if (strcasecmp(uuid, PAC_SOURCE_UUID) &&
> > + strcasecmp(uuid, BAA_SERVICE_UUID))
> > + return -ENODEV;
> > +
> > if (volume < 0 || volume > 255)
> > return -EINVAL;
> >
> > @@ -2802,7 +2818,6 @@ void media_transport_update_device_volume(struct btd_device *dev,
> > if (dev == NULL || volume < 0)
> > return;
> >
> > -#ifdef HAVE_A2DP
> > /* Attempt to locate the transport to set its volume */
> > for (l = transports; l; l = l->next) {
> > struct media_transport *transport = l->data;
> > @@ -2811,16 +2826,19 @@ void media_transport_update_device_volume(struct btd_device *dev,
> > continue;
> >
> > /* 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)) {
> > +#ifdef HAVE_A2DP
> > + if (media_endpoint_get_sep(transport->endpoint)) {
> > media_transport_update_volume(transport, volume);
> > break;
> > }
> > - }
> > #endif
> >
> > + /* This is sink volume */
> > + if (strcasecmp(uuid, PAC_SOURCE_UUID) == 0 ||
> > + strcasecmp(uuid, BAA_SERVICE_UUID) == 0)
> > + media_transport_update_volume(transport, volume);
> > + }
>
> Current code sounds quite messy with respect to callbacks, we should
> really only add the callback if the feature is supported per UUID,
> perhaps we have to rethink how media.c interfaces with profiles to
> avoid having to do #ifdef everywhere.
Sure, this is something that will need to be revisited as AICS
integration has to be completed, however it could make sense to have
whatever quick fix in the meanwhile as the current code is wrong.
>
> > btd_device_set_volume(dev, volume);
> > }
> >
> > --
> > 2.51.1
> >
> >
>
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-26 17:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-25 17:15 [PATCH BlueZ] transport: fix VCP volume updating and sink volumes Pauli Virtanen
2025-11-25 18:32 ` [BlueZ] " bluez.test.bot
2025-11-26 15:48 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2025-11-26 16:51 ` Pauli Virtanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox