Received-SPF: pass (google.com: best guess record for domain of linux-bluetooth-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 26 Nov 2019 08:46:10 +0100
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali.rohar@gmail.com>
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Pavel Machek <pavel@ucw.cz>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>,
        Johan Hedberg <johan.hedberg@gmail.com>
Subject: Re: HCI Set custom bandwidth for AuriStream SCO codec
Message-ID: <20191126074610.wy5pgjgskv6rxr7y@pali>
References: <20190718100939.bwl26qcfxe6ppcto@pali>
 <20191027220945.wmb3g55wtrmqbnmz@pali>
 <1CFFA8EF-1B2A-466E-8901-BFB849F20442@holtmann.org>
 <20191119171342.mwfzszu7xwabi7to@pali>
 <392B6AA9-7512-472A-8F22-D86F2A7F1EDC@holtmann.org>
 <20191120074411.xtyw34kb3gc73zvk@pali>
 <20191121224455.orhslaa6zdmlwe43@pali>
 <390FC814-FBC2-4225-82C2-0CB866F01471@holtmann.org>
 <20191124111313.gaqjyxmmxigmwmpr@pali>
 <8A14D8FE-A2B1-437E-B7A8-FAE5B39ADC5E@holtmann.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;
        protocol="application/pgp-signature"; boundary="7lb2z4g7eugkw5ds"
Content-Disposition: inline
In-Reply-To: <8A14D8FE-A2B1-437E-B7A8-FAE5B39ADC5E@holtmann.org>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk


--7lb2z4g7eugkw5ds
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tuesday 26 November 2019 08:24:04 Marcel Holtmann wrote:
> Hi Pali,
>=20
> >>> I played more with C99 flexible arrays and seems that gcc supports it
> >>> without any problems. I'm sending another attempt of API implementati=
on,
> >>> now with more fields which are needed for Enhanced Setup Synchronous
> >>> Connection command. This command is not supported by kernel yet, but
> >>> should be easy to add it. So my ioctl API is prepared for it. Enhanced
> >>> Setup Synchronous Connection command would be needed to use hardware
> >>> mSBC codec encoder/decoder.
> >>>=20
> >>> --=20
> >>> Pali Roh=C3=A1r
> >>> pali.rohar@gmail.com
> >>> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetoot=
h/bluetooth.h
> >>> index fabee6db0abb..29590c6749d5 100644
> >>> --- a/include/net/bluetooth/bluetooth.h
> >>> +++ b/include/net/bluetooth/bluetooth.h
> >>> @@ -116,12 +116,49 @@ struct bt_voice {
> >>> 	__u16 setting;
> >>> };
> >>>=20
> >>> -#define BT_VOICE_TRANSPARENT			0x0003
> >>> -#define BT_VOICE_CVSD_16BIT			0x0060
> >>> -
> >>> #define BT_SNDMTU		12
> >>> #define BT_RCVMTU		13
> >>>=20
> >>> +#define BT_VOICE_SETUP		14
> >>> +#define BT_VOICE_PKT_TYPE_CAP_SCO	BIT(0)
> >>> +#define BT_VOICE_PKT_TYPE_CAP_ESCO	BIT(1)
> >>> +struct bt_voice_pkt_type {
> >>> +	__u8 capability; /* bitmask of BT_VOICE_PKT_TYPE_CAP_* */
> >>> +	__u8 retrans_effort;
> >>> +	__u16 pkt_type;
> >>> +	__u16 max_latency;
> >>> +};
> >>> +#define BT_VOICE_SETUP_FEATURE_CONFIG		BIT(0) /* Additional configur=
ation fields after voice_settings are set (including other features) */
> >>> +#define BT_VOICE_SETUP_FEATURE_ADD_SCO		BIT(1) /* Can use Add Synchr=
onous Connection */
> >>> +#define BT_VOICE_SETUP_FEATURE_SETUP_SCO	BIT(2) /* Can use Setup Syn=
chronous Connection */
> >>> +#define BT_VOICE_SETUP_FEATURE_ENH_SETUP_SCO	BIT(3) /* Can use Enhan=
ced Setup Synchronous Connection */
> >>> +struct bt_voice_setup {
> >>> +	__u16 voice_setting;
> >>> +	__u8 features; /* bitmask of BT_VOICE_SETUP_FEATURE_* */
> >>> +	__u8 pkt_types_count;
> >>> +	__u32 tx_bandwidth;
> >>> +	__u32 rx_bandwidth;
> >>> +	__u32 input_bandwidth;
> >>> +	__u32 output_bandwidth;
> >>> +	__u8 tx_coding_format[5];
> >>> +	__u8 rx_coding_format[5];
> >>> +	__u8 input_coding_format[5];
> >>> +	__u8 output_coding_format[5];
> >>> +	__u16 tx_codec_frame_size;
> >>> +	__u16 rx_codec_frame_size;
> >>> +	__u16 input_coded_data_size;
> >>> +	__u16 output_coded_data_size;
> >>> +	__u8 input_pcm_data_format;
> >>> +	__u8 output_pcm_data_format;
> >>> +	__u8 input_pcm_msb_position;
> >>> +	__u8 output_pcm_msb_position;
> >>> +	__u8 input_data_path;
> >>> +	__u8 output_data_path;
> >>> +	__u8 input_unit_size;
> >>> +	__u8 output_unit_size;
> >>> +	struct bt_voice_pkt_type pkt_types[];
> >>> +};
> >>> +
> >>=20
> >> lets not mush these together. One of these are air codecs and setup de=
fined by a profile, the other are local codec path defined by the platform.
> >=20
> > You are right that air codecs are defined by profile and local codecs by
> > platform / bluetooth adapter. But ...
> >=20
> >> You will also not have multiple local codec path. That will be one and=
 they will not be negotiated. The eSCO settings however will be negotiated.
> >=20
> > Above structure specify exactly one codec setup and then multiple packet
> > types. See that flexible array is only for packet types, not for whole
> > codec structure.
> >=20
> > And all above parameters are required for Enhanced Setup Synchronous
> > Connection command. So kernel needs to know what should put into
> > Enhanced Setup Synchronous Connection command when creating a new SCO
> > connection. So for supporting Enhanced Setup Synchronous Connection
> > command userspace needs to pass all above parameters to kernel.
> >=20
> > And usage of Enhanced Setup Synchronous Connection is required when we
> > want to use in-hardware encoding/decoding of mSBC codec. E.g. new
> > Thinkpads already have bluetooth adapter which supports encoding and
> > decoding of mSBC codec in hardware.
> >=20
> > So above structure as I define is really needed. Do you see it now?
>=20
> the information are needed, but not that struct. It makes no sense to han=
d this problem of configuring the PCM data path correctly to the profile co=
de. So I am not going to do that. The profile itself has no interest in how=
 the platform is built. What you are going to do is configure possible PCM =
data path options and then the kernel will use that information for Enhance=
d Sync Setup. However the profile should only provide the over-the-air sett=
ings since that is what is going to be negotiated during the profile setup.

Userspace needs to know if kernel expects sound data (via write()) in
mSBC or in PCM_s16le. Also userspace needs to know if kernel provides
(via read()) sound data in mSBC, PCM_s16le or in other format. Otherwise
userspace is not possible to send any data to remote headset correctly.

--=20
Pali Roh=C3=A1r
pali.rohar@gmail.com

--7lb2z4g7eugkw5ds
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iF0EABECAB0WIQS4VrIQdKium2krgIWL8Mk9A+RDUgUCXdzYQAAKCRCL8Mk9A+RD
UnRxAJ4jv+c5ivd+AOx5bpxotEEM/OTleACeI9/NZkOW4LgwvbHdCTpeAPNeqOw=
=US3p
-----END PGP SIGNATURE-----

--7lb2z4g7eugkw5ds--