CSA2: User space aspect

CSA2: User space aspect

[Michael Knudsen]

I'm currently looking into CSA2 and how it should be exposed to user
space applications, so I want to make sure I am not duplicating any
existing effort in this area.

I've been doing some prestudying and my current thought is to expose
this as socket options for the following parameters:

	Transmit_Coding_Format
	Receive_Coding_Format

	Input_Coding_Format
	Output_Coding_Format

	Input_Data_Path
	Output_Data_Path

If anyone else has an ideas or opinions about this, please speak up,
otherwise I'll try coming up with an interface specification with
more details.

-m.

Re: CSA2: User space aspect

[Michael Knudsen]

On 2012-11-08 14:36, Michael Knudsen wrote:
> If anyone else has an ideas or opinions about this, please speak up,
> otherwise I'll try coming up with an interface specification with
> more details.

This diff shows the direction I'm heading:

diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..a565a4d 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -51,6 +51,42 @@ struct sco_conninfo {
 	__u8  dev_class[3];
 };
 
+/* Audio format setting */
+#define SCO_HOST_FORMAT	0x04
+#define SCO_AIR_FORMAT	0x05
+
+#define SCO_FORMAT_ULAW		0x00
+#define SCO_FORMAT_ALAW		0x01
+#define SCO_FORMAT_CVSD		0x02
+#define SCO_FORMAT_TRANSPARENT	0x03 
+#define SCO_FORMAT_PCM		0x05 
+#define SCO_FORMAT_MSBC		0x05 
+#define SCO_FORMAT_VENDOR	0xff
+struct sco_format_vendor {
+	__u16 vendor;
+	__u16 codec;
+};
+
+struct sco_format {
+	__u8                     in_format;
+	struct sco_format_vendor in_vendor;
+
+	__u8                     out_format;
+	struct sco_format_vendor out_vendor;
+};
+
+#define SCO_CODECS		0x06
+struct sco_codecs {
+	__u8  count;
+	__u8 *codec;
+};
+
+#define SCO_CODECS_VENDOR	0x07
+struct sco_codecs_vendor {
+	__u8                      count;
+	struct sco_format_vendor *format;
+};
+
 /* ---- SCO connections ---- */
 struct sco_conn {
 	struct hci_conn	*hcon;
@@ -74,6 +110,8 @@ struct sco_pinfo {
 	struct bt_sock	bt;
 	__u32		flags;
 	struct sco_conn	*conn;
+	struct sco_format host_format;
+	struct sco_format air_format;
 };
 
 #endif /* __SCO_H */

Basically, this adds four socket options (I'll do the audio path
stuff as well once this is done):

	SCO_AIR_FORMAT
	SCO_HOST_FORMAT
	SCO_CODECS (ro)
	SCO_CODECS_VENDOR (ro)

The SCO_CODECS ones provide the application with a list of codecs
supported by the hdev as indicated in the HCI_Read_Local_Supported_Codecs
command response, and if the hdev does not support this command a
default of linear PCM, CVSD, and transparent will be provided.

Because the result length is variable, the idea is that the application-
provided structure is modified by the kernel to hold the actual number
of results so the application can allocate a buffer accordingly, e.g.:

	struct sco_codecs sc;
	
	memset(&sc, 0, sizeof(sc));
	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);

	sk.codecs = malloc(sk.count);
	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);

The SCO_*_FORMAT ones allows the application to set the parameters that
are to be used on host-controller and controller-controller paths.  While
the spec requires the pairs (host input/output, air input/output) to be
identical, I don't see a reason to enforce this in the API, thus all are
set independently.

So, before I spend any more time on this.. comments?

-m.

Re: CSA2: User space aspect

[Marcel Holtmann]

Hi Michael,

> > If anyone else has an ideas or opinions about this, please speak up,
> > otherwise I'll try coming up with an interface specification with
> > more details.
> 
> This diff shows the direction I'm heading:
> 
> diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
> index 1e35c43..a565a4d 100644
> --- a/include/net/bluetooth/sco.h
> +++ b/include/net/bluetooth/sco.h
> @@ -51,6 +51,42 @@ struct sco_conninfo {
>  	__u8  dev_class[3];
>  };
>  
> +/* Audio format setting */
> +#define SCO_HOST_FORMAT	0x04
> +#define SCO_AIR_FORMAT	0x05
> +
> +#define SCO_FORMAT_ULAW		0x00
> +#define SCO_FORMAT_ALAW		0x01
> +#define SCO_FORMAT_CVSD		0x02
> +#define SCO_FORMAT_TRANSPARENT	0x03 
> +#define SCO_FORMAT_PCM		0x05 
> +#define SCO_FORMAT_MSBC		0x05 
> +#define SCO_FORMAT_VENDOR	0xff
> +struct sco_format_vendor {
> +	__u16 vendor;
> +	__u16 codec;
> +};
> +
> +struct sco_format {
> +	__u8                     in_format;
> +	struct sco_format_vendor in_vendor;
> +
> +	__u8                     out_format;
> +	struct sco_format_vendor out_vendor;
> +};
> +
> +#define SCO_CODECS		0x06
> +struct sco_codecs {
> +	__u8  count;
> +	__u8 *codec;
> +};
> +
> +#define SCO_CODECS_VENDOR	0x07
> +struct sco_codecs_vendor {
> +	__u8                      count;
> +	struct sco_format_vendor *format;
> +};
> +
>  /* ---- SCO connections ---- */
>  struct sco_conn {
>  	struct hci_conn	*hcon;
> @@ -74,6 +110,8 @@ struct sco_pinfo {
>  	struct bt_sock	bt;
>  	__u32		flags;
>  	struct sco_conn	*conn;
> +	struct sco_format host_format;
> +	struct sco_format air_format;
>  };
>  
>  #endif /* __SCO_H */
> 
> Basically, this adds four socket options (I'll do the audio path
> stuff as well once this is done):
> 
> 	SCO_AIR_FORMAT
> 	SCO_HOST_FORMAT
> 	SCO_CODECS (ro)
> 	SCO_CODECS_VENDOR (ro)
> 
> The SCO_CODECS ones provide the application with a list of codecs
> supported by the hdev as indicated in the HCI_Read_Local_Supported_Codecs
> command response, and if the hdev does not support this command a
> default of linear PCM, CVSD, and transparent will be provided.

please to not attempt to use socket options as ioctl. They are called
options for a reason.

Getting the list of supported codecs should be done via mgmt interface
command and it should be only done once.

> Because the result length is variable, the idea is that the application-
> provided structure is modified by the kernel to hold the actual number
> of results so the application can allocate a buffer accordingly, e.g.:
> 
> 	struct sco_codecs sc;
> 	
> 	memset(&sc, 0, sizeof(sc));
> 	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);
> 
> 	sk.codecs = malloc(sk.count);
> 	getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);

We are not doing that. I have no intention to map kernel memory to
userspace memory and back with socket options.

> The SCO_*_FORMAT ones allows the application to set the parameters that
> are to be used on host-controller and controller-controller paths.  While
> the spec requires the pairs (host input/output, air input/output) to be
> identical, I don't see a reason to enforce this in the API, thus all are
> set independently.
> 
> So, before I spend any more time on this.. comments?

Please ask yourself the question when and how the SCO data from the
socket is actually affected. I still have not seen you provide the
semantics of how the socket would be used afterwards. Especially on
impact for the application establishing the SCO socket.

In a more important question, is this static one time fits all selection
or is this actually to be dynamic on every new connection establishment.

Regards

Marcel

RE: CSA2: User space aspect

[Michael Knudsen]

> please to not attempt to use socket options as ioctl. They are
> called options for a reason.
> 
> Getting the list of supported codecs should be done via mgmt
> interface command and it should be only done once.

Okay, I'll take a stab at this, then.

I expect that socket options are still fine for actually
setting the configuration? 

> Please ask yourself the question when and how the SCO data from the
> socket is actually affected. I still have not seen you provide the
> semantics of how the socket would be used afterwards. Especially on
> impact for the application establishing the SCO socket.

There are two cases, connect() and accept().

For connect(), this is pretty simple:

	sk = socket(..);
	setsockopt(sk, SOL_SCO, SCO_AIR_FORMAT, ...);
	connect(sk, ..);

For accept(), it is the intention that this will be used in
conjunction with BT_DEFER_SETUP such that we can set the codecs
as per how we just negotiated with the peer:

	sk = socket(..);
	bind(sk, ..);
	setsockopt(sk, SOL_SCO, BT_DEFER_SETUP, 1);
	listen(sk, ..);
	peer = accept(sk, ..);
	setsockopt(peer, SOL_SCO, SCO_AIR_FORMAT, SCO_FORMAT_MSBC);
	recvmsg(peer, ..);

> In a more important question, is this static one time fits all
> selection or is this actually to be dynamic on every new connection
> establishment.

Both (defer/non-defer).  The socket options will set the parameters
that are used by the stack whenever it sets up or accepts a connection.

Did this answer your questions?

-m.