On Thursday 18 July 2019 12:09:39 Pali Rohár wrote: > On Friday 12 July 2019 20:59:22 Marcel Holtmann wrote: > > Hi Pali, > > > > >>>>>>>>> to be honest, I would rather see WBS implementation finally > > >>>>>>>>> reach PA before we start digging into this. > > >>>>>>>> > > >>>>>>>> First I want to finish improving A2DP codec support in pulseaudio. Later > > >>>>>>>> I can look at HSP/HFP profiles. Ideally it should have modular/plugin > > >>>>>>>> extensible design. So the aim is that adding new codec would be very > > >>>>>>>> simple, without need to hack something related to mSBC/WBC, AuriStream > > >>>>>>>> or any other codec. > > >>>>>>> > > >>>>>>> Well HSP don't have support for codec negotiation, but yes a modular > > >>>>>>> design is probably recommended. > > >>>>>>> > > >>>>>>>> But for AuriStream I need to set custom SCO parameters as described > > >>>>>>>> below and currently kernel does not support it. This is why I'm asking > > >>>>>>>> how kernel can export for userspace configuration of SCO parameters... > > >>>>>>> > > >>>>>>> We can always come up with socket options but we got to see the value > > >>>>>>> it would bring since AuriStream don't look that popular among > > >>>>>>> headsets, at least Ive never seem any device advertising it like > > >>>>>>> apt-X, etc. > > >>>>>> > > >>>>>> Pali clearly has such device and he is willing to work on it. Surely > > >>>>>> that means it is popular enough to be supported...? > > >>>>> > > >>>>> Just put AT+CSRSF=0,0,0,0,0,7 to google search and you would see that > > >>>>> not only I have such device... > > >>>>> > > >>>>> So I would really would like to see that kernel finally stops blocking > > >>>>> usage of this AuriStream codec. > > >>>> > > >>>> we need to figure out on how we do the kernel API to allow you this specific setting. > > >>> > > >>> Hi Marcel! Kernel API for userspace should be simple. Just add two > > >>> ioctls for retrieving and setting structure with custom parameters: > > >>> > > >>> syncPktTypes = 0x003F > > >>> bandwidth = 4000 > > >>> max_latency = 16 > > >>> voice_settings = 0x63 > > >>> retx_effort = 2 > > >>> > > >>> Or add more ioctls, one ioctl per parameter. There is already only ioctl > > >>> for voice settings and moreover it is whitelisted only for two values. > > >> > > >> it is not that simple actually. Most profiles define a certain set of parameters and then they try to configure better settings and only fallback to a specification defined default as last resort. > > > > > > Ok. I see that there is another "example" configuration for AuriStream > > > with just different syncPktTypes = 0x02BF and bandwidth = 3850. > > > > > > So it really is not simple as it can be seen. > > > > currently the stepping for mSBC and CVSD are hard-coded in esco_param_cvsd and esco_param_msbc arrays in hci_conn.c and then selected by the ->setting parameter. > > > > So either we provide an new socket option (for example BT_VOICE_EXT) or we extend BT_VOICE to allow providing the needed information. However this needs to be flexible array size since we should then be able to encode multiple stepping that are tried in order. > > > > My preference is that we extend BT_VOICE and not introduce a new socket option. So feel free to propose how we can load the full tables into the SCO socket. I mean we are not really far off actually. The only difference is that currently the tables are in the hci_conn.c file and selected by the provided voice->setting. However nothing really stops us from providing the full table via user space. > > Ok. I will look at it and I will try to propose how to extend current > BT_VOICE ioctl API for supporting all those new parameters. Below is inline MIME part with POC patch which try to implement a new IOCTL (currently named BT_VOICE_SETUP) for configuring voice sco settings. It uses flexible array of parameters , but with maximally 10 array members (due to usage of static array storage). cvsd codec uses 7 different fallback settings (see voice_setup_cvsd), so for POC 10 should be enough. Because a new IOCL has different members then old BT_VOICE I rather decided to introduce a new IOCTL and not hacking old IOCTL to accept two different structures. Please let me know what do you think about this API, if this is a way how to continue or if something different is needed. -- Pali Rohár pali.rohar@gmail.com