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

[Jean-Baptiste Kempf]

On 28 Nov, Ron Wright wrote :
> already included in equalizer_presets.h.  I purposely avoided including
> math.h to avoid compilation overhead when it's not needed.  If I understand

Seriously?

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

compilation overhead in the case of VLC?
I'm all for VLC to stay lean, but come on, math.h is included almost
everywhere already.

> 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?

I prefer the single list of options.
I believe Remi is right, but come on, this is a rarely used option.

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

No, I meant a single-list of options and deprecate the checkbox.


-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device