Hi Luiz, On Tue, May 5, 2020 at 7:48 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Marcel and Alain, > > On Tue, May 5, 2020 at 8:47 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > > > Friendly ping on this series. > > > > > > On Tue, May 5, 2020 at 11:46 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > > > > > Friendly ping on this series. > > > > > > On Fri, Apr 17, 2020 at 5:03 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > >> > > >> On Fri, Apr 17, 2020 at 4:58 PM Luiz Augusto von Dentz > > >> <luiz.dentz@xxxxxxxxx> wrote: > > >> > > > >> > Hi Alain, > > >> > > > >> > On Fri, Apr 17, 2020 at 11:22 AM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > >> > > > > >> > > On Fri, Apr 17, 2020 at 2:16 PM Luiz Augusto von Dentz > > >> > > <luiz.dentz@xxxxxxxxx> wrote: > > >> > > > > > >> > > > Hi Alain, > > >> > > > > > >> > > > On Thu, Apr 16, 2020 at 5:34 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > >> > > > > > > >> > > > > HI Luiz, > > >> > > > > > > >> > > > > On Thu, Apr 16, 2020 at 8:29 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > >> > > > >> > > >> > > > >> Hi Alain, > > >> > > > >> > > >> > > > >> On Thu, Apr 16, 2020 at 1:32 PM Alain Michaud <alainmichaud@xxxxxxxxxx> wrote: > > >> > > > >> > > > >> > > > >> > Friendly ping on this series. > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > On Fri, Apr 3, 2020 at 11:33 AM Alain Michaud <alainm@xxxxxxxxxxxx> wrote: > > >> > > > >> > > > > >> > > > >> > > This change adds an adapter Api to report the controller's > > >> > > > >> > > widebandspeech enabled state. > > >> > > > >> > > >> > > > >> I wonder if this shouldn't be queried over SCO socket, or simple fail > > >> > > > >> with BT_VOICE when using BT_VOICE_TRANSPARENT which is how what is > > >> > > > >> normally used when using mSBC. > > >> > > > > > > >> > > > > I think there is value in both. > > >> > > > > > >> > > > Can you expand on that? I think this might generate confusion if the > > >> > > > property indicates support for it but HFP implementation don't support > > >> > > > it, since the later is usually implemented as a external profile so we > > >> > > > don't have the features it may support, or perhaps the intention here > > >> > > > is to actually indicate when it is in use? > > >> > > > > >> > > This is a signal that the adapter supports it and has everything > > >> > > enabled to support it. Driver indicated it supports it and erroneous > > >> > > data reporting was enabled. The profile has it's own state which may > > >> > > indicate if msbc will be used, but this will be on a per connection > > >> > > basis and is independent from this adapter property. > > >> > > > > >> > > The value in this property is to support diagnostic UX about > > >> > > controller capabilities/state and also allow profiles that are > > >> > > implemented outside of bluetoothd to see which codec it can attempt to > > >> > > negotiate with the device. > > >> > > > >> > For diagnosic I think we would be better off with some dedicated > > >> > interface to query this, as for the later the information we are > > >> > giving does not actually tell anything about the codec support, which > > >> > was part of my original argument that for the likes of HFP and other > > >> > profiles using it it might not be enough and they still need to use > > >> > BT_VOICE in order to enable the use of custom codecs, if you take > > >> > ofono for example it does implement support for wideband speech > > >> > already and it would completely disregard this property which can give > > >> > the false impression that wideband speech cannot be activated when in > > >> > fact it can, it just don't have erroneous data reporting enable, so > > >> > perhaps we should indicate the actual adapter feature (e.g. > > >> > ErrnoneousDataReporting) not the profile feature here, so the profile > > >> > implementation can check weather this would disable use of wideband > > >> > speech or not, futhermore we should probably report the errors back to > > >> > the SCO socket or is that just for diagnostic and cannot be used to > > >> > adjust the streaming? > > >> > > >> My original patch actually had this MGMT feature called erroneous data > > >> reporting and Marcel argued against it. If you both agree, then I'm > > >> happy to rename all of this to erroneous data reporting. We'd still > > >> need some way for the driver support to be messaged some other way > > >> though. see: > > >> > > >> if (id->driver_info & BTUSB_WIDEBAND_SPEECH) > > >> set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks); > > So this will get a little bit confusing if we don't have a HFP > implementation that actually does implement the profile parts to > enable wideband speech so I wonder if we should rename it to the > underline feature it enables, that said this sort of feature is > probably easier to be queried over the socket itself rather than over > D-Bus so it can be used in conjunction with the likes of BT_VOICE, > also regarding the erroneous data should that be also enabled by the > HFP profile, because depending on the platform it may not support > wideband speech so enabling erroneous data automatically may result in > artifacs, specially in cases where the codecs don't have error > correction or data loss concealment to handle data with possible error > and lost data respectively, btw we don't seem to be parsing the > SCO/ESCO flags and even if we do this properly we need to find a way > to notify them over the socket so things like plc works properly. This indeed makes sense, but we'll need to decide how erroneous data reporting gets enabled. The SCO socket is a per connection scope thing while erroneous data reporting is a controller level thing. > > > >> > > >> > > >> > > > > > >> > > > > > >> > > > >> > > >> > > > >> > > >> > > > >> > > --- > > >> > > > >> > > > > >> > > > >> > > doc/adapter-api.txt | 8 ++++++++ > > >> > > > >> > > 1 file changed, 8 insertions(+) > > >> > > > >> > > > > >> > > > >> > > diff --git a/doc/adapter-api.txt b/doc/adapter-api.txt > > >> > > > >> > > index acae032d9..d8865e795 100644 > > >> > > > >> > > --- a/doc/adapter-api.txt > > >> > > > >> > > +++ b/doc/adapter-api.txt > > >> > > > >> > > @@ -326,3 +326,11 @@ Properties string Address [readonly] > > >> > > > >> > > > > >> > > > >> > > Local Device ID information in modalias format > > >> > > > >> > > used by the kernel and udev. > > >> > > > >> > > + > > >> > > > >> > > + boolean WidebandSpeechEnabled [readonly] > > >> > > > >> > > + > > >> > > > >> > > + Returns true if the adapter's wideband speech feature is > > >> > > > >> > > + supported and enabled. > > >> > > > >> > > >> > > > >> There seems to be some extra empty lines here. > > >> > > > > > > >> > > > > ACK, will fix. > > >> > > > >> > > >> > > > >> > > >> > > > >> > > + > > >> > > > >> > > + > > >> > > > >> > > + > > >> > > > >> > > -- > > >> > > > >> > > 2.26.0.292.g33ef6b2f38-goog > > >> > > > >> > > > > >> > > > >> > > >> > > > >> > > >> > > > >> > > >> > > > >> -- > > >> > > > >> Luiz Augusto von Dentz > > >> > > > > > >> > > > > > >> > > > > > >> > > > -- > > >> > > > Luiz Augusto von Dentz > > >> > > > >> > > > >> > > > >> > -- > > >> > Luiz Augusto von Dentz > > > > -- > Luiz Augusto von Dentz
