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

[Hyperion]

19.10.2019, 18:42, "Pali Rohár" <pali.rohar at gmail.com>:
> 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.
>
>>  > Automatic mode is also main objection against usage of SBC codec (it
>>  > does not specify, say or enforce specific bitrate or quality; it can be
>>  > anything) and also reason why there are vendor codecs like aptX.
>>  > Defining SBC LQ, MQ, HQ or XQ just allows to compare it with other
>>  > codecs and guarantee same settings and quality across all devices.
>>
>>  Doesn't the automatic mode have the benefit that it automatically
>>  adapts to bad radio conditions so that users get the best quality
>>  possible without needing to fiddle with any options in case the initial
>>  bitrate is too high? So it's not entirely pointless.
>
> Yes, but it make sense only for lower bitpool values. Higher bitpool
> increase size of SBC frames and with larger SBC frames there would be
> lot of wasted space in bluetooth packets as pulseaudio pulseaudio does
> not support SBC fragmentation. There are only some higher bitpool values
> which make sense to use.
>
> Plus pulseaudio's implementation of (current) automatic mode only
> decrease bitpool. It never increase it.

Yes, it's a good improvement of your patch !

>
> So yes, it is not pointless, but in current state not very useful for
> higher bitpool values.
>
> --
> Pali Rohár
> pali.rohar at gmail.com