<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks a lot for the reply</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
><span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">If
the preset files are expected to be shared between users, then the</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">database.h
stuff isn't good, because different users can have their</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">pulseaudio
configured with different database formats. I like the "ini-</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">style"
configuration file style that pulseaudio uses for .conf files.</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">There
are no helpers for writing those files, though, but that's</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">probably
not a big issue.</span></div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I can write a parser for ini-style file however since PA is multiplatform I need some information about how to store user and system settings. System settings can be hardcoded at least, but the directory of user config depends on the platform I think.</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
>I<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important"><span> </span>would
love to have the equalizer as a LADSPA plugin</span></div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
My fear is that a LADSPA plugin will be too hard to use for a lot of desktop users. I think that a GNU desktop user would like to have a fully working audio equalizer in his distribution and PA is default in almost all GNU distributions. Configuring a LADSPA
plugin may be hard and boring for the average user and GNU will continue to don't have a standard equalizer. Beyond the issues you've already listed.</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
> <span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">It's
not very uncommon that some core</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">change
requires changes in all sinks, so even if the module is perfect</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">and
doesn't require maintenance in form of bug fixes, there are other</span><br style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255)">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important">kinds
of real maintenance costs.</span></div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
As far as I know the actual equalizer is deprecated so if this mine equalizer will be adequate I think that the actual can be substitute and the number of modules to maintain will not change.</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Andrea993</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<span style="color: rgb(51, 51, 51); font-family: "Segoe UI", "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; background-color: rgb(255, 255, 255); display: inline !important"><br>
</span></div>
<div style="font-family:Calibri,Helvetica,sans-serif; font-size:12pt; color:rgb(0,0,0)">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>Da:</b> pulseaudio-discuss <pulseaudio-discuss-bounces@lists.freedesktop.org> per conto di Tanu Kaskinen <tanuk@iki.fi><br>
<b>Inviato:</b> marted́ 6 novembre 2018 21:18<br>
<b>A:</b> General PulseAudio Discussion<br>
<b>Oggetto:</b> Re: [pulseaudio-discuss] New equalizer module (module-eqpro-sink), some questions</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">On Mon, 2018-11-05 at 00:14 +0500, Alexander E. Patrakov wrote:<br>
> Andrea A <Andrea69x@hotmail.it>:<br>
> > I'm writing a new equalizer module for PA,<br>
> > <a href="https://github.com/andrea993/audioeqpro/blob/master/pulsemodule/module-eqpro-sink.c">
https://github.com/andrea993/audioeqpro/blob/master/pulsemodule/module-eqpro-sink.c</a><br>
> > I've almost done but I need some information from developer about how to proceed.<br>
> <br>
> Thanks for attempting a contribution. I have attempted to answer your<br>
> questions regarding the integration, please read below. However, see<br>
> the end of this email for the biggest reason why I am against<br>
> accepting this module or any future form of it (but my "no" holds very<br>
> little weight, so feel free to ignore it).<br>
> <br>
> However, in order for the module to be accepted (barring the big<br>
> objection at the bottom of this email), we need to review the DSP<br>
> part, and not just the integration part. It would help if you provide,<br>
> in the form of comments in the source code, some references where the<br>
> math comes from. And use more descriptive variable names, such as K -><br>
> extra_gain. Also, I think it would make sense to use a struct of 10<br>
> well-named floats instead of eqp->c.<br>
> <br>
> > First of all, I see that current equalizer module manages<br>
> > "autoloaded" have I to manage it? What it does exactly? Old<br>
> > equalizer check "autoloaded" state in a callback "may_move_to",<br>
> > what is it? Have I to implement this callback and manage<br>
> > "autoloaded" like the current equalizer module?<br>
> <br>
> It is set by module_filter_apply. The intended effect is to prevent<br>
> moving the output of the equalizer to a different sink - i.e. if it<br>
> was autoloaded for "Built-in Audio Analog Stereo" then you cannot move<br>
> it to "HDMI Digital Stereo" using pavucontrol. See<br>
> module-virtual-surround-sink.c for known-good usage. Although, I don't<br>
> know any user of module_filter_apply.<br>
> <br>
> Regarding the may_move_to callback, it is called when a user tries to<br>
> move the equalizer output to a different sink. Please at least prevent<br>
> creating a loop, i.e. moving the output to the equalizer itself.<br>
<br>
There's no need to worry about loops, pa_sink_input_may_move_to()<br>
already checks that (except loops built using module-loopback aren't<br>
checked, but Andrea probably isn't going to solve that problem anyway,<br>
or if he is, it's better to solve that in pa_sink_input_move_to()<br>
rather than in individual modules).<br>
<br>
> > After the "autoloaded" management I can send the equalizer as a<br>
> > patch, however I've some questions about how to add my equalizer<br>
> > GUI to the PA branch. Should the GUI remains an external program or<br>
> > the GUI will be inserted to the mainline sources? In the second<br>
> > scenario how the GUI should be inserted? Which is the correct<br>
> > directory in the sources tree and what about the GUI makefile and<br>
> > the GUI installation directory?<br>
> <br>
> PulseAudio currently does not depend on any GUI toolkit (well, except<br>
> the old equalizer GUI). Personally, I am fine with this GUI (or maybe<br>
> two GUIs - one for GNOME and MATE and XFCE, one for KDE) being in<br>
> external repositories.<br>
<br>
GUIs should go to external repositories. qpaeq is an exception, and<br>
probably not that well justified exception. One reason qpaeq made its<br>
way to the main pulseaudio repo was that it's just a simple python<br>
script that doesn't need much support from the build system.<br>
<br>
> > The equalizer needs the messages patches from George Chini<br>
> > <a href="https://patchwork.freedesktop.org/series/41390/">https://patchwork.freedesktop.org/series/41390/</a><br>
> > Have I to write this information as a patch comment or other?<br>
> <br>
> Patch comment.<br>
<br>
I'm not sure what "patch comment" means, but the information doesn't<br>
belong in the commit message. If the module is submitted as a merge<br>
request in GitLab, the information can be written to the merge request<br>
description or added as a separate comment in the discussion section.<br>
If the module is submitted via email, the information can be added<br>
below the "---" line in the patch (this stuff is explained at<br>
<a href="https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/">https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/</a><br>
).<br>
<br>
> > I would like to add some configuration files to my module, for example to load and store equalizer preset, is there a PA specific file format (and directory to store file) to do this?<br>
> <br>
> There are database files in ~/.config/pulse. There are multiple<br>
> backends supported, see the --with-database=... configure argument.<br>
> The abstraction layer is in src/pulsecore/database.h. Not sure if this<br>
> is suitable for your needs.<br>
<br>
If the preset files are expected to be shared between users, then the<br>
database.h stuff isn't good, because different users can have their<br>
pulseaudio configured with different database formats. I like the "ini-<br>
style" configuration file style that pulseaudio uses for .conf files.<br>
There are no helpers for writing those files, though, but that's<br>
probably not a big issue.<br>
<br>
You mentioned presets only as an example, do you have other kinds of<br>
configuration in mind? I'd expect the module arguments to provide all<br>
the necessary configuration.<br>
<br>
> > Execuse me for the wall of questions and thanks in advance.<br>
> <br>
> You are welcome.<br>
> <br>
> Anyway, just a small nitpick: the rewind callback is implemented<br>
> incorrectly. The real problem is - nobody implements it correctly,<br>
> especially because the comment in the template module-virtual-sink.c<br>
> suggest doing such a stupid thing as resetting the filter. And, at<br>
> least for the case of a resampler, users other than me do notice the<br>
> imperfection, see <a href="https://bugs.freedesktop.org/show_bug.cgi?id=50113">
https://bugs.freedesktop.org/show_bug.cgi?id=50113</a> .<br>
> There are two solutions that I would accept as "proper". 1 - store the<br>
> history of your input and/or state, restore it when asked to rewind. 2<br>
> - do not pretend to support rewinds (but in this case, please limit<br>
> the latency to something like 20-30 ms, so hat PulseAudio reacts<br>
> quickly to the new streams). In the name of simplicity, and because<br>
> the power-saving argument behind the original rewind operation does<br>
> not hold if there is non-trivial processing, I would prefer option 2.<br>
<br>
In the name of simplicity, I'd strongly prefer option 2.<br>
<br>
> Big warning: I have not tested the module.<br>
> <br>
> And here at the bottom of the email, let me explain why I think<br>
> keeping this module outside of PulseAudio, in a different form, may be<br>
> a better idea.<br>
> <br>
> The reason is that, by accepting this module, we are implicitly taking<br>
> the responsibility to support it inside the tree. And, you are the<br>
> best person to support it. So there is an additional (avoidable!)<br>
> latency between the time when you develop improvements and the time<br>
> when users see them: namely, the time for someone else to understand<br>
> and review your code, for PulseAudio team to make a release, and for<br>
> distributions to package it.<br>
> <br>
> A better alternative, in my opinion, would be to create a LADSPA<br>
> plugin instead. PulseAudio already has module-lasdpa-sink since ages<br>
> (even with D-Bus interface to change parameters at runtime), so your<br>
> filter will be available also to all users of old PulseAudio versions.<br>
> It will be also available to users of pure ALSA-based setup, if they<br>
> still exist. You can publish improvements any time you want, without<br>
> needing any potentially slow review from PulseAudio maintainers (but<br>
> feel free to contact me privately if you do need a review), and your<br>
> module will be quick to compile, because it is separated from the rest<br>
> of PulseAudio. You can then publish a GUI application that loads the<br>
> module into PulseAudio and then controls its filter via D-Bus. And you<br>
> don't need to care about rewinds and may_move_to and all other<br>
> pulseaudio-specific boilerplate. Sounds like a win-win situation.<br>
> Could you please investigate this approach?<br>
<br>
I would love to have the equalizer as a LADSPA plugin rather than yet<br>
another separate filter sink. It's not very uncommon that some core<br>
change requires changes in all sinks, so even if the module is perfect<br>
and doesn't require maintenance in form of bug fixes, there are other<br>
kinds of real maintenance costs.<br>
<br>
The LADSPA plugin approach isn't without issues, however:<br>
<br>
LADSPA doesn't seem to support parametrized plugin instantiation,<br>
meaning that the number of bands needs to be fixed. This can be<br>
mitigated by creating a few separate plugins with different band<br>
counts, but that of course can't scale to support arbitrary band<br>
counts. But maybe a few common cases is good enough?<br>
<br>
There is a D-Bus interface for changing the parameters, but it's rather<br>
limited. It may very well be enough for a specialized GUI that knows<br>
exactly what plugin it's dealing with, though. But the D-Bus protocol<br>
has some serious issues, such as having no API stability guarantees. I<br>
think the D-Bus protocol can be regarded kind of deprecated, it's not<br>
likely to ever become the preferred protocol as originally envisioned.<br>
Implementing a control interface using the message API from Georg for<br>
the LADSPA sink would be an awesome contribution in itself, especially<br>
if it's more complete than the D-Bus one. "More complete" means<br>
supporting introspection: applications should be able to enumerate the<br>
LADSPA sinks, find out which plugins they have loaded (including label<br>
and name information) and what controls the plugins have (including the<br>
control name and type).<br>
<br>
-- <br>
Tanu<br>
<br>
<a href="https://www.patreon.com/tanuk">https://www.patreon.com/tanuk</a><br>
<a href="https://liberapay.com/tanuk">https://liberapay.com/tanuk</a><br>
<br>
_______________________________________________<br>
pulseaudio-discuss mailing list<br>
pulseaudio-discuss@lists.freedesktop.org<br>
<a href="https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss">https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss</a><br>
</div>
</span></font></div>
</body>
</html>