Hi,<div><br></div><div>    I have attached the patch addressing the review comments mentioned below.<span class="Apple-style-span" style="border-collapse: collapse; font-family: arial, sans-serif; font-size: 13px; "> I have developed the video filter based on virtualdub. Pls see <a href="http://neuron2.net/deflick/flick.html" target="_blank" style="color: rgb(0, 0, 204); ">http://neuron2.net/deflick/flick.html</a> for more details about the video filter and sample videos</span></div>
<div><br></div><div>-Dharani<br><br><div class="gmail_quote">On Sun, Apr 3, 2011 at 1:18 PM, Rémi Duraffort <span dir="ltr"><<a href="mailto:ivoire@videolan.org">ivoire@videolan.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Some small remarks.<br>
<div class="im"><br>
> +/*****************************************************************************<br>
> + * antiflicker.c : antiflicker video effect plugin for vlc<br>
> + *****************************************************************************<br>
> + * Copyright (C) 2000-2008 the VideoLAN team<br>
</div>Wrong date.<br>
<div class="im"><br>
> + * $Id:<br>
> + *<br>
> + * Authors: S Dharani Prabhu<br>
> + * Email:   <a href="mailto:dharani.prabhu.s@gmail.com">dharani.prabhu.s@gmail.com</a><br>
</div>We usually write "Authors: name <email><br>
<div class="im"><br>
> +/*****************************************************************************<br>
> + * filter_sys_t: Distort video output method descriptor<br>
> + *****************************************************************************<br>
> + * This structure is part of the video output thread descriptor.<br>
> + * It describes the Distort specific properties of an output thread.<br>
> + *****************************************************************************/<br>
> +struct filter_sys_t<br>
> +{<br>
> +    vlc_mutex_t lock;<br>
> +    uint8_t i_window_size;<br>
> +    uint8_t i_softening;<br>
> +    uint8_t* p_old_data;<br>
> +    uint32_t ia_luminance_data[MAX_WINDOW_SZ];<br>
> +};<br>
</div>Not really important here, but that's better to have bigger struc<br>
members at the begining and smaller ones at the end (help avoiding hole<br>
in the structure).<br>
<div class="im"><br>
> +/*****************************************************************************<br>
> + * Destroy: destroy Distort video thread output method<br>
> + *****************************************************************************<br>
> + * Terminate an output method created by DistortCreateOutputMethod<br>
> + *****************************************************************************/<br>
> +static void Destroy( vlc_object_t *p_this )<br>
> +{<br>
> +    filter_t *p_filter = (filter_t *)p_this;<br>
> +<br>
> +    var_DelCallback(p_filter,FILTER_PREFIX "winsz",<br>
> +        AntiFlickerCallback, p_filter->p_sys);<br>
> +    var_DelCallback(p_filter,FILTER_PREFIX "sftn",<br>
> +        AntiFlickerCallback, p_filter->p_sys);<br>
> +    vlc_mutex_destroy( &p_filter->p_sys->lock );<br>
<br>
</div>p_filter->p_sys->p_old_data is never freed AFAIK.<br>
<div class="im"><br>
> +    free( p_filter->p_sys );<br>
> +}<br>
<br>
<br>
</div><div class="im">> +/*****************************************************************************<br>
> + * Filter: adjust the luminance value and renders<br>
> + *****************************************************************************<br>
> + * The function uses moving average of past frames to adjust the luminance<br>
> + * of current frame also applies temporaral smoothening if enabled.<br>
> + *****************************************************************************/<br>
> +static picture_t *Filter( filter_t *p_filter, picture_t *p_pic )<br>
> +{<br>
</div>> [...]<br>
<div class="im">> +    /******* Temporal softening phase. Adapted from code by Steven Don ******/<br>
> +    uint8_t *src1, *src2;<br>
> +    long diff, ofs, sum;<br>
> +<br>
> +    if( !p_filter->p_sys->p_old_data )<br>
> +    {<br>
> +        p_filter->p_sys->p_old_data =<br>
> +          malloc( (i_num_lines * i_out_pitch + i_num_cols) * sizeof(uint8_t) );<br>
> +        memset( p_filter->p_sys->p_old_data, 0,<br>
> +                  (i_num_lines * i_out_pitch + i_num_cols) * sizeof(uint8_t) );<br>
> +<br>
</div>You can use calloc instead of malloc+memset(0)<br>
Moreover I cannot find where p_old_data is freed.<br>
<br>
<br>
I cannot judge for the algorithm itself.<br>
<br>
<br>
Best regards<br>
<font color="#888888"><br>
--<br>
Rémi Duraffort | ivoire<br>
<a href="http://ivoire.dinauz.org/blog/" target="_blank">http://ivoire.dinauz.org/blog/</a><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>
</font></blockquote></div><br></div>