Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On 20.04.19 01:02, Alexander E. Patrakov wrote: сб, 20 апр. 2019 г. в 00:15, Georg Chini : On 19.04.19 18:23, Alexander E. Patrakov wrote: пт, 19 апр. 2019 г. в 14:13, Tanu Kaskinen : If the plugin gets the number of bands during the initialization, it can create the appropriate number of non-array control ports. Interleaved audio ports aren't needed either, because PulseAudio can do the deinterleaving before passing the audio to the plugin (like module-ladspa-sink already does). If one's going to write an LV2 plugin, it's best to use standard port types so that all hosts will be able to use the plugin. In addition, I think that "if the plugin gets the number of bands [at runtime]" is a very big "if" for an IIR-based equalizer. So big that I would rather hard-code it, or treat equalizers with a different number of bands as completely different plugins. The reason is that the required slope of inter-band transition (i.e., effectively, the filter order) is a function of the width of each band. Implement a filter with a too-low order, and it won't be able to isolate (and thus control thegain of) a frequency band selectively enough. Implement a filter with a too-high order, and its frequency response will be too steppy. My understanding is that Andrea's equalizer algorithm supports an arbitrary number of bands (certainly within some sane limits). Please correct me if I am wrong. It supports arbitrary amount of bands, but not arbitrary filter order for a band. Just to reiterate, the whole filter is a cascade of some per-band filters of the same order. Each filter in the cascade is implemented, according to the dissertation, as a cut-and-boost filter, which is based on a band-pass filter, which is in turn based on a shelving Butterworth filter of order 2M. Hopefully I haven't missed any order-doubling here. I have not thoroughly checked whether the implementation actually follows the dissertation - this is complicated by the Italian language that I don't know, and cryptic variable names. The majority of the dissertation does contain formulas for an arbitrary even order (2M), but on page 41, it starts talking about filters of order 8 only. And, if I understand correctly, it doesn't match the code, which is hard-coded to implement only filters of order 4, and it is not only a matter of increasing M and M2 in the file, because then xn[i] must be set in eq_filter() for i >= 4. Another sign of this mismatch between the dissertation and the code is the different length of the "c" array - 10 in the code and 21 in algorithm 6.2 on page 43 (page numbers here are presented as printed at the bottom, not as displayed in PDF viewers' left pane). Because of the fixed order, there are indeed limits on the useful number of bands. E.g., given that the minimum and maximum supported gains in the band are -12 and 12 dB (at least this is what's mentioned in the dissertation), it means that the gain must increase by 24 dB between the centers of the neighboring bands. With the default 10-band filter, i.e. with one octave per band, this means that 24 dB per octave is the maximum supported slope, which is roughly right for a 4th order filter. In other words, more than 10 bands cannot be useful with 4th order filters and 24 dB of gain difference between the neighboring bands. I understand that it is odd to set the gain in two neighboring octaves to differ by that much. For a 5-band equalizer with the same gain limits, filters of 2nd order would be sufficient and preferable. Also, the order of the filter could be lowered if the gain limits are set lower. I can't really comment on this, my knowledge is not sufficient. But what you are basically saying is that the equalizer does not hold what Andrea claims it can do and that it is only useful as a 10-band equalizer. I wonder why her work was then accepted as a master thesis. Besides, consumer electronic devices (TVs, HDMI receivers, Hi-Fi amplifiers) just do not have equalizers with a variable number of bands, for usability reasons. I don't see a reason for PulseAudio to be different. You can't add sliders on the fly on a consumer electronic device but you can do so in software. Why should we be guided by a behavior that is governed by physical limits? If the algorithm supports it, I see no reason why it should not be implemented. (As said above naturally within some sane limits, you are right that it would make no sense to have for example 50 bands.) Sliders in a consumer electronics device such as a TV are just virtual widgets on the screen, i.e. software, not physical knobs. Again, their number is fixed because of usable and "intuitive" UI. I guess there are enough people who would wish for more flexibility. Surely you could go for several different plugins, but I do not see the reason why the code should be duplicated a couple of times. The goal is to have one plugin and not a collection. I just don't like the idea that you have to duplicate things only because
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
сб, 20 апр. 2019 г. в 00:15, Georg Chini : > > On 19.04.19 18:23, Alexander E. Patrakov wrote: > > пт, 19 апр. 2019 г. в 14:13, Tanu Kaskinen : > >> If the plugin gets the number of bands during the > >> initialization, it can create the appropriate number of non-array > >> control ports. Interleaved audio ports aren't needed either, because > >> PulseAudio can do the deinterleaving before passing the audio to the > >> plugin (like module-ladspa-sink already does). If one's going to write > >> an LV2 plugin, it's best to use standard port types so that all hosts > >> will be able to use the plugin. > > In addition, I think that "if the plugin gets the number of bands [at > > runtime]" is a very big "if" for an IIR-based equalizer. So big that I > > would rather hard-code it, or treat equalizers with a different number > > of bands as completely different plugins. The reason is that the > > required slope of inter-band transition (i.e., effectively, the filter > > order) is a function of the width of each band. Implement a filter > > with a too-low order, and it won't be able to isolate (and thus > > control thegain of) a frequency band selectively enough. Implement a > > filter with a too-high order, and its frequency response will be too > > steppy. > > My understanding is that Andrea's equalizer algorithm supports > an arbitrary number of bands (certainly within some sane limits). > Please correct me if I am wrong. It supports arbitrary amount of bands, but not arbitrary filter order for a band. Just to reiterate, the whole filter is a cascade of some per-band filters of the same order. Each filter in the cascade is implemented, according to the dissertation, as a cut-and-boost filter, which is based on a band-pass filter, which is in turn based on a shelving Butterworth filter of order 2M. Hopefully I haven't missed any order-doubling here. I have not thoroughly checked whether the implementation actually follows the dissertation - this is complicated by the Italian language that I don't know, and cryptic variable names. The majority of the dissertation does contain formulas for an arbitrary even order (2M), but on page 41, it starts talking about filters of order 8 only. And, if I understand correctly, it doesn't match the code, which is hard-coded to implement only filters of order 4, and it is not only a matter of increasing M and M2 in the file, because then xn[i] must be set in eq_filter() for i >= 4. Another sign of this mismatch between the dissertation and the code is the different length of the "c" array - 10 in the code and 21 in algorithm 6.2 on page 43 (page numbers here are presented as printed at the bottom, not as displayed in PDF viewers' left pane). Because of the fixed order, there are indeed limits on the useful number of bands. E.g., given that the minimum and maximum supported gains in the band are -12 and 12 dB (at least this is what's mentioned in the dissertation), it means that the gain must increase by 24 dB between the centers of the neighboring bands. With the default 10-band filter, i.e. with one octave per band, this means that 24 dB per octave is the maximum supported slope, which is roughly right for a 4th order filter. In other words, more than 10 bands cannot be useful with 4th order filters and 24 dB of gain difference between the neighboring bands. I understand that it is odd to set the gain in two neighboring octaves to differ by that much. For a 5-band equalizer with the same gain limits, filters of 2nd order would be sufficient and preferable. Also, the order of the filter could be lowered if the gain limits are set lower. > > Besides, consumer electronic devices (TVs, HDMI receivers, Hi-Fi > > amplifiers) just do not have equalizers with a variable number of > > bands, for usability reasons. I don't see a reason for PulseAudio to > > be different. > > > You can't add sliders on the fly on a consumer electronic > device but you can do so in software. Why should we be guided > by a behavior that is governed by physical limits? If the algorithm > supports it, I see no reason why it should not be implemented. > (As said above naturally within some sane limits, you are right > that it would make no sense to have for example 50 bands.) Sliders in a consumer electronics device such as a TV are just virtual widgets on the screen, i.e. software, not physical knobs. Again, their number is fixed because of usable and "intuitive" UI. > Surely you could go for several different plugins, but I do not > see the reason why the code should be duplicated a couple > of times. The goal is to have one plugin and not a collection. > I just don't like the idea that you have to duplicate things only > because of the limitation of the surrounding framework. > That's why I am looking for a way to dynamically adapt the > number of control ports or - which seems to be supported > by LV2 - use a control port that is an array. And my point is exactly that there is a reason, filter
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
пн, 8 апр. 2019 г. в 15:05, Alexander E. Patrakov : > > пн, 8 апр. 2019 г. в 13:08, Alexander E. Patrakov : > > > I have looked again at the paper, and have a DSP question. > > > > The thesis starts with designing a shelf filter. That is, a filter > > that contains a flat area at low frequencies, a flat area at high > > frequencies (but with a different gain), and some transition in > > between. Then, for some unknown reason, it is transformed into a > > band-pass shelving filter. Then there is some talk how to "stitch" > > together several band-pass shelving filters. > > > > Why would one need to do this at all? I.e., why can't one just cascade > > several shelf filters, designed according to the correct transition > > frequencies, with the height of the shelf being the dB difference of > > the desired gains of the neighboring bands? > > Just for some context: the reason why I asked is the ripple on Figure > 2.5 that seems to be avoidable in my approach. Sorry for the stupid question. This is answered in Chapter 5. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On 19.04.19 18:23, Alexander E. Patrakov wrote: пт, 19 апр. 2019 г. в 14:13, Tanu Kaskinen : If the plugin gets the number of bands during the initialization, it can create the appropriate number of non-array control ports. Interleaved audio ports aren't needed either, because PulseAudio can do the deinterleaving before passing the audio to the plugin (like module-ladspa-sink already does). If one's going to write an LV2 plugin, it's best to use standard port types so that all hosts will be able to use the plugin. In addition, I think that "if the plugin gets the number of bands [at runtime]" is a very big "if" for an IIR-based equalizer. So big that I would rather hard-code it, or treat equalizers with a different number of bands as completely different plugins. The reason is that the required slope of inter-band transition (i.e., effectively, the filter order) is a function of the width of each band. Implement a filter with a too-low order, and it won't be able to isolate (and thus control thegain of) a frequency band selectively enough. Implement a filter with a too-high order, and its frequency response will be too steppy. My understanding is that Andrea's equalizer algorithm supports an arbitrary number of bands (certainly within some sane limits). Please correct me if I am wrong. Besides, consumer electronic devices (TVs, HDMI receivers, Hi-Fi amplifiers) just do not have equalizers with a variable number of bands, for usability reasons. I don't see a reason for PulseAudio to be different. You can't add sliders on the fly on a consumer electronic device but you can do so in software. Why should we be guided by a behavior that is governed by physical limits? If the algorithm supports it, I see no reason why it should not be implemented. (As said above naturally within some sane limits, you are right that it would make no sense to have for example 50 bands.) Surely you could go for several different plugins, but I do not see the reason why the code should be duplicated a couple of times. The goal is to have one plugin and not a collection. I just don't like the idea that you have to duplicate things only because of the limitation of the surrounding framework. That's why I am looking for a way to dynamically adapt the number of control ports or - which seems to be supported by LV2 - use a control port that is an array. Another example where an array would be useful as a control port is the virtual-surround-sink. The HRIR data used by the filter is an array of floats which could vary in size. The focus of this mail thread is no longer the equalizer, even though the subject may suggest that and it is often used as an example. The starting point of the discussion was that I did a consolidation of the virtual sinks we have in pulseaudio (see !88) and the result shows that many of the filters we have could be further consolidated to a single plugin sink. The question then was which kind of plugin format we should use? Naturally it would be better to use some standard API instead of inventing our own. And if we move to a plugin-sink, the plugin API should also support the features of the new equalizer, so that this can be integrated as well if Andrea is willing to contribute to a plugin. Tanu suggested the LV2 standard, so I took a look into it. My understanding is, that the CVPort is the type of control structure I have been looking for, though Tanu says otherwise. There is however another limitation I see with the LV2 standard, which is that it does not support interleaved audio channels. This is not fatal, but at least inconvenient. It looks like massive overhead to me if you have to de-interlace the audio data, then run multiple instances of the filter and finally have to interleave the data again. I think a plugin standard for pulseaudio should be able to handle interleaved data. ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
пт, 19 апр. 2019 г. в 14:13, Tanu Kaskinen : > If the plugin gets the number of bands during the > initialization, it can create the appropriate number of non-array > control ports. Interleaved audio ports aren't needed either, because > PulseAudio can do the deinterleaving before passing the audio to the > plugin (like module-ladspa-sink already does). If one's going to write > an LV2 plugin, it's best to use standard port types so that all hosts > will be able to use the plugin. In addition, I think that "if the plugin gets the number of bands [at runtime]" is a very big "if" for an IIR-based equalizer. So big that I would rather hard-code it, or treat equalizers with a different number of bands as completely different plugins. The reason is that the required slope of inter-band transition (i.e., effectively, the filter order) is a function of the width of each band. Implement a filter with a too-low order, and it won't be able to isolate (and thus control thegain of) a frequency band selectively enough. Implement a filter with a too-high order, and its frequency response will be too steppy. Besides, consumer electronic devices (TVs, HDMI receivers, Hi-Fi amplifiers) just do not have equalizers with a variable number of bands, for usability reasons. I don't see a reason for PulseAudio to be different. -- Alexander E. Patrakov ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On 19.04.19 16:56, Tanu Kaskinen wrote: On Fri, 2019-04-19 at 12:03 +0200, Georg Chini wrote: On 19.04.19 11:13, Tanu Kaskinen wrote: On Tue, 2019-04-16 at 21:40 +0200, Georg Chini wrote: On 16.04.19 19:19, Tanu Kaskinen wrote: On Thu, 2019-04-11 at 20:42 +0200, Georg Chini wrote: On 11.04.19 19:36, Tanu Kaskinen wrote: If you want a better plugin standard, are you aware of LV2 and PipeWire's SPA (the latter doesn't seem to be properly documented yet, but to my understanding it's supposed to have a stable and flexible API)? Arun already suggested the pipewire SPA. I took a look, but it seems not very simple compared to LADSPA. I could not really understand how it works and it appears to support a lot more than just filters. LV2 would also be an option, although it too is pretty complex compared to LADSPA. But at least it's documented and has examples. I just took a look and on the first glance LV2 seems similar to LADSPA. I have to dig into the details though, maybe control arrays and interleaved audio ports are possible there. I'm pretty sure they are possible, but neither of those features are necessary. If the plugin gets the number of bands during the initialization, it can create the appropriate number of non-array control ports. Interleaved audio ports aren't needed either, because PulseAudio can do the deinterleaving before passing the audio to the plugin (like module-ladspa-sink already does). If one's going to write an LV2 plugin, it's best to use standard port types so that all hosts will be able to use the plugin. The problem here is that the number of ports must be known before the initialization because it is something which is already provided by the plugin descriptor. So there seems to be no way to change that number dynamically unless I misread the documentation. But looking at the LV2 standard, it supplies the port type lv2:CVPort (see https://gitlab.com/lv2/lv2/blob/master/lv2/core/lv2core.ttl, line 256) which is what I have been looking for if I read the documentation right. I don't think CVPorts are relevant for this discussion. As far as I can tell, they just provide a different kind of control port, one that can use audio signal as input. You wanted a port that can take an array of values, CVPorts isn't that. The documentation basically says it is an array of floats which is not audio but control data. So it is very relevant here. This is exactly what is needed if you want to support a variable number of bands because you need a variable number of gains. You seem to be right about the requirement to declare all ports in advance. I thought dynamic ports would be the primary benefit of using LV2 rather than creating a custom API based on LADSPA. Concerning interleaved audio format: Up to now I found nothing that explicitly forbids interleaved audio though it appears that the plugins usually provide one audio port per channel. You can't use a plain AudioPort for interleaved audio, because hosts will assume that it operates with mono audio. You can probably define a subclass of AudioPort with different semantics, but then hosts other than PulseAudio won't be able to use the plugin (unless they adopt your extension). Other hosts could still use the plugin because mono would be perfectly acceptable. But I agree that we should not implement something that is not in the specification. What should be possible however is writing an LV2 extension that allows interleaved ports. If hosts do not support this extension, the plugins would be considered mono but could still work. PA can surely deinterleave the input and interleave the output but to me it looks like completely unnecessary copying around of buffers within a real time thread. With interleaved channels, you could directly pass the input and output buffers. Why should we copy the data twice if it can be avoided? Additionally, using one port per channel makes it impossible to adapt the number of channels dynamically when loading the plugin for the reason given above. The reason I suggested deinterleaving in PulseAudio was to allow the plugin to be compatible with other hosts. Without native support for dynamic ports in LV2, such compatibility seems to be hard/impossible to achieve, however. My intention would have been to support both, plugins that use one audio port per channel and plugins that use interleaved channels. Most of the plugins can be implemented as mono plugins and instantiated according to the number of channels, so compatibility would be possible. The question is, which part of LV2 should PA support? Only the core specification or extensions as well? If yes, which extensions? As for dynamically changing channels, I don't see the use case for that. With dynamically I meant choosing the number of channels at the time of instance creation. I don't want to change it at run-time. You say that your extension allows full integration of Andrea's equalizer, but I don't see how it allows
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On Fri, 2019-04-19 at 12:03 +0200, Georg Chini wrote: > On 19.04.19 11:13, Tanu Kaskinen wrote: > > On Tue, 2019-04-16 at 21:40 +0200, Georg Chini wrote: > > > On 16.04.19 19:19, Tanu Kaskinen wrote: > > > > On Thu, 2019-04-11 at 20:42 +0200, Georg Chini wrote: > > > > > On 11.04.19 19:36, Tanu Kaskinen wrote: > > > > > > If you want a better plugin standard, are you aware of LV2 > > > > > > and PipeWire's SPA (the latter doesn't seem to be properly > > > > > > documented > > > > > > yet, but to my understanding it's supposed to have a stable and > > > > > > flexible API)? > > > > > Arun already suggested the pipewire SPA. I took a look, but it > > > > > seems not very simple compared to LADSPA. I could not really > > > > > understand how it works and it appears to support a lot more > > > > > than just filters. > > > > LV2 would also be an option, although it too is pretty complex compared > > > > to LADSPA. But at least it's documented and has examples. > > > I just took a look and on the first glance LV2 seems similar > > > to LADSPA. I have to dig into the details though, maybe control > > > arrays and interleaved audio ports are possible there. > > I'm pretty sure they are possible, but neither of those features are > > necessary. If the plugin gets the number of bands during the > > initialization, it can create the appropriate number of non-array > > control ports. Interleaved audio ports aren't needed either, because > > PulseAudio can do the deinterleaving before passing the audio to the > > plugin (like module-ladspa-sink already does). If one's going to write > > an LV2 plugin, it's best to use standard port types so that all hosts > > will be able to use the plugin. > > The problem here is that the number of ports must be known before > the initialization because it is something which is already provided by > the plugin descriptor. So there seems to be no way to change that > number dynamically unless I misread the documentation. But looking > at the LV2 standard, it supplies the port type lv2:CVPort (see > https://gitlab.com/lv2/lv2/blob/master/lv2/core/lv2core.ttl, line 256) > which is what I have been looking for if I read the documentation > right. I don't think CVPorts are relevant for this discussion. As far as I can tell, they just provide a different kind of control port, one that can use audio signal as input. You wanted a port that can take an array of values, CVPorts isn't that. You seem to be right about the requirement to declare all ports in advance. I thought dynamic ports would be the primary benefit of using LV2 rather than creating a custom API based on LADSPA. > Concerning interleaved audio format: Up to now I found nothing > that explicitly forbids interleaved audio though it appears that the > plugins usually provide one audio port per channel. You can't use a plain AudioPort for interleaved audio, because hosts will assume that it operates with mono audio. You can probably define a subclass of AudioPort with different semantics, but then hosts other than PulseAudio won't be able to use the plugin (unless they adopt your extension). > PA can surely deinterleave the input and interleave the output > but to me it looks like completely unnecessary copying around > of buffers within a real time thread. With interleaved channels, > you could directly pass the input and output buffers. Why should > we copy the data twice if it can be avoided? Additionally, using > one port per channel makes it impossible to adapt the number > of channels dynamically when loading the plugin for the reason > given above. The reason I suggested deinterleaving in PulseAudio was to allow the plugin to be compatible with other hosts. Without native support for dynamic ports in LV2, such compatibility seems to be hard/impossible to achieve, however. As for dynamically changing channels, I don't see the use case for that. > > > > > > You say that your extension allows full integration of Andrea's > > > > > > equalizer, but I don't see how it allows the host to tell the plugin > > > > > > how many channels and how many frequency bands it should initialize. > > > > > For an interleaved audio port, there would be another control > > > > > port which holds the number of (interleaved) channels. So > > > > > this port would allow you to change the number of channels. > > > > > You could for example have an audio port named "Input" > > > > > and a control port "Number of input channels". Then the > > > > > get_info_port() function would return the index of the > > > > > "Number of input channels" control when called with the > > > > > "Input" port as argument. Or the other way round: If you > > > > > set "Number of input channels" to 6 the plugin will expect > > > > > 6 channels in the interleaved audio port (and you know > > > > > which control port sets the number of channels because > > > > > you can get it via the get_info_port() function. > > > > > > > > > > The same applies to the number of
Re: [pulseaudio-discuss] [PATCH v8 07/12] bluetooth: Add A2DP aptX and aptX HD codecs support
On Fri, 2019-04-19 at 12:52 +0200, Pali Rohár wrote:
> On Friday 19 April 2019 11:41:22 Tanu Kaskinen wrote:
> > On Sat, 2019-04-06 at 11:16 +0200, Pali Rohár wrote:
> > > +static size_t decode_buffer(void *codec_info, const uint8_t
> > > *input_buffer, size_t input_size, uint8_t *output_buffer, size_t
> > > output_size, size_t *processed) {
> > > +struct aptx_context *aptx_c = (struct aptx_context *) codec_info;
> > > +
> > > +const uint8_t *p;
> > > +uint8_t *d;
> > > +size_t to_write, to_decode;
> > > +
> > > +p = input_buffer;
> > > +to_decode = input_size;
> > > +
> > > +d = output_buffer;
> > > +to_write = output_size;
> > > +
> > > +while (PA_LIKELY(to_decode > 0)) {
> >
> > Is it intentional that encode_buffer() checks both to_encode and
> > to_write in the while loop condition, but decode_buffer() only checks
> > to_decode? If so, why does encode_buffer() need to be extra careful but
> > decode_buffer() doesn't?
>
> These checks are similar/same as in SBC codec code. And SBC code was
> there prior to my work. So personally I do not know.
Ok, let's find out if this code needs changing.
The output buffer size when decoding is determined by
get_read_block_size(), which returns a value corresponding to the read
MTU (the MTU is in the compressed domain, the block size is in the
uncompressed domain).
The input buffer size is potentially up to twice the read MTU.
It doesn't seem impossible that the socket could have more data queued
than just one MTU. Therefore this doesn't seem safe, because
decode_buffer() may overrun the output buffer. The solution isn't to
check to_write, however, because that would lead to some audio getting
dropped. I think the input buffer size should be limited to roughly one
MTU just like the output buffer size is limited to roughly one MTU. The
exact logic is codec dependent, so the codec implementation should
decide both the input and output buffer sizes, and it should be ensured
that the output buffer is big enough to fully decode a full input
buffer.
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v8 07/12] bluetooth: Add A2DP aptX and aptX HD codecs support
On Friday 19 April 2019 11:41:22 Tanu Kaskinen wrote:
> On Sat, 2019-04-06 at 11:16 +0200, Pali Rohár wrote:
> > This patch provides support for aptX and aptX HD codecs in bluetooth A2DP
> > profile. It uses open source LGPLv2.1+ licensed libopenaptx library which
> > can be found at https://github.com/pali/libopenaptx.
> >
> > aptX for s24 stereo samples provides fixed 6:1 compression ratio and
> > bitrate 352.8 kbit/s, aptX HD provides fixed 4:1 compression ratio and
> > bitrate 529.2 kbit/s.
> >
> > According to soundexpert research, aptX codec used in bluetooth A2DP is no
> > better than SBC High Quality settings. And you cannot hear difference
> > between aptX and SBC High Quality, aptX is just a copper-less overpriced
> > audio cable.
> >
> > aptX HD is high-bitrate version of aptX. It has clearly noticeable increase
> > in sound quality (not dramatic though taking into account the increase in
> > bitrate).
> >
> > http://soundexpert.org/news/-/blogs/audio-quality-of-bluetooth-aptx
> > ---
> > configure.ac| 36 +++
> > src/Makefile.am | 6 +
> > src/modules/bluetooth/a2dp-codec-aptx.c | 445
> >
> > src/modules/bluetooth/a2dp-codec-util.c | 8 +
> > 4 files changed, 495 insertions(+)
> > create mode 100644 src/modules/bluetooth/a2dp-codec-aptx.c
>
> > +static bool can_accept_capabilities_common(const a2dp_aptx_t
> > *capabilities, uint32_t vendor_id, uint16_t codec_id) {
> > +if (!(capabilities->frequency & (APTX_SAMPLING_FREQ_16000 |
> > APTX_SAMPLING_FREQ_32000 |
> > + APTX_SAMPLING_FREQ_44100 |
> > APTX_SAMPLING_FREQ_48000)))
> > +return false;
> > +
> > +if (A2DP_GET_VENDOR_ID(capabilities->info) != vendor_id ||
> > A2DP_GET_CODEC_ID(capabilities->info) != codec_id)
> > +return false;
> > +
> > +if (!(capabilities->channel_mode & APTX_CHANNEL_MODE_STEREO))
>
> Why is mono support not implemented? Is it a limitation of libopenaptx?
> It's not a big shortcoming, but it would be nice to support mono too.
It is limitation of libopenaptx. aptX in bluetooth stores in some bits
parity checks and I was not able to find any mono encoded sample of
aptX. So I do not know how aptX mono should be properly encapsulated in
bluetooth a2dp. And for obvious reason there is no (public)
documentation about aptX on Internet.
So once somebody shows us how aptX mono for bluetooth a2dp looks like I
can implement missing mono support in libopenaptx and in pulseaudio.
> > +static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer,
> > size_t input_size, uint8_t *output_buffer, size_t output_size, size_t
> > *processed) {
> > +struct aptx_context *aptx_c = (struct aptx_context *) codec_info;
> > +
> > +const uint8_t *p;
> > +uint8_t *d;
> > +size_t to_write, to_decode;
> > +
> > +p = input_buffer;
> > +to_decode = input_size;
> > +
> > +d = output_buffer;
> > +to_write = output_size;
> > +
> > +while (PA_LIKELY(to_decode > 0)) {
>
> Is it intentional that encode_buffer() checks both to_encode and
> to_write in the while loop condition, but decode_buffer() only checks
> to_decode? If so, why does encode_buffer() need to be extra careful but
> decode_buffer() doesn't?
These checks are similar/same as in SBC codec code. And SBC code was
there prior to my work. So personally I do not know.
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: PGP signature
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] License problem: LDAC codec & pulseaudio
On Fri, 2019-04-19 at 00:17 +0200, Pali Rohár wrote: > On Saturday 23 February 2019 17:59:57 Tanu Kaskinen wrote: > > On Sat, 2019-02-23 at 16:06 +0100, Pali Rohár wrote: > > > Hello, > > > > > > I would like to discuss about licence problems with usage of LDAC > > > encoder libldac in pulseaudio. LDAC is a codec used by some Sony > > > bluetooth headsets and it would be nice to have support for it after my > > > patch series for Modular API for A2DP codecs is merged. > > > > > > There is only one released LDAC encoder. It is libldac which can be > > > found at: https://android.googlesource.com/platform/external/libldac/ > > > It is licensed under Apache License Version 2.0. > > > > > > If I understood correctly pulseaudio is licensed under LGPL v2.1 or > > > later with some exceptions when some optional GPL dependences are > > > enabled then server part is licensed under GPL v2 or later. So it can be > > > distributed under GPL v3, right? Please correct me if I'm wrong. > > > > > > According to FSF https://www.gnu.org/licenses/license-list.html#apache2 > > > Apache License Version 2.0 is compatible with GPL v3, but is > > > incompatible with GPL v2. > > > > > > So, would it be feasible to write LDAC specific code for pulseaudio > > > under GPL v3 license (in separate files) and make compile time option to > > > enable/disable GPL v3 LDAC support? When enabled it would mean that > > > compiled binary of puleaudio server and server modules are forced to be > > > distributed under GPL v3 and thanks to compatibility with Apache License > > > Version 2.0, it would be possible to use libdac library. When disabled > > > then it would be like before (without LDAC and with license like > > > before). Am I correct? > > > > Yes, I believe you're correct, and at least I am fine with adding LDAC > > support via libldac. > > I asked people in SUSE and I got answer that it should be ok if there is > no GPLv2-only code. So has pulseaudio some GPLv2-only code or is all code > compatible with GPLv3 (i.e. GPLv2-or-later)? PulseAudio doesn't have any GPL code, but it has optional GPL dependencies, all of which seem to be either GPLv2-or-later or GPLv3- or-later (gdbm is the one that is nowadays licensed under GPLv3). The LICENSE file could be more clear on these details... You suggested writing LDAC specific code under GPLv3, but since LGPLv2.1-or-later is convertible to GPLv3, I think the LDAC code should be under LGPLv2.1-or-later like the rest of the code base. -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On 19.04.19 11:13, Tanu Kaskinen wrote: On Tue, 2019-04-16 at 21:40 +0200, Georg Chini wrote: On 16.04.19 19:19, Tanu Kaskinen wrote: On Thu, 2019-04-11 at 20:42 +0200, Georg Chini wrote: On 11.04.19 19:36, Tanu Kaskinen wrote: If you want a better plugin standard, are you aware of LV2 and PipeWire's SPA (the latter doesn't seem to be properly documented yet, but to my understanding it's supposed to have a stable and flexible API)? Arun already suggested the pipewire SPA. I took a look, but it seems not very simple compared to LADSPA. I could not really understand how it works and it appears to support a lot more than just filters. LV2 would also be an option, although it too is pretty complex compared to LADSPA. But at least it's documented and has examples. I just took a look and on the first glance LV2 seems similar to LADSPA. I have to dig into the details though, maybe control arrays and interleaved audio ports are possible there. I'm pretty sure they are possible, but neither of those features are necessary. If the plugin gets the number of bands during the initialization, it can create the appropriate number of non-array control ports. Interleaved audio ports aren't needed either, because PulseAudio can do the deinterleaving before passing the audio to the plugin (like module-ladspa-sink already does). If one's going to write an LV2 plugin, it's best to use standard port types so that all hosts will be able to use the plugin. The problem here is that the number of ports must be known before the initialization because it is something which is already provided by the plugin descriptor. So there seems to be no way to change that number dynamically unless I misread the documentation. But looking at the LV2 standard, it supplies the port type lv2:CVPort (see https://gitlab.com/lv2/lv2/blob/master/lv2/core/lv2core.ttl, line 256) which is what I have been looking for if I read the documentation right. Concerning interleaved audio format: Up to now I found nothing that explicitly forbids interleaved audio though it appears that the plugins usually provide one audio port per channel. PA can surely deinterleave the input and interleave the output but to me it looks like completely unnecessary copying around of buffers within a real time thread. With interleaved channels, you could directly pass the input and output buffers. Why should we copy the data twice if it can be avoided? Additionally, using one port per channel makes it impossible to adapt the number of channels dynamically when loading the plugin for the reason given above. You say that your extension allows full integration of Andrea's equalizer, but I don't see how it allows the host to tell the plugin how many channels and how many frequency bands it should initialize. For an interleaved audio port, there would be another control port which holds the number of (interleaved) channels. So this port would allow you to change the number of channels. You could for example have an audio port named "Input" and a control port "Number of input channels". Then the get_info_port() function would return the index of the "Number of input channels" control when called with the "Input" port as argument. Or the other way round: If you set "Number of input channels" to 6 the plugin will expect 6 channels in the interleaved audio port (and you know which control port sets the number of channels because you can get it via the get_info_port() function. The same applies to the number of bands. There must be a control port which reflects the number of elements in the control array which is the same as the number of bands. Both values can be set to convenient defaults if the host does not supply them (like 5 bands and 2 channels). Ok, so the idea is to do the configuration while the filter is running. I think it would be better to do the configuration in the plugin setup phase. I imagine that would simplify the control port allocoation and management, since the setup doesn't have to run in the IO thread where malloc() is not allowed. I don't see much benefit in doing this kind of configuration while the filter is running, since the filter state most likely has to be reset anyway when the number of EQ bands is changed. There could be a function for getting a description of what options the plugin accepts, and a setup function for setting the options. Why do you think that the filter must be configured while it is running? In case of the equalizer the number of channels and also the number of bands are known before the filter is run. The LADSPA standard says all control ports must be connected (and valid) before you can run the filter. If a parameter changes at runtime, the filter must be reset like the current ladspa-sink does. Control ports are used for realtime parameter changes, so that's why I thought the intention was to set the parameters while the filter is running. I think it would be much clearer and easier to document the expected
Re: [pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions
On Tue, 2019-04-16 at 21:40 +0200, Georg Chini wrote: > On 16.04.19 19:19, Tanu Kaskinen wrote: > > On Thu, 2019-04-11 at 20:42 +0200, Georg Chini wrote: > > > On 11.04.19 19:36, Tanu Kaskinen wrote: > > > > If you want a better plugin standard, are you aware of LV2 > > > > and PipeWire's SPA (the latter doesn't seem to be properly documented > > > > yet, but to my understanding it's supposed to have a stable and > > > > flexible API)? > > > Arun already suggested the pipewire SPA. I took a look, but it > > > seems not very simple compared to LADSPA. I could not really > > > understand how it works and it appears to support a lot more > > > than just filters. > > LV2 would also be an option, although it too is pretty complex compared > > to LADSPA. But at least it's documented and has examples. > > I just took a look and on the first glance LV2 seems similar > to LADSPA. I have to dig into the details though, maybe control > arrays and interleaved audio ports are possible there. I'm pretty sure they are possible, but neither of those features are necessary. If the plugin gets the number of bands during the initialization, it can create the appropriate number of non-array control ports. Interleaved audio ports aren't needed either, because PulseAudio can do the deinterleaving before passing the audio to the plugin (like module-ladspa-sink already does). If one's going to write an LV2 plugin, it's best to use standard port types so that all hosts will be able to use the plugin. > > > > You say that your extension allows full integration of Andrea's > > > > equalizer, but I don't see how it allows the host to tell the plugin > > > > how many channels and how many frequency bands it should initialize. > > > For an interleaved audio port, there would be another control > > > port which holds the number of (interleaved) channels. So > > > this port would allow you to change the number of channels. > > > You could for example have an audio port named "Input" > > > and a control port "Number of input channels". Then the > > > get_info_port() function would return the index of the > > > "Number of input channels" control when called with the > > > "Input" port as argument. Or the other way round: If you > > > set "Number of input channels" to 6 the plugin will expect > > > 6 channels in the interleaved audio port (and you know > > > which control port sets the number of channels because > > > you can get it via the get_info_port() function. > > > > > > The same applies to the number of bands. There must be a > > > control port which reflects the number of elements in the > > > control array which is the same as the number of bands. > > > > > > Both values can be set to convenient defaults if the host does > > > not supply them (like 5 bands and 2 channels). > > Ok, so the idea is to do the configuration while the filter is running. > > I think it would be better to do the configuration in the plugin setup > > phase. I imagine that would simplify the control port allocoation and > > management, since the setup doesn't have to run in the IO thread where > > malloc() is not allowed. I don't see much benefit in doing this kind of > > configuration while the filter is running, since the filter state most > > likely has to be reset anyway when the number of EQ bands is changed. > > > > There could be a function for getting a description of what options the > > plugin accepts, and a setup function for setting the options. > > > Why do you think that the filter must be configured while it is > running? In case of the equalizer the number of channels and > also the number of bands are known before the filter is run. > The LADSPA standard says all control ports must be connected > (and valid) before you can run the filter. If a parameter changes > at runtime, the filter must be reset like the current ladspa-sink > does. Control ports are used for realtime parameter changes, so that's why I thought the intention was to set the parameters while the filter is running. I think it would be much clearer and easier to document the expected host behaviour if the parameter configuration was not implemented via control ports. -- Tanu https://www.patreon.com/tanuk https://liberapay.com/tanuk ___ pulseaudio-discuss mailing list pulseaudio-discuss@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
Re: [pulseaudio-discuss] [PATCH v8 07/12] bluetooth: Add A2DP aptX and aptX HD codecs support
On Sat, 2019-04-06 at 11:16 +0200, Pali Rohár wrote:
> This patch provides support for aptX and aptX HD codecs in bluetooth A2DP
> profile. It uses open source LGPLv2.1+ licensed libopenaptx library which
> can be found at https://github.com/pali/libopenaptx.
>
> aptX for s24 stereo samples provides fixed 6:1 compression ratio and
> bitrate 352.8 kbit/s, aptX HD provides fixed 4:1 compression ratio and
> bitrate 529.2 kbit/s.
>
> According to soundexpert research, aptX codec used in bluetooth A2DP is no
> better than SBC High Quality settings. And you cannot hear difference
> between aptX and SBC High Quality, aptX is just a copper-less overpriced
> audio cable.
>
> aptX HD is high-bitrate version of aptX. It has clearly noticeable increase
> in sound quality (not dramatic though taking into account the increase in
> bitrate).
>
> http://soundexpert.org/news/-/blogs/audio-quality-of-bluetooth-aptx
> ---
> configure.ac| 36 +++
> src/Makefile.am | 6 +
> src/modules/bluetooth/a2dp-codec-aptx.c | 445
>
> src/modules/bluetooth/a2dp-codec-util.c | 8 +
> 4 files changed, 495 insertions(+)
> create mode 100644 src/modules/bluetooth/a2dp-codec-aptx.c
> +static bool can_accept_capabilities_common(const a2dp_aptx_t *capabilities,
> uint32_t vendor_id, uint16_t codec_id) {
> +if (!(capabilities->frequency & (APTX_SAMPLING_FREQ_16000 |
> APTX_SAMPLING_FREQ_32000 |
> + APTX_SAMPLING_FREQ_44100 |
> APTX_SAMPLING_FREQ_48000)))
> +return false;
> +
> +if (A2DP_GET_VENDOR_ID(capabilities->info) != vendor_id ||
> A2DP_GET_CODEC_ID(capabilities->info) != codec_id)
> +return false;
> +
> +if (!(capabilities->channel_mode & APTX_CHANNEL_MODE_STEREO))
Why is mono support not implemented? Is it a limitation of libopenaptx?
It's not a big shortcoming, but it would be nice to support mono too.
> +static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer,
> size_t input_size, uint8_t *output_buffer, size_t output_size, size_t
> *processed) {
> +struct aptx_context *aptx_c = (struct aptx_context *) codec_info;
> +
> +const uint8_t *p;
> +uint8_t *d;
> +size_t to_write, to_decode;
> +
> +p = input_buffer;
> +to_decode = input_size;
> +
> +d = output_buffer;
> +to_write = output_size;
> +
> +while (PA_LIKELY(to_decode > 0)) {
Is it intentional that encode_buffer() checks both to_encode and
to_write in the while loop condition, but decode_buffer() only checks
to_decode? If so, why does encode_buffer() need to be extra careful but
decode_buffer() doesn't?
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
