[vlc-devel] [PATCH] Add new API to libvlc for persistent equalizer settings.

[Rémi Denis-Courmont]

Le jeudi 6 septembre 2012 22:06:53, Mark Lee a écrit :
> Hello,
> 
> On 6 September 2012 17:49, Rémi Denis-Courmont <remi at remlab.net> wrote:
> > > The latest version of the patch does set values on the aout, but it
> > > uses
> > 
> > an
> > 
> > > additional variable on the media player to track whether or not the
> > > equalizer should automatically be enabled when playing new media.
> > 
> > Yes but the way to do that is to set both, not add ugly callbacks in the
> > core
> > code.
> 
> Is it OK to do something like this (in aout_New() in
> src/audio_output/common.c):

Again: No. Do NOT add ugly callbacks in the core code like this.

(Volume is _not_ a good example. The UI never sets the volume variable 
directly.)

> if (var_Type (p_parent, "equalizer-bands"))
> {
>      var_Create (aout, "equalizer-preamp", VLC_VAR_FLOAT |
> VLC_VAR_DOINHERIT);
>      var_Create (aout, "equalizer-bands", VLC_VAR_STRING |
> VLC_VAR_DOINHERIT);
>      var_AddCallback (p_parent, "equalizer-preamp", var_Copy, aout);
>      var_AddCallback (p_parent, "equalizer-bands", var_Copy, aout);
> }
> 
> Or am I not supposed to change anything in the core, and I must instead add
> the callbacks from the libvlc side?

Yes.

Your patch makes crash-prone assumption that could crash VLC.

-- 
Rémi Denis-Courmont
http://www.remlab.net/