[FFmpeg-devel] make swscale's palette functions public

[Stefano Sabatini]

On date Saturday 2010-05-15 17:06:02 +0200, Reinhard Tartler encoded:
> On Fri, May 14, 2010 at 09:16:28 (CEST), Reinhard Tartler wrote:
> 
> >> its probably a good idea but the functions need to be documented in
> >> teh header
> >
> > In this case I propose the following patch for swscale:
> 
> Updated Patch with improved documentation by Attila:
> 
> Index: swscale.c
> ===================================================================
> --- swscale.c	(revision 31179)
> +++ swscale.c	(working copy)
> @@ -1424,12 +1424,12 @@
>  
>      if (usePal(srcFormat)) {
>          switch (dstFormat) {
> -        case PIX_FMT_RGB32  : conv = palette8topacked32; break;
> -        case PIX_FMT_BGR32  : conv = palette8topacked32; break;
> -        case PIX_FMT_BGR32_1: conv = palette8topacked32; break;
> -        case PIX_FMT_RGB32_1: conv = palette8topacked32; break;
> -        case PIX_FMT_RGB24  : conv = palette8topacked24; break;
> -        case PIX_FMT_BGR24  : conv = palette8topacked24; break;
> +        case PIX_FMT_RGB32  : conv = sws_palette8topacked32; break;
> +        case PIX_FMT_BGR32  : conv = sws_palette8topacked32; break;
> +        case PIX_FMT_BGR32_1: conv = sws_palette8topacked32; break;
> +        case PIX_FMT_RGB32_1: conv = sws_palette8topacked32; break;
> +        case PIX_FMT_RGB24  : conv = sws_palette8topacked24; break;
> +        case PIX_FMT_BGR24  : conv = sws_palette8topacked24; break;
>          }
>      }
>  
> @@ -1962,3 +1962,70 @@
>      return sws_scale(c, src, srcStride, srcSliceY, srcSliceH, dst, dstStride);
>  }
>  #endif
> +
> +/**
> + * Convert the palette to the same packet 32-bit format as the palette
> + */

What's the point of duplicating docs in the implementation?

> +void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> +    long i;
> +
> +    for (i=0; i<num_pixels; i++)
> +        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
> +}
[...]
> Index: swscale.h
> ===================================================================
> --- swscale.h	(revision 31179)
> +++ swscale.h	(working copy)
> @@ -302,4 +302,58 @@
>                                          int flags, SwsFilter *srcFilter,
>                                          SwsFilter *dstFilter, const double *param);
>  
> +/**
> + * Convert an 8bit paletted frame into an RGB32 frame
> + * @param src: source frame buffer
> + * @param dst: destination frame buffer
> + * @param num_pixels: number of pixels to convert
> + * @param palette: array, [256] of RGB32 entries
> + */
> +void sws_palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);

Convert -> ConvertS, please use third person. Also add an empty line
before the first @param, the ":" at the end of the param name is
inconsistent with the FFmpeg doxies.

Same comments apply to the other doxies in the patch.

[...]
>  #endif /* SWSCALE_SWSCALE_H */
> Index: rgb2rgb.c
> ===================================================================
> --- rgb2rgb.c	(revision 31179)
> +++ rgb2rgb.c	(working copy)
> @@ -207,63 +207,34 @@
>          rgb2rgb_init_C();
>  }
>  
> -/**
> - * Convert the palette to the same packet 32-bit format as the palette
> - */
>  void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
>  {
> -    long i;
> -
> -    for (i=0; i<num_pixels; i++)
> -        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
> +    sws_palette8topacked32(src, dst, num_pixels, palette);
>  }

Why not directly use sws_palette8XXX() in the code, and get rid of
these senseless wrapper functions?

They've never been public, so it should be safe to just remove
them. If you want to keep them for the sake of misbehaving apps using
the libav* internal API (hi MPlayer!) please mark them with:
#if LIBSWCALE_VERSION_MAJOR < 1
...
#endif

But I don't think this is going to be any good, applications using
internal API are doomed to be broken anyway, if not here somewhere
else (see my eval.c patches).

>  void rgb32to24(const uint8_t *src, uint8_t *dst, long src_size)
> Index: rgb2rgb.h
> ===================================================================
> --- rgb2rgb.h	(revision 31179)
> +++ rgb2rgb.h	(working copy)
> @@ -4,7 +4,7 @@
>   *               Software YUV to YUV converter
>   *               Software YUV to RGB converter
>   *  Written by Nick Kurshev.
> - *  palette & YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
> + *  YUV & runtime CPU stuff by Michael (michaelni at gmx.at)
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -66,6 +66,7 @@
>  void shuffle_bytes_3012(const uint8_t *src, uint8_t *dst, long src_size);
>  void shuffle_bytes_3210(const uint8_t *src, uint8_t *dst, long src_size);
>  
> +/* deprecated, use the public versions in swscale.h */
>  void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>  void palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
>  void palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);

Regards.
-- 
FFmpeg = Fabulous & Fantastic Muttering Portable Elitist Geisha