26.10.2019, 20:23, "Hyperion" <h1p8r10n@xxxxxxxxxx>: > 26.10.2019, 14:39, "Tanu Kaskinen" <tanuk@xxxxxx>: >> 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
