diff mbox series

[BlueZ,v2,1/3] audio/transport: Add volume callback to BAP transport

Message ID 20221007061223.46114-1-sathish.narasimman@intel.com
State New
Headers show
Series [BlueZ,v2,1/3] audio/transport: Add volume callback to BAP transport | expand

Commit Message

Sathish Narasimman Oct. 7, 2022, 6:12 a.m. UTC
Initialize set_volume and get_volume to BAP transport and update the
volume when media_transport_update_device_volume is called.
---
 profiles/audio/transport.c | 98 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

Comments

bluez.test.bot@gmail.com Oct. 7, 2022, 7:35 a.m. UTC | #1
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=683646

---Test result---

Test Summary:
CheckPatch                    PASS      2.41 seconds
GitLint                       PASS      1.40 seconds
Prep - Setup ELL              PASS      31.92 seconds
Build - Prep                  PASS      0.66 seconds
Build - Configure             PASS      10.33 seconds
Build - Make                  PASS      963.09 seconds
Make Check                    PASS      13.13 seconds
Make Check w/Valgrind         PASS      347.55 seconds
Make Distcheck                PASS      292.36 seconds
Build w/ext ELL - Configure   PASS      10.51 seconds
Build w/ext ELL - Make        PASS      101.36 seconds
Incremental Build w/ patches  PASS      359.03 seconds
Scan Build                    PASS      624.27 seconds



---
Regards,
Linux Bluetooth
Sathish Narasimman Oct. 9, 2022, 8:10 a.m. UTC | #2
Hi

On Fri, Oct 7, 2022 at 1:26 PM <bluez.test.bot@gmail.com> wrote:
>
> 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=683646
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      2.41 seconds
> GitLint                       PASS      1.40 seconds
> Prep - Setup ELL              PASS      31.92 seconds
> Build - Prep                  PASS      0.66 seconds
> Build - Configure             PASS      10.33 seconds
> Build - Make                  PASS      963.09 seconds
> Make Check                    PASS      13.13 seconds
> Make Check w/Valgrind         PASS      347.55 seconds
> Make Distcheck                PASS      292.36 seconds
> Build w/ext ELL - Configure   PASS      10.51 seconds
> Build w/ext ELL - Make        PASS      101.36 seconds
> Incremental Build w/ patches  PASS      359.03 seconds
> Scan Build                    PASS      624.27 seconds
>
>
>

Please help to review the changes


> ---
> Regards,
> Linux Bluetooth
>

Regards
Sathish N
Luiz Augusto von Dentz Oct. 10, 2022, 7:58 p.m. UTC | #3
Hi Sathish,

On Thu, Oct 6, 2022 at 11:26 PM Sathish Narasimman
<sathish.narasimman@intel.com> wrote:
>
> Initialize set_volume and get_volume to BAP transport and update the
> volume when media_transport_update_device_volume is called.
> ---
>  profiles/audio/transport.c | 98 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 98 insertions(+)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 41339da51e17..46b936c965bf 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_vol)(struct media_transport *transport,
> +                                               int8_t volume);
> +       int8_t                  (*get_vol)(struct media_transport *transport);

Lets have volume in the function name instead of just vol above.

>         GDestroyNotify          destroy;
>         void                    *data;
>  };
> @@ -717,6 +721,7 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
>         return a2dp->volume >= 0;
>  }
>
> +
>  static gboolean get_volume(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -769,6 +774,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 +1027,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 },
> @@ -1387,6 +1445,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 +1486,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 +1497,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_vol = set_volume_bap;
> +       transport->get_vol = get_volume_bap;

Im not seeing any initialization for A2DP or that doesn't need
dedicated callbacks?

>         transport->destroy = free_bap;
>
>         return 0;
> @@ -1537,6 +1623,11 @@ 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;
> +
> +               /* Get BAP transport volume */
> +               if (transport->get_vol)
> +                       return transport->get_vol(transport);
> +
>                 if (transport->device != dev)
>                         continue;
>
> @@ -1560,6 +1651,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;
> +
> +               /* Set BAP transport volume */
> +               if (transport->set_vol) {
> +                       transport->set_vol(transport, volume);
> +                       return;
> +               }
> +
>                 if (transport->device != dev)
>                         continue;
>
> --
> 2.25.1
>
Sathish Narasimman Oct. 11, 2022, 3:54 a.m. UTC | #4
Hi Luiz

On Tue, Oct 11, 2022 at 1:45 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Sathish,
>
> On Thu, Oct 6, 2022 at 11:26 PM Sathish Narasimman
> <sathish.narasimman@intel.com> wrote:
> >
> > Initialize set_volume and get_volume to BAP transport and update the
> > volume when media_transport_update_device_volume is called.
> > ---
> >  profiles/audio/transport.c | 98 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 98 insertions(+)
> >
> > diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> > index 41339da51e17..46b936c965bf 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_vol)(struct media_transport *transport,
> > +                                               int8_t volume);
> > +       int8_t                  (*get_vol)(struct media_transport *transport);
>
> Lets have volume in the function name instead of just vol above.
For the present indentation if we use set_volume it exceeds 80 spaces.
Not sure if we can split the like this
"struct media_transport
*transport"
in 2 lines. If the above is allowed,I will make the changes and submit.

>
> >         GDestroyNotify          destroy;
> >         void                    *data;
> >  };
> > @@ -717,6 +721,7 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
> >         return a2dp->volume >= 0;
> >  }
> >
> > +
> >  static gboolean get_volume(const GDBusPropertyTable *property,
> >                                         DBusMessageIter *iter, void *data)
> >  {
> > @@ -769,6 +774,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 +1027,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 },
> > @@ -1387,6 +1445,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 +1486,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 +1497,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_vol = set_volume_bap;
> > +       transport->get_vol = get_volume_bap;
>
> Im not seeing any initialization for A2DP or that doesn't need
> dedicated callbacks?
yes, I have missed it just to keep bap changes. will update in v3
>
> >         transport->destroy = free_bap;
> >
> >         return 0;
> > @@ -1537,6 +1623,11 @@ 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;
> > +
> > +               /* Get BAP transport volume */
> > +               if (transport->get_vol)
> > +                       return transport->get_vol(transport);
> > +
> >                 if (transport->device != dev)
> >                         continue;
> >
> > @@ -1560,6 +1651,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;
> > +
> > +               /* Set BAP transport volume */
> > +               if (transport->set_vol) {
> > +                       transport->set_vol(transport, volume);
> > +                       return;
> > +               }
> > +
> >                 if (transport->device != dev)
> >                         continue;
> >
> > --
> > 2.25.1
> >
>
>
> --
> Luiz Augusto von Dentz

Sathish N
diff mbox series

Patch

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 41339da51e17..46b936c965bf 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_vol)(struct media_transport *transport,
+						int8_t volume);
+	int8_t			(*get_vol)(struct media_transport *transport);
 	GDestroyNotify		destroy;
 	void			*data;
 };
@@ -717,6 +721,7 @@  static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
 	return a2dp->volume >= 0;
 }
 
+
 static gboolean get_volume(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -769,6 +774,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 +1027,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 },
@@ -1387,6 +1445,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 +1486,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 +1497,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_vol = set_volume_bap;
+	transport->get_vol = get_volume_bap;
 	transport->destroy = free_bap;
 
 	return 0;
@@ -1537,6 +1623,11 @@  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;
+
+		/* Get BAP transport volume */
+		if (transport->get_vol)
+			return transport->get_vol(transport);
+
 		if (transport->device != dev)
 			continue;
 
@@ -1560,6 +1651,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;
+
+		/* Set BAP transport volume */
+		if (transport->set_vol) {
+			transport->set_vol(transport, volume);
+			return;
+		}
+
 		if (transport->device != dev)
 			continue;