[Ffmpeg-devel] Some possible libswscale cleanups?

[Luca Abeni]

Hi Michael,

On Thu, 2006-12-21 at 16:12 +0100, Michael Niedermayer wrote:
[...]
> > /usr/include/stdlib.h:559: warning: previous declaration of `calloc'
> > In file included from /tmp/ffmpeg/libavutil/common.h:35,
> >                  from /tmp/ffmpeg/libavutil/avutil.h:44,
> >                  from swscale_internal.h:28,
> >                  from swscale.c:76:
> > [...]
> > I think "#include <malloc.h>" is not needed now (it was needed when
> > libswscale used memalign()), so the attached remove_useless_include.diff
> > removes it, eliminating the warning.
> 
> ok
Thanks, I'll commit it this evening.

[...]
> > swscale.c:788: warning: unused variable `b'
> > swscale.c:788: warning: unused variable `r'
> > (and I see no easy way to remove them).
> 
> attribute_unused ?
I'll have a look at it; thanks

[...]
> > I also noticed that swscale.c contains some 
> > FFMIN(FFMAX(..., 0), 255);
> > Maybe it would be good to change them in clip_uint8()? If yes, let me
> > know and I'll provide a patch.
> 
> yes
Ok, I'll work on a patch and post it in the next days

[...]
> >  #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];\
> 
> void* + int != random_type* + int
Ughh... This is embarassing :( I just added "(type *)" without looking
at the code (and when I tested it "type" probably was "uint8_t").
I think the correct version should be
-			b = c->table_bU[U];\
+			r = (type *)c->table_rV[V];\
+			g = (type *)(c->table_gU[U] + c->table_gV[V]);\
right?

[...]
> > +	uint16_t *srcPtr= (uint16_t *)src[0];
> > +	uint16_t *dstPtr= (uint16_t *)dst[0] + dstStride[0]*y/2;
> 
> this also doesnt look correct for the same reason (/2 vs. 16bits)
Ooops... I missed some parenthesys again :(
It should be
uint16_t *dstPtr= (uint16_t *)(dst[0] + dstStride[0]*y/2);
right?

I'll perform some more tests, verifying that the correct code is
generated, and post an updated patch.


				Thanks,
					Luca
-- 
_____________________________________________________________________________
Copy this in your signature, if you think it is important:
                               N O    W A R ! ! !