[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