[x264-devel] Fix some integer overflows/signedness errors found by IOC

Jason Garrett-Glaser git at videolan.org
Sat Oct 22 02:30:23 CEST 2011


x264 | branch: master | Jason Garrett-Glaser <jason at x264.com> | Mon Oct 10 17:44:31 2011 -0700| [2697313a6f223f0b270ba9533c6b47967fa7d246] | committer: Jason Garrett-Glaser

Fix some integer overflows/signedness errors found by IOC
The only real bug here is in slicetype.c, which may or may not affect real encodes.

> http://git.videolan.org/gitweb.cgi/x264.git/?a=commit;h=2697313a6f223f0b270ba9533c6b47967fa7d246
---

 common/bitstream.h  |    2 +-
 common/frame.c      |    4 ++--
 common/macroblock.c |    6 ++----
 common/macroblock.h |    6 +++---
 encoder/me.c        |    6 +++---
 encoder/slicetype.c |    2 +-
 6 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/common/bitstream.h b/common/bitstream.h
index 8920b89..1a15338 100644
--- a/common/bitstream.h
+++ b/common/bitstream.h
@@ -48,7 +48,7 @@ typedef struct bs_s
     uint8_t *p;
     uint8_t *p_end;
 
-    intptr_t cur_bits;
+    uintptr_t cur_bits;
     int     i_left;    /* i_count number of available bits */
     int     i_bits_encoded; /* RD only */
 } bs_t;
diff --git a/common/frame.c b/common/frame.c
index 6edbf55..254b962 100644
--- a/common/frame.c
+++ b/common/frame.c
@@ -409,8 +409,8 @@ int x264_frame_copy_picture( x264_t *h, x264_frame_t *dst, x264_picture_t *src )
 static void ALWAYS_INLINE pixel_memset( pixel *dst, pixel *src, int len, int size )
 {
     uint8_t *dstp = (uint8_t*)dst;
-    uint8_t  v1 = *src;
-    uint16_t v2 = size == 1 ? v1 + (v1 <<  8) : M16( src );
+    uint32_t v1 = *src;
+    uint32_t v2 = size == 1 ? v1 + (v1 <<  8) : M16( src );
     uint32_t v4 = size <= 2 ? v2 + (v2 << 16) : M32( src );
     int i = 0;
     len *= size;
diff --git a/common/macroblock.c b/common/macroblock.c
index de6dfd8..fd1b90b 100644
--- a/common/macroblock.c
+++ b/common/macroblock.c
@@ -1422,10 +1422,8 @@ static void x264_macroblock_deblock_strength_mbaff( x264_t *h, uint8_t (*bs)[8][
 
                 if( !h->param.b_cabac && h->pps->b_transform_8x8_mode && h->mb.mb_transform_size[mbn_xy] )
                 {
-                    int nnz_top0 = M16( &nnz[mbn_xy][8] ) | M16( &nnz[mbn_xy][12] );
-                    int nnz_top1 = M16( &nnz[mbn_xy][10] ) | M16( &nnz[mbn_xy][14] );
-                    nnz_top[0] = nnz_top[1] = nnz_top0 ? 0x0101 : 0;
-                    nnz_top[2] = nnz_top[3] = nnz_top1 ? 0x0101 : 0;
+                    nnz_top[0] = nnz_top[1] = M16( &nnz[mbn_xy][ 8] ) || M16( &nnz[mbn_xy][12] );
+                    nnz_top[2] = nnz_top[3] = M16( &nnz[mbn_xy][10] ) || M16( &nnz[mbn_xy][14] );
                 }
 
                 for( int i = 0; i < 4; i++ )
diff --git a/common/macroblock.h b/common/macroblock.h
index 6484393..8b30053 100644
--- a/common/macroblock.h
+++ b/common/macroblock.h
@@ -348,7 +348,7 @@ void x264_mb_predict_mv_ref16x16( x264_t *h, int i_list, int i_ref, int16_t mvc[
 void x264_mb_mc( x264_t *h );
 void x264_mb_mc_8x8( x264_t *h, int i8 );
 
-static ALWAYS_INLINE uint32_t pack16to32( int a, int b )
+static ALWAYS_INLINE uint32_t pack16to32( uint32_t a, uint32_t b )
 {
 #if WORDS_BIGENDIAN
    return b + (a<<16);
@@ -356,7 +356,7 @@ static ALWAYS_INLINE uint32_t pack16to32( int a, int b )
    return a + (b<<16);
 #endif
 }
-static ALWAYS_INLINE uint32_t pack8to16( int a, int b )
+static ALWAYS_INLINE uint32_t pack8to16( uint32_t a, uint32_t b )
 {
 #if WORDS_BIGENDIAN
    return b + (a<<8);
@@ -364,7 +364,7 @@ static ALWAYS_INLINE uint32_t pack8to16( int a, int b )
    return a + (b<<8);
 #endif
 }
-static ALWAYS_INLINE uint32_t pack8to32( int a, int b, int c, int d )
+static ALWAYS_INLINE uint32_t pack8to32( uint32_t a, uint32_t b, uint32_t c, uint32_t d )
 {
 #if WORDS_BIGENDIAN
    return d + (c<<8) + (b<<16) + (a<<24);
diff --git a/encoder/me.c b/encoder/me.c
index 6d3d865..75ae29d 100644
--- a/encoder/me.c
+++ b/encoder/me.c
@@ -957,7 +957,7 @@ static void refine_subpel( x264_t *h, x264_me_t *m, int hpel_iters, int qpel_ite
     }\
 }
 
-#define SATD_THRESH 17/16
+#define SATD_THRESH(cost) (cost+(cost>>4))
 
 /* Don't unroll the BIME_CACHE loop. I couldn't find any way to force this
  * other than making its iteration count not a compile-time constant. */
@@ -1063,7 +1063,7 @@ static void ALWAYS_INLINE x264_me_refine_bidir( x264_t *h, x264_me_t *m0, x264_m
                          + p_cost_m0x[m0x] + p_cost_m0y[m0y] + p_cost_m1x[m1x] + p_cost_m1y[m1y];
                 if( rd )
                 {
-                    if( cost < bcost * SATD_THRESH )
+                    if( cost < SATD_THRESH(bcost) )
                     {
                         bcost = X264_MIN( cost, bcost );
                         M32( cache0_mv ) = pack16to32_mask(m0x,m0y);
@@ -1146,7 +1146,7 @@ void x264_me_refine_bidir_rd( x264_t *h, x264_me_t *m0, x264_me_t *m1, int i_wei
 
 #define COST_MV_RD( mx, my, satd, do_dir, mdir ) \
 { \
-    if( satd <= bsatd * SATD_THRESH ) \
+    if( satd <= SATD_THRESH(bsatd) ) \
     { \
         uint64_t cost; \
         M32( cache_mv ) = pack16to32_mask(mx,my); \
diff --git a/encoder/slicetype.c b/encoder/slicetype.c
index 74ad07a..8a1ebb1 100644
--- a/encoder/slicetype.c
+++ b/encoder/slicetype.c
@@ -790,7 +790,7 @@ static int x264_slicetype_frame_cost( x264_t *h, x264_mb_analysis_t *a,
     {
         // arbitrary penalty for I-blocks after B-frames
         int nmb = NUM_MBS;
-        i_score += i_score * frames[b]->i_intra_mbs[b-p0] / (nmb * 8);
+        i_score += (uint64_t)i_score * frames[b]->i_intra_mbs[b-p0] / (nmb * 8);
     }
     return i_score;
 }



More information about the x264-devel mailing list