[Ffmpeg-devel] libswscale cleanups and warning fixes

[Luca Abeni]

Hi Michael,

On Sun, 2006-12-24 at 15:40 +0100, Michael Niedermayer wrote:
[...]
> >  #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?
Well, my patch is changing c->table* from void * to uint8_t *, so gcc is
complaining when type is not uint8_t:
swscale_template.c:1698: warning: assignment from incompatible pointer type
and similar warnings.
I do not know how meaningful these warnings are, but I was trying to
reduce the verbosity of the build.

> [...]
> > 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
Good; I am going to commit this part in a short time.

[...]
> > -	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?
same as above: to remove "assignment from incompatible pointer type"
warnings.

[...]
> >      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
Yes it is; I did not change the type of the variable.

[...]
> > -	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?
Well, I only wanted to apply the "unused" attribute to U and V (I do not
see the "unused variable" warning for Y). So, I ended up with this funny
"int attribute_unused U, attribute_unused V, Y;" but I see now that the
"unused" attribute is probably also applied to Y. Both "int
attribute_unused U,V,Y" and "attribute_unused int U,V,Y" work. Which
form is preferred? So, I think the best thing would be
attribute_unused int U,V;	(or int attribute_unused)
int Y;
Would this be ok? I'll prepare a new patch in a short time (I think the
same applies to r,g,b, right?).

[...]
> >                          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
Ok; I'll commit this soon.

> [clippatch]
> looks ok
So I can commit it, right?


			Thanks,
				Luca