19.10.2019, 18:42, "Pali Rohár" <pali.rohar@xxxxxxxxx>: > 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@xxxxxxxxx _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
