| 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 |
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
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
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 >
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 --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;