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

[Arun Raghavan]

On 2 December 2014 at 17:52, David Henningsson
<david.henningsson at canonical.com> wrote:
>
>
> 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'd amend that last to say "If you are facing problems with your
audio, try unloading it as a potential workaround". Also probably use
"audible" instead of "hearable".

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

Also sounds sensible to me.

-- Arun