2020-11-16 23:42:15

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [RFC BlueZ 2/2] main.conf: Add option to configure AVDP session/stream channel modes

From: Luiz Augusto von Dentz <[email protected]>

This adds a new group AVDTP where platform can confure the prefered
modes L2CAP channel for both session (signalling) and stream
(transport). For better backward compatibility the default modes of
boths
---
profiles/audio/a2dp.c | 5 +----
profiles/audio/avdtp.c | 14 ++-----------
src/btd.h | 7 +++++++
src/main.c | 45 ++++++++++++++++++++++++++++++++++++++++++
src/main.conf | 13 ++++++++++++
5 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 626f61d34..59d11a0aa 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -2324,10 +2324,7 @@ static bool a2dp_server_listen(struct a2dp_server *server)
if (server->io)
return true;

- if (btd_opts.mps == MPS_OFF)
- mode = BT_IO_MODE_BASIC;
- else
- mode = BT_IO_MODE_STREAMING;
+ mode = btd_opts.avdtp.session_mode;

server->io = bt_io_listen(NULL, confirm_cb, server, NULL, &err,
BT_IO_OPT_SOURCE_BDADDR,
diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 619b94e29..ff0a124c3 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -2603,12 +2603,7 @@ static int send_req(struct avdtp *session, gboolean priority,
int err, timeout;

if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) {
- BtIOMode mode;
-
- if (btd_opts.mps == MPS_OFF)
- mode = BT_IO_MODE_BASIC;
- else
- mode = BT_IO_MODE_ERTM;
+ BtIOMode mode = btd_opts.avdtp.session_mode;

session->io = l2cap_connect(session, mode);
if (!session->io) {
@@ -2807,12 +2802,7 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre
struct seid_rej *resp, int size)
{
struct avdtp_local_sep *sep = stream->lsep;
- BtIOMode mode;
-
- if (btd_opts.mps == MPS_OFF)
- mode = BT_IO_MODE_BASIC;
- else
- mode = BT_IO_MODE_STREAMING;
+ BtIOMode mode = btd_opts.avdtp.stream_mode;

stream->io = l2cap_connect(session, mode);
if (!stream->io) {
diff --git a/src/btd.h b/src/btd.h
index c98414e35..a3247e4fd 100644
--- a/src/btd.h
+++ b/src/btd.h
@@ -84,6 +84,11 @@ struct btd_defaults {
struct btd_le_defaults le;
};

+struct btd_avdtp_opts {
+ uint8_t session_mode;
+ uint8_t stream_mode;
+};
+
struct btd_opts {
char *name;
uint32_t class;
@@ -112,6 +117,8 @@ struct btd_opts {
uint8_t gatt_channels;
enum mps_mode_t mps;

+ struct btd_avdtp_opts avdtp;
+
uint8_t key_size;

enum jw_repairing_t jw_repairing;
diff --git a/src/main.c b/src/main.c
index e6c4d861e..33c0f0d15 100644
--- a/src/main.c
+++ b/src/main.c
@@ -34,6 +34,7 @@
#include "lib/sdp.h"

#include "gdbus/gdbus.h"
+#include "btio/btio.h"

#include "log.h"
#include "backtrace.h"
@@ -137,6 +138,12 @@ static const char *gatt_options[] = {
NULL
};

+static const char *avdtp_options[] = {
+ "SessionMode",
+ "StreamMode",
+ NULL
+};
+
static const struct group_table {
const char *name;
const char **options;
@@ -146,6 +153,7 @@ static const struct group_table {
{ "LE", le_options },
{ "Policy", policy_options },
{ "GATT", gatt_options },
+ { "AVDTP", avdtp_options },
{ }
};

@@ -744,6 +752,40 @@ static void parse_config(GKeyFile *config)
btd_opts.gatt_channels = val;
}

+ str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ DBG("SessionMode=%s", str);
+
+ if (!strcmp(str, "basic"))
+ btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
+ else if (!strcmp(str, "ertm"))
+ btd_opts.avdtp.session_mode = BT_IO_MODE_ERTM;
+ else {
+ DBG("Invalid mode option: %s", str);
+ btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
+ }
+ }
+
+ val = g_key_file_get_integer(config, "AVDTP", "StreamMode", &err);
+ if (err) {
+ DBG("%s", err->message);
+ g_clear_error(&err);
+ } else {
+ DBG("StreamMode=%s", str);
+
+ if (!strcmp(str, "basic"))
+ btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
+ else if (!strcmp(str, "streaming"))
+ btd_opts.avdtp.stream_mode = BT_IO_MODE_STREAMING;
+ else {
+ DBG("Invalid mode option: %s", str);
+ btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
+ }
+ }
+
parse_br_config(config);
parse_le_config(config);
}
@@ -780,6 +822,9 @@ static void init_defaults(void)
btd_opts.gatt_cache = BT_GATT_CACHE_ALWAYS;
btd_opts.gatt_mtu = BT_ATT_MAX_LE_MTU;
btd_opts.gatt_channels = 3;
+
+ btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
+ btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
}

static void log_handler(const gchar *log_domain, GLogLevelFlags log_level,
diff --git a/src/main.conf b/src/main.conf
index 54f6a36bd..d3bc61441 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -200,6 +200,19 @@
# Default to 3
#Channels = 3

+[AVDTP]
+# AVDTP L2CAP Signalling Channel Mode.
+# Possible values:
+# basic: Use L2CAP Basic Mode
+# ertm: Use L2CAP Enhanced Retransmission Mode
+#SessionMode = basic
+
+# AVDTP L2CAP Transport Channel Mode.
+# Possible values:
+# basic: Use L2CAP Basic Mode
+# streaming: Use L2CAP Streaming Mode
+#StreamMode = basic
+
[Policy]
#
# The ReconnectUUIDs defines the set of remote services that should try
--
2.26.2


2020-11-17 14:45:17

by Alain Michaud

[permalink] [raw]
Subject: Re: [RFC BlueZ 2/2] main.conf: Add option to configure AVDP session/stream channel modes

On Mon, Nov 16, 2020 at 7:26 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds a new group AVDTP where platform can confure the prefered
> modes L2CAP channel for both session (signalling) and stream
> (transport). For better backward compatibility the default modes of
> boths

Reviewed-by: Alain Michaud <[email protected]>
Tested-by: Alain Michaud <[email protected]>

> ---
> profiles/audio/a2dp.c | 5 +----
> profiles/audio/avdtp.c | 14 ++-----------
> src/btd.h | 7 +++++++
> src/main.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> src/main.conf | 13 ++++++++++++
> 5 files changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index 626f61d34..59d11a0aa 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -2324,10 +2324,7 @@ static bool a2dp_server_listen(struct a2dp_server *server)
> if (server->io)
> return true;
>
> - if (btd_opts.mps == MPS_OFF)
> - mode = BT_IO_MODE_BASIC;
> - else
> - mode = BT_IO_MODE_STREAMING;
> + mode = btd_opts.avdtp.session_mode;
>
> server->io = bt_io_listen(NULL, confirm_cb, server, NULL, &err,
> BT_IO_OPT_SOURCE_BDADDR,
> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index 619b94e29..ff0a124c3 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -2603,12 +2603,7 @@ static int send_req(struct avdtp *session, gboolean priority,
> int err, timeout;
>
> if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) {
> - BtIOMode mode;
> -
> - if (btd_opts.mps == MPS_OFF)
> - mode = BT_IO_MODE_BASIC;
> - else
> - mode = BT_IO_MODE_ERTM;
> + BtIOMode mode = btd_opts.avdtp.session_mode;
>
> session->io = l2cap_connect(session, mode);
> if (!session->io) {
> @@ -2807,12 +2802,7 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre
> struct seid_rej *resp, int size)
> {
> struct avdtp_local_sep *sep = stream->lsep;
> - BtIOMode mode;
> -
> - if (btd_opts.mps == MPS_OFF)
> - mode = BT_IO_MODE_BASIC;
> - else
> - mode = BT_IO_MODE_STREAMING;
> + BtIOMode mode = btd_opts.avdtp.stream_mode;
>
> stream->io = l2cap_connect(session, mode);
> if (!stream->io) {
> diff --git a/src/btd.h b/src/btd.h
> index c98414e35..a3247e4fd 100644
> --- a/src/btd.h
> +++ b/src/btd.h
> @@ -84,6 +84,11 @@ struct btd_defaults {
> struct btd_le_defaults le;
> };
>
> +struct btd_avdtp_opts {
> + uint8_t session_mode;
> + uint8_t stream_mode;
> +};
> +
> struct btd_opts {
> char *name;
> uint32_t class;
> @@ -112,6 +117,8 @@ struct btd_opts {
> uint8_t gatt_channels;
> enum mps_mode_t mps;
>
> + struct btd_avdtp_opts avdtp;
> +
> uint8_t key_size;
>
> enum jw_repairing_t jw_repairing;
> diff --git a/src/main.c b/src/main.c
> index e6c4d861e..33c0f0d15 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -34,6 +34,7 @@
> #include "lib/sdp.h"
>
> #include "gdbus/gdbus.h"
> +#include "btio/btio.h"
>
> #include "log.h"
> #include "backtrace.h"
> @@ -137,6 +138,12 @@ static const char *gatt_options[] = {
> NULL
> };
>
> +static const char *avdtp_options[] = {
> + "SessionMode",
> + "StreamMode",
> + NULL
> +};
> +
> static const struct group_table {
> const char *name;
> const char **options;
> @@ -146,6 +153,7 @@ static const struct group_table {
> { "LE", le_options },
> { "Policy", policy_options },
> { "GATT", gatt_options },
> + { "AVDTP", avdtp_options },
> { }
> };
>
> @@ -744,6 +752,40 @@ static void parse_config(GKeyFile *config)
> btd_opts.gatt_channels = val;
> }
>
> + str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err);
> + if (err) {
> + DBG("%s", err->message);
> + g_clear_error(&err);
> + } else {
> + DBG("SessionMode=%s", str);
> +
> + if (!strcmp(str, "basic"))
> + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> + else if (!strcmp(str, "ertm"))
> + btd_opts.avdtp.session_mode = BT_IO_MODE_ERTM;
> + else {
> + DBG("Invalid mode option: %s", str);
> + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> + }
> + }
> +
> + val = g_key_file_get_integer(config, "AVDTP", "StreamMode", &err);
> + if (err) {
> + DBG("%s", err->message);
> + g_clear_error(&err);
> + } else {
> + DBG("StreamMode=%s", str);
> +
> + if (!strcmp(str, "basic"))
> + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> + else if (!strcmp(str, "streaming"))
> + btd_opts.avdtp.stream_mode = BT_IO_MODE_STREAMING;
> + else {
> + DBG("Invalid mode option: %s", str);
> + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> + }
> + }
> +
> parse_br_config(config);
> parse_le_config(config);
> }
> @@ -780,6 +822,9 @@ static void init_defaults(void)
> btd_opts.gatt_cache = BT_GATT_CACHE_ALWAYS;
> btd_opts.gatt_mtu = BT_ATT_MAX_LE_MTU;
> btd_opts.gatt_channels = 3;
> +
> + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> }
>
> static void log_handler(const gchar *log_domain, GLogLevelFlags log_level,
> diff --git a/src/main.conf b/src/main.conf
> index 54f6a36bd..d3bc61441 100644
> --- a/src/main.conf
> +++ b/src/main.conf
> @@ -200,6 +200,19 @@
> # Default to 3
> #Channels = 3
>
> +[AVDTP]
> +# AVDTP L2CAP Signalling Channel Mode.
> +# Possible values:
> +# basic: Use L2CAP Basic Mode
> +# ertm: Use L2CAP Enhanced Retransmission Mode
> +#SessionMode = basic
> +
> +# AVDTP L2CAP Transport Channel Mode.
> +# Possible values:
> +# basic: Use L2CAP Basic Mode
> +# streaming: Use L2CAP Streaming Mode
> +#StreamMode = basic
> +
> [Policy]
> #
> # The ReconnectUUIDs defines the set of remote services that should try
> --
> 2.26.2
>

2020-11-17 17:32:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [RFC BlueZ 2/2] main.conf: Add option to configure AVDP session/stream channel modes

Hi Alain,

On Tue, Nov 17, 2020 at 6:43 AM Alain Michaud <[email protected]> wrote:
>
> On Mon, Nov 16, 2020 at 7:26 PM Luiz Augusto von Dentz
> <[email protected]> wrote:
> >
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > This adds a new group AVDTP where platform can confure the prefered
> > modes L2CAP channel for both session (signalling) and stream
> > (transport). For better backward compatibility the default modes of
> > boths
>
> Reviewed-by: Alain Michaud <[email protected]>
> Tested-by: Alain Michaud <[email protected]>
>
> > ---
> > profiles/audio/a2dp.c | 5 +----
> > profiles/audio/avdtp.c | 14 ++-----------
> > src/btd.h | 7 +++++++
> > src/main.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> > src/main.conf | 13 ++++++++++++
> > 5 files changed, 68 insertions(+), 16 deletions(-)
> >
> > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> > index 626f61d34..59d11a0aa 100644
> > --- a/profiles/audio/a2dp.c
> > +++ b/profiles/audio/a2dp.c
> > @@ -2324,10 +2324,7 @@ static bool a2dp_server_listen(struct a2dp_server *server)
> > if (server->io)
> > return true;
> >
> > - if (btd_opts.mps == MPS_OFF)
> > - mode = BT_IO_MODE_BASIC;
> > - else
> > - mode = BT_IO_MODE_STREAMING;
> > + mode = btd_opts.avdtp.session_mode;
> >
> > server->io = bt_io_listen(NULL, confirm_cb, server, NULL, &err,
> > BT_IO_OPT_SOURCE_BDADDR,
> > diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> > index 619b94e29..ff0a124c3 100644
> > --- a/profiles/audio/avdtp.c
> > +++ b/profiles/audio/avdtp.c
> > @@ -2603,12 +2603,7 @@ static int send_req(struct avdtp *session, gboolean priority,
> > int err, timeout;
> >
> > if (session->state == AVDTP_SESSION_STATE_DISCONNECTED) {
> > - BtIOMode mode;
> > -
> > - if (btd_opts.mps == MPS_OFF)
> > - mode = BT_IO_MODE_BASIC;
> > - else
> > - mode = BT_IO_MODE_ERTM;
> > + BtIOMode mode = btd_opts.avdtp.session_mode;
> >
> > session->io = l2cap_connect(session, mode);
> > if (!session->io) {
> > @@ -2807,12 +2802,7 @@ static gboolean avdtp_open_resp(struct avdtp *session, struct avdtp_stream *stre
> > struct seid_rej *resp, int size)
> > {
> > struct avdtp_local_sep *sep = stream->lsep;
> > - BtIOMode mode;
> > -
> > - if (btd_opts.mps == MPS_OFF)
> > - mode = BT_IO_MODE_BASIC;
> > - else
> > - mode = BT_IO_MODE_STREAMING;
> > + BtIOMode mode = btd_opts.avdtp.stream_mode;
> >
> > stream->io = l2cap_connect(session, mode);
> > if (!stream->io) {
> > diff --git a/src/btd.h b/src/btd.h
> > index c98414e35..a3247e4fd 100644
> > --- a/src/btd.h
> > +++ b/src/btd.h
> > @@ -84,6 +84,11 @@ struct btd_defaults {
> > struct btd_le_defaults le;
> > };
> >
> > +struct btd_avdtp_opts {
> > + uint8_t session_mode;
> > + uint8_t stream_mode;
> > +};
> > +
> > struct btd_opts {
> > char *name;
> > uint32_t class;
> > @@ -112,6 +117,8 @@ struct btd_opts {
> > uint8_t gatt_channels;
> > enum mps_mode_t mps;
> >
> > + struct btd_avdtp_opts avdtp;
> > +
> > uint8_t key_size;
> >
> > enum jw_repairing_t jw_repairing;
> > diff --git a/src/main.c b/src/main.c
> > index e6c4d861e..33c0f0d15 100644
> > --- a/src/main.c
> > +++ b/src/main.c
> > @@ -34,6 +34,7 @@
> > #include "lib/sdp.h"
> >
> > #include "gdbus/gdbus.h"
> > +#include "btio/btio.h"
> >
> > #include "log.h"
> > #include "backtrace.h"
> > @@ -137,6 +138,12 @@ static const char *gatt_options[] = {
> > NULL
> > };
> >
> > +static const char *avdtp_options[] = {
> > + "SessionMode",
> > + "StreamMode",
> > + NULL
> > +};
> > +
> > static const struct group_table {
> > const char *name;
> > const char **options;
> > @@ -146,6 +153,7 @@ static const struct group_table {
> > { "LE", le_options },
> > { "Policy", policy_options },
> > { "GATT", gatt_options },
> > + { "AVDTP", avdtp_options },
> > { }
> > };
> >
> > @@ -744,6 +752,40 @@ static void parse_config(GKeyFile *config)
> > btd_opts.gatt_channels = val;
> > }
> >
> > + str = g_key_file_get_string(config, "AVDTP", "SessionMode", &err);
> > + if (err) {
> > + DBG("%s", err->message);
> > + g_clear_error(&err);
> > + } else {
> > + DBG("SessionMode=%s", str);
> > +
> > + if (!strcmp(str, "basic"))
> > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> > + else if (!strcmp(str, "ertm"))
> > + btd_opts.avdtp.session_mode = BT_IO_MODE_ERTM;
> > + else {
> > + DBG("Invalid mode option: %s", str);
> > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> > + }
> > + }
> > +
> > + val = g_key_file_get_integer(config, "AVDTP", "StreamMode", &err);
> > + if (err) {
> > + DBG("%s", err->message);
> > + g_clear_error(&err);
> > + } else {
> > + DBG("StreamMode=%s", str);
> > +
> > + if (!strcmp(str, "basic"))
> > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> > + else if (!strcmp(str, "streaming"))
> > + btd_opts.avdtp.stream_mode = BT_IO_MODE_STREAMING;
> > + else {
> > + DBG("Invalid mode option: %s", str);
> > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> > + }
> > + }
> > +
> > parse_br_config(config);
> > parse_le_config(config);
> > }
> > @@ -780,6 +822,9 @@ static void init_defaults(void)
> > btd_opts.gatt_cache = BT_GATT_CACHE_ALWAYS;
> > btd_opts.gatt_mtu = BT_ATT_MAX_LE_MTU;
> > btd_opts.gatt_channels = 3;
> > +
> > + btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
> > + btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
> > }
> >
> > static void log_handler(const gchar *log_domain, GLogLevelFlags log_level,
> > diff --git a/src/main.conf b/src/main.conf
> > index 54f6a36bd..d3bc61441 100644
> > --- a/src/main.conf
> > +++ b/src/main.conf
> > @@ -200,6 +200,19 @@
> > # Default to 3
> > #Channels = 3
> >
> > +[AVDTP]
> > +# AVDTP L2CAP Signalling Channel Mode.
> > +# Possible values:
> > +# basic: Use L2CAP Basic Mode
> > +# ertm: Use L2CAP Enhanced Retransmission Mode
> > +#SessionMode = basic
> > +
> > +# AVDTP L2CAP Transport Channel Mode.
> > +# Possible values:
> > +# basic: Use L2CAP Basic Mode
> > +# streaming: Use L2CAP Streaming Mode
> > +#StreamMode = basic
> > +
> > [Policy]
> > #
> > # The ReconnectUUIDs defines the set of remote services that should try
> > --
> > 2.26.2
> >

Pushed.



--
Luiz Augusto von Dentz