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

[Michael Niedermayer]

On Sun, May 16, 2010 at 10:32:38PM +0200, Reinhard Tartler wrote:
> On Sun, May 16, 2010 at 20:58:36 (CEST), Michael Niedermayer wrote:
[...]
> >> >
> >> > [...]
> >> >> +/* 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.
> >> 
> >> okay, used attribute_deprecated then.
> >> 
> >> 
> >> BTW, during my work I noticed that palette8torgb16 and palette8torgb15
> >> (and their bgr counterparts) implement exactly the same code. Is this
> >> really intended?
> >
> > certainly not
> > this should definitly be fixed before making them officially public
> 
> AFAIUI, you have written these functions. Could you perhaps take a look
> at them?

what do you want me to do /help?
what iam seeing is that we just need
3 functions, one for 32bit, one for 24bit and one for 16bit


> 
> >> 
> >> 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,58 @@
> >>      return sws_scale(c, src, srcStride, srcSliceY, srcSliceH, dst, dstStride);
> >>  }
> >>  #endif
> >> +
> >> +/* Convert the palette to the same packed 32-bit format as the palette */
> >
> > "convert a to the same format as a"
> > is a circular reference
> 
> This is just a moving around of a comment of the previous author (i.e.,
> you). What would be the correct comment here? Or should it be dropped?
> 
> >
> >> +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]];
> >> +}

Convert the palette indexed data in src to 32bit packed pixel data in dst.
This is done by substituting each byte from src through lookup in palette.



> >> +
> >> +/* 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 RBG16 */
> >
> > rbg?
> 
> again, I also guess you meant RGB here.

you need to guess that? ;)
the function name contains "rgb"


> 
> >> +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]]);
> >
> > the bswap can be merged into the palette
> >
> 
> I'm not sure what you mean here, how can this be done without breaking
> applications that already use this function (e.g. mplayer)?

it cant
but the function is not needed for the new "sws_" api.
it should stay (deprecated) and eventually removed with the next major bump
thhrough #if "VERSION" < "next major"

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- 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/698004df/attachment.pgp>