[FFmpeg-devel] remove palette8torgb15 and palette8torgb15, was: make swscale's palette functions public

[Reimar Döffinger]

On Mon, May 31, 2010 at 04:40:24PM +0200, Reinhard Tartler wrote:
> On Mo, Mai 24, 2010 at 21:55:04 (CEST), Reinhard Tartler wrote:
> 
> > sorry for the cross post, but this is a proposed patch that affects both
> > projects.
> >
> > On Sun, May 16, 2010 at 20:58:36 (CEST), Michael Niedermayer wrote:
> >>> 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
> >
> > Disclaimer: I'm not sure if this code duplication is intentional or
> > not. I read Michaels comment that it is not. Looking at their users, it
> > looks to me that they can be rewritten to use their 16bit variants. In
> > this case, mplayer code could be simplified like this:
> > Index: libmpcodecs/vf_palette.c
> > ===================================================================
> > --- libmpcodecs/vf_palette.c	(revision 31208)
> > +++ libmpcodecs/vf_palette.c	(working copy)
> > @@ -114,11 +114,6 @@
> >  	// no stride conversion needed
> >  	switch(IMGFMT_RGB_DEPTH(dmpi->imgfmt)){
> >  	case 15:
> > -	    if (IMGFMT_IS_BGR(dmpi->imgfmt))
> > -		palette8tobgr15(mpi->planes[0],dmpi->planes[0],mpi->h*mpi->w,mpi->planes[1]);
> > -	    else
> > -		palette8torgb15(mpi->planes[0],dmpi->planes[0],mpi->h*mpi->w,mpi->planes[1]);
> > -	    break;
> >  	case 16:
> >  	    if (IMGFMT_IS_BGR(dmpi->imgfmt))
> >  		palette8tobgr16(mpi->planes[0],dmpi->planes[0],mpi->h*mpi->w,mpi->planes[1]);
> > @@ -145,11 +140,6 @@
> >  	    unsigned char* dst=dmpi->planes[0]+y*dmpi->stride[0];
> >  	    switch(IMGFMT_RGB_DEPTH(dmpi->imgfmt)){
> >  	    case 15:
> > -		if (IMGFMT_IS_BGR(dmpi->imgfmt))
> > -		    palette8tobgr15(src,dst,mpi->w,mpi->planes[1]);
> > -		else
> > -		    palette8torgb15(src,dst,mpi->w,mpi->planes[1]);
> > -		break;
> >  	    case 16:
> >  		if (IMGFMT_IS_BGR(dmpi->imgfmt))
> >  		    palette8tobgr16(src,dst,mpi->w,mpi->planes[1]);
> > Index: libswscale/rgb2rgb.c
> > ===================================================================
> > --- libswscale/rgb2rgb.c	(revision 31208)
> > +++ libswscale/rgb2rgb.c	(working copy)
> > @@ -250,22 +250,6 @@
> >          ((uint16_t *)dst)[i] = bswap_16(((const uint16_t *)palette)[src[i]]);
> >  }
> >  
> > -/**
> > - * Palette is assumed to contain BGR15, see rgb32to15 to convert the palette.
> > - */
> > -void 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]];
> > -}
> > -void 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]]);
> > -}
> > -
> >  void rgb32to24(const uint8_t *src, uint8_t *dst, long src_size)
> >  {
> >      long i;
> > Index: libswscale/rgb2rgb.h
> > ===================================================================
> > --- libswscale/rgb2rgb.h	(revision 31208)
> > +++ libswscale/rgb2rgb.h	(working copy)
> > @@ -70,8 +70,6 @@
> >  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);
> >  void palette8tobgr16(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
> > -void palette8torgb15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
> > -void palette8tobgr15(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
> >  
> >  /**
> >   * Height should be a multiple of 2 and width should be a multiple of 16.
> >
> >
> > In case the 15bit variants are supposed to contain different code, could
> > someone please elaborate, or ideally, fix the code?
> 
> ping?

Seems fine, though I think vf_palette might not be 100% working anymore anyway.
At the very least it is not really useful with FFmpeg decoders since they all use the
same palette format.