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

Sat Oct 26 12:54:02 UTC 2019

On Saturday 26 October 2019 15:39:51 Tanu Kaskinen wrote:
> 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.
> Is your point that we should not add the module argument at all, if it
> will be superseded by a different mechanism?

Yes.

> 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?

Yes. For established connection, bluetooth packet size is fixed. SBC
packet size depends on SBC bitpool (and other) parameters. And because
pulseaudio does not support generating fragmented SBC packets, it can
only fill those SBC frames which fits into bluetooth packet size.

> Isn't the solution then to ensure that we
> always reduce the bitpool enough to get a smaller bluetooth packet
> size?

That can be solution. But easier is to pre-calculate bitpool parameters.

> "There are only some higher bitpool values which make sense to use."
> What are those values specifically?

For higher bitrates, these settings make sense:

For 2M EDR:
bitpool 38, dual channel
bitpool 76, stereo/joint stereo

For 3M EDR:
bitpool 47, dual channel
bitpool 93, joint stereo
bitpool 94, stereo

You can use online SBC calculator to calculate SBC parameters:
https://btcodecs.valdikss.org.ru/sbc-bitrate-calculator/

-- 
Pali Rohár
pali.rohar at gmail.com