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

[Tanu Kaskinen]

On Sat, 2019-04-20 at 11:38 +0200, Georg Chini wrote:
> On 20.04.19 11:06, Tanu Kaskinen wrote:
> > On Fri, 2019-04-19 at 17:52 +0200, Georg Chini wrote:
> > > On 19.04.19 16:56, Tanu Kaskinen wrote:
> > > > Changing the number of eq bands isn't quite like changing regular
> > > > control values. The plugin probably has some per-band data, which has
> > > > to be reallocated when the number of bands changes. malloc() isn't
> > > > allowed in the IO thread. Also, all gain values assigned to the bands
> > > > previously are likely useless, because the band frequencies change. The
> > > > host will likely have to set all bands to the default gain value, so in
> > > > effect changing the band count is like starting from scratch, which is
> > > > very different from changing a regular control value.
> > > Yes, you are right, it would be like starting from scratch. It
> > > would however be not the primary goal to change the number
> > > of bands at run-time, but to be able to define the number of
> > > bands dynamically when the instance is created. Because this
> > > would affect the number of necessary ports (per band gains)
> > > it is not possible with the current definition unless you have
> > > a control port that can be an array. As a side effect, the band
> > > count could also be changed dynamically at run-time.
> > > 
> > > Why would malloc() not be allowed in the IO-thread? It's not
> > > allowed within the run() function, but that's a different thing.
> > Why is it a different thing? malloc() is not allowed in the run()
> > function, because the function is expected to be run in a realtime
> > thread, and malloc() is not realtime-safe. The IO thread is a realtime
> > thread, so the same limitations apply also outside the run() function.
> 
> PA uses malloc() in the IO-thread, so are we doing things wrong?
> I think using malloc() when a parameter changes is not interfering
> with real-time operation because the filter must be reset after
> a parameter change anyway.

Where exactly is malloc() used? Sounds like a bug.

Using malloc() when changing parameters does interfere with realtime
operation, because there may be more audio paths to the hardware sink
than just the ladspa filter. There may be streams that aren't filtered.

> > > > If the eq band count was an initialization parameter rather than a
> > > > control port, the IO thread limitations wouldn't become an issue, and
> > > > it would be explicit that changing the eq band count means starting
> > > > from scratch. It should still be possible to change initialization
> > > > parameters at runtime, that would just mean that a new plugin instance
> > > > is created and the old instance is removed.
> > > > 
> > > It is not possible to have that kind of initialization parameter.
> > > That is the main problem. As explained above, changing the
> > > number of bands would require changing the number of control
> > > ports.
> > If we're adding new stuff to the LADSPA interface anyway, we can surely
> > add a function that sets initialization parameters (and a function for
> > querying what initialization parameters the plugin has). We can then
> > specify that the control ports are to be created only after the
> > initialization parameters have been set.
> > 
> Well, after evaluation of the feedback I have been getting so far,
> I do not think I will make an attempt to create some plugin-sink.
> The existing standards do not fit to what I have in mind and
> if I read Alexander right they even intentionally do not support
> those features.
> 
> Inventing a PA internal standard does not make sense, because
> your main argument against implementing the equalizer as a module
> was that you did not want to host the DSP code inside PA. If we
> did our own standard, new filters would again be bound to PA.

New filters would be specific to PA, yes, but not maintained by us, and
new filters wouldn't have to go through the review process, so there
would still be some benefit in having a PA specific stable plugin API.

-- 
Tanu

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