[pulseaudio-discuss] [PATCH] Warn on attempts to build and load module-equalizer-sink

[David Henningsson]

On 2014-12-02 10:15, Alexander E. Patrakov wrote:
> 02.12.2014 14:02, David Henningsson wrote:
>>
>>
>> On 2014-12-01 17:14, Alexander E. Patrakov wrote:
>>> +    AS_HELP_STRING([--with-fftw],[Build FFTW-using module
>>> (equalizer). Note: the module has known-wrong DSP logic]))
>>> +AS_IF([test "x$HAVE_FFTW" = "x1"], ENABLE_FFTW="yes (BAD IDEA)",
>>> ENABLE_FFTW=no)
>>> +===== WARNING WARNING WARNING WARNING WARNING WARNING WARNING =====
>>> +You have fftw support enabled. fftw is used only by
>>> +module-equalizer-sink, which has many bugs, including wrong DSP
>>> +logic. See details at:
>>> +
>>> +http://lists.freedesktop.org/archives/pulseaudio-discuss/2014-March/020174.html
>>>
>>>
>>> +https://bugs.freedesktop.org/show_bug.cgi?id=41465
>>> +https://bugs.freedesktop.org/show_bug.cgi?id=54881
>>> +https://bugs.freedesktop.org/show_bug.cgi?id=69229
>>> +
>>> +It is suggested that you reconsider, unless you are a DSP
>>> +specialist willing to fix or rewrite the code and submit patches.
>>
>> Please take it down a notch or two.
>
> I can remove links to freedesktop bugs. Would that help?
>
>> Ok, so module-equalizer-sink has known crashers. So has
>> module-dbus-protocol [1], but we don't go all nuts like the above over
>> module-dbus-protocol.
>>
>> Ok, so module-equalizer-sink has "wrong DSP logic". But yet, people find
>> it useful. Hence, DSP logic seems to be something that concerns DSP
>> people more than casual listeners, who does not even know what "wrong
>> DSP logic" means in practice. And since you seem to be the closest to a
>> DSP specialist we have in this community, I'm sure it would be more
>> appreciated if you spend time fixing the sink, rather than trying to
>> take it down.
>
> I understand, but I don't have time to fix it, either. And I will never
> have it unless I change a job - which I have already tried and failed.
>
> But OK - I realize that I have already spent too much time trying to
> take it down.
>
>> That said, I don't mind a warning message or two when trying to load a
>> module that has known crashers, and that we don't have time/energy to
>> fix. But the extent of the warning messages above makes this look more
>> like "vendetta" and less like "politely inform the user". IMO.
>
> IMHO the messages in the module and in the configure script have
> different target audiences. The message in the module is indeed targeted
> at users and should indeed be reworded to be more to the point to inform
> them that there are known bugs. The message in the configure script is
> not targeted at users, it targets distributors, and thus needs a
> different wording than a message to users.

So, to get constructive here, how about this when you load the module:

pa_log_warn("module-equalizer-sink is currently unsupported, and can 
sometimes cause PulseAudio to crash and/or have hearable artifacts. Use 
at your own risk.");

(I'm assuming that "hearable artifacts" is the practical implication of 
"wrong DSP logic".)

I'm not sure we need to add anything for the target distributors. We 
could potentially add notices about module-dbus-protocol and 
module-equalizer-sink in the release notes, under a "known issues" 
section or similar.

>
>>
>> As for building by default, I don't have a strong opinion on whether we
>> do that or not - distros will disable or re-enable as they see fit
>> anyhow.
>>
>
>

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic