Attached the changes ...<div><br></div><div>-Dharani<br><br><div class="gmail_quote">On Sun, May 8, 2011 at 5:48 PM, Laurent Aimar <span dir="ltr"><<a href="mailto:fenrir@elivagar.org">fenrir@elivagar.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Hi,<br>
<br>
> +struct filter_sys_t<br>
> +{<br>
> +    int ia_luminance_data[MAX_WINDOW_SZ];<br>
> +    vlc_mutex_t lock;<br>
> +    uint8_t i_window_size;<br>
> +    uint8_t i_softening;<br>
 Using int (or unsigned maybe) would be more natural (same in Filter() function).<br>
<div class="im">> +    uint8_t* p_old_data;<br>
</div>> +};<br>
<div class="im">> +<br>
<br>
> +/*****************************************************************************<br>
> + * Create: allocates Distort video thread output method<br>
> + *****************************************************************************<br>
> + * This function allocates and initializes a Distort vout method.<br>
> + *****************************************************************************/<br>
> +static int Create( vlc_object_t *p_this )<br>
> +{<br>
> +    filter_t *p_filter = (filter_t *)p_this;<br>
> +<br>
> +    /* Allocate structure */<br>
</div>> +    p_filter->p_sys = malloc( sizeof( *(p_filter->p_sys) ) );<br>
<div class="im">> +    if( p_filter->p_sys == NULL )<br>
> +        return VLC_ENOMEM;<br>
> +<br>
> +    p_filter->pf_video_filter = Filter;<br>
> +<br>
> +    /* Initialize the arguments */<br>
> +    p_filter->p_sys->i_window_size = var_CreateGetIntegerCommand( p_filter,<br>
</div>> +                                               FILTER_PREFIX "window-size" );<br>
<div class="im">> +    p_filter->p_sys->i_softening = var_CreateGetIntegerCommand( p_filter,<br>
</div>> +                                               FILTER_PREFIX "softening-size" );<br>
> +<br>
> +    p_filter->p_sys->p_old_data = calloc( p_filter->fmt_in.video.i_width *<br>
> +     (p_filter->fmt_in.video.i_height+1),sizeof(*p_filter->p_sys->p_old_data) );<br>
<div class="im">> +<br>
> +    memset( p_filter->p_sys->ia_luminance_data, 0,<br>
> +                    sizeof(p_filter->p_sys->ia_luminance_data) );<br>
</div>> +    p_filter->p_sys->ia_luminance_data[p_filter->p_sys->i_window_size - 1] = 256;<br>
<div class="im">> +<br>
> +    vlc_mutex_init( &p_filter->p_sys->lock );<br>
</div>> +    var_AddCallback(p_filter,FILTER_PREFIX "window-size",<br>
<div class="im">> +        AntiFlickerCallback, p_filter->p_sys);<br>
</div>> +    var_AddCallback(p_filter,FILTER_PREFIX "softening-size",<br>
<div class="im">> +        AntiFlickerCallback, p_filter->p_sys);<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +}<br>
</div> I didn't saw it the first time, but you need to check the video type and only<br>
accept the ones you support (probably only planar YUV).<br>
<div class="im"><br>
> +/*****************************************************************************<br>
> + * GetLuminanceAvg : The funtion returns the luminance average for a picture<br>
> + *****************************************************************************/<br>
</div>> +static int GetLuminanceAvg( picture_t * p_pic,uint x_offset,uint y_offset )<br>
<div class="im">> +{<br>
> +    uint8_t *p_yplane_out = p_pic->p[Y_PLANE].p_pixels;<br>
> +<br>
> +    int i_num_lines = p_pic->p[Y_PLANE].i_visible_lines;<br>
> +    int i_num_cols = p_pic->p[Y_PLANE].i_visible_pitch;<br>
> +    int i_in_pitch = p_pic->p[Y_PLANE].i_pitch;<br>
</div>> +<br>
> +    if( i_num_lines == 0 || i_num_cols == 0 )<br>
> +        return 0;<br>
<div class="im">> +<br>
> +    uint32_t lum_sum = 0;<br>
</div> Using 'unsigned' would be more natural here.<br>
> +    for( int i_line = y_offset ; i_line < i_num_lines ; ++i_line )<br>
> +    {<br>
> +        for( int i_col = x_offset ; i_col < i_num_cols; ++i_col )<br>
<div class="im">> +        {<br>
> +            lum_sum += p_yplane_out[i_line*i_in_pitch+i_col];<br>
> +        }<br>
> +    }<br>
</div>> +    return lum_sum / ( i_num_lines * i_num_cols );<br>
It might improve the quality to round to the nearest integer the returned<br>
value, no?<br>
> +}<br>
 I am sorry, but I mix up between fields of video_format_t and fields of plane_t.<br>
Could you just revert that change (ie x/y_offset are always 0 here).<br>
<div class="im"><br>
> +/*****************************************************************************<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>
> +    if( !p_pic ) return NULL;<br>
> +<br>
> +    picture_t *p_outpic = filter_NewPicture( p_filter );<br>
> +    if( !p_outpic )<br>
> +    {<br>
</div><div class="im">> +        picture_Release( p_pic );<br>
> +        return NULL;<br>
> +    }<br>
> +<br>
> +    /****************** Get variables *************************/<br>
> +<br>
</div>> +    uint8_t i_window_size;<br>
> +    uint8_t i_softening;<br>
unsigned or int.<br>
<div class="im">> +<br>
> +    vlc_mutex_lock( &p_filter->p_sys->lock );<br>
> +    i_window_size = p_filter->p_sys->i_window_size;<br>
> +    i_softening = p_filter->p_sys->i_softening;<br>
> +    vlc_mutex_unlock( &p_filter->p_sys->lock );<br>
> +<br>
> +    uint8_t *p_yplane_in = p_pic->p[Y_PLANE].p_pixels;<br>
> +    uint8_t *p_yplane_out = p_outpic->p[Y_PLANE].p_pixels;<br>
> +    bool scene_changed = false;<br>
> +<br>
> +    int16_t i_num_lines = p_pic->p[Y_PLANE].i_visible_lines;<br>
> +    int16_t i_num_cols = p_pic->p[Y_PLANE].i_visible_pitch;<br>
</div>unsigned or int.<br>
<div class="im"><br>
> +    int16_t i_in_pitch = p_pic->p[Y_PLANE].i_pitch;<br>
> +    int16_t i_out_pitch = p_outpic->p[Y_PLANE].i_pitch;<br>
</div>int for pitches.<br>
<div class="im">> +<br>
> +    /******** Get the luminance average for the current picture ********/<br>
</div>> +    int lum_avg = GetLuminanceAvg(p_pic,p_filter->fmt_in.video.i_x_offset,<br>
> +        p_filter->fmt_in.video.i_y_offset );<br>
> +    lum_avg = lum_avg > 255 ? 255 : lum_avg;<br>
 I think there's no need to check against 255, by design it will be<br>
bound to 0-255.<br>
> +<br>
> +    /*Identify as scene change if the luminance average deviates<br>
> +     more than the threshold value or if it is the first frame*/<br>
> +<br>
> +    if( abs(lum_avg - p_filter->p_sys-><br>
<div class="im">> +        ia_luminance_data[i_window_size - 1]) > SCENE_CHANGE_THRESHOLD<br>
</div>> +        || p_filter->p_sys->ia_luminance_data[i_window_size - 1] == 256)<br>
<div class="im">> +    {<br>
> +        scene_changed = true;<br>
> +    }<br>
> +<br>
> +    /******* Compute the adjustment factor using moving average ********/<br>
</div>> +    float scale = 1.0;<br>
<div class="im">> +<br>
> +    if ( scene_changed )<br>
> +    {<br>
> +        //reset the luminance data<br>
> +        for (int i = 0; i < i_window_size; ++i)<br>
> +            p_filter->p_sys->ia_luminance_data[i] = lum_avg;<br>
> +    }<br>
> +    else<br>
> +    {<br>
> +        for (int i = 0; i < i_window_size-1 ; ++i)<br>
> +            p_filter->p_sys->ia_luminance_data[i] =<br>
> +                           p_filter->p_sys->ia_luminance_data[i+1];<br>
> +<br>
> +        p_filter->p_sys->ia_luminance_data[i_window_size - 1] = lum_avg;<br>
> +<br>
> +        if (lum_avg > 0)<br>
> +        {<br>
> +             scale = 1.0f/lum_avg;<br>
> +             double filt = 0;<br>
> +             for (int i = 0; i < i_window_size; i++)<br>
</div>> +                  filt += (float) p_filter->p_sys->ia_luminance_data[i];<br>
> +             scale *= (filt/i_window_size);<br>
You could merged the 2 scale modifications (easier to understand).<br>
> +        }<br>
> +    }<br>
> +<br>
> +<br>
> +    if( scene_changed )<br>
> +        plane_CopyPixels( &p_outpic->p[Y_PLANE], &p_pic->p[Y_PLANE] );<br>
> +    else<br>
> +    {<br>
> +        /******* Apply the adjustment factor to each pixel on Y_PLANE ********/<br>
> +        uint8_t shift = 8;<br>
> +        int scale_num = __MIN(scale,255) * ( 1 << shift );<br>
<div class="im">> +<br>
> +        for( int16_t i_line = 0 ; i_line < i_num_lines ; i_line++ )<br>
> +        {<br>
> +            for( int16_t i_col = 0; i_col < i_num_cols  ; i_col++ )<br>
</div>unsigned or int (same as i_num_lines/i_num_cols).<br>
<div class="im">> +            {<br>
> +                uint8_t pixel_data = p_yplane_in[i_line*i_in_pitch+i_col];<br>
</div>> +                int pixel_val = ( scale_num * pixel_data +<br>
> +                       (1<<(shift -1)) ) >> shift;<br>
<div class="im">> +                p_yplane_out[i_line*i_out_pitch+i_col] =<br>
</div>> +                       (pixel_val>255) ? 255:pixel_val;<br>
> +            }<br>
> +        }<br>
> +    }<br>
 You could merged the 2 "if/else" blocks.<br>
<br>
> +<br>
<div class="im">> +    /***************** Copy the UV plane as such *****************************/<br>
> +    plane_CopyPixels( &p_outpic->p[U_PLANE], &p_pic->p[U_PLANE] );<br>
> +    plane_CopyPixels( &p_outpic->p[V_PLANE], &p_pic->p[V_PLANE] );<br>
> +<br>
> +    if (scene_changed || i_softening == 0)<br>
> +    {<br>
> +       return CopyInfoAndRelease( p_outpic, p_pic );<br>
> +    }<br>
> +<br>
> +    /******* Temporal softening phase. Adapted from code by Steven Don ******/<br>
> +    uint8_t *src1, *src2;<br>
> +    long diff, ofs, sum;<br>
</div>int, you could alo move diff and sum inside the loop<br>
<div class="im">> +<br>
> +    ofs = (i_num_lines * i_out_pitch + i_num_cols);<br>
> +    src1 = p_outpic->p[Y_PLANE].p_pixels;<br>
> +    src2 = p_filter->p_sys->p_old_data;<br>
> +<br>
> +    do<br>
> +    {<br>
> +        diff = abs(*src1 - *src2);<br>
> +        if (diff < i_softening)<br>
> +        {<br>
> +            if (diff > (i_softening >> 1))<br>
> +            {<br>
> +                sum = *src1 + *src1 + *src2;<br>
> +                *src2 = sum / 3;<br>
> +            }<br>
> +        }<br>
> +        else<br>
> +        {<br>
> +            *src2 = *src1;<br>
> +        }<br>
> +        *src1 = *src2;<br>
> +        src1++; src2++;<br>
> +    } while (--ofs);<br>
</div> That does not work, you need to iterate over lines/cols to correctly take care<br>
that p_outpic->p[Y_PLANE].p_pixels uses p_outpic->p[Y_PLANE].i_pitch and<br>
p_filter->p_sys->p_old_data uses p_filter->fmt_out.i_width as pitch.<br>
<div class="im"><br>
> +<br>
> +    return CopyInfoAndRelease( p_outpic, p_pic );<br>
> +}<br>
<br>
</div>Sorry for the few points I missed the first time.<br>
<div><div></div><div class="h5"><br>
Regards,<br>
<br>
--<br>
fenrir<br>
<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>
</div></div></blockquote></div><br></div>