[FFmpeg-devel] [PATCH] cleanup swscale
Michael Niedermayer
michaelni
Thu Jun 7 23:49:09 CEST 2007
Hi
On Wed, Jun 06, 2007 at 08:55:37PM +0200, Luca Barbato wrote:
> Michael Niedermayer wrote:
> > Hi
> >
> > On Wed, Jun 06, 2007 at 12:06:32AM +0200, Luca Barbato wrote:
> >> here a first patch that makes swscale less ugly, the next steps will be
> >> making a template file for x86 by svn cp and make the scalar template C
> >> only.
> >>
>
> try 2
>
> lu
>
>
> --
>
> Luca Barbato
>
> Gentoo/linux Gentoo/PPC
> http://dev.gentoo.org/~lu_zero
>
> Index: swscale.c
> ===================================================================
> --- swscale.c (revision 23482)
> +++ swscale.c (working copy)
> @@ -366,9 +366,13 @@
> }
> #endif
>
> -static inline void yuv2yuvXinC(int16_t *lumFilter, int16_t **lumSrc, int lumFilterSize,
> - int16_t *chrFilter, int16_t **chrSrc, int chrFilterSize,
> - uint8_t *dest, uint8_t *uDest, uint8_t *vDest, int dstW, int chrDstW)
> +static inline void yuv2yuvXinC(int16_t *lumFilter,
> + int16_t **lumSrc, int lumFilterSize,
> + int16_t *chrFilter,
> + int16_t **chrSrc, int chrFilterSize,
> + uint8_t *dest,
> + uint8_t *uDest, uint8_t *vDest,
> + int dstW, int chrDstW)
this is less readable not more than the original code
> {
> //FIXME Optimize (just quickly writen not opti..)
> int i;
> @@ -399,9 +403,13 @@
> }
> }
>
> -static inline void yuv2nv12XinC(int16_t *lumFilter, int16_t **lumSrc, int lumFilterSize,
> - int16_t *chrFilter, int16_t **chrSrc, int chrFilterSize,
> - uint8_t *dest, uint8_t *uDest, int dstW, int chrDstW, int dstFormat)
> +static inline void yuv2nv12XinC(int16_t *lumFilter,
> + int16_t **lumSrc, int lumFilterSize,
> + int16_t *chrFilter,
> + int16_t **chrSrc, int chrFilterSize,
> + uint8_t *dest,
> + uint8_t *uDest, int dstW, int chrDstW,
> + int dstFormat)
same here
[...]
>
> -static inline void yuv2packedXinC(SwsContext *c, int16_t *lumFilter, int16_t **lumSrc, int lumFilterSize,
> - int16_t *chrFilter, int16_t **chrSrc, int chrFilterSize,
> +static inline void yuv2packedXinC(SwsContext *c,
> + int16_t *lumFilter,
> + int16_t **lumSrc, int lumFilterSize,
> + int16_t *chrFilter,
> + int16_t **chrSrc, int chrFilterSize,
static inline void yuv2packedXinC(SwsContext *c,
int16_t *lumFilter, int16_t **lumSrc, int lumFilterSize,
int16_t *chrFilter, int16_t **chrSrc, int chrFilterSize,
[...]
> @@ -1617,8 +1634,10 @@
> }
>
> /* {RGB,BGR}{15,16,24,32} -> {RGB,BGR}{15,16,24,32} */
> -static int rgb2rgbWrapper(SwsContext *c, uint8_t* src[], int srcStride[], int srcSliceY,
> - int srcSliceH, uint8_t* dst[], int dstStride[]){
> +static int rgb2rgbWrapper(SwsContext *c,
> + uint8_t* src[], int srcStride[],
> + int srcSliceY, int srcSliceH,
> + uint8_t* dst[], int dstStride[]){
this too is worse than it was
also note you can reorder the parameters for static funtions as you see fit
static int rgb2rgbWrapper(SwsContext *c,
uint8_t* src[], int srcStride[],
uint8_t* dst[], int dstStride[],
int srcSliceY, int srcSliceH){
being an option
note there are many similar cases
[...]
> int length= c->srcW;
> int y= srcSliceY;
> @@ -1917,7 +1948,10 @@
> * @param fullRange if 1 then the luma range is 0..255 if 0 its 16..235
> * @return -1 if not supported
> */
> -int sws_setColorspaceDetails(SwsContext *c, const int inv_table[4], int srcRange, const int table[4], int dstRange, int brightness, int contrast, int saturation){
> +int sws_setColorspaceDetails(SwsContext *c, const int inv_table[4],
> + int srcRange, const int table[4],
> + int dstRange,
> + int brightness, int contrast, int saturation){
int sws_setColorspaceDetails(SwsContext *c,
const int inv_table[4], int srcRange,
const int table[4], int dstRange,
int brightness, int contrast, int saturation){
being the obvios choice
[...]
> void sws_freeVec(SwsVector *a);
>
> @@ -131,11 +145,12 @@
> float lumaSarpen, float chromaSharpen,
> float chromaHShift, float chromaVShift,
> int verbose);
> +
> void sws_freeFilter(SwsFilter *filter);
>
> struct SwsContext *sws_getCachedContext(struct SwsContext *context,
> int srcW, int srcH, int srcFormat,
> - int dstW, int dstH, int dstFormat, int flags,
> - SwsFilter *srcFilter, SwsFilter *dstFilter, double *param);
> -
> + int dstW, int dstH, int dstFormat,
> + int flags, SwsFilter *srcFilter,
> + SwsFilter *dstFilter, double *param);
this also has a random line break feel to it, hardly better than the
original
struct SwsContext *sws_getCachedContext(struct SwsContext *context,
int srcW, int srcH, int srcFormat,
int dstW, int dstH, int dstFormat,
int flags,
SwsFilter *srcFilter, SwsFilter *dstFilter,
double *param);
might be better
[...]
> #ifdef HAVE_MMX
> if (c->flags & SWS_ACCURATE_RND){
> if (uDest){
> - YSCALEYUV2YV12X_ACCURATE( 0, CHR_MMX_FILTER_OFFSET, uDest, chrDstW)
> - YSCALEYUV2YV12X_ACCURATE(4096, CHR_MMX_FILTER_OFFSET, vDest, chrDstW)
> + YSCALEYUV2YV12X_ACCURATE( 0, CHR_MMX_FILTER_OFFSET,
> + uDest, chrDstW)
> + YSCALEYUV2YV12X_ACCURATE(4096, CHR_MMX_FILTER_OFFSET,
> + vDest, chrDstW)
> }
less readable than it was
[...]
> static inline void RENAME(yuv2yuv1)(int16_t *lumSrc, int16_t *chrSrc,
> - uint8_t *dest, uint8_t *uDest, uint8_t *vDest, long dstW, long chrDstW)
> + uint8_t *dest, uint8_t *uDest,
> + uint8_t *vDest, long dstW, long chrDstW)
putting dest and uDest on the same line and vDest on the next is ugly
[...]
note, breaking lines while making the code less readable is NOT ok
i will not accpet such a patch, making the code more readable and
while doing that trying to keep things <80chars is ok
--
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: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070607/05a0ccad/attachment.pgp>
More information about the ffmpeg-devel
mailing list