[vlc-devel] [PATCH] libvlc API: Add equalizer to audio.c

[Cyril MATHE]

Le mardi 07 juillet 2009 à 01:37 -0700, Pierre d'Herbemont a écrit :
> On Mon, Jul 6, 2009 at 1:11 AM, Cyril MATHE<cmathe at actech-innovation.com> wrote:
> > Hello,
> >
> > This patch adds new feature to audio.c : equalizer.
> >
> > libvlc_audio_set_eqz_enable(int b_enable): enable or disable equalizer
> > libvlc_audio_set_eqz_preset(int i_preset): set an equalizer preset
> > libvlc_audio_set_eqz(libvlc_audio_eqz_t eqz): set an equalizer
> > libvlc_audio_get_eqz_preset(int i_preset): get an equalizer preset
> > libvlc_audio_get_eqz(): get current equalizer
> 
> I don't really like the fact that there is one structure, and multiple
> non obvious functions.
I agree with you
> 
> There should be only one set/get_eqz and no set/get_eqz_blabl or no
> set_eqz and multiple set_eqz_blabla.
> 
> The structure shouldn't have type prefix for the members as well.
> 
> Can all the functions be justified? Should the naming be more obvious?
In equalizer you can do two kind of action:
first you can change a band, for example you want to set 20 dB to 1kHz
band, this is what is done by libvlc_set_eqz and libvlc_get_eqz()
second, you can set a preset like flat, fullbass, pop..., this is what
is done by libvlc_set_eqz_preset(i_preset) and libvlc_get_preset(),
i_preset corresponding to the preset you want to choose.

I don't think that it is a good idea (even if it is possible) to group
libvlc_set_eqz and libvlc_set_eqz_preset together (the same for
getters), because they don't really do the same thing. But if you think
that it is more consistent to have one structure and only one getter and
setter I can change it.

libvlc_eqz_enable() to enable equalizer
> 
> I haven't looked at the implementation, but I also like to see persistence.
> 
> Pierre.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>