[PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[Kiran K]

Define a flag in struct avdtp and set it based on
the definition of env variable USE_OFFLOAD
---
 profiles/audio/avdtp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index d3dfbf96dda3..b6feac0ba4d5 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -409,6 +409,9 @@ struct avdtp {
 
 	/* Attempt stream setup instead of disconnecting */
 	gboolean stream_setup;
+
+	/* use offload for transport */
+	gboolean use_offload;
 };
 
 static GSList *state_callbacks = NULL;
@@ -2425,6 +2428,7 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 							struct queue *lseps)
 {
 	struct avdtp *session;
+	char *use_offload;
 
 	session = g_new0(struct avdtp, 1);
 
@@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
 
 	session->version = get_version(session);
 
+	use_offload = getenv("USE_OFFLOAD");
+	if (use_offload && !strncmp(use_offload, "1", 1))
+		session->use_offload = TRUE;
+
 	if (!chan)
 		return session;
 
-- 
2.17.1

[PATCH v1 2/5] avdtp: Add support for offload msft open command

[Kiran K]

In a2dp offload use case, controller needs to be sent
MSFT avdtp command after opening media transport channel
---
 lib/bluetooth.h        |  3 +++
 profiles/audio/avdtp.c | 48 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 0fcf412c6c6b..59ef178a563a 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -158,6 +158,9 @@ struct bt_codecs {
 	struct bt_codec codecs[];
 } __attribute__((packed));
 
+#define BT_MSFT_OPEN	20
+#define BT_MSFT_CLOSE	23
+
 /* Connection and socket states */
 enum {
 	BT_CONNECTED = 1, /* Equal to TCP_ESTABLISHED to make net code happy */
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index b6feac0ba4d5..1cd4b4472b08 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2352,6 +2352,47 @@ static uint16_t get_version(struct avdtp *session)
 	return ver;
 }
 
+static gboolean avdtp_offload_open(struct avdtp_stream *stream)
+{
+	int sock;
+	struct avdtp_service_capability *cap;
+	GSList *l;
+
+	if (!stream->io)
+		return FALSE;
+
+	sock = g_io_channel_unix_get_fd(stream->io);
+
+	for (l = stream->caps; l ; l = g_slist_next(l)) {
+		cap = l->data;
+
+		if (cap->category != AVDTP_MEDIA_CODEC)
+			continue;
+		break;
+	}
+
+	if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
+		       sizeof(*cap) + cap->length))
+		return FALSE;
+
+	return TRUE;
+}
+
+static gboolean avdtp_offload_close(struct avdtp_stream *stream)
+{
+	int sock;
+
+	if (!stream->io)
+		return FALSE;
+
+	sock = g_io_channel_unix_get_fd(stream->io);
+
+	if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_CLOSE, 0, 0))
+		return FALSE;
+
+	return TRUE;
+}
+
 static void avdtp_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 {
 	struct avdtp *session = user_data;
@@ -2385,6 +2426,13 @@ static void avdtp_connect_cb(GIOChannel *chan, GError *err, gpointer user_data)
 			session->pending_open ? "transport" : "signaling",
 			address);
 
+	if (session->pending_open && session->use_offload) {
+		if (!avdtp_offload_open(session->pending_open)) {
+			avdtp_offload_close(session->pending_open);
+			goto failed;
+		}
+	}
+
 	if (session->state == AVDTP_SESSION_STATE_CONNECTING) {
 		DBG("AVDTP imtu=%u, omtu=%u", session->imtu, session->omtu);
 
-- 
2.17.1

[PATCH v1 3/5] avdtp: Add support for offload msft start command

[Kiran K]

Send MSFT avdtp start command to trigger a2dp offload
streaming after sending remote AVDTP start command.
---
 lib/bluetooth.h        |  1 +
 profiles/audio/avdtp.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 59ef178a563a..32a3b5f0c2b9 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -159,6 +159,7 @@ struct bt_codecs {
 } __attribute__((packed));
 
 #define BT_MSFT_OPEN	20
+#define BT_MSFT_START	21
 #define BT_MSFT_CLOSE	23
 
 /* Connection and socket states */
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 1cd4b4472b08..1ae96e3e8a07 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2910,6 +2910,21 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre
 	return TRUE;
 }
 
+static gboolean avdtp_offload_start(struct avdtp_stream *stream)
+{
+	int sock;
+
+	if (!stream->io)
+		return FALSE;
+
+	sock = g_io_channel_unix_get_fd(stream->io);
+
+	if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_START, 0, 0))
+		return FALSE;
+
+	return TRUE;
+}
+
 static gboolean avdtp_start_resp(struct avdtp *session,
 					struct avdtp_stream *stream,
 					struct seid_rej *resp, int size)
@@ -2924,6 +2939,9 @@ static gboolean avdtp_start_resp(struct avdtp *session,
 	if (sep->state != AVDTP_STATE_STREAMING)
 		avdtp_sep_set_state(session, sep, AVDTP_STATE_STREAMING);
 
+	if (session->use_offload)
+		avdtp_offload_start(stream);
+
 	return TRUE;
 }
 
-- 
2.17.1

[PATCH v1 4/5] avdtp: Add support for offload msft suspend command

[Kiran K]

In a2dp offload use case, send MSFT avdtp suspend command
followed by remote AVDTP suspend command.
---
 lib/bluetooth.h        |  1 +
 profiles/audio/avdtp.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 32a3b5f0c2b9..2bd253aeeed4 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -160,6 +160,7 @@ struct bt_codecs {
 
 #define BT_MSFT_OPEN	20
 #define BT_MSFT_START	21
+#define BT_MSFT_SUSPEND 22
 #define BT_MSFT_CLOSE	23
 
 /* Connection and socket states */
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 1ae96e3e8a07..d5e8c7856be3 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2958,6 +2958,21 @@ static gboolean avdtp_close_resp(struct avdtp *session,
 	return TRUE;
 }
 
+static gboolean avdtp_offload_suspend(struct avdtp_stream *stream)
+{
+	int sock;
+
+	if (!stream->io)
+		return FALSE;
+
+	sock = g_io_channel_unix_get_fd(stream->io);
+
+	if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_SUSPEND, 0, 0))
+		return FALSE;
+
+	return TRUE;
+}
+
 static gboolean avdtp_suspend_resp(struct avdtp *session,
 					struct avdtp_stream *stream,
 					void *data, int size)
@@ -2969,6 +2984,9 @@ static gboolean avdtp_suspend_resp(struct avdtp *session,
 	if (sep->cfm && sep->cfm->suspend)
 		sep->cfm->suspend(session, sep, stream, NULL, sep->user_data);
 
+	if (session->use_offload)
+		avdtp_offload_suspend(stream);
+
 	return TRUE;
 }
 
-- 
2.17.1

[PATCH v1 5/5] avdtp: Add support for offload msft close command

[Kiran K]

In a2dp offload use case, send MSFT avdtp close
command followed by remote AVDTP close command.
---
 profiles/audio/avdtp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index d5e8c7856be3..3ee5f30967f0 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2953,6 +2953,9 @@ static gboolean avdtp_close_resp(struct avdtp *session,
 
 	avdtp_sep_set_state(session, sep, AVDTP_STATE_CLOSING);
 
+	if (session->use_offload)
+		avdtp_offload_close(stream);
+
 	close_stream(stream);
 
 	return TRUE;
-- 
2.17.1

RE: [v1,1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      7.21 seconds
GitLint                       PASS      4.81 seconds
Prep - Setup ELL              PASS      42.19 seconds
Build - Prep                  PASS      0.53 seconds
Build - Configure             PASS      7.91 seconds
Build - Make                  FAIL      138.55 seconds
Make Check                    FAIL      1.60 seconds
Make Distcheck                PASS      214.63 seconds
Build w/ext ELL - Configure   PASS      8.04 seconds
Build w/ext ELL - Make        FAIL      128.87 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:10501: check] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
profiles/audio/avdtp.c: In function ‘avdtp_connect_cb’:
profiles/audio/avdtp.c:2374:6: error: ‘cap’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
 2374 |  if (setsockopt(sock, SOL_BLUETOOTH, BT_MSFT_OPEN, cap,
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2375 |          sizeof(*cap) + cap->length))
      |          ~~~~~~~~~~~~~~~~~~~~~~~~~~~
profiles/audio/avdtp.c:2358:35: note: ‘cap’ was declared here
 2358 |  struct avdtp_service_capability *cap;
      |                                   ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8640: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4175: all] Error 2




---
Regards,
Linux Bluetooth

Re: [PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[Luiz Augusto von Dentz]

Hi Kiran,

On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
>
> Define a flag in struct avdtp and set it based on
> the definition of env variable USE_OFFLOAD
> ---
>  profiles/audio/avdtp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index d3dfbf96dda3..b6feac0ba4d5 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -409,6 +409,9 @@ struct avdtp {
>
>         /* Attempt stream setup instead of disconnecting */
>         gboolean stream_setup;
> +
> +       /* use offload for transport */
> +       gboolean use_offload;
>  };
>
>  static GSList *state_callbacks = NULL;
> @@ -2425,6 +2428,7 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>                                                         struct queue *lseps)
>  {
>         struct avdtp *session;
> +       char *use_offload;
>
>         session = g_new0(struct avdtp, 1);
>
> @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
>
>         session->version = get_version(session);
>
> +       use_offload = getenv("USE_OFFLOAD");
> +       if (use_offload && !strncmp(use_offload, "1", 1))
> +               session->use_offload = TRUE;
> +

We already have a configuration for experimental flags:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#n118

So you just have to check if experimental is enabled, or the offload
UUID, in adapter.c, also perhaps we should have something like
btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
care of doing all the checking if that had been enabled in the kernel
or not.

>         if (!chan)
>                 return session;
>
> --
> 2.17.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[Luiz Augusto von Dentz]

Hi Kiran,

On Mon, Nov 15, 2021 at 11:42 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Kiran,
>
> On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
> >
> > Define a flag in struct avdtp and set it based on
> > the definition of env variable USE_OFFLOAD
> > ---
> >  profiles/audio/avdtp.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index d3dfbf96dda3..b6feac0ba4d5 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -409,6 +409,9 @@ struct avdtp {
> >
> >         /* Attempt stream setup instead of disconnecting */
> >         gboolean stream_setup;
> > +
> > +       /* use offload for transport */
> > +       gboolean use_offload;
> >  };
> >
> >  static GSList *state_callbacks = NULL;
> > @@ -2425,6 +2428,7 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> >                                                         struct queue *lseps)
> >  {
> >         struct avdtp *session;
> > +       char *use_offload;
> >
> >         session = g_new0(struct avdtp, 1);
> >
> > @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> >
> >         session->version = get_version(session);
> >
> > +       use_offload = getenv("USE_OFFLOAD");
> > +       if (use_offload && !strncmp(use_offload, "1", 1))
> > +               session->use_offload = TRUE;
> > +
>
> We already have a configuration for experimental flags:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#n118

Correction, we may need to introduce yet another experimental UUID
given the UUID above is just about codec offload not MSFT A2DP offload
which may require a completely different set of commands.

> So you just have to check if experimental is enabled, or the offload
> UUID, in adapter.c, also perhaps we should have something like
> btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
> care of doing all the checking if that had been enabled in the kernel
> or not.
>
> >         if (!chan)
> >                 return session;
> >
> > --
> > 2.17.1
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

RE: [PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp offload

[K, Kiran]

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Tuesday, November 16, 2021 1:15 AM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Von Dentz, Luiz
> <luiz.von.dentz@intel.com>
> Subject: Re: [PATCH v1 1/5] avdtp: Add a flag in struct avdtp to control a2dp
> offload
> 
> Hi Kiran,
> 
> On Mon, Nov 15, 2021 at 11:42 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Kiran,
> >
> > On Mon, Nov 15, 2021 at 1:36 AM Kiran K <kiran.k@intel.com> wrote:
> > >
> > > Define a flag in struct avdtp and set it based on the definition of
> > > env variable USE_OFFLOAD
> > > ---
> > >  profiles/audio/avdtp.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c index
> > > d3dfbf96dda3..b6feac0ba4d5 100644
> > > --- a/profiles/audio/avdtp.c
> > > +++ b/profiles/audio/avdtp.c
> > > @@ -409,6 +409,9 @@ struct avdtp {
> > >
> > >         /* Attempt stream setup instead of disconnecting */
> > >         gboolean stream_setup;
> > > +
> > > +       /* use offload for transport */
> > > +       gboolean use_offload;
> > >  };
> > >
> > >  static GSList *state_callbacks = NULL; @@ -2425,6 +2428,7 @@ struct
> > > avdtp *avdtp_new(GIOChannel *chan, struct btd_device *device,
> > >                                                         struct queue
> > > *lseps)  {
> > >         struct avdtp *session;
> > > +       char *use_offload;
> > >
> > >         session = g_new0(struct avdtp, 1);
> > >
> > > @@ -2436,6 +2440,10 @@ struct avdtp *avdtp_new(GIOChannel *chan,
> > > struct btd_device *device,
> > >
> > >         session->version = get_version(session);
> > >
> > > +       use_offload = getenv("USE_OFFLOAD");
> > > +       if (use_offload && !strncmp(use_offload, "1", 1))
> > > +               session->use_offload = TRUE;
> > > +
> >
> > We already have a configuration for experimental flags:
> >
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/main.conf#
> > n118
> 
> Correction, we may need to introduce yet another experimental UUID given
> the UUID above is just about codec offload not MSFT A2DP offload which
> may require a completely different set of commands.
> 
Ok. I will introduce a new UUID for a2dp offload codecs.

> > So you just have to check if experimental is enabled, or the offload
> > UUID, in adapter.c, also perhaps we should have something like
> > btd_adapter_experimental_is_enabled(adapter, uuid) so it would take
> > care of doing all the checking if that had been enabled in the kernel
> > or not.
> >
> > >         if (!chan)
> > >                 return session;
> > >
> > > --
> > > 2.17.1
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
> 
> 
> 
> --
> Luiz Augusto von Dentz

Thanks,
Kiran