[vlc-devel] [PATCH] Chorus Flanger audio filter

[Laurent Aimar]

Hi,

On Thu, May 14, 2009, Srikanth Raju wrote:
> +/**
> + * Open: initialize and create stuff
> + * @param p_this
> + */
> +static int Open( vlc_object_t *p_this )
> +{
> +    aout_filter_t *p_filter = (aout_filter_t*)p_this;
> +    aout_filter_sys_t *p_sys;
> +
> +    if ( !AOUT_FMTS_SIMILAR( &p_filter->input, &p_filter->output ) )
> +    {
> +        msg_Err( p_filter, "input and output formats are not similar" );
> +        return VLC_EGENERIC;
> +    }
> +
> +    if( p_filter->input.i_format != VLC_FOURCC('f','l','3','2' ) ||
> +        p_filter->output.i_format != VLC_FOURCC('f','l','3','2') )
> +    {
> +        p_filter->input.i_format = VLC_FOURCC('f','l','3','2');
> +        p_filter->output.i_format = VLC_FOURCC('f','l','3','2');
> +        msg_Warn( p_filter, "bad input or output format" );
> +    }
Please use the new VLC_CODEC_FL32 define for theses.

> +    p_sys->i_channels       = aout_FormatNbChannels( &p_filter->input );
> +    p_sys->f_delayTime      = config_GetFloat( p_this, "delay-time" );
> +    p_sys->f_sweepDepth     = config_GetFloat( p_this, "sweep-depth" );
> +    p_sys->f_sweepRate      = config_GetFloat( p_this, "sweep-rate" );
> +    p_sys->f_feedbackGain   = config_GetFloat( p_this, "feedback-gain" );
> +    p_sys->f_dryLevel       = config_GetFloat( p_this, "dry-mix" );
> +    p_sys->f_wetLevel       = config_GetFloat( p_this, "wet-mix" );
 You must not use config_Get* but var_CreateGetFloat.

> +    if( p_sys->f_sweepRate < 0.0 ){
> +        msg_Err( p_filter, "Sweep Rate is invalid" );
> +        free( p_sys );
> +        return VLC_EGENERIC;
> +    }
 and
> +    if( p_sys->i_bufferLength <= 0 )
> +    {
> +        msg_Err( p_filter, "Delay-time, Sample rate or Channels was incorrectly set. Hmmm..." );
> +        free(p_sys);
> +        return VLC_EGENERIC;
> +    }
 Do not mix two coding style ( {} and () placement/spacing )

(There are other place where coding style are mixed).

> +    if( p_sys->f_sweepDepth == 0.0 || p_filter->input.i_rate == 0.0 ) {
 I am not sure how to replace that, but testing float against a a specific
value is often wrong.

> +inline float small_value(){
> +    return ( 1.0 / 16777216.0 ); // allows for 2^-24, should be enough for 24-bit DACs at least :)
> +}
 Coding style + missing static.

> +inline void sanitize( float * f_value ){
> +    if ( fabs( *f_value ) < small_value() )
> +        *f_value = 0.0f;
> +}
 Same here.

Regards,

-- 
fenrir