[FFmpeg-devel] Trans.: a64multienc.c and drawutils.c optimisations

[Stefano Sabatini]

On date Wednesday 2011-12-28 04:35:04 +0100, yann.lepetitcorps at free.fr encoded:
> Selon yann.lepetitcorps at free.fr:

Please have a look at:
http://ffmpeg.org/developer.html#Submitting-patches-1

The important points are "avoid tabs, keep a consistent style, and
separate patches for separate changes".

[...]
> diff --git a/libavfilter/drawutils.c b/libavfilter/drawutils.c
> index bf308a1..8776920 100644
> --- a/libavfilter/drawutils.c
> +++ b/libavfilter/drawutils.c
> @@ -38,6 +38,44 @@ int ff_fill_rgba_map(uint8_t *rgba_map, enum PixelFormat pix_fmt)
>      return 0;
>  }
>  
> +void ff_memset8(char *dst, char val, int num)
> +{
> +    memset(dst, val, num);
> +}
> +
> +void ff_memset16(int16_t *dst, int16_t val, int num)
> +{
> +    int i;
> +
> +    for(i=0;i<num;i++)
> +        *dst++ = val;
> +}
> +
> +void ff_memset32(int32_t *dst, int32_t val, int num)
> +{
> +    int i;
> +
> +    for(i=0;i<num;i++)
> +        *dst++ = val;
> +}
> +
> +
> +void ff_memset24(char *dst, char *src, int num)
> +{
> +    int i;
> +
> +    for (i = 0; i < num; i++, dst += 3)
> +        memcpy(dst, src, 3);
> +}
> +
> +void ff_memset_sized(char *dst, char *src, int num, int stepsize)
> +{
> +    int i;
> +
> +    for (i = 0; i < num; i++, dst += stepsize)
> +        memcpy(dst, src, stepsize);
> +}
> +
>  int ff_fill_line_with_color(uint8_t *line[4], int pixel_step[4], int w, uint8_t dst_color[4],
>                              enum PixelFormat pix_fmt, uint8_t rgba_color[4],
>                              int *is_packed_rgba, uint8_t rgba_map_ptr[4])
> @@ -55,8 +93,25 @@ int ff_fill_line_with_color(uint8_t *line[4], int pixel_step[4], int w, uint8_t
>              dst_color[rgba_map[i]] = rgba_color[i];
>  
>          line[0] = av_malloc(w * pixel_step[0]);
> -        for (i = 0; i < w; i++)
> -            memcpy(line[0] + i * pixel_step[0], dst_color, pixel_step[0]);
> + 
> +        switch(av_get_bits_per_pixel(pix_desc))
> +        {
> +            case 8  : ff_memset8((char *)line[0], dst_color[0], w);
> +		      break;
> +
> +            case 16 : ff_memset16((int16_t *)line[0], *(int16_t *)dst_color, w);
> +                      break;
> +
> +            case 24 : ff_memset24((char *)line[0], (char *)dst_color, w);
> +	              break;
> +
> +            case 32 : ff_memset32((int32_t *)line[0], *(int32_t *)dst_color, w);
> +                      break;
> +
> +            default : ff_memset_sized((char *)line[0], (char *)dst_color, w, pixel_step[0]);
> +                      break;
> +        }

Function calls are expensive, so we should avoid them when they are
not strictly necessary, even if in this case the call is not very
performance critical, and I like a simple code path, in other words
you can put the code in the switch without wrapping it around exported
functions, especially if they are not going to be used in other parts
of the project.

And as Michael suggested, START/STOP_TIMER in libavutil/timer.h can be
used for benchmarking the code.

[...]
-- 
FFmpeg = Faithless Faithless Maxi Puritan Ecumenical Generator