| Message ID | c304d0b98ed0ce4504549e43a99adcfcaca77521.1745771127.git.pav@iki.fi |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ,v3,1/2] org.bluez.Media: add SupportedFeatures | 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=957508 ---Test result--- Test Summary: CheckPatch PENDING 0.25 seconds GitLint PENDING 0.29 seconds BuildEll PASS 20.68 seconds BluezMake PASS 2695.02 seconds MakeCheck PASS 20.48 seconds MakeDistcheck PASS 200.77 seconds CheckValgrind PASS 278.41 seconds CheckSmatch PASS 306.31 seconds bluezmakeextell PASS 129.96 seconds IncrementalBuild PENDING 0.19 seconds ScanBuild PASS 934.57 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
Hi Pauli, On Sun, Apr 27, 2025 at 12:25 PM Pauli Virtanen <pav@iki.fi> wrote: > > Add org.bluez.Media.SupportedFeatures. Add feature tx-timestamping. > --- > > Notes: > v3: > - fix #includes > > v2: > - use SIOCETHTOOL to get kernel support > > profiles/audio/media.c | 74 ++++++++++++++++++++++++++++++++++++++++++ > src/adapter.h | 3 ++ > 2 files changed, 77 insertions(+) > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > index 69c6dc671..07264cfa1 100644 > --- a/profiles/audio/media.c > +++ b/profiles/audio/media.c > @@ -18,6 +18,17 @@ > #include <errno.h> > #include <inttypes.h> > > +#include <time.h> > +#include <stdio.h> > +#include <unistd.h> > +#include <linux/errqueue.h> > +#include <linux/net_tstamp.h> > +#include <linux/ethtool.h> > +#include <linux/sockios.h> > +#include <net/if.h> > +#include <sys/socket.h> > +#include <sys/ioctl.h> > + > #include <glib.h> > > #include "lib/bluetooth.h" > @@ -81,6 +92,7 @@ struct media_adapter { > #ifdef HAVE_AVRCP > GSList *players; /* Players list */ > #endif > + int so_timestamping; > }; > > struct endpoint_request { > @@ -3340,8 +3352,69 @@ static gboolean supported_uuids(const GDBusPropertyTable *property, > return TRUE; > } I would add a comment regarding the assumption that this depends on bluetoothd not threatening the errqueue events as errors, which is why it is important to have this exposed otherwise the clients don't know it can be safely enabled with use of ETHTOOL_GET_TS_INFO. > +static bool probe_tx_timestamping(struct media_adapter *adapter) > +{ > + struct ifreq ifr = {}; > + struct ethtool_ts_info cmd = {}; > + int sk = -1; > + > + if (adapter->so_timestamping != -1) > + goto done; > + > + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "hci%u", > + btd_adapter_get_index(adapter->btd_adapter)); > + ifr.ifr_data = (void *)&cmd; > + cmd.cmd = ETHTOOL_GET_TS_INFO; I'd probably add a comment here saying that if one does support SIOCETHTOOL over L2CAP then it must be enabled to other MediaTransports, e.g. ISO. > + sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP); > + if (sk < 0) > + goto error; > + if (ioctl(sk, SIOCETHTOOL, &ifr)) > + goto error; > + close(sk); > + > + adapter->so_timestamping = cmd.so_timestamping; > + > +done: > + return adapter->so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE; > + > +error: > + if (sk >= 0) > + close(sk); > + adapter->so_timestamping = 0; > + return false; > +} > + > +static const struct { > + const char *name; > + bool (*probe)(struct media_adapter *adapter); > +} features[] = { > + { "tx-timestamping", probe_tx_timestamping }, > +}; > + > +static gboolean supported_features(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct media_adapter *adapter = data; > + DBusMessageIter entry; > + size_t i; > + > + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, > + DBUS_TYPE_STRING_AS_STRING, &entry); > + > + for (i = 0; i < ARRAY_SIZE(features); ++i) > + if (features[i].probe(adapter)) > + dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, > + &features[i].name); > + > + dbus_message_iter_close_container(iter, &entry); > + > + return TRUE; > +} > + > static const GDBusPropertyTable media_properties[] = { > { "SupportedUUIDs", "as", supported_uuids }, > + { "SupportedFeatures", "as", supported_features }, > { } > }; > > @@ -3383,6 +3456,7 @@ int media_register(struct btd_adapter *btd_adapter) > adapter = g_new0(struct media_adapter, 1); > adapter->btd_adapter = btd_adapter_ref(btd_adapter); > adapter->apps = queue_new(); > + adapter->so_timestamping = -1; > > if (!g_dbus_register_interface(btd_get_dbus_connection(), > adapter_get_path(btd_adapter), > diff --git a/src/adapter.h b/src/adapter.h > index 6b2bc28f6..9371cdefb 100644 > --- a/src/adapter.h > +++ b/src/adapter.h > @@ -262,6 +262,9 @@ bool btd_le_connect_before_pairing(void); > > bool btd_adapter_has_settings(struct btd_adapter *adapter, uint32_t settings); > > +int btd_adapter_get_so_timestamping(struct btd_adapter *adapter, int proto, > + int *so_timestamping); > + > enum experimental_features { > EXP_FEAT_DEBUG = 1 << 0, > EXP_FEAT_LE_SIMULT_ROLES = 1 << 1, > -- > 2.49.0 > >
Hi Pauli, On Sun, Apr 27, 2025 at 12:25 PM Pauli Virtanen <pav@iki.fi> wrote: > > Add org.bluez.Media.SupportedFeatures. Add feature tx-timestamping. > --- > > Notes: > v3: > - fix #includes > > v2: > - use SIOCETHTOOL to get kernel support > > profiles/audio/media.c | 74 ++++++++++++++++++++++++++++++++++++++++++ > src/adapter.h | 3 ++ > 2 files changed, 77 insertions(+) > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > index 69c6dc671..07264cfa1 100644 > --- a/profiles/audio/media.c > +++ b/profiles/audio/media.c > @@ -18,6 +18,17 @@ > #include <errno.h> > #include <inttypes.h> > > +#include <time.h> > +#include <stdio.h> > +#include <unistd.h> > +#include <linux/errqueue.h> > +#include <linux/net_tstamp.h> > +#include <linux/ethtool.h> > +#include <linux/sockios.h> > +#include <net/if.h> > +#include <sys/socket.h> > +#include <sys/ioctl.h> > + > #include <glib.h> > > #include "lib/bluetooth.h" > @@ -81,6 +92,7 @@ struct media_adapter { > #ifdef HAVE_AVRCP > GSList *players; /* Players list */ > #endif > + int so_timestamping; > }; > > struct endpoint_request { > @@ -3340,8 +3352,69 @@ static gboolean supported_uuids(const GDBusPropertyTable *property, > return TRUE; > } One thing that just occurred to me, can ETHTOOL_GET_TS_INFO be written as well? If it can we could make this just an ioctl operation where we attempt to enable so_timestamping field and then the kernel checks if it can be enabled, that way we don't have to introduce another to D-Bus since so_timestamping would only be enabled if bluetoothd had enabled it, that said we need to confirm that would fail with older kernels. > +static bool probe_tx_timestamping(struct media_adapter *adapter) > +{ > + struct ifreq ifr = {}; > + struct ethtool_ts_info cmd = {}; > + int sk = -1; > + > + if (adapter->so_timestamping != -1) > + goto done; > + > + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "hci%u", > + btd_adapter_get_index(adapter->btd_adapter)); > + ifr.ifr_data = (void *)&cmd; > + cmd.cmd = ETHTOOL_GET_TS_INFO; > + > + sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP); > + if (sk < 0) > + goto error; > + if (ioctl(sk, SIOCETHTOOL, &ifr)) > + goto error; > + close(sk); > + > + adapter->so_timestamping = cmd.so_timestamping; > + > +done: > + return adapter->so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE; > + > +error: > + if (sk >= 0) > + close(sk); > + adapter->so_timestamping = 0; > + return false; > +} > + > +static const struct { > + const char *name; > + bool (*probe)(struct media_adapter *adapter); > +} features[] = { > + { "tx-timestamping", probe_tx_timestamping }, > +}; > + > +static gboolean supported_features(const GDBusPropertyTable *property, > + DBusMessageIter *iter, void *data) > +{ > + struct media_adapter *adapter = data; > + DBusMessageIter entry; > + size_t i; > + > + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, > + DBUS_TYPE_STRING_AS_STRING, &entry); > + > + for (i = 0; i < ARRAY_SIZE(features); ++i) > + if (features[i].probe(adapter)) > + dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, > + &features[i].name); > + > + dbus_message_iter_close_container(iter, &entry); > + > + return TRUE; > +} > + > static const GDBusPropertyTable media_properties[] = { > { "SupportedUUIDs", "as", supported_uuids }, > + { "SupportedFeatures", "as", supported_features }, > { } > }; > > @@ -3383,6 +3456,7 @@ int media_register(struct btd_adapter *btd_adapter) > adapter = g_new0(struct media_adapter, 1); > adapter->btd_adapter = btd_adapter_ref(btd_adapter); > adapter->apps = queue_new(); > + adapter->so_timestamping = -1; > > if (!g_dbus_register_interface(btd_get_dbus_connection(), > adapter_get_path(btd_adapter), > diff --git a/src/adapter.h b/src/adapter.h > index 6b2bc28f6..9371cdefb 100644 > --- a/src/adapter.h > +++ b/src/adapter.h > @@ -262,6 +262,9 @@ bool btd_le_connect_before_pairing(void); > > bool btd_adapter_has_settings(struct btd_adapter *adapter, uint32_t settings); > > +int btd_adapter_get_so_timestamping(struct btd_adapter *adapter, int proto, > + int *so_timestamping); > + > enum experimental_features { > EXP_FEAT_DEBUG = 1 << 0, > EXP_FEAT_LE_SIMULT_ROLES = 1 << 1, > -- > 2.49.0 > >
Hi Luiz, ma, 2025-04-28 kello 10:49 -0400, Luiz Augusto von Dentz kirjoitti: > On Sun, Apr 27, 2025 at 12:25 PM Pauli Virtanen <pav@iki.fi> wrote: > > > > Add org.bluez.Media.SupportedFeatures. Add feature tx-timestamping. > > --- > > > > Notes: > > v3: > > - fix #includes > > > > v2: > > - use SIOCETHTOOL to get kernel support > > > > profiles/audio/media.c | 74 ++++++++++++++++++++++++++++++++++++++++++ > > src/adapter.h | 3 ++ > > 2 files changed, 77 insertions(+) > > > > diff --git a/profiles/audio/media.c b/profiles/audio/media.c > > index 69c6dc671..07264cfa1 100644 > > --- a/profiles/audio/media.c > > +++ b/profiles/audio/media.c > > @@ -18,6 +18,17 @@ > > #include <errno.h> > > #include <inttypes.h> > > > > +#include <time.h> > > +#include <stdio.h> > > +#include <unistd.h> > > +#include <linux/errqueue.h> > > +#include <linux/net_tstamp.h> > > +#include <linux/ethtool.h> > > +#include <linux/sockios.h> > > +#include <net/if.h> > > +#include <sys/socket.h> > > +#include <sys/ioctl.h> > > + > > #include <glib.h> > > > > #include "lib/bluetooth.h" > > @@ -81,6 +92,7 @@ struct media_adapter { > > #ifdef HAVE_AVRCP > > GSList *players; /* Players list */ > > #endif > > + int so_timestamping; > > }; > > > > struct endpoint_request { > > @@ -3340,8 +3352,69 @@ static gboolean supported_uuids(const GDBusPropertyTable *property, > > return TRUE; > > } > > One thing that just occurred to me, can ETHTOOL_GET_TS_INFO be written > as well? If it can we could make this just an ioctl operation where we > attempt to enable so_timestamping field and then the kernel checks if > it can be enabled, that way we don't have to introduce another to > D-Bus since so_timestamping would only be enabled if bluetoothd had > enabled it, that said we need to confirm that would fail with older > kernels. There does not appear to be ETHTOOL_SET_TS_INFO or equivalent for netdev, the information comes from drivers where it is hardcoded. So if we want that, it would be a new API. Designing new APIs, one might want instead to take another shot at the NO_POLL_ERRQUEUE thing, but maybe lower in the netdev core stack. > > +static bool probe_tx_timestamping(struct media_adapter *adapter) > > +{ > > + struct ifreq ifr = {}; > > + struct ethtool_ts_info cmd = {}; > > + int sk = -1; > > + > > + if (adapter->so_timestamping != -1) > > + goto done; > > + > > + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "hci%u", > > + btd_adapter_get_index(adapter->btd_adapter)); > > + ifr.ifr_data = (void *)&cmd; > > + cmd.cmd = ETHTOOL_GET_TS_INFO; > > + > > + sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET, BTPROTO_L2CAP); > > + if (sk < 0) > > + goto error; > > + if (ioctl(sk, SIOCETHTOOL, &ifr)) > > + goto error; > > + close(sk); > > + > > + adapter->so_timestamping = cmd.so_timestamping; > > + > > +done: > > + return adapter->so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE; > > + > > +error: > > + if (sk >= 0) > > + close(sk); > > + adapter->so_timestamping = 0; > > + return false; > > +} > > + > > +static const struct { > > + const char *name; > > + bool (*probe)(struct media_adapter *adapter); > > +} features[] = { > > + { "tx-timestamping", probe_tx_timestamping }, > > +}; > > + > > +static gboolean supported_features(const GDBusPropertyTable *property, > > + DBusMessageIter *iter, void *data) > > +{ > > + struct media_adapter *adapter = data; > > + DBusMessageIter entry; > > + size_t i; > > + > > + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, > > + DBUS_TYPE_STRING_AS_STRING, &entry); > > + > > + for (i = 0; i < ARRAY_SIZE(features); ++i) > > + if (features[i].probe(adapter)) > > + dbus_message_iter_append_basic(&entry, DBUS_TYPE_STRING, > > + &features[i].name); > > + > > + dbus_message_iter_close_container(iter, &entry); > > + > > + return TRUE; > > +} > > + > > static const GDBusPropertyTable media_properties[] = { > > { "SupportedUUIDs", "as", supported_uuids }, > > + { "SupportedFeatures", "as", supported_features }, > > { } > > }; > > > > @@ -3383,6 +3456,7 @@ int media_register(struct btd_adapter *btd_adapter) > > adapter = g_new0(struct media_adapter, 1); > > adapter->btd_adapter = btd_adapter_ref(btd_adapter); > > adapter->apps = queue_new(); > > + adapter->so_timestamping = -1; > > > > if (!g_dbus_register_interface(btd_get_dbus_connection(), > > adapter_get_path(btd_adapter), > > diff --git a/src/adapter.h b/src/adapter.h > > index 6b2bc28f6..9371cdefb 100644 > > --- a/src/adapter.h > > +++ b/src/adapter.h > > @@ -262,6 +262,9 @@ bool btd_le_connect_before_pairing(void); > > > > bool btd_adapter_has_settings(struct btd_adapter *adapter, uint32_t settings); > > > > +int btd_adapter_get_so_timestamping(struct btd_adapter *adapter, int proto, > > + int *so_timestamping); > > + > > enum experimental_features { > > EXP_FEAT_DEBUG = 1 << 0, > > EXP_FEAT_LE_SIMULT_ROLES = 1 << 1, > > -- > > 2.49.0 > > > > >
diff --git a/doc/org.bluez.Media.rst b/doc/org.bluez.Media.rst index ecd982652..ef13a56a9 100644 --- a/doc/org.bluez.Media.rst +++ b/doc/org.bluez.Media.rst @@ -7,7 +7,7 @@ BlueZ D-Bus Media API documentation ----------------------------------- :Version: BlueZ -:Date: September 2023 +:Date: April 2025 :Manual section: 5 :Manual group: Linux System Administration @@ -131,3 +131,16 @@ array{string} SupportedUUIDs [readonly] List of 128-bit UUIDs that represents the supported Endpoint registration. + +array{string} SupportedFeatures [readonly] +`````````````````````````````````````````` + + List of strings that represent supported special features. + Possible values: + + :"tx-timestamping": + + Bluetooth TX timestamping in media stream sockets is + supported by BlueZ and kernel. Applications may check + kernel support for specific timestamp types via + SIOCETHTOOL.
Add SupportedFeatures property for feature information that applications cannot find otherwise. Add feature tx-timestamping. Applications cannot enable it on old BlueZ versions without that feature, as it requires special handling on BlueZ side. --- Notes: v3: - no change v2: - mention user application can check tstamp types itself doc/org.bluez.Media.rst | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)