[vlc-devel] Dynamic range compressor patch

[Laurent Aimar]

Hi,

On Sun, Jun 27, 2010 at 10:24:25AM -0500, Ron Wright wrote:
> I didn't like the delay in the attack and release settings for peak
> compression, so I have attached the updated compressor core patch that resolves
> the problem.
 A few remarks (but probably the last ones :)

> +typedef struct
> +{
> +    struct
> +    {
> +        float *chan_vals;
 float chan_vals[AOUT_CHAN_MAX]
will avoid the calloc/free and will also be faster (no pointer deferencings).

> +    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;
> +    }
 I think they could be merged (AOUT_FMTS_IDENTICAL will test the format too) but
as I see that it is always done in 2 steps in other modules, anyone know why ?

> +    p_sys = p_filter->p_sys = calloc( 1, sizeof(struct filter_sys_t) );
sizeof(*p_sys) is less dangerous IMHO.

> +    count = p_sys->count;
[...]
> +        if( ( count++ & 3 ) == 3 )
> +        {
[...]
> +        }
[...]
> +    p_sys->count = count;
 As it is uses only once, it might not be worth it to use a temporary
variable, but do as you prefer.

> +static float db2lin( float db, filter_sys_t * p_sys )
> +{
> +    float scale = ( db - DB_MIN ) * LIN_TABLE_SIZE / ( DB_MAX - DB_MIN );
> +    int base = f_round( scale - 0.5f );
> +    float ofs = scale - base;
> +    float *lin_data = p_sys->lin_data;
> +
> +    if( base < 1 )
> +    {
> +        return 0.0f;
> +    }
> +    else if( base > LIN_TABLE_SIZE - 3 )
> +    {
> +        return lin_data[LIN_TABLE_SIZE - 2];
> +    }
> +
> +#ifdef DB_DEFAULT_CUBE
> +    return cube_interp( ofs, lin_data[base - 1],
> +                             lin_data[base],
> +                             lin_data[base + 1],
> +                             lin_data[base + 2] );
> +#else
> +    return ( 1.0f - ofs ) * lin_data[base] + ofs * lin_data[base + 1];
> +#endif 
Trailing whitespace.

> +static void buffer_process( float * p_buf, int i_channels, float gain,
> +                            float mug, lookahead * la )
> +{
> +    for( int pos_chan = 0; pos_chan < i_channels; pos_chan++ )
> +    {
> +        float x = p_buf[pos_chan];
> +        p_buf[pos_chan] = la->buffer[la->pos].chan_vals[pos_chan] * gain * mug;
> +        la->buffer[la->pos].chan_vals[pos_chan] = x;
> +    }    
Trailing whitespace.
> +    la->pos = ( la->pos + 1 ) & ( LOOKAHEAD_SIZE - 1 );
> +}

 Otherwise, could you send the patches using git format-patch ? It is easier
to apply and allow to commit without loosing the authorship.

 Other than that, I don't have anything to add, good work !

Regards,

-- 
fenrir