[vlc-devel] [PATCH v2 4/9] equalizer: Add support for extra sets of ISO bands

[Ron Wright]

On 11/28/2015 05:44 PM, Jean-Baptiste Kempf wrote:
> On 28 Nov, Ron Wright wrote :
>> The EQZ_INCLUDE_MATH_ROUTINES define is needed because I wanted to make it
>> optional to include math.h before including equalizer_presets.h when
>> EqzGetTrueFrequency(), which requires math.h, is not used.  At the same
> Why?

I just explained why.  I wanted to guarantee performance by making the 
function definition inline, and if I don't mask it out, then a compiler 
error will result if the user doesn't include math.h because math.h 
isn't already included in equalizer_presets.h.  I purposely avoided 
including math.h to avoid compilation overhead when it's not needed.  If 
I understand the meaning of your comment correctly, it sounds like you 
want me to include math.h in the equalizer_presets.h file to avoid the 
EQZ_INCLUDE_MATH_ROUTINES define.  I was thinking of doing that, but I 
wasn't sure whether that was okay because the header file originally 
didn't have any other header inclusions in it, and it could introduce 
some additional compilation overhead.

>>> I'd say you need a selector with a few choices.
>> What's wrong with the selector that's already present?  It allows the user
>> to select from 10, 15, or 31 bands.
> Because you have now one checkbox, plus a spinbox? And all the choices
> of the spinbox are not valid...

Rémi suggested that I keep the new code backwards compatible with the 
old settings, and that was one of the solutions I came up with while 
keeping the option to use VLC frequency bands when using 10 bands.  I 
guess another solution would be to use a single list of options (10-band 
VLC, 10-band ISO, 15-band ISO, 31-band ISO), and then read the old 
equalizer-vlcfreqs variable and apply it to the new option list format.  
What do you think?

As for your second comment, I'm guessing you want me to use an integer 
variable and an integer list instead of a string variable and a string 
list to make the choices of the spinbox valid.  I'm not sure what you 
exactly mean by your concern that the choices of the spinbox are not valid.

Regards,

Ron Wright