[vlc-devel] [PATCH 1/2] Anti-flicker filter initial version

[Rémi Duraffort]

Some small remarks.

> +/*****************************************************************************
> + * antiflicker.c : antiflicker video effect plugin for vlc
> + *****************************************************************************
> + * Copyright (C) 2000-2008 the VideoLAN team
Wrong date.

> + * $Id: 
> + *
> + * Authors: S Dharani Prabhu
> + * Email:   dharani.prabhu.s at gmail.com
We usually write "Authors: name <email>

> +/*****************************************************************************
> + * filter_sys_t: Distort video output method descriptor
> + *****************************************************************************
> + * This structure is part of the video output thread descriptor.
> + * It describes the Distort specific properties of an output thread.
> + *****************************************************************************/
> +struct filter_sys_t
> +{
> +    vlc_mutex_t lock;
> +    uint8_t i_window_size;
> +    uint8_t i_softening;
> +    uint8_t* p_old_data;
> +    uint32_t ia_luminance_data[MAX_WINDOW_SZ];
> +};
Not really important here, but that's better to have bigger struc
members at the begining and smaller ones at the end (help avoiding hole
in the structure).

> +/*****************************************************************************
> + * Destroy: destroy Distort video thread output method
> + *****************************************************************************
> + * Terminate an output method created by DistortCreateOutputMethod
> + *****************************************************************************/
> +static void Destroy( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t *)p_this;
> +
> +    var_DelCallback(p_filter,FILTER_PREFIX "winsz",
> +        AntiFlickerCallback, p_filter->p_sys);
> +    var_DelCallback(p_filter,FILTER_PREFIX "sftn",
> +        AntiFlickerCallback, p_filter->p_sys);
> +    vlc_mutex_destroy( &p_filter->p_sys->lock );

p_filter->p_sys->p_old_data is never freed AFAIK.

> +    free( p_filter->p_sys );
> +}


> +/*****************************************************************************
> + * Filter: adjust the luminance value and renders
> + *****************************************************************************
> + * The function uses moving average of past frames to adjust the luminance 
> + * of current frame also applies temporaral smoothening if enabled.
> + *****************************************************************************/
> +static picture_t *Filter( filter_t *p_filter, picture_t *p_pic )
> +{
> [...]
> +    /******* Temporal softening phase. Adapted from code by Steven Don ******/ 
> +    uint8_t *src1, *src2;
> +    long diff, ofs, sum;
> +
> +    if( !p_filter->p_sys->p_old_data )
> +    {
> +        p_filter->p_sys->p_old_data = 
> +          malloc( (i_num_lines * i_out_pitch + i_num_cols) * sizeof(uint8_t) );
> +        memset( p_filter->p_sys->p_old_data, 0, 
> +                  (i_num_lines * i_out_pitch + i_num_cols) * sizeof(uint8_t) );
> +        
You can use calloc instead of malloc+memset(0)
Moreover I cannot find where p_old_data is freed.


I cannot judge for the algorithm itself.


Best regards

-- 
Rémi Duraffort | ivoire
http://ivoire.dinauz.org/blog/