[pulseaudio-discuss] R: New equalizer module (module-eqpro-sink), some questions

Tue Apr 16 17:19:29 UTC 2019

On Thu, 2019-04-11 at 20:42 +0200, Georg Chini wrote:
> On 11.04.19 19:36, Tanu Kaskinen wrote:
> > On Thu, 2019-04-11 at 15:16 +0200, Georg Chini wrote:
> > > On 08.04.19 09:27, Georg Chini wrote:
> > > > On 05.04.19 13:29, Tanu Kaskinen wrote:
> > > > > On Tue, 2019-04-02 at 20:28 +0200, Georg Chini wrote:
> > > > > > On 06.11.18 22:14, Andrea A wrote:
> > > > > > Hi Andrea,
> > > > > > 
> > > > > > 
> > > > > > maybe there is a chance now to have your equalizer included as a
> > > > > > module.
> > > > > > The messaging API patches
> > > > > > should have their final form (at least I do not think the public
> > > > > > functions will change anymore) and today
> > > > > > I submitted a patch series that consolidates the code of the current
> > > > > > virtual sinks and moves the common
> > > > > > code to a separate file. Using the common code should significantly
> > > > > > reduce the maintenance cost of an
> > > > > > additional sink.
> > > > > > 
> > > > > > So if you are still interested to have it included, at least I would
> > > > > > welcome a new patch.
> > > > > > 
> > > > > > 
> > > > > > Arun, Tanu, what do you think?
> > > > > I think it would anyway make sense to make one or more LADSPA plugins
> > > > > out of the equalizer code (I say one or more, because of the lack of
> > > > > parametrization support in LADSPA). That way the equalizer would be
> > > > > available also to other software than just PulseAudio (I'm thinking
> > > > > PipeWire in particular).
> > > > > 
> > > > > If a suitable LADSPA plugin existed, we might or might not still need a
> > > > > separate equalizer module, but in any case we wouldn't need to maintain
> > > > > the DSP code in PulseAudio. If there's some reason why module-ladspa-
> > > > > sink isn't (and can't become) suitable for implementing the integration
> > > > > in PulseAudio, then a specialized module is fine.
> > > > > 
> > > > > I'm not saying that I'm dead against hosting the DSP code in
> > > > > PulseAudio, but I'd certainly prefer not to.
> > > > > 
> > > > It surely would make sense to have one or several  LADSPA
> > > > plugins, but for me a good equalizer should be an integral
> > > > part of pulseaudio. And as you say yourself, the full flexibility
> > > > cannot be achieved by a single LADSPA plugin. The equalizer
> > > > we are currently providing is buggy and completely unsupported.
> > > > The new equalizer would at least be fully documented, so that
> > > > it is possible to maintain. Additionally I agree with Andrea that
> > > > handling LADSPA plugins is somewhat cumbersome. From a
> > > > user point of view, a module is much easier to handle.
> > > I have taken a more detailed look on the LADSPA standard and
> > > to me it appears like you would not only need different plugins for
> > > different numbers of equalizer channels but in addition also
> > > for different number of audio channels. Both, the number of
> > > single-value parameters and number of input-/output-channels
> > > seem to be fixed, so producing a bunch of plugins is rather
> > > impractical.
> > An equalizer plugin doesn't need multiple channels, one mono plugin can
> > be instantiated for each channel. Or does this equalizer have some
> > cross-channel effects?
> 
> You are right, that would not be necessary. Only some plugin
> that does up- or down-mixing of channels would need that.
> 
> > > I wonder if there is a chance to extend the standard a bit to
> > > allow a variable number of audio channels and allow control
> > > ports to be arrays. It can be done with two more constants and
> > > one additional function, see attached diff. This extension would
> > > allow to reduce many of our virtual sinks to one plugin-sink and
> > > also allow full integration of Andrea's equalizer.
> > Do you have in mind actually extending the LADSPA standard (i.e.
> > something to be promoted to other projects as well), or just creating a
> > new custom (i.e. non-standard) plugin API for PulseAudio that is based
> > on LADSPA?
> 
> It would be actually best if the standard could be extended, but
> I do not know how feasible this is. The extension I provided will
> not break any old plugin and would simplify the handling in many
> cases.
> The next best thing would be to do it only for PA.

I doubt the wider Linux audio community is very interested in extending
LADSPA, since LV2 already exists as a successor.

Defining a PA specific filter API would be worth doing, IMO, if it can
be kept simple (compared to the alternatives). Of course anyone would
be welcome to use it in other projects too (the API should be stable,
so there should be no problems with maintaining plugin compatibility).

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

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

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk