[MPlayer-dev-eng] [PATCH] remove limits from af_equalizer

[Corey Hickey]

Reynaldo H. Verdejo Pinochet wrote:
>>>>It's not really a high priority of mine -- I just noticed that it could
>>>>be done and probably should. Here are the reasons:
>>>>
>>>>* It's possible to pass af_equalizer one or more suboptions of 12 that
>>>>causes it to clip with audio that is loud but still of fairly "usual"
>>>>volume. So, a maximum of 12 doesn't necessarily protect users from
>>>>themselves.
>>>>
>>>>* Conversely, values higher than 12 may not cause clipping with quieter
>>>>audio. In this case, 12 is an artificial restriction.
>>>>
>>>>* af_volume is an easy way to enforce clipping if necessary and can use
>>>>soft clipping, which may be desirable.
>>>
>>>
>>>All this leaves me the impresion youre measuring efectivity/usability
>>>of the filter by means of his ability to *clip* rather than his hability 
>>>to correctly filter/boost a given freq range.
>>
>>Actually, af_equalizer doesn't clip on it's own. By the time I wrote my
>>last email I had gotten mixed up and thought it had. What I should have
>>said instead of "cause clipping" is "cause out-of-range values to be
>>sent to the sound card". Sorry about that.
>>
>>To restate my point, I'll say that af_equalizer already is capable of
>>generating out-of-range values, so the current limits are not effective.
>>Moreover, those limits can be too restrictive with audio that is fairly
>>quiet.
>>
>>
>>>AFAIK eq filters are used mainly to work over a given dynamic range
>>>using cutoff freqs to (among other things) *avoid* clipping
>>
>>I actually know very little about traditional audio equipment -- I'm
>>just approaching this from the standpoint of making the filter more
>>flexible.
>>
>>
>>>>* Suboptions with values much lower than 12 have hardly any effect.
>>>>While they aren't really useful, there's no danger in allowing them.
>>>
>>>may a wider bost range be enough? not that Im against leaving it unrestricted
>>>but I wanna be sure why ;) especially on changes that can lead users
>>>to wrong / bad results.
>>
>>Probably a wider range would be fine, but I consider it unnecessary. Any
>>value chosen would be arbitrary, and any value high enough to avoid
>>restricting the amplification of a quiet source would be high enough to
>>be damaging to a loud source.
>>
>>The current limits encompass most of the useful values, and I noted that
>>in my patch to the man page. I've attached a new patch that describes
>>more accurately what happens with suboption values that are too high.
>>
>>-Corey
> 
> 
> Ok Corey, if flexibility is the issue I'll back you on this one, still
> I'll need you to make some measurements to determine a db range that
> will (~overall) *work* without making a mess of the output, add that 
> range to the man page and Ill commit if no one complains.

Sorry this took me so long -- I got caught up in preparing for the Doom9
codec comparison and had to set this aside for a while.

The application of my other patch to af_equalizer (which removed an
unnecessary divide by 40) makes af_equalizer way more sensitive than it
used to be. Useful values are only between -2 and 2, whereas they should
be between -12 and 12. Setting a suboption to 3 shouldn't cause
out-of-range values. I don't think my other patch was wrong, but I think
it only fixes half the bug. I spent a while looking at af_equalizer.c,
but I didn't find a solution. I can understand the source ok, but I
don't know how it should be.

I wanted to fix it myself before I sent in another patch removing the
limits, but at this point I have to give up. As it is right now, with
af_equalizer's sensitivity, the limits are way higher than they need to
be anyhow. If that bug is fixed, then my previous patch
(equalizer-remove-limits2.diff) will probably be relevant. Otherwise,
you might as well drop it.

-Corey