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

Sat Oct 26 18:30:03 UTC 2019

26.10.2019, 20:23, "Hyperion" <h1p8r10n at yandex.com>:
> 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.

Also : there should be a "XQ automatic" mode (my patch), and it should be the default when XQ is 
enabled through the module arg : cause it will not fail with devices that can only do XQ at 2x32 
dual channel : cause it's negotiated. "fixed bitpool" modes (Pali's patch) : will fail with such a device.
Default modes (both legacy and XQ) should be negotiated (aka "automatic).

>
>>  Is your point that we should not add the module argument at all, if it
>>  will be superseded by a different mechanism? That is, an option to
>>  enable XQ in the automatic bitpool mode is fine, but the option should
>>  be configurable at run-time (via the message API) rather than a module
>>  argument?
>>
>>>   > > 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.
>>>
>>>   So yes, it is not pointless, but in current state not very useful for
>>>   higher bitpool values.
>>
>>  I'm not sure I understand. Why do we care about wasted space in
>>  bluetooth packets? Do you mean that there are only a few bluetooth
>>  packet sizes, and decreasing the bitpool doesn't help if the bluetooth
>>  packet size stays the same? Isn't the solution then to ensure that we
>>  always reduce the bitpool enough to get a smaller bluetooth packet
>>  size?
>>
>>  "There are only some higher bitpool values which make sense to use."
>>  What are those values specifically?
>>
>>  --
>>  Tanu
>>
>>  https://www.patreon.com/tanuk
>>  https://liberapay.com/tanuk
>
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss