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

[Laurent Aimar]

Hi,

> +struct filter_sys_t
> +{
> +    int ia_luminance_data[MAX_WINDOW_SZ];
> +    vlc_mutex_t lock;
> +    uint8_t i_window_size;
> +    uint8_t i_softening;
 Using int (or unsigned maybe) would be more natural (same in Filter() function).
> +    uint8_t* p_old_data;
> +};
> +

> +/*****************************************************************************
> + * Create: allocates Distort video thread output method
> + *****************************************************************************
> + * This function allocates and initializes a Distort vout method.
> + *****************************************************************************/
> +static int Create( vlc_object_t *p_this )
> +{
> +    filter_t *p_filter = (filter_t *)p_this;
> +
> +    /* Allocate structure */
> +    p_filter->p_sys = malloc( sizeof( *(p_filter->p_sys) ) );
> +    if( p_filter->p_sys == NULL )
> +        return VLC_ENOMEM;
> +
> +    p_filter->pf_video_filter = Filter;
> +
> +    /* Initialize the arguments */
> +    p_filter->p_sys->i_window_size = var_CreateGetIntegerCommand( p_filter,
> +                                               FILTER_PREFIX "window-size" );
> +    p_filter->p_sys->i_softening = var_CreateGetIntegerCommand( p_filter,
> +                                               FILTER_PREFIX "softening-size" );
> +
> +    p_filter->p_sys->p_old_data = calloc( p_filter->fmt_in.video.i_width *
> +     (p_filter->fmt_in.video.i_height+1),sizeof(*p_filter->p_sys->p_old_data) );
> +
> +    memset( p_filter->p_sys->ia_luminance_data, 0, 
> +                    sizeof(p_filter->p_sys->ia_luminance_data) );
> +    p_filter->p_sys->ia_luminance_data[p_filter->p_sys->i_window_size - 1] = 256;
> +  
> +    vlc_mutex_init( &p_filter->p_sys->lock ); 
> +    var_AddCallback(p_filter,FILTER_PREFIX "window-size",
> +        AntiFlickerCallback, p_filter->p_sys);
> +    var_AddCallback(p_filter,FILTER_PREFIX "softening-size",
> +        AntiFlickerCallback, p_filter->p_sys);
> + 
> +    return VLC_SUCCESS;
> +}
 I didn't saw it the first time, but you need to check the video type and only
accept the ones you support (probably only planar YUV).

> +/*****************************************************************************
> + * GetLuminanceAvg : The funtion returns the luminance average for a picture
> + *****************************************************************************/
> +static int GetLuminanceAvg( picture_t * p_pic,uint x_offset,uint y_offset )
> +{
> +    uint8_t *p_yplane_out = p_pic->p[Y_PLANE].p_pixels;
> +
> +    int i_num_lines = p_pic->p[Y_PLANE].i_visible_lines;
> +    int i_num_cols = p_pic->p[Y_PLANE].i_visible_pitch;
> +    int i_in_pitch = p_pic->p[Y_PLANE].i_pitch;
> +   
> +    if( i_num_lines == 0 || i_num_cols == 0 )
> +        return 0;
> +
> +    uint32_t lum_sum = 0;
 Using 'unsigned' would be more natural here.
> +    for( int i_line = y_offset ; i_line < i_num_lines ; ++i_line )
> +    {
> +        for( int i_col = x_offset ; i_col < i_num_cols; ++i_col )
> +        {
> +            lum_sum += p_yplane_out[i_line*i_in_pitch+i_col];
> +        }   
> +    }
> +    return lum_sum / ( i_num_lines * i_num_cols );
It might improve the quality to round to the nearest integer the returned
value, no?
> +}
 I am sorry, but I mix up between fields of video_format_t and fields of plane_t.
Could you just revert that change (ie x/y_offset are always 0 here).

> +/*****************************************************************************
> + * 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 )
> +{
> +    if( !p_pic ) return NULL;
> +
> +    picture_t *p_outpic = filter_NewPicture( p_filter );
> +    if( !p_outpic )
> +    {
> +        picture_Release( p_pic );
> +        return NULL;
> +    }
> +
> +    /****************** Get variables *************************/
> +
> +    uint8_t i_window_size;
> +    uint8_t i_softening;
unsigned or int.
> +
> +    vlc_mutex_lock( &p_filter->p_sys->lock );
> +    i_window_size = p_filter->p_sys->i_window_size;
> +    i_softening = p_filter->p_sys->i_softening;
> +    vlc_mutex_unlock( &p_filter->p_sys->lock );
> +
> +    uint8_t *p_yplane_in = p_pic->p[Y_PLANE].p_pixels;
> +    uint8_t *p_yplane_out = p_outpic->p[Y_PLANE].p_pixels;
> +    bool scene_changed = false;
> +
> +    int16_t i_num_lines = p_pic->p[Y_PLANE].i_visible_lines;
> +    int16_t i_num_cols = p_pic->p[Y_PLANE].i_visible_pitch;
unsigned or int.

> +    int16_t i_in_pitch = p_pic->p[Y_PLANE].i_pitch;
> +    int16_t i_out_pitch = p_outpic->p[Y_PLANE].i_pitch;
int for pitches.
> +
> +    /******** Get the luminance average for the current picture ********/
> +    int lum_avg = GetLuminanceAvg(p_pic,p_filter->fmt_in.video.i_x_offset,
> +        p_filter->fmt_in.video.i_y_offset );
> +    lum_avg = lum_avg > 255 ? 255 : lum_avg;
 I think there's no need to check against 255, by design it will be
bound to 0-255.
> +
> +    /*Identify as scene change if the luminance average deviates
> +     more than the threshold value or if it is the first frame*/
> +
> +    if( abs(lum_avg - p_filter->p_sys->
> +        ia_luminance_data[i_window_size - 1]) > SCENE_CHANGE_THRESHOLD 
> +        || p_filter->p_sys->ia_luminance_data[i_window_size - 1] == 256)
> +    {
> +        scene_changed = true;
> +    }
> +
> +    /******* Compute the adjustment factor using moving average ********/
> +    float scale = 1.0;
> +
> +    if ( scene_changed )
> +    {
> +        //reset the luminance data
> +        for (int i = 0; i < i_window_size; ++i) 
> +            p_filter->p_sys->ia_luminance_data[i] = lum_avg;
> +    }
> +    else
> +    {
> +        for (int i = 0; i < i_window_size-1 ; ++i) 
> +            p_filter->p_sys->ia_luminance_data[i] = 
> +                           p_filter->p_sys->ia_luminance_data[i+1];
> +
> +        p_filter->p_sys->ia_luminance_data[i_window_size - 1] = lum_avg;
> +        
> +        if (lum_avg > 0)
> +        {
> +             scale = 1.0f/lum_avg;
> +             double filt = 0;
> +             for (int i = 0; i < i_window_size; i++)
> +                  filt += (float) p_filter->p_sys->ia_luminance_data[i];
> +             scale *= (filt/i_window_size);
You could merged the 2 scale modifications (easier to understand).
> +        }
> +    }
> +
> +
> +    if( scene_changed )
> +        plane_CopyPixels( &p_outpic->p[Y_PLANE], &p_pic->p[Y_PLANE] );
> +    else
> +    {
> +        /******* Apply the adjustment factor to each pixel on Y_PLANE ********/
> +        uint8_t shift = 8; 
> +        int scale_num = __MIN(scale,255) * ( 1 << shift );
> +
> +        for( int16_t i_line = 0 ; i_line < i_num_lines ; i_line++ )
> +        {
> +            for( int16_t i_col = 0; i_col < i_num_cols  ; i_col++ )
unsigned or int (same as i_num_lines/i_num_cols).
> +            {
> +                uint8_t pixel_data = p_yplane_in[i_line*i_in_pitch+i_col];
> +                int pixel_val = ( scale_num * pixel_data + 
> +                       (1<<(shift -1)) ) >> shift;
> +                p_yplane_out[i_line*i_out_pitch+i_col] = 
> +                       (pixel_val>255) ? 255:pixel_val;
> +            }
> +        }
> +    }
 You could merged the 2 "if/else" blocks.

> +   
> +    /***************** Copy the UV plane as such *****************************/
> +    plane_CopyPixels( &p_outpic->p[U_PLANE], &p_pic->p[U_PLANE] );
> +    plane_CopyPixels( &p_outpic->p[V_PLANE], &p_pic->p[V_PLANE] );
> +
> +    if (scene_changed || i_softening == 0)
> +    {
> +       return CopyInfoAndRelease( p_outpic, p_pic );
> +    }
> +
> +    /******* Temporal softening phase. Adapted from code by Steven Don ******/ 
> +    uint8_t *src1, *src2;
> +    long diff, ofs, sum;
int, you could alo move diff and sum inside the loop
> +
> +    ofs = (i_num_lines * i_out_pitch + i_num_cols);
> +    src1 = p_outpic->p[Y_PLANE].p_pixels;
> +    src2 = p_filter->p_sys->p_old_data;
> +
> +    do
> +    {
> +        diff = abs(*src1 - *src2);
> +        if (diff < i_softening)
> +        {
> +            if (diff > (i_softening >> 1))
> +            {
> +                sum = *src1 + *src1 + *src2;
> +                *src2 = sum / 3;
> +            }
> +        }
> +        else
> +        {
> +            *src2 = *src1;
> +        }
> +        *src1 = *src2;
> +        src1++; src2++;
> +    } while (--ofs);
 That does not work, you need to iterate over lines/cols to correctly take care
that p_outpic->p[Y_PLANE].p_pixels uses p_outpic->p[Y_PLANE].i_pitch and
p_filter->p_sys->p_old_data uses p_filter->fmt_out.i_width as pitch.

> +
> +    return CopyInfoAndRelease( p_outpic, p_pic );
> +}

Sorry for the few points I missed the first time.

Regards,

-- 
fenrir