[FFmpeg-devel] make swscale's palette functions public
Michael Niedermayer
michaelni
Sun May 16 13:39:28 CEST 2010
On Sat, May 15, 2010 at 06:39:59PM +0200, Reinhard Tartler wrote:
> On Sat, May 15, 2010 at 17:49:28 (CEST), Stefano Sabatini wrote:
>
> > 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?
>
> Perhaps Attila can comment on this better than me since he suggested
> that. The comment is not really duplicated, this comment is about the
> implementation, the documentation in the header is for users.
>
> >> +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.
>
> changed
>
> > [...]
> >> #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
>
> good idea, implemented.
>
> > 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).
>
> Yes, and I think this is undesireable. I can tell you more about the
> breakage as soon as I'll upload an ffmpeg 0.6 package for ubuntu/debian.
>
> >
> >> 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.
>
> 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
> + */
is this an english sentance?
> +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]];
> +}
> +
> +/**
> + * Palette format: ABCD -> dst format: ABC
> + */
> +void sws_palette8topacked24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> + long i;
> +
> + for (i=0; i<num_pixels; i++) {
> + //FIXME slow?
> + dst[0]= palette[src[i]*4+0];
> + dst[1]= palette[src[i]*4+1];
> + dst[2]= palette[src[i]*4+2];
> + dst+= 3;
> + }
> +}
> +
> +/**
> + * Palette is assumed to contain BGR16
> + */
> +void sws_palette8torgb16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> + long i;
> + for (i=0; i<num_pixels; i++)
> + ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
> +}
> +
> +/**
> + * Palette is assumed to contain BGR16
> + */
> +void sws_palette8tobgr16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> + long i;
> + for (i=0; i<num_pixels; i++)
> + ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
> +}
> +
> +/**
> + * Palette is assumed to contain RGB15
> + */
> +void sws_palette8torgb15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> + long i;
> + for (i=0; i<num_pixels; i++)
> + ((uint16_t *)dst)[i] = ((const uint16_t *)palette)[src[i]];
> +}
> +
> +/**
> + * Palette is assumed to contain BGR15
> + */
> +void sws_palette8tobgr15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> +{
> + long i;
> + for (i=0; i<num_pixels; i++)
> + ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
> +}
> Index: swscale.h
> ===================================================================
> --- swscale.h (revision 31179)
> +++ swscale.h (working copy)
> @@ -302,4 +302,64 @@
> int flags, SwsFilter *srcFilter,
> SwsFilter *dstFilter, const double *param);
>
> +/**
> + * Converts an 8bit paletted frame into an RGB32 frame
thats not complete, BGR and others can use this too
> + *
> + * @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);
> +
> +/**
> + * Converts an 8bit paletted frame into an RGB24 frame
same
> + *
> + * @param src source frame buffer
> + * @param dst destination frame buffer
> + * @param num_pixels number of pixels to convert
> + * @param palette array, [256] of RGB24 entries
it should be const uint8_t palette[256] in the code then
[...]
> +/* 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);
there is a attribute for deprecation and we have some wraper macro for
non gcc compatibility.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100516/4d75ba1a/attachment.pgp>
More information about the ffmpeg-devel
mailing list