[Ffmpeg-devel] libswscale cleanups and warning fixes

Michael Niedermayer michaelni
Sun Dec 24 15:40:33 CET 2006


Hi

On Sun, Dec 24, 2006 at 02:09:03PM +0100, Luca Abeni wrote:
> Hi all,
> 
> here are some patches to remove some warnings during libswscale build,
> and some other cleanups:
> 
> 1) remove_void_pointers-1.diff: eliminates void pointers arithmetic from
> libswscale. Respect to the last version I posted, I fixed some missing
> parenthesis, and I verified that the generated code before and after
> applying the patch is exactly the same. I went for this solution
> (instead of removing "entry_size *" where c->table_gV is initalized)
> because it does not change the generated code, so I can verify that is
> is correct. If some other solution is preferrable, let me know and I'll
> update the patch. I also added yuv2rgb.c to the patch (I initially
> missed it because I was not compiling it).
> 
> 2) unused_var.diff: add some attribute_unused. Fixes some warnings, and
> does not change the generated code (I checked that)
> 
> 3) int_pointer.diff: add some casts to int32_t to remove warnings. I
> assume the casts are correct, because I copied them from similar code
> (some lines after the patched code). Again, the generated code does not
> change.
> 
> 4) cast_fix.diff: this one changes the generated code. But after reading
> it, I believe the previous code is incorrect, and the new one looks
> correct to me (of course, I can be wrong ;-). Unfortunately, I found no
> way to test this part of code.
> Kostya, if I understand correctly you are the author of this code... Can
> you have a look?
> 
> 5) minmax_remove.diff: change all the occurrences of "FFMIN(FFMAX())" to
> clip_uint8() or clip()
> 
> If one of these patches must be changed, or is useless, let me know and
> I'll fix it / drop it.
> 
> 
> 			Thanks,
> 				Luca

> Index: swscale.c
> ===================================================================
> --- swscale.c.orig	2006-12-22 19:16:49.173862336 +0100
> +++ swscale.c	2006-12-22 19:16:53.705173472 +0100
> @@ -417,9 +417,9 @@
>                          
>  #define YSCALE_YUV_2_RGBX_C(type) \
>  			YSCALE_YUV_2_PACKEDX_C(type)\
> -			r = c->table_rV[V];\
> -			g = c->table_gU[U] + c->table_gV[V];\
> -			b = c->table_bU[U];\
> +			r = (type *)c->table_rV[V];\
> +			g = (type *)(c->table_gU[U] + c->table_gV[V]);\
> +			b = (type *)c->table_bU[U];\
>  
>  #define YSCALE_YUV_2_PACKED2_C \
>  		for(i=0; i<(dstW>>1); i++){\
> @@ -432,9 +432,9 @@
>  #define YSCALE_YUV_2_RGB2_C(type) \
>  			YSCALE_YUV_2_PACKED2_C\
>  			type *r, *b, *g;\
> -			r = c->table_rV[V];\
> -			g = c->table_gU[U] + c->table_gV[V];\
> -			b = c->table_bU[U];\
> +			r = (type *)c->table_rV[V];\
> +			g = (type *)(c->table_gU[U] + c->table_gV[V]);\
> +			b = (type *)c->table_bU[U];\
>  
>  #define YSCALE_YUV_2_PACKED1_C \
>  		for(i=0; i<(dstW>>1); i++){\
> @@ -447,9 +447,9 @@
>  #define YSCALE_YUV_2_RGB1_C(type) \
>  			YSCALE_YUV_2_PACKED1_C\
>  			type *r, *b, *g;\
> -			r = c->table_rV[V];\
> -			g = c->table_gU[U] + c->table_gV[V];\
> -			b = c->table_bU[U];\
> +			r = (type *)c->table_rV[V];\
> +			g = (type *)(c->table_gU[U] + c->table_gV[V]);\
> +			b = (type *)c->table_bU[U];\
>  
>  #define YSCALE_YUV_2_PACKED1B_C \
>  		for(i=0; i<(dstW>>1); i++){\
> @@ -462,9 +462,9 @@
>  #define YSCALE_YUV_2_RGB1B_C(type) \
>  			YSCALE_YUV_2_PACKED1B_C\
>  			type *r, *b, *g;\
> -			r = c->table_rV[V];\
> -			g = c->table_gU[U] + c->table_gV[V];\
> -			b = c->table_bU[U];\
> +			r = (type *)c->table_rV[V];\
> +			g = (type *)(c->table_gU[U] + c->table_gV[V]);\
> +			b = (type *)c->table_bU[U];\

why are these needed? if c->table* isnt void anymore?


[...]
> Index: swscale_internal.h
> ===================================================================
> --- swscale_internal.h.orig	2006-12-22 19:16:49.173862336 +0100
> +++ swscale_internal.h	2006-12-22 19:16:53.705173472 +0100
> @@ -101,10 +101,10 @@
>  	int dstY;
>  	int flags;
>  	void * yuvTable;			// pointer to the yuv->rgb table start so it can be freed()
> -	void * table_rV[256];
> -	void * table_gU[256];
> +	uint8_t * table_rV[256];
> +	uint8_t * table_gU[256];
>  	int    table_gV[256];
> -	void * table_bU[256];
> +	uint8_t * table_bU[256];

ok, feel free to commit anytime



>  
>  	//Colorspace stuff
>  	int contrast, brightness, saturation;	// for sws_getColorspaceDetails
> Index: yuv2rgb.c
> ===================================================================
> --- yuv2rgb.c.orig	2006-12-22 18:36:47.071037472 +0100
> +++ yuv2rgb.c	2006-12-22 19:20:53.164770080 +0100
> @@ -213,9 +213,9 @@
>  #define RGB(i)					\
>  	U = pu[i];				\
>  	V = pv[i];				\
> -	r = c->table_rV[V];			\
> -	g = c->table_gU[U] + c->table_gV[V];		\
> -	b = c->table_bU[U];
> +	r = (void *)c->table_rV[V];			\
> +	g = (void *)(c->table_gU[U] + c->table_gV[V]);		\
> +	b = (void *)c->table_bU[U];

same question, why is this needed?


>  
>  #define DST1(i)					\
>  	Y = py_1[2*i];				\
> @@ -832,10 +832,10 @@
>      }
>  
>      for (i = 0; i < 256; i++) {
> -	c->table_rV[i] = table_r + entry_size * div_round (crv * (i-128), 76309);
> -	c->table_gU[i] = table_g + entry_size * div_round (cgu * (i-128), 76309);
> +	c->table_rV[i] = (uint8_t *)table_r + entry_size * div_round (crv * (i-128), 76309);
> +	c->table_gU[i] = (uint8_t *)table_g + entry_size * div_round (cgu * (i-128), 76309);
>  	c->table_gV[i] = entry_size * div_round (cgv * (i-128), 76309);
> -	c->table_bU[i] = table_b + entry_size * div_round (cbu * (i-128), 76309);
> +	c->table_bU[i] = (uint8_t *)table_b + entry_size * div_round (cbu * (i-128), 76309);
>      }

ok if table_* is void


>  
>      av_free(c->yuvTable);

> Index: libswscale/yuv2rgb.c
> ===================================================================
> --- libswscale.orig/yuv2rgb.c	2006-12-22 21:57:35.926051024 +0100
> +++ libswscale/yuv2rgb.c	2006-12-22 22:01:12.234167192 +0100
> @@ -265,14 +265,14 @@
>      for(y=0; y<srcSliceH; y+=2){\
>  	dst_type *dst_1= (dst_type*)(dst[0] + (y+srcSliceY  )*dstStride[0]);\
>  	dst_type *dst_2= (dst_type*)(dst[0] + (y+srcSliceY+1)*dstStride[0]);\
> -	dst_type *r, *g, *b;\
> +	dst_type attribute_unused *r, *g, attribute_unused *b;\
>  	uint8_t *py_1= src[0] + y*srcStride[0];\
>  	uint8_t *py_2= py_1 + srcStride[0];\
>  	uint8_t *pu= src[1] + (y>>1)*srcStride[1];\
>  	uint8_t *pv= src[2] + (y>>1)*srcStride[2];\
>  	unsigned int h_size= c->dstW>>3;\
>  	while (h_size--) {\
> -	    int U, V, Y;\
> +	    int attribute_unused U, attribute_unused V, Y;\

hmm int attribute_unused U,V,Y; or attribute_unused int U,V,Y;
doenst work?

[...]

> Index: libswscale/swscale_template.c
> ===================================================================
> --- libswscale.orig/swscale_template.c	2006-12-22 21:54:29.309421064 +0100
> +++ libswscale/swscale_template.c	2006-12-22 23:47:50.610466144 +0100
> @@ -3107,15 +3107,15 @@
>  		int i;
>              if(flags & SWS_ACCURATE_RND){
>                          for(i=0; i<vLumFilterSize; i+=2){
> -                                lumMmxFilter[2*i+0]= lumSrcPtr[i  ];
> -                                lumMmxFilter[2*i+1]= lumSrcPtr[i+(vLumFilterSize>1)];
> +                                lumMmxFilter[2*i+0]= (int32_t)lumSrcPtr[i  ];
> +                                lumMmxFilter[2*i+1]= (int32_t)lumSrcPtr[i+(vLumFilterSize>1)];
>                                  lumMmxFilter[2*i+2]=
>                                  lumMmxFilter[2*i+3]= vLumFilter[dstY*vLumFilterSize + i    ]
>                                                  + (vLumFilterSize>1 ? vLumFilter[dstY*vLumFilterSize + i + 1]<<16 : 0);
>                          }
>                          for(i=0; i<vChrFilterSize; i+=2){
> -                                chrMmxFilter[2*i+0]= chrSrcPtr[i  ];
> -                                chrMmxFilter[2*i+1]= chrSrcPtr[i+(vChrFilterSize>1)];
> +                                chrMmxFilter[2*i+0]= (int32_t)chrSrcPtr[i  ];
> +                                chrMmxFilter[2*i+1]= (int32_t)chrSrcPtr[i+(vChrFilterSize>1)];
>                                  chrMmxFilter[2*i+2]=
>                                  chrMmxFilter[2*i+3]= vChrFilter[chrDstY*vChrFilterSize + i    ]
>                                                  + (vChrFilterSize>1 ? vChrFilter[chrDstY*vChrFilterSize + i + 1]<<16 : 0);


looks ok commit anytime



[clippatch]
looks ok

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/20061224/8b2a9eb5/attachment.pgp>



More information about the ffmpeg-devel mailing list