[pulseaudio-discuss] [PATCH v13 10/10] bluetooth: policy: Treat bi-directional A2DP profiles as suitable for VOIP

[Tanu Kaskinen]

On Sat, 2019-10-26 at 20:23 +0200, Hyperion wrote:
> 
> 26.10.2019, 14:39, "Tanu Kaskinen" <tanuk at iki.fi>:
> > On Sat, 2019-10-19 at 18:42 +0200, Pali Rohár wrote:
> > >  On Saturday 19 October 2019 19:27:19 Tanu Kaskinen wrote:
> > >  > On Sat, 2019-10-19 at 18:16 +0200, Pali Rohár wrote:
> > >  > > On Saturday 19 October 2019 19:07:44 Tanu Kaskinen wrote:
> > >  > > > On Sat, 2019-10-19 at 17:20 +0200, Pali Rohár wrote:
> > >  > > > > On Friday 18 October 2019 15:29:43 Tanu Kaskinen wrote:
> > >  > > > > > On Thu, 2019-10-17 at 15:34 +0200, Hyperion wrote:
> > >  > > > > > > Regression would mean that some devices can't connect anymore : this
> > >  > > > > > > won't happen if a workaround is provided, and this workaround won't
> > >  > > > > > > be used often.
> > >  > > > > > >
> > >  > > > > > > Most (99% ?) of the devices will work correctly with my patch (many
> > >  > > > > > > of them in XQ mode, and some in legacy mode because they will fall
> > >  > > > > > > back to legacy bitpool during negociation)
> > >  > > > > > >
> > >  > > > > > > The remaining (1% ?) : will need a simple boolean swicth in one of
> > >  > > > > > > the PA config files to restrict negociation to legacy bitpool (a
> > >  > > > > > > module option ? or daemon.conf ?).
> > >  > > > > > >
> > >  > > > > > > I think it's really "simple", efficient, and not dependent of any
> > >  > > > > > > upcoming Bluez feature.
> > >  > > > > > >
> > >  > > > > > > "The complex solution is always the best until one find a simpler one"
> > >  > > > > >
> > >  > > > > > I don't know the number of users who use bluetooth headsets with
> > >  > > > > > PulseAudio, but even just 1% regression rate can mean quite a few
> > >  > > > > > unhappy users. When your headset suddenly stops working, it's not
> > >  > > > > > trivial to figure out that you may need to pass a special argument to
> > >  > > > > > module-bluetooth-discover in order to make it work again.
> > >  > > > > >
> > >  > > > > > It would be better to have a module argument to enable the XQ settings.
> > >  > > > >
> > >  > > > > Main question, do we really need this special "settings"? Because my
> > >  > > > > patch series introduce also SBC XQ profile and basically replaces above
> > >  > > > > module parameter, by runtime configuration.
> > >  > > > >
> > >  > > > > For me above solution looks like a hack. It adds some module parameter
> > >  > > > > for tweaking configuration. And what would happen with that parameter
> > >  > > > > after we have "proper" support for multiple codecs? Do we need to
> > >  > > > > maintain backward compatibility? Or would we remove that configuration
> > >  > > > > and therefore revert to state prior existence of new module parameter
> > >  > > > > (which is current situation)?
> > >  > > >
> > >  > > > After your patches there's still the "automatic bitpool" mode
> > >  > > > available, right?
> > >  > >
> > >  > > Yes, I wanted to have it there for legacy/backward compatibility reasons
> > >  > > for those devices which could be broken with new settings. That is the
> > >  > > reason I do not wanted to touch Automatic mode, to have exact same
> > >  > > behavior as in current (and older) pulseaudio versions.
> > >  > >
> > >  > > But if automatic mode is going to be changed, I do not see reason for
> > >  > > keeping it (the argument for backward compatibility would not apply
> > >  > > anymore, if it is going to be changed). My patch series with new A2DP
> > >  > > API can fully replace that automatic mode.
> > >  >
> > >  > I don't see how the proposed option changes anything about
> > >  > compatibility. The option will be disabled by default, so the default
> > >  > behaviour will be the same as always.
> > > 
> > >  And what should happen after support for multiple A2DP codecs (from my
> > >  patch series) would be there? Basically it obsoletes that config option.
> > >  As all such settings can be set at runtime.
> > 
> > If the "enable XQ in the automatic bitpool mode" option is made run-
> > time configurable, then yes, the module argument will become obsolete.
> 
> I don't think so : by default Pali's patch first connects using one of the 2 XQ modes : so 
> it will fail with devices not "XQ able". Since we are having this discussion only because
> we don't want ANY regression (even as rare as the devices that can't do bitpool 38 dual
> , and even for users who don't know how to switch SBC modes) : then the default mode 
> of operations shouldn't allow XQ at all. 

I think we're talking about different things here.

A mode for automatic bitpool is needed and will have to be the default.
XQ bitpool values need to be disabled by default. An option for
enabling XQ bitpool values in the automatic mode would be useful, so I
hope we will have that.

When I said that "the module argument will become obsolete" didn't mean
that the XQ option itself would become obsolete. If the option can be
configured at run-time, for example with "pactl enable-sbc-xq true" or
with pavucontrol, then that will not be implemented via a module
argument. It will be implemented using the message API (which is still
under review), and the option value will be stored in a database.
Having a module argument doesn't add any value in this scheme, which is
why I said the module argument will become obsolete.

This depends on someone actually implementing the interface for setting
the XQ option, however. I hope someone will promise to do that. It
could be you, or it could be Pali. I'm not promising to do it myself.
If nobody promises to implement that, then I'm going to accept a module
argument, because having something is better than having nothing.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk