[vlc-devel] [PATCH 2/4] Adds the compressor module to VLC

[Laurent Aimar]

Hi,

On Thu, Jul 01, 2010 at 03:33:00AM -0500, Ronald Wright wrote:
> +vlc_module_begin()
> +    set_shortname( _("Compressor") )
> +    set_description( _("Dynamic range compressor") )
> +    set_capability( "audio filter", 1 )
 It must be 0 as explained in reply to another of your patches.

> +    add_float( "compressor-lo-pregain", 0.0, NULL, LO_TEXT,
> +               LO_LONGTEXT, false )
> +    add_float( "compressor-mid-pregain", 0.0, NULL, MID_TEXT,
> +               MID_LONGTEXT, false )
> +    add_float( "compressor-hi-pregain", 0.0, NULL, HI_TEXT,
> +               HI_LONGTEXT, false )

 I don't see why you have to do the equalizer part inside your module.
I think that normalizing the audio and applying equalization are two
completly independant tasks, and as such should be done by two
independant plugin. Furthermore, we already have 2 equalizers in VLC.

> +static int Open( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t*)p_this;
> +    vlc_object_t *p_aout = p_filter->p_parent;
> +    float f_sample_rate = p_filter->fmt_in.audio.i_rate;
> +    struct filter_sys_t *p_sys;
> +    float f_num;
> +
> +    if( p_filter->fmt_in.audio.i_format != VLC_CODEC_FL32 ||
> +        p_filter->fmt_out.audio.i_format != VLC_CODEC_FL32 )
> +    {
> +        p_filter->fmt_in.audio.i_format = VLC_CODEC_FL32;
> +        p_filter->fmt_out.audio.i_format = VLC_CODEC_FL32;
> +        msg_Warn( p_filter, "bad input or output format" );
> +        return VLC_EGENERIC;
> +    }
> +    if( !AOUT_FMTS_SIMILAR( &p_filter->fmt_in.audio,
> +                            &p_filter->fmt_out.audio ) )
> +    {
> +        p_filter->fmt_out.audio = p_filter->fmt_in.audio;
> +        msg_Warn( p_filter, "input and output formats are not similar" );
> +        return VLC_EGENERIC;
> +    }
> +
> +    /* Initialize the filter parameter structure */
> +    p_sys = p_filter->p_sys = calloc( 1, sizeof(*p_sys) );
> +    if( !p_sys )
> +    {
> +        return VLC_ENOMEM;
> +    }
> +
> +    /* Initialize the attack lookup table */
> +    p_sys->pf_as[0] = 1.0f;
> +    for( int i = 1; i < A_TBL; i++ )
> +    {
> +        p_sys->pf_as[i] = expf( -1.0f / ( f_sample_rate * i / A_TBL ) );
> +    }
> +
> +    /* Calculate the RMS and lookahead sizes from the sample rate */
> +    f_num = 0.01f * f_sample_rate;
> +    p_sys->rms.i_count = Round( Clamp( 0.5f * f_num, 1.0f, RMS_BUF_SIZE ) );
> +    p_sys->la.i_count = Round( Clamp( f_num, 1.0f, LOOKAHEAD_SIZE ) );
> +
> +    /* Initialize decibel lookup tables */
> +    DbInit( p_sys );
> +
> +    /* Restore the last saved settings */
> +    p_sys->f_lo_pregain  =
> +           var_CreateGetFloat( p_aout, "compressor-lo-pregain" );
> +    p_sys->f_mid_pregain =
> +           var_CreateGetFloat( p_aout, "compressor-mid-pregain" );
> +    p_sys->f_hi_pregain  =
> +           var_CreateGetFloat( p_aout, "compressor-hi-pregain" );
> +    p_sys->f_rms_peak    = var_CreateGetFloat( p_aout, "compressor-rms-peak" );
> +    p_sys->f_attack      = var_CreateGetFloat( p_aout, "compressor-attack" );
> +    p_sys->f_release     = var_CreateGetFloat( p_aout, "compressor-release" );
> +    p_sys->f_threshold   = var_CreateGetFloat( p_aout, "compressor-threshold" );
> +    p_sys->f_ratio       = var_CreateGetFloat( p_aout, "compressor-ratio" );
> +    p_sys->f_knee        = var_CreateGetFloat( p_aout, "compressor-knee" );
> +    p_sys->f_makeup_gain =
> +           var_CreateGetFloat( p_aout, "compressor-makeup-gain" );
> +
> +    /* Initialize the mutex */
> +    vlc_mutex_init( &p_sys->lock );
> +
> +    /* Add our own callbacks */
> +    var_AddCallback( p_aout, "compressor-lo-pregain", LoPregainCallback,
> +                     p_sys );
> +    var_AddCallback( p_aout, "compressor-mid-pregain", MidPregainCallback,
> +                     p_sys );
> +    var_AddCallback( p_aout, "compressor-hi-pregain", HiPregainCallback,
> +                     p_sys );
> +    var_AddCallback( p_aout, "compressor-rms-peak", RMSPeakCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-attack", AttackCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-release", ReleaseCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-threshold", ThresholdCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-ratio", RatioCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-knee", KneeCallback, p_sys );
> +    var_AddCallback( p_aout, "compressor-makeup-gain", MakeupGainCallback,
> +                     p_sys );
> +
> +    /* Set the filter function */
> +    p_filter->pf_audio_filter = DoWork;
> +
> +    /* At this stage, we are ready! */
> +    msg_Dbg( p_filter, "compressor successfully initialized" );
> +    return VLC_SUCCESS;
> +}
> +
> +/*****************************************************************************
> + * Close: destroy interface
> + *****************************************************************************/
> +
> +static void Close( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t*)p_this;
> +    vlc_object_t *p_aout = p_filter->p_parent;
> +    struct filter_sys_t *p_sys = p_filter->p_sys;
> +
> +    /* Remove our callbacks */
> +    var_DelCallback( p_aout, "compressor-rms-peak", RMSPeakCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-attack", AttackCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-release", ReleaseCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-threshold", ThresholdCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-ratio", RatioCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-knee", KneeCallback, p_sys );
> +    var_DelCallback( p_aout, "compressor-makeup-gain", MakeupGainCallback,
> +                     p_sys );
 I think you forgot some deletes.
Maybe creating a table with the variables names and the associated callbacks
would  simplify it ? Otherwise, adding the missing one is enough.

Regards,

-- 
fenrir