29.10.2019, 10:24, "Pali Rohár" <pali.rohar@xxxxxxxxx>: > On Monday 28 October 2019 17:12:45 Hyperion wrote: >> 28.10.2019, 16:11, "Tanu Kaskinen" <tanuk@xxxxxx>: >> > On Sat, 2019-10-26 at 20:23 +0200, Hyperion wrote: >> >> 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. >> > >> > I think we're talking about different things here. >> > >> > A mode for automatic bitpool is needed and will have to be the default. >> > XQ bitpool values need to be disabled by default. An option for >> > enabling XQ bitpool values in the automatic mode would be useful, so I >> > hope we will have that. >> > >> > When I said that "the module argument will become obsolete" didn't mean >> > that the XQ option itself would become obsolete. If the option can be >> > configured at run-time, for example with "pactl enable-sbc-xq true" or >> > with pavucontrol, then that will not be implemented via a module >> > argument. It will be implemented using the message API (which is still >> > under review), and the option value will be stored in a database. >> > Having a module argument doesn't add any value in this scheme, which is >> > why I said the module argument will become obsolete. >> > >> > This depends on someone actually implementing the interface for setting >> > the XQ option, however. I hope someone will promise to do that. It >> > could be you, or it could be Pali. I'm not promising to do it myself. >> > If nobody promises to implement that, then I'm going to accept a module >> > argument, because having something is better than having nothing. > > I have already implemented it! It is in my patch series. Currently > selection is done by choosing PA profile (because there is no other > mechanism yet). > >> It's not obvious to get details of the A2DP protocol without reading the RFC, >> so I'm going to give a few tracks : >> >> - Bluetooth packet size is not constant : BT traffic is splited in radio slots >> that have fixed time lengh : so for a given bandwidth : slots size vary > > Plus there is retransmission at low bluetooth layer when they are lost. > > Also radio slots are shared across all bluetooth traffic between host > and device. So not only A2DP packets are transmitted, but also other > packets, like established HSP / HFP or anything else. > >> - Bitpool is dynamically reduced when the bandwidth decrease : for example >> when the emitter go away from the receiver. >> - Bitrate is related to bitpool but depends on other parameters (see the attached sheet) >> - Bluetooth A2DP emiters (SRC) usually specify a maximum value for Bitpool. > > It specify both minimum and maximum (range) > >> This value is not the value that will be negociated but rather a >> highest negociation value. > > Result of negotiation is bitpool range, which must be continuous subset > of intersect of supported range of host and device. > > And then emitter choose any bitpool value from negotiated range, > probably always the highest value from that range. > >> - SBC XQ is standard SBC codec operating at high bitrates and thus reaching >> the transparent audio transport quality of AptX (HD) . It can be achieved either >> in STEREO MODE, with bitpool ~ 76, or in DUAL CHANNEL MODE, with bitpool ~ 38 >> per channel (simplified) >> - "Legacy automatic" is the barbarous name I gave to the traditional negociated algorithm >> with max bitpool=53 and STEREO MODE prefered to DUAL CHANNEL MODE : it is implemented >> in current PA >> - "XQ automatic" is the barbarous name I gave to the negociated algorithm >> implemented in my patch with max bitpool=38 per channel, and DUAL CHANNEL >> prefered to STEREO MODE (simplified) >> - "Forced XQ" modes are several algorithms implemented by Pali with fixed >> bitpool values : not negociated : if the receiver can do the fixed bitpool it works, else >> it fails (simplified) > > It is negotiated too. If remote side does not announce that it (may) > support "forced XQ" then it is not available for PA. > >> Now the "quaity switch" idea : >> The "quaity switch" (aka module param or pactl API call, all good for me) is a way >> to switch between "Legacy automatic" and something else giving better quality. >> The "something else" could be : >> - "XQ automatic" + several "Forced XQ" modes (my latest idea, relies on Pali's code + my code + "quaity switch") >> - "XQ automatic" alone (my former idea, relies on my "simple" patch + "quaity switch") >> - several "Forced XQ" modes alone (Pali's idea, relies on Pali's code +"quaity switch") > > My proposed API in PA server allows to define any settings or > combination for SBC codec. So it would be easy to define any such > "SBC profile" with my patches. So it's all fine for me. Plus the fact that you implemented the bitpool automatic recovery grow up when bandwidth grow : it's perfect. I would just suggest to add an automatic XQ profile (inspired from my patch if you want to) and make it default when XQ is available on SNK side. This way we don't have to wait for Bluez to provide codec switching and it allows for a 14.0 PA release at the date already scheduled by Tanu. JP > >> Pali : please fix the above in case I did some mistakes >> JP >> >> > >> > -- >> > Tanu >> > >> > https://www.patreon.com/tanuk >> > https://liberapay.com/tanuk > > -- > Pali Rohár > pali.rohar@xxxxxxxxx _______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
