[PATCH BlueZ 0/3] transport: distinguish BAP mic and playback volumes

[PATCH BlueZ 0/3] transport: distinguish BAP mic and playback volumes

[Pauli Virtanen]

Currently we are confusing VCP and MICP volumes.

We are also mixing up AVRCP and VCP volumes.

As microphone volumes are commonly 100% and sound server may set them,
we end up with spurious 100% playback volumes.

Fix this, and also do some cleanup of #ifdefs in the volume code.

Pauli Virtanen (3):
  shared/bap: add bt_bap_stream_is_server
  transport: clean up volume set/get for A2DP and VCP
  transport: distinguish BAP mic and playback volumes

 profiles/audio/avrcp.c     |  10 +--
 profiles/audio/media.c     |   2 +-
 profiles/audio/media.h     |  10 +++
 profiles/audio/transport.c | 137 +++++++++++++++++++++----------------
 profiles/audio/transport.h |  10 +--
 profiles/audio/vcp.c       |  12 ++--
 profiles/audio/vcp.h       |  21 +++++-
 src/shared/bap.c           |   6 ++
 src/shared/bap.h           |   2 +
 9 files changed, 133 insertions(+), 77 deletions(-)

-- 
2.52.0

[PATCH BlueZ 1/3] shared/bap: add bt_bap_stream_is_server

[Pauli Virtanen]

Add function for determining whether a given stream is a BAP Server
stream or not.
---
 src/shared/bap.c | 6 ++++++
 src/shared/bap.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 6a35e4e1d..d0425318c 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -4270,6 +4270,12 @@ uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
 	return 0x00;
 }
 
+bool bt_bap_stream_is_server(struct bt_bap_stream *stream)
+{
+	/* Stream is a BAP Server stream (not Broadcast or BAP Client) */
+	return !stream->client;
+}
+
 static void notify_pac_removed(void *data, void *user_data)
 {
 	struct bt_bap_pac_changed *changed = data;
diff --git a/src/shared/bap.h b/src/shared/bap.h
index 80e91f10a..983b9d9a6 100644
--- a/src/shared/bap.h
+++ b/src/shared/bap.h
@@ -113,6 +113,8 @@ struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
 
 uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
 
+bool bt_bap_stream_is_server(struct bt_bap_stream *stream);
+
 struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
 
 /* Session related function */
-- 
2.52.0

RE: transport: distinguish BAP mic and playback volumes

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1865 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=1038262

---Test result---

Test Summary:
CheckPatch                    PENDING   0.30 seconds
GitLint                       PENDING   0.27 seconds
BuildEll                      PASS      20.50 seconds
BluezMake                     PASS      658.19 seconds
MakeCheck                     PASS      22.50 seconds
MakeDistcheck                 PASS      245.23 seconds
CheckValgrind                 PASS      306.15 seconds
CheckSmatch                   WARNING   354.28 seconds
bluezmakeextell               PASS      183.48 seconds
IncrementalBuild              PENDING   0.25 seconds
ScanBuild                     PASS      1048.98 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:312:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/3] shared/bap: add bt_bap_stream_is_server

[Luiz Augusto von Dentz]

Hi Pauli,

On Sun, Jan 4, 2026 at 3:51 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Add function for determining whether a given stream is a BAP Server
> stream or not.
> ---
>  src/shared/bap.c | 6 ++++++
>  src/shared/bap.h | 2 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 6a35e4e1d..d0425318c 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -4270,6 +4270,12 @@ uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
>         return 0x00;
>  }
>
> +bool bt_bap_stream_is_server(struct bt_bap_stream *stream)
> +{

It is probably a good idea to check if the stream is valid first
before attempting to check if the client field is valid.

> +       /* Stream is a BAP Server stream (not Broadcast or BAP Client) */
> +       return !stream->client;

Hmm, so the assumption here is that stream->client is set for
broadcast as well? Perhaps the terminology is misleading here, perhaps
initiator would have been better, otherwise this may be confused with
BAP client/server role which only applies to unicast, anyway Im fine
rewording it later but I think it would be clearer to have something
like bt_bap_stream_is_initiator.

> +}
> +
>  static void notify_pac_removed(void *data, void *user_data)
>  {
>         struct bt_bap_pac_changed *changed = data;
> diff --git a/src/shared/bap.h b/src/shared/bap.h
> index 80e91f10a..983b9d9a6 100644
> --- a/src/shared/bap.h
> +++ b/src/shared/bap.h
> @@ -113,6 +113,8 @@ struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
>
>  uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
>
> +bool bt_bap_stream_is_server(struct bt_bap_stream *stream);
> +
>  struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
>
>  /* Session related function */
> --
> 2.52.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/3] shared/bap: add bt_bap_stream_is_server

[Pauli Virtanen]

Hi,

ma, 2026-01-05 kello 13:45 -0500, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Sun, Jan 4, 2026 at 3:51 PM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > Add function for determining whether a given stream is a BAP Server
> > stream or not.
> > ---
> >  src/shared/bap.c | 6 ++++++
> >  src/shared/bap.h | 2 ++
> >  2 files changed, 8 insertions(+)
> > 
> > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > index 6a35e4e1d..d0425318c 100644
> > --- a/src/shared/bap.c
> > +++ b/src/shared/bap.c
> > @@ -4270,6 +4270,12 @@ uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
> >         return 0x00;
> >  }
> > 
> > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream)
> > +{
> 
> It is probably a good idea to check if the stream is valid first
> before attempting to check if the client field is valid.

Yes

> > +       /* Stream is a BAP Server stream (not Broadcast or BAP Client) */
> > +       return !stream->client;
> 
> Hmm, so the assumption here is that stream->client is set for
> broadcast as well? Perhaps the terminology is misleading here, perhaps
> initiator would have been better, otherwise this may be confused with
> BAP client/server role which only applies to unicast, anyway Im fine
> rewording it later but I think it would be clearer to have something
> like bt_bap_stream_is_initiator.

Yes, client=true is set for bcast in bap_bcast_stream_new().

This is intented to returns whether the stream is BAP Unicast Server
stream, hence the is_server() suffix, but maybe it's slightly strange
API with broadcast.

Maybe bt_bap_stream_is_initiator() is clearer here,

Local is Initiator Sink (Unicast only) -> MICP
Local is Initiator Source (Unicast or Broadcast) -> VCP
Local is Acceptor Sink (Unicast or Broadcast) -> VCS
Local is Acceptor Source (Unicast only) -> MICS

where Initiator is assumed to also be Commander, with the definitions
from CAP.

> > +}
> > +
> >  static void notify_pac_removed(void *data, void *user_data)
> >  {
> >         struct bt_bap_pac_changed *changed = data;
> > diff --git a/src/shared/bap.h b/src/shared/bap.h
> > index 80e91f10a..983b9d9a6 100644
> > --- a/src/shared/bap.h
> > +++ b/src/shared/bap.h
> > @@ -113,6 +113,8 @@ struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
> > 
> >  uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
> > 
> > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream);
> > +
> >  struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> > 
> >  /* Session related function */
> > --
> > 2.52.0
> > 
> > 
> 

-- 
Pauli Virtanen

Re: [PATCH BlueZ 1/3] shared/bap: add bt_bap_stream_is_server

[Luiz Augusto von Dentz]

Hi Pauli,

On Mon, Jan 5, 2026 at 2:09 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> ma, 2026-01-05 kello 13:45 -0500, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Sun, Jan 4, 2026 at 3:51 PM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > Add function for determining whether a given stream is a BAP Server
> > > stream or not.
> > > ---
> > >  src/shared/bap.c | 6 ++++++
> > >  src/shared/bap.h | 2 ++
> > >  2 files changed, 8 insertions(+)
> > >
> > > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > > index 6a35e4e1d..d0425318c 100644
> > > --- a/src/shared/bap.c
> > > +++ b/src/shared/bap.c
> > > @@ -4270,6 +4270,12 @@ uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
> > >         return 0x00;
> > >  }
> > >
> > > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream)
> > > +{
> >
> > It is probably a good idea to check if the stream is valid first
> > before attempting to check if the client field is valid.
>
> Yes
>
> > > +       /* Stream is a BAP Server stream (not Broadcast or BAP Client) */
> > > +       return !stream->client;
> >
> > Hmm, so the assumption here is that stream->client is set for
> > broadcast as well? Perhaps the terminology is misleading here, perhaps
> > initiator would have been better, otherwise this may be confused with
> > BAP client/server role which only applies to unicast, anyway Im fine
> > rewording it later but I think it would be clearer to have something
> > like bt_bap_stream_is_initiator.
>
> Yes, client=true is set for bcast in bap_bcast_stream_new().
>
> This is intented to returns whether the stream is BAP Unicast Server
> stream, hence the is_server() suffix, but maybe it's slightly strange
> API with broadcast.
>
> Maybe bt_bap_stream_is_initiator() is clearer here,
>
> Local is Initiator Sink (Unicast only) -> MICP
> Local is Initiator Source (Unicast or Broadcast) -> VCP
> Local is Acceptor Sink (Unicast or Broadcast) -> VCS
> Local is Acceptor Source (Unicast only) -> MICS
>
> where Initiator is assumed to also be Commander, with the definitions
> from CAP.

Yep, I guess that is clearer than reusing the client in the context of
stream, or perhaps MICP/VCP have role definitions for these?

>
> > > +}
> > > +
> > >  static void notify_pac_removed(void *data, void *user_data)
> > >  {
> > >         struct bt_bap_pac_changed *changed = data;
> > > diff --git a/src/shared/bap.h b/src/shared/bap.h
> > > index 80e91f10a..983b9d9a6 100644
> > > --- a/src/shared/bap.h
> > > +++ b/src/shared/bap.h
> > > @@ -113,6 +113,8 @@ struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
> > >
> > >  uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
> > >
> > > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream);
> > > +
> > >  struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> > >
> > >  /* Session related function */
> > > --
> > > 2.52.0
> > >
> > >
> >
>
> --
> Pauli Virtanen



-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/3] shared/bap: add bt_bap_stream_is_server

[Pauli Virtanen]

Hi,

ma, 2026-01-05 kello 14:29 -0500, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Mon, Jan 5, 2026 at 2:09 PM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > Hi,
> > 
> > ma, 2026-01-05 kello 13:45 -0500, Luiz Augusto von Dentz kirjoitti:
> > > Hi Pauli,
> > > 
> > > On Sun, Jan 4, 2026 at 3:51 PM Pauli Virtanen <pav@iki.fi> wrote:
> > > > 
> > > > Add function for determining whether a given stream is a BAP Server
> > > > stream or not.
> > > > ---
> > > >  src/shared/bap.c | 6 ++++++
> > > >  src/shared/bap.h | 2 ++
> > > >  2 files changed, 8 insertions(+)
> > > > 
> > > > diff --git a/src/shared/bap.c b/src/shared/bap.c
> > > > index 6a35e4e1d..d0425318c 100644
> > > > --- a/src/shared/bap.c
> > > > +++ b/src/shared/bap.c
> > > > @@ -4270,6 +4270,12 @@ uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream)
> > > >         return 0x00;
> > > >  }
> > > > 
> > > > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream)
> > > > +{
> > > 
> > > It is probably a good idea to check if the stream is valid first
> > > before attempting to check if the client field is valid.
> > 
> > Yes
> > 
> > > > +       /* Stream is a BAP Server stream (not Broadcast or BAP Client) */
> > > > +       return !stream->client;
> > > 
> > > Hmm, so the assumption here is that stream->client is set for
> > > broadcast as well? Perhaps the terminology is misleading here, perhaps
> > > initiator would have been better, otherwise this may be confused with
> > > BAP client/server role which only applies to unicast, anyway Im fine
> > > rewording it later but I think it would be clearer to have something
> > > like bt_bap_stream_is_initiator.
> > 
> > Yes, client=true is set for bcast in bap_bcast_stream_new().
> > 
> > This is intented to returns whether the stream is BAP Unicast Server
> > stream, hence the is_server() suffix, but maybe it's slightly strange
> > API with broadcast.
> > 
> > Maybe bt_bap_stream_is_initiator() is clearer here,
> > 
> > Local is Initiator Sink (Unicast only) -> MICP
> > Local is Initiator Source (Unicast or Broadcast) -> VCP
> > Local is Acceptor Sink (Unicast or Broadcast) -> VCS
> > Local is Acceptor Source (Unicast only) -> MICS
> > 
> > where Initiator is assumed to also be Commander, with the definitions
> > from CAP.
> 
> Yep, I guess that is clearer than reusing the client in the context of
> stream, or perhaps MICP/VCP have role definitions for these?

MICP/VCP have role definitions but they appear equivalent to just
Client/Server but in different words. AFAIK they don't associate them
with BAP streams.

I'd maybe go with CAP terminology here.

> > 
> > > > +}
> > > > +
> > > >  static void notify_pac_removed(void *data, void *user_data)
> > > >  {
> > > >         struct bt_bap_pac_changed *changed = data;
> > > > diff --git a/src/shared/bap.h b/src/shared/bap.h
> > > > index 80e91f10a..983b9d9a6 100644
> > > > --- a/src/shared/bap.h
> > > > +++ b/src/shared/bap.h
> > > > @@ -113,6 +113,8 @@ struct iovec *bt_bap_pac_get_metadata(struct bt_bap_pac *pac);
> > > > 
> > > >  uint8_t bt_bap_stream_get_type(struct bt_bap_stream *stream);
> > > > 
> > > > +bool bt_bap_stream_is_server(struct bt_bap_stream *stream);
> > > > +
> > > >  struct bt_bap_stream *bt_bap_pac_get_stream(struct bt_bap_pac *pac);
> > > > 
> > > >  /* Session related function */
> > > > --
> > > > 2.52.0
> > > > 
> > > > 
> > > 
> > 
> > --
> > Pauli Virtanen
> 
> 

-- 
Pauli Virtanen

[PATCH BlueZ 2/3] transport: clean up volume set/get for A2DP and VCP

[Pauli Virtanen]

Cleanup uuid comparisons in device transport volume set/get. These are
unnecessary because the transport->ops already do the equivalent check.

Use separate functions for set/get AVRCP volume, to avoid mixing up
AVRCP and VCP volumes.

For VCP send volume update notifications on all transports, since same
volume is listed on all.

To avoid interspersing #ifdefs for A2DP/VCP in this code, define dummy
functions in headers that behave accordingly.

Fix the bt_audio_vcp_get/set_volume to properly indicate when VCP is not
present.
---
 profiles/audio/avrcp.c     |  10 ++--
 profiles/audio/media.c     |   2 +-
 profiles/audio/media.h     |  10 ++++
 profiles/audio/transport.c | 116 +++++++++++++++++++------------------
 profiles/audio/transport.h |  10 ++--
 profiles/audio/vcp.c       |  12 ++--
 profiles/audio/vcp.h       |  21 ++++++-
 7 files changed, 105 insertions(+), 76 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 14889e4b4..724b46c59 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1672,7 +1672,7 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
 		len = 1;
 		break;
 	case AVRCP_EVENT_VOLUME_CHANGED:
-		volume = media_transport_get_device_volume(dev);
+		volume = media_transport_get_a2dp_volume(dev);
 		if (volume < 0)
 			goto err;
 
@@ -1795,7 +1795,7 @@ static uint8_t avrcp_handle_set_absolute_volume(struct avrcp *session,
 
 	volume = pdu->params[0] = pdu->params[0] & 0x7F;
 
-	media_transport_update_device_volume(session->dev, volume);
+	media_transport_set_a2dp_volume(session->dev, volume);
 
 	return AVC_CTYPE_ACCEPTED;
 
@@ -3809,7 +3809,7 @@ static void avrcp_volume_changed(struct avrcp *session,
 	volume = pdu->params[1] & 0x7F;
 
 	/* Always attempt to update the transport volume */
-	media_transport_update_device_volume(session->dev, volume);
+	media_transport_set_a2dp_volume(session->dev, volume);
 }
 
 static void avrcp_status_changed(struct avrcp *session,
@@ -4284,7 +4284,7 @@ static void target_init(struct avrcp *session)
 		player->sessions = g_slist_prepend(player->sessions, session);
 
 		init_volume = btd_device_get_volume(session->dev);
-		media_transport_update_device_volume(session->dev, init_volume);
+		media_transport_set_a2dp_volume(session->dev, init_volume);
 	}
 
 	session->supported_events |= (1 << AVRCP_EVENT_STATUS_CHANGED) |
@@ -4646,7 +4646,7 @@ static gboolean avrcp_handle_set_volume(struct avctp *conn, uint8_t code,
 	volume = pdu->params[0] & 0x7F;
 
 	/* Always attempt to update the transport volume */
-	media_transport_update_device_volume(session->dev, volume);
+	media_transport_set_a2dp_volume(session->dev, volume);
 
 	return FALSE;
 }
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index ad9eb7beb..cc0029750 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -583,7 +583,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 		return FALSE;
 
 	init_volume = btd_device_get_volume(device);
-	media_transport_update_volume(transport, init_volume);
+	media_transport_set_a2dp_volume(device, init_volume);
 
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
 						MEDIA_ENDPOINT_INTERFACE,
diff --git a/profiles/audio/media.h b/profiles/audio/media.h
index 28174a017..1c43075ba 100644
--- a/profiles/audio/media.h
+++ b/profiles/audio/media.h
@@ -18,7 +18,17 @@ typedef void (*media_endpoint_cb_t) (struct media_endpoint *endpoint,
 int media_register(struct btd_adapter *btd_adapter);
 void media_unregister(struct btd_adapter *btd_adapter);
 
+#ifdef HAVE_A2DP
 struct a2dp_sep *media_endpoint_get_sep(struct media_endpoint *endpoint);
+#else
+struct a2dp_sep;
+static inline struct a2dp_sep *media_endpoint_get_sep(
+						struct media_endpoint *endpoint)
+{
+	return NULL;
+}
+#endif
+
 const char *media_endpoint_get_uuid(struct media_endpoint *endpoint);
 bool media_endpoint_get_delay_reporting(struct media_endpoint *endpoint);
 uint8_t media_endpoint_get_codec(struct media_endpoint *endpoint);
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index fc23cf33d..7fc8d8525 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1094,6 +1094,22 @@ static void set_delay_report(const GDBusPropertyTable *property,
 }
 #endif /* HAVE_A2DP */
 
+static int media_transport_get_volume(struct media_transport *transport,
+								int *volume)
+{
+	if (transport->ops && transport->ops->get_volume) {
+		int ret = transport->ops->get_volume(transport);
+
+		if (ret < 0)
+			return ret;
+
+		*volume = ret;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
 {
 	struct media_transport *transport = data;
@@ -1105,16 +1121,6 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
 	return volume >= 0;
 }
 
-int media_transport_get_volume(struct media_transport *transport, int *volume)
-{
-	if (transport->ops && transport->ops->get_volume) {
-		*volume = transport->ops->get_volume(transport);
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
 static gboolean get_volume(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -2307,24 +2313,16 @@ 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;
-#endif /* HAVE_VCP */
 }
 
 static int transport_bap_set_volume(struct media_transport *transport,
 								int volume)
 {
-#ifdef HAVE_VCP
 	if (volume < 0 || volume > 255)
 		return -EINVAL;
 
-	return bt_audio_vcp_set_volume(transport->device, volume) ? 0 : -EIO;
-#else
-	return -ENODEV;
-#endif /* HAVE_VCP */
+	return bt_audio_vcp_set_volume(transport->device, volume);
 }
 
 static void transport_bap_destroy(void *data)
@@ -2739,46 +2737,33 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
 	return transport->device;
 }
 
-void media_transport_update_volume(struct media_transport *transport,
-								int volume)
+static void media_transport_emit_volume(struct media_transport *transport)
 {
-	if (volume < 0)
+	int volume;
+
+	if (media_transport_get_volume(transport, &volume))
 		return;
 
-#ifdef HAVE_A2DP
-	if (media_endpoint_get_sep(transport->endpoint)) {
-		struct a2dp_transport *a2dp = transport->data;
-
-		if (volume > 127)
-			return;
-
-		/* Check if volume really changed */
-		if (a2dp->volume == volume)
-			return;
-
-		a2dp->volume = volume;
-	}
-#endif
 	g_dbus_emit_property_changed(btd_get_dbus_connection(),
 					transport->path,
 					MEDIA_TRANSPORT_INTERFACE, "Volume");
 }
 
-int media_transport_get_device_volume(struct btd_device *dev)
+int media_transport_get_a2dp_volume(struct btd_device *dev)
 {
 	GSList *l;
 
 	if (dev == NULL)
 		return -1;
 
-#ifdef HAVE_A2DP
 	/* 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 */
+		/* A2DP only */
 		if (media_endpoint_get_sep(transport->endpoint)) {
 			int volume;
 
@@ -2788,40 +2773,57 @@ int media_transport_get_device_volume(struct btd_device *dev)
 			return -1;
 		}
 	}
-#endif
 
-	/* If transport volume doesn't exists use device_volume */
+	/* If no transport, use device volume. This is a workaround for the lack
+	 * of ordering between AVRCP and A2DP session start. (Note BAP+VCP do
+	 * not have this issue.)
+	 */
 	return btd_device_get_volume(dev);
 }
 
-void media_transport_update_device_volume(struct btd_device *dev,
-								int volume)
+void media_transport_set_a2dp_volume(struct btd_device *dev, int volume)
 {
 	GSList *l;
 
-	if (dev == NULL || volume < 0)
+	if (dev == NULL || volume < 0 || volume > 127)
 		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;
-		const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+		struct a2dp_transport *a2dp;
+
+		if (transport->device != dev)
+			continue;
+		if (!media_endpoint_get_sep(transport->endpoint))
+			continue;
+
+		a2dp = transport->data;
+		if (a2dp->volume != volume) {
+			a2dp->volume = volume;
+			media_transport_emit_volume(transport);
+		}
+		break;
+	}
+
+	btd_device_set_volume(dev, volume);
+}
+
+void media_transport_volume_changed(struct btd_device *dev)
+{
+	GSList *l;
+
+	if (dev == NULL)
+		return;
+
+	for (l = transports; l; l = l->next) {
+		struct media_transport *transport = l->data;
+
 		if (transport->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)) {
-			media_transport_update_volume(transport, volume);
-			break;
-		}
+		media_transport_emit_volume(transport);
 	}
-#endif
-
-	btd_device_set_volume(dev, volume);
 }
 
 const char *media_transport_stream_path(void *stream)
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 7c107281a..90e079c4c 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -21,8 +21,6 @@ void media_transport_destroy(struct media_transport *transport);
 const char *media_transport_get_path(struct media_transport *transport);
 void *media_transport_get_stream(struct media_transport *transport);
 struct btd_device *media_transport_get_dev(struct media_transport *transport);
-int media_transport_get_volume(struct media_transport *transport,
-					int *volume);
 void media_transport_update_delay(struct media_transport *transport,
 							uint16_t delay);
 void media_transport_update_volume(struct media_transport *transport,
@@ -30,7 +28,9 @@ void media_transport_update_volume(struct media_transport *transport,
 void transport_get_properties(struct media_transport *transport,
 							DBusMessageIter *iter);
 
-int media_transport_get_device_volume(struct btd_device *dev);
-void media_transport_update_device_volume(struct btd_device *dev,
-								int volume);
+int media_transport_get_a2dp_volume(struct btd_device *dev);
+void media_transport_set_a2dp_volume(struct btd_device *dev, int volume);
+
+void media_transport_volume_changed(struct btd_device *dev);
+
 const char *media_transport_stream_path(void *stream);
diff --git a/profiles/audio/vcp.c b/profiles/audio/vcp.c
index 277c9bbc3..00ee2b64b 100644
--- a/profiles/audio/vcp.c
+++ b/profiles/audio/vcp.c
@@ -107,7 +107,7 @@ 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, volume);
+		media_transport_volume_changed(data->device);
 }
 
 static void vcp_data_add(struct vcp_data *data)
@@ -165,24 +165,24 @@ static void vcp_data_remove(struct vcp_data *data)
 	}
 }
 
-uint8_t bt_audio_vcp_get_volume(struct btd_device *device)
+int bt_audio_vcp_get_volume(struct btd_device *device)
 {
 	struct vcp_data *data = queue_find(sessions, match_device, device);
 
 	if (data)
 		return bt_vcp_get_volume(data->vcp);
 
-	return 0;
+	return -ENODEV;
 }
 
-bool bt_audio_vcp_set_volume(struct btd_device *device, uint8_t volume)
+int bt_audio_vcp_set_volume(struct btd_device *device, uint8_t volume)
 {
 	struct vcp_data *data = queue_find(sessions, match_device, device);
 
 	if (data)
-		return bt_vcp_set_volume(data->vcp, volume);
+		return bt_vcp_set_volume(data->vcp, volume) ? 0 : -EIO;
 
-	return FALSE;
+	return -ENODEV;
 }
 
 static void vcp_remote_client_detached(struct bt_vcp *vcp, void *user_data)
diff --git a/profiles/audio/vcp.h b/profiles/audio/vcp.h
index cf7935d1a..b538cebf0 100644
--- a/profiles/audio/vcp.h
+++ b/profiles/audio/vcp.h
@@ -8,5 +8,22 @@
  *
  */
 
-uint8_t bt_audio_vcp_get_volume(struct btd_device *device);
-bool bt_audio_vcp_set_volume(struct btd_device *device, uint8_t volume);
+#ifdef HAVE_VCP
+
+int bt_audio_vcp_get_volume(struct btd_device *device);
+int bt_audio_vcp_set_volume(struct btd_device *device, uint8_t volume);
+
+#else
+
+static inline int bt_audio_vcp_get_volume(struct btd_device *device)
+{
+	return -ENODEV;
+}
+
+static inline int bt_audio_vcp_set_volume(struct btd_device *device,
+								uint8_t volume)
+{
+	return -ENODEV;
+}
+
+#endif
-- 
2.52.0

[PATCH BlueZ 3/3] transport: distinguish BAP mic and playback volumes

[Pauli Virtanen]

BAP Unicast has two kinds of relevant volumes: playback and microphone.
Client Sink and Server Source generally correspond to microphone volume.
Broadcast Source and Sink do not have microphone volume.  Microphone
volumes shall use MICP, not VCP, but currently we confuse them.

Fix by distinguishing the VCP / MICP cases. The shared/micp
implementation is incomplete, so leave those volumes unimplemented also
in transport.

This fixes setting volume on microphone transport changing the playback
volume.
---
 profiles/audio/transport.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 7fc8d8525..d9f884710 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -2311,9 +2311,25 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
 	bap_update_links(transport);
 }
 
+static bool transport_bap_is_playback(struct media_transport *transport)
+{
+	struct bap_transport *bap = transport->data;
+	const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+
+	/* BAP Server: local ucast source = "mic" */
+	if (bt_bap_stream_is_server(bap->stream))
+		return strcasecmp(uuid, PAC_SOURCE_UUID) != 0;
+
+	/* BAP Client / Broadcast: local ucast sink = "mic" */
+	return strcasecmp(uuid, PAC_SINK_UUID) != 0;
+}
+
 static int transport_bap_get_volume(struct media_transport *transport)
 {
-	return bt_audio_vcp_get_volume(transport->device);
+	if (transport_bap_is_playback(transport))
+		return bt_audio_vcp_get_volume(transport->device);
+	else
+		return -ENOTSUP; /* TODO: MICP */
 }
 
 static int transport_bap_set_volume(struct media_transport *transport,
@@ -2322,7 +2338,10 @@ static int transport_bap_set_volume(struct media_transport *transport,
 	if (volume < 0 || volume > 255)
 		return -EINVAL;
 
-	return bt_audio_vcp_set_volume(transport->device, volume);
+	if (transport_bap_is_playback(transport))
+		return bt_audio_vcp_set_volume(transport->device, volume);
+	else
+		return -ENOTSUP; /* TODO: MICP */
 }
 
 static void transport_bap_destroy(void *data)
-- 
2.52.0