[FFmpeg-cvslog] r27730 - in trunk/libswscale: swscale.c swscale_internal.h
Michael Niedermayer
michaelni
Tue Oct 21 03:28:33 CEST 2008
On Mon, Oct 20, 2008 at 08:05:26PM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
> > On Mon, Oct 20, 2008 at 06:27:36PM +0200, Vitor Sessak wrote:
> >> Michael Niedermayer wrote:
> >>> On Sun, Oct 19, 2008 at 08:25:41PM +0200, Vitor Sessak wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Sat, Oct 18, 2008 at 11:55:58PM +0200, Vitor Sessak wrote:
> >>>>>> vitor wrote:
> >>>>>>> Author: vitor
> >>>>>>> Date: Wed Oct 8 19:46:22 2008
> >>>>>>> New Revision: 27730
> >>>>>>> Log:
> >>>>>>> Add a new unscaled PAL8 -> RGB converter.
> >>>>>> 1000l for me for this patch. I tested it, but without noticing that
> >>>>>> typing "make" do _not_ rebuild swscale-example. Actually it made a
> >>>>>> small regression in {PAL,RGB8,BGR8,RGB4,BGR4}->{RGB32, BGR32}.
> >>>>>>
> >>>>>> Three possibilities:
> >>>>>>
> >>>>>> 1- Revert it
> >>>>>> 2- sws_fix10l.diff: fix the actual bug (in rgb2rgb.c)
> >>>>>> 3- sws_hack.diff: just fix my breakage
> >>>>> i do not think 2 or 3 fix the code
> >>>>> the palette is in RGB32 format IIRC so one side of the #ifdef has to be
> >>>>> wrong. Also the rgb24 would need ifdefs which it does not have, or the
> >>>>> palette would have to be converted into a differnt byte order or the
> >>>>> switch case would need to be changed a little
> >>>> AFAIK, before the palette was either in BGR32 or RGB32 depending on
> >>>> alignment. Now I've made everything alignment independent. Also, if I
> >>>> understand correctly the code, I think that swscaler uses a slightly
> >>>> unusual definition of RGB/BGR.
> >>> the swscaler uses the same definition as lav* uses, and its very similar
> >>> to mplayers except some rgb <-> bgr flips
> >>> These definitions have also been choosen that way for efficiency and
> >>> simplicity.
> >>> anyway, this patch doesnt look correct either
> >>> i suggest you read the definition of the formats in avutils.h
> >> Documented header files are a bliss =)
> >>
> >> Hopefully correct version attached.
> >>
> >> -Vitor
> >
> > [...]
> >> void palette8tobgr32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> >> {
> >> long i;
> >> - for (i=0; i<num_pixels; i++)
> >> - {
> >> - #ifdef WORDS_BIGENDIAN
> >> - dst[3]= palette[src[i]*4+0];
> >> - dst[2]= palette[src[i]*4+1];
> >> - dst[1]= palette[src[i]*4+2];
> >> - #else
> >> - //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]= 0; /* do we need this cleansing? */
> >> - #endif
> >>
> >> - dst+= 4;
> >> - }
> >> + for (i=0; i<num_pixels; i++)
> >> + ((uint32_t *) dst)[i] =
> >> + bswap_32(((const uint32_t *) palette)[src[i]]) >> 8;
> >> }
> >
> > i think it would be faster to bswap 256 entries in palette than
> > width*height, besides this would allow the same code to be used for
> > rgb & bgr
>
> Yes, but it would have to be done in swscale(). Isn't having both
> pal_rgb[] and pal_bgr[] in the context a bit of an overkill?
yep, what about a pal_native ?
there are btw 4 32bit rgb/bgr formats so its not 2 vs 1 but 4 vs 1
> Also
> palette8tobgr32() is a public function, so I think it should either be
> fixed or removed...
i dont mind removing broken and redundant functions
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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-cvslog/attachments/20081021/d08bb996/attachment.pgp>
More information about the ffmpeg-cvslog
mailing list