[x264-devel] Re: x264: svn commit r618 (pengvado)

[Loïc Le Loarer]

Hi,

The Friday 12 January 2007 à 00:55:52 +0100, Subversion daemon wrote:
> r618 | pengvado | 2007-01-12 00:55:51 +0100 (Fri, 12 Jan 2007) | 2 lines
> make array_non_zero() compatible with -fstrict-aliasing
> 
>  > http://trac.videolan.org/x264/changeset/618

+#define array_non_zero(a) array_non_zero_int(a, sizeof(a))
+static inline int array_non_zero_int( void *v, int i_count )

On the computers I have access to (one linux i386, one linux amd64 and
one sparc 64), with gcc compiler, sizeof returns a value of type size_t,
not int. size_t is unsigned and 32 bits on 32 bits computers and 64 bits
on 64 bits computers.

Moreover, I agree with Limin Wang, the line "i_count >>= 2;" is wrong,
it should be "i_count /= sizeof(int);" which will be optimized on most
systems to the same thing.

The fact that an int* is used to interate in the void* array means that
its size should be a multiple of sizeof(int), which it is currently in
all usages of array_non_zero but is not really clean.

Here is the patch for first and second issue:

--- encoder/macroblock.h        (révision 618)
+++ encoder/macroblock.h        (copie de travail)
@@ -55,11 +55,11 @@
 void x264_denoise_dct( x264_t *h, int16_t *dct );

 #define array_non_zero(a) array_non_zero_int(a, sizeof(a))
-static inline int array_non_zero_int( void *v, int i_count )
+static inline int array_non_zero_int( void *v, size_t i_count )
 {
-    int i;
+    size_t i;
     int *x = v;
-    i_count >>= 2;
+    i_count /= sizeof(int);
     for( i = 0; i < i_count; i++ )
         if( x[i] ) return 1;
     return 0;


Best regards.

-- 
Loïc

"heaven is not a place, it's a feeling"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.videolan.org/pipermail/x264-devel/attachments/20070112/5e308b07/attachment.pgp