Hello,<br><div class="gmail_quote">
<br>
I have made many drastic changes in the compressor.c file 
and some minor changes in the UI code.  Here are some of the changes:<br>
<br>
1. Added VLC copyright and $id$ tag to the banner.<br>
2. Made the filter structure, RMS envelope structure, and attack lookup 
table contiguous in memory so that only one memory allocation and one 
memory deallocation is needed.<br>
3. Reorganized the functions so that they don't appear out of place.<br>
4. Removed two wasteful members (amplitude and gain reduction) from the 
filter structure (this is probably used to implement output meters in 
certain LADSPA frontends).<br>
5. The release of the mutex lock in the DoWork function now occurs 
immediately after retrieving the shared values.<br>
6. Replaced the logic in the callback 
functions with clipping functions.<br>
7. Removed useless casts.<br>
8. Changed default values.<br>

<br>Here are some notes to take into consideration:<br>
<br>
1. The f_max and f_clamp functions work as expected.  Proof:<br>
<br>
f_max (with arguments x and a):<br>
<br>
If x < a, then 0.5 * ((x - a) + fabs(x - a)) + a = 0.5 * ((x - a) - (x -
 a)) + a = a<br>
If x >= a, then 0.5 * ((x - a) + fabs(x - a)) + a = 0.5 * ((x - a) + (x - 
a)) + a = 0.5*2*(x - a) + a = x<br>
<br>
f_clamp (with arguments x, a, and b):<br>
<br>
If x < a, then 0.5 * (fabs(x - a) + a + b - fabs(x - b)) = 0.5 * (-(x
 - a) + a + b + (x - b)) = 0.5 * (2*a - x + b + x - b) = a<br>
If a <= x < b, then 0.5 * (fabs(x - a) + a + b - fabs(x - b)) = 0.5 * 
((x - a) + a + b + (x - b)) = 0.5 * (x + b + x - b) = x<br>
If x >= b, then 0.5 * (fabs(x - a) + a + b - fabs(x - b)) = 0.5 * ((x - a) +
 a + b - (x - b)) = 0.5 * (x + b - x + b) = b<br>
<br>
These are branchless clipping functions from Laurent de Soras.<br>
<br>
2. A buffer overflow will not occur when either attack or release 
converted to seconds is multiplied by (A_TBL - 1), with the resulting value used as an 
index to retrieve the attack (or release) value from the lookup table.  The
 maximum of the maximum values of attack and release is 800, and A_TBL 
is 256, so 800 * 0.001 * 255 = 204.  This means that the index will neither 
go beyond 255 nor even reach 255.  The minimum of the minimum values of 
attack and release is 1.5.  However, the real minimum value used in the code is 2.  Nevertheless, there will also be no
 buffer underflow, since this value is positive.<br>
<br><br>
If you wish to see the original LADSPA SWH code, look here: 
<a href="http://github.com/swh/ladspa" target="_blank">http://github.com/swh/ladspa</a>.  The compressor is based on code in 
sc4_1882.xml along with the necessary helper functions in other files.<br>
<br>
Attached are the updated compressor core and UI patches.<br>
<br>
Regards,<br>
<br>
Ronald Wright<div><div></div><div class="h5"><br><br><div class="gmail_quote">On Thu, Jun 24, 2010 at 2:05 PM, Laurent Aimar <span dir="ltr"><<a href="mailto:fenrir@elivagar.org" target="_blank">fenrir@elivagar.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div>On Mon, Jun 21, 2010 at 11:28:55PM -0500, Ron Wright wrote:<br>
</div>> +/*****************************************************************************<br>
> + * Open: initialize interface<br>
> + *****************************************************************************/<br>
> +<br>
> +static int Open( vlc_object_t *p_this )<br>
> +{<br>
> +    filter_t *p_filter = (filter_t*)p_this;<br>
> +    vlc_object_t *p_aout = p_filter->p_parent;<br>
 I see what you need it but I find it really really ugly. Dunno how to<br>
avoid that. Anyone with an idea ?<br>
<br>
> +    struct filter_sys_t *p_sys;<br>
> +    float amp;<br>
> +    float *as = NULL;<br>
 Useless init.<br>
> +    unsigned int count;<br>
> +    float env;<br>
> +    float env_peak;<br>
> +    float env_rms;<br>
> +    float gain;<br>
> +    float gain_t;<br>
> +    rms_env *rms = NULL;<br>
 Useless init.<br>
> +    float sum;<br>
> +<br>
> +    sample_rate = (float)p_filter->fmt_in.audio.i_rate;<br>
 Useless cast.<br>
<br>
> +    rms = rms_env_new();<br>
> +    if ( !rms )<br>
> +    {<br>
> +        free( p_sys );<br>
> +        return VLC_ENOMEM;<br>
> +    }<br>
> +<br>
> +    sum = 0.0f;<br>
> +    amp = 0.0f;<br>
> +    gain = 0.0f;<br>
> +    gain_t = 0.0f;<br>
> +    env = 0.0f;<br>
> +    env_rms = 0.0f;<br>
> +    env_peak = 0.0f;<br>
> +    count = 0;<br>
> +<br>
> +    as = malloc( A_TBL * sizeof(float) );<br>
 calloc( A_TBL, sizeof(*as) )<br>
<br>
> +    if ( !as )<br>
> +    {<br>
> +        rms_env_free( rms );<br>
> +        free( p_sys );<br>
> +        return VLC_ENOMEM;<br>
> +    }<br>
 But moving as and rms into the context (instead of allocating them<br>
independantly) would avoid thoses 2 mallocs and so simplify the code<br>
(only one error path).<br>
<br>
> +    as[0] = 1.0f;<br>
> +    for ( i = 1; i < A_TBL; i++ ) {<br>
> +        as[i] = expf( -1.0f / ( sample_rate * (float)i / (float)A_TBL ) );<br>
 Useless float casts.<br>
<br>
> +    p_sys->amp = amp;<br>
> +    p_sys->as = as;<br>
> +    p_sys->count = count;<br>
> +    p_sys->env = env;<br>
> +    p_sys->env_peak = env_peak;<br>
> +    p_sys->env_rms = env_rms;<br>
> +    p_sys->gain = gain;<br>
> +    p_sys->gain_t = gain_t;<br>
> +    p_sys->rms = rms;<br>
> +    p_sys->sum = sum;<br>
 I think you call remove most of the local variables and directly init p_sys.<br>
<br>
> +    vlc_mutex_init( &p_sys->lock );<br>
> +<br>
> +    p_sys->rms_peak    = var_CreateGetFloat( p_aout, "compressor-rms-peak" );<br>
> +    p_sys->attack      = var_CreateGetFloat( p_aout, "compressor-attack" );<br>
> +    p_sys->release     = var_CreateGetFloat( p_aout, "compressor-release" );<br>
> +    p_sys->threshold   = var_CreateGetFloat( p_aout, "compressor-threshold" );<br>
> +    p_sys->ratio       = var_CreateGetFloat( p_aout, "compressor-ratio" );<br>
> +    p_sys->knee        = var_CreateGetFloat( p_aout, "compressor-knee" );<br>
> +    p_sys->makeup_gain = var_CreateGetFloat( p_aout, "compressor-makeup-gain" );<br>
> +    p_sys->amplitude   = 0;<br>
> +    p_sys->gain_red    = 0;<br>
> +<br>
> +    /* Add our own callbacks */<br>
> +    var_AddCallback( p_aout, "compressor-rms-peak", RMSPeakCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-attack", AttackCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-release", ReleaseCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-threshold", ThresholdCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-ratio", RatioCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-knee", KneeCallback, p_sys );<br>
> +    var_AddCallback( p_aout, "compressor-makeup-gain", MakeupGainCallback,<br>
> +                     p_sys );<br>
> +<br>
> +    msg_Dbg( p_filter, "compressor successfully initialized" );<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static float f_db2lin_cube( float db, filter_sys_t * p_sys )<br>
> +{<br>
> +    float scale = ( db - DB_MIN ) * (float)LIN_TABLE_SIZE / ( DB_MAX - DB_MIN );<br>
> +    int base = f_round( scale - 0.5f );<br>
> +    float ofs = scale - base;<br>
> +    float *lin_data = p_sys->lin_data;<br>
> +<br>
> +    if ( base < 1 )<br>
> +    {<br>
> +        return 0.0f;<br>
> +    }<br>
> +    else if ( base > LIN_TABLE_SIZE - 3 )<br>
> +    {<br>
> +        return lin_data[LIN_TABLE_SIZE - 2];<br>
> +    }<br>
> +<br>
> +    return cube_interp( ofs, lin_data[base - 1],<br>
> +                             lin_data[base],<br>
> +                             lin_data[base + 1],<br>
> +                             lin_data[base + 2] );<br>
> +}<br>
> +<br>
> +static float f_db2lin_lerp( float db, filter_sys_t * p_sys )<br>
> +{<br>
> +    float scale = ( db - DB_MIN ) * (float)LIN_TABLE_SIZE / ( DB_MAX - DB_MIN );<br>
> +    int base = f_round( scale - 0.5f );<br>
> +    float ofs = scale - base;<br>
> +    float *lin_data = p_sys->lin_data;<br>
> +<br>
> +    if ( base < 1 )<br>
> +    {<br>
> +        return 0.0f;<br>
> +    }<br>
> +    else if ( base > LIN_TABLE_SIZE - 3 )<br>
> +    {<br>
> +        return lin_data[LIN_TABLE_SIZE - 2];<br>
> +    }<br>
> +<br>
> +    return ( 1.0f - ofs ) * lin_data[base] + ofs * lin_data[base + 1];<br>
> +}<br>
 f_db2lin_cube and f_db2lin_lerp could be merged and simply use the define for<br>
the only specific line.<br>
<br>
> +static float f_lin2db_cube( float lin, filter_sys_t * p_sys )<br>
> +{<br>
> +    float scale = ( lin - LIN_MIN ) * (float)DB_TABLE_SIZE /<br>
> +        ( LIN_MAX - LIN_MIN );<br>
> +    int base = f_round( scale - 0.5f );<br>
> +    float ofs = scale - base;<br>
> +    float *db_data = p_sys->db_data;<br>
> +<br>
> +    if ( base < 2 )<br>
> +    {<br>
> +        return db_data[2] * scale * 0.5f - 23.0f * ( 2.0f - scale );<br>
> +    }<br>
> +    else if ( base > DB_TABLE_SIZE - 3 )<br>
> +    {<br>
> +        return db_data[DB_TABLE_SIZE - 2];<br>
> +    }<br>
> +<br>
> +    return cube_interp( ofs, db_data[base - 1],<br>
> +                             db_data[base],<br>
> +                             db_data[base + 1],<br>
> +                             db_data[base + 2] );<br>
> +}<br>
> +<br>
> +static float f_lin2db_lerp( float lin, filter_sys_t * p_sys )<br>
> +{<br>
> +    float scale = ( lin - LIN_MIN ) * (float)DB_TABLE_SIZE /<br>
> +        ( LIN_MAX - LIN_MIN );<br>
> +    int base = f_round( scale - 0.5f );<br>
> +    float ofs = scale - base;<br>
> +    float *db_data = p_sys->db_data;<br>
> +<br>
> +    if ( base < 2 )<br>
> +    {<br>
> +        return db_data[2] * scale * 0.5f - 23.0f * ( 2.0f - scale );<br>
> +    }<br>
> +    else if ( base > DB_TABLE_SIZE - 2 )<br>
> +    {<br>
> +        return db_data[DB_TABLE_SIZE - 1];<br>
> +    }<br>
> +<br>
> +    return ( 1.0f - ofs ) * db_data[base] + ofs * db_data[base + 1];<br>
> +}<br>
 Same here (dunno if the -2 vs -3 is a bug or not, it doesn't match<br>
f_db2lin_cube vs f_db2lin_lerp).<br>
<br>
> +static void round_to_zero( volatile float *f )<br>
> +{<br>
> +    *f += 1e-18;<br>
> +    *f -= 1e-18;<br>
> +}<br>
 I don't think volatile is needed here.<br>
 Btw, what do you want to do with it ? Is it to remove not normalized number ?<br>
<br>
> +static float f_max( float x, float a )<br>
> +{<br>
> +    x -= a;<br>
> +    x += fabs( x );<br>
> +    x *= 0.5;<br>
> +    x += a;<br>
> +<br>
> +    return x;<br>
> +}<br>
 Is it really a max like function ?<br>
<br>
> +static block_t * DoWork( filter_t * p_filter, block_t * p_in_buf )<br>
> +{<br>
> +    int i_samples = p_in_buf->i_nb_samples;<br>
> +    int i_channels = aout_FormatNbChannels( &p_filter->fmt_in.audio );<br>
> +    float *p_out = (float*)p_in_buf->p_buffer;<br>
> +    float *p_in =  (float*)p_in_buf->p_buffer;<br>
> +<br>
> +    float rms_peak, attack, release, threshold, ratio, knee, makeup_gain;<br>
> +    float amp, *as, env, env_peak, env_rms, gain, gain_t, sum;<br>
> +    unsigned int count;<br>
> +    rms_env *rms;<br>
> +<br>
> +    float ga, gr, rs, mug, knee_min, knee_max, ef_a, ef_ai;<br>
> +<br>
> +    int pos, pos_chan;<br>
> +<br>
> +    /* Current configuration */<br>
> +    struct filter_sys_t *p_sys = p_filter->p_sys;<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +<br>
> +    /* RMS/peak (float value) */<br>
> +    rms_peak = p_sys->rms_peak;<br>
> +<br>
> +    /* Attack time (ms) (float value) */<br>
> +    attack = p_sys->attack;<br>
> +<br>
> +    /* Release time (ms) (float value) */<br>
> +    release = p_sys->release;<br>
> +<br>
> +    /* Threshold level (dB) (float value) */<br>
> +    threshold = p_sys->threshold;<br>
> +<br>
> +    /* Ratio (n:1) (float value) */<br>
> +    ratio = p_sys->ratio;<br>
> +<br>
> +    /* Knee radius (dB) (float value) */<br>
> +    knee = p_sys->knee;<br>
> +<br>
> +    /* Makeup gain (dB) (float value) */<br>
> +    makeup_gain = p_sys->makeup_gain;<br>
<br>
 As you made a copy of all the variable protected by the lock, you could<br>
release it here (instead of later). It is always good to minimized the time<br>
you hold a lock when possible.<br>
<br>
> +<br>
> +    amp = p_sys->amp;<br>
> +    as = p_sys->as;<br>
> +    count = p_sys->count;<br>
> +    env = p_sys->env;<br>
> +    env_peak = p_sys->env_peak;<br>
> +    env_rms = p_sys->env_rms;<br>
> +    gain = p_sys->gain;<br>
> +    gain_t = p_sys->gain_t;<br>
 I would prefer if you don't use _t for a variable.<br>
<br>
> +    rms = p_sys->rms;<br>
> +    sum = p_sys->sum;<br>
> +<br>
> +    ga = attack < 2.0f ? 0.0f<br>
> +                       : as[f_round( attack * 0.001f * (float)( A_TBL - 1 ) )];<br>
> +    gr = as[f_round( release * 0.001f * (float)( A_TBL - 1 ) )];<br>
 Are thoses as[] accesses guaranted to be insided bound ? (besides, with float<br>
I am a bit paranoiac).<br>
<br>
> +    rs = ( ratio - 1.0f ) / ratio;<br>
> +    mug = db2lin( makeup_gain, p_sys );<br>
> +    knee_min = db2lin( threshold - knee, p_sys );<br>
> +    knee_max = db2lin( threshold + knee, p_sys );<br>
> +    ef_a = ga * 0.25f;<br>
> +    ef_ai = 1.0f - ef_a;<br>
> +<br>
> +    for ( pos = 0; pos < i_samples; pos++ )<br>
> +    {<br>
> +        float lev_in = fabs( p_in[0] );<br>
> +        for( pos_chan = 1; pos_chan < i_channels; pos_chan++ )<br>
> +        {<br>
> +            lev_in = f_max( lev_in, fabs( p_in[pos_chan] ) );<br>
> +        }<br>
> +        sum += lev_in * lev_in;<br>
> +<br>
> +        if ( amp > env_rms )<br>
> +        {<br>
> +            env_rms = env_rms * ga + amp * ( 1.0f - ga );<br>
> +        }<br>
> +        else<br>
> +        {<br>
> +            env_rms = env_rms * gr + amp * ( 1.0f - gr );<br>
> +        }<br>
> +        round_to_zero( &env_rms );<br>
> +        if ( lev_in > env_peak )<br>
> +        {<br>
> +            env_peak = env_peak * ga + lev_in * ( 1.0f - ga );<br>
> +        }<br>
> +        else<br>
> +        {<br>
> +            env_peak = env_peak * gr + lev_in * ( 1.0f - gr );<br>
> +        }<br>
> +        round_to_zero( &env_peak );<br>
> +        if ( ( count++ & 3 ) == 3 )<br>
> +        {<br>
> +            amp = rms_env_process( rms, sum * 0.25f );<br>
> +            sum = 0.0f;<br>
> +            if ( isnan( env_rms ) )<br>
> +            {<br>
> +                /* This can happen sometimes, but I don't know why */<br>
> +                env_rms = 0.0f;<br>
 Maybe because your env_rms equation diverges (easy with float) or one<br>
of the other numbers used is a NaN.<br>
> +            }<br>
> +<br>
> +            env = LIN_INTERP( rms_peak, env_rms, env_peak );<br>
> +<br>
> +            if ( env <= knee_min )<br>
> +            {<br>
> +                gain_t = 1.0f;<br>
> +            }<br>
> +            else if ( env < knee_max )<br>
> +            {<br>
> +                const float x = -( threshold - knee<br>
> +                                             - lin2db( env, p_sys ) ) / knee;<br>
> +                gain_t = db2lin( -knee * rs * x * x * 0.25f, p_sys );<br>
> +            }<br>
> +            else<br>
> +            {<br>
> +                gain_t = db2lin( ( threshold - lin2db( env, p_sys ) ) * rs,<br>
> +                                 p_sys );<br>
> +            }<br>
> +        }<br>
> +<br>
> +        gain = gain * ef_a + gain_t * ef_ai;<br>
> +<br>
> +        for ( pos_chan = 0; pos_chan < i_channels; pos_chan++ )<br>
> +        {<br>
> +            p_out[pos_chan] = p_in[pos_chan] * gain * mug;<br>
> +        }<br>
> +<br>
> +        p_in  += i_channels;<br>
> +        p_out += i_channels;<br>
 You may also one only one pointer (but not needed).<br>
<br>
> +    }<br>
> +<br>
> +    p_sys->sum = sum;<br>
> +    p_sys->amp = amp;<br>
> +    p_sys->gain = gain;<br>
> +    p_sys->gain_t = gain_t;<br>
> +    p_sys->env = env;<br>
> +    p_sys->env_rms = env_rms;<br>
> +    p_sys->env_peak = env_peak;<br>
> +    p_sys->count = count;<br>
> +<br>
> +    p_sys->amplitude = lin2db( env, p_sys );<br>
> +    p_sys->gain_red = lin2db( gain, p_sys );<br>
> +<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return p_in_buf;<br>
> +}<br>
> +<br>
> +/*****************************************************************************<br>
> + * Callback functions<br>
> + *****************************************************************************/<br>
> +<br>
> +static int RMSPeakCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                            vlc_value_t oldval, vlc_value_t newval,<br>
> +                            void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 0.0 )<br>
> +    {<br>
> +        newval.f_float = 0.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 1.0 )<br>
> +    {<br>
> +        newval.f_float = 1.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->rms_peak = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int AttackCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                           vlc_value_t oldval, vlc_value_t newval,<br>
> +                           void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 1.5 )<br>
> +    {<br>
> +        newval.f_float = 1.5;<br>
> +    }<br>
> +    else if ( newval.f_float > 400.0 )<br>
> +    {<br>
> +        newval.f_float = 400.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->attack = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int ReleaseCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                            vlc_value_t oldval, vlc_value_t newval,<br>
> +                            void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 2.0 )<br>
> +    {<br>
> +        newval.f_float = 2.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 800.0 )<br>
> +    {<br>
> +        newval.f_float = 800.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->release = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int ThresholdCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                              vlc_value_t oldval, vlc_value_t newval,<br>
> +                              void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < -30.0 )<br>
> +    {<br>
> +        newval.f_float = -30.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 0.0 )<br>
> +    {<br>
> +        newval.f_float = 0.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->threshold = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int RatioCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                          vlc_value_t oldval, vlc_value_t newval,<br>
> +                          void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 1.0 )<br>
> +    {<br>
> +        newval.f_float = 1.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 20.0 )<br>
> +    {<br>
> +        newval.f_float = 20.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->ratio = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int KneeCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                         vlc_value_t oldval, vlc_value_t newval,<br>
> +                         void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 1.0 )<br>
> +    {<br>
> +        newval.f_float = 1.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 10.0 )<br>
> +    {<br>
> +        newval.f_float = 10.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->knee = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
> +<br>
> +static int MakeupGainCallback( vlc_object_t *p_this, char const *psz_cmd,<br>
> +                               vlc_value_t oldval, vlc_value_t newval,<br>
> +                               void * p_data )<br>
> +{<br>
> +    VLC_UNUSED(p_this); VLC_UNUSED(psz_cmd); VLC_UNUSED(oldval);<br>
> +    filter_sys_t *p_sys = p_data;<br>
> +<br>
> +    if ( newval.f_float < 0.0 )<br>
> +    {<br>
> +        newval.f_float = 0.0;<br>
> +    }<br>
> +    else if ( newval.f_float > 24.0 )<br>
> +    {<br>
> +        newval.f_float = 24.0;<br>
> +    }<br>
> +<br>
> +    vlc_mutex_lock( &p_sys->lock );<br>
> +    p_sys->makeup_gain = newval.f_float;<br>
> +    vlc_mutex_unlock( &p_sys->lock );<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
 Doing a small float cliping function would reduce the code (and so improve<br>
readability) IMHO.<br>
 You may also create only one callback and check the value of psz_cmd inside<br>
but that depends on one preferences.<br>
<br>
<br>
Regards,<br>
<font color="#888888"><br>
--<br>
fenrir<br>
</font><div><div></div><div><br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="http://mailman.videolan.org/listinfo/vlc-devel" target="_blank">http://mailman.videolan.org/listinfo/vlc-devel</a><br>
</div></div></blockquote></div><br>
</div></div></div><br>