[Ffmpeg-devel] qtrle bug

Reimar Döffinger Reimar.Doeffinger
Wed Oct 12 18:11:54 CEST 2005


Hi,
On Wed, Oct 12, 2005 at 11:51:41AM +0200, matthieu castet wrote:
> Ok, I found the bug,
> 
> rle_code was an signed char, and when it was '*4' in the check it 
> overflow and become negative.

Are you sure? Because I'm rather certain the bug is elsewhere, namely at
rle_code = -rle_code;
This also explains why the problem happens quite rarely, namely when
rle_code == -128.
The attached patch would fix it as well, making rle_code 'signed char' everywhere
for consistency, though I think it is quite obfuscated and your fix probably is
better (I just did it for fun :-) ).
So somebody please say what to do about it *g*.

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: libavcodec/qtrle.c
===================================================================
RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/qtrle.c,v
retrieving revision 1.7
diff -u -r1.7 qtrle.c
--- libavcodec/qtrle.c	13 Aug 2005 17:46:09 -0000	1.7
+++ libavcodec/qtrle.c	12 Oct 2005 16:14:15 -0000
@@ -78,7 +78,7 @@
     int header;
     int start_line;
     int lines_to_change;
-    int rle_code;
+    signed char rle_code;
     int row_ptr, pixel_ptr;
     int row_inc = s->frame.linesize[0];
     unsigned char pi1, pi2, pi3, pi4, pi5, pi6, pi7, pi8;  /* 8 palette indices */
@@ -122,7 +122,6 @@
                 CHECK_PIXEL_PTR(0);  /* make sure pixel_ptr is positive */
             } else if (rle_code < 0) {
                 /* decode the run length code */
-                rle_code = -rle_code;
                 /* get the next 4 bytes from the stream, treat them as palette
                  * indices, and output them rle_code times */
                 CHECK_STREAM_PTR(4);
@@ -135,9 +134,9 @@
                 pi7 = ((s->buf[stream_ptr]) >> 4) & 0x0f;
                 pi8 = (s->buf[stream_ptr++]) & 0x0f;
 
-                CHECK_PIXEL_PTR(rle_code * 8);
+                CHECK_PIXEL_PTR(-rle_code * 8);
 
-                while (rle_code--) {
+                while (rle_code++) {
                     rgb[pixel_ptr++] = pi1;
                     rgb[pixel_ptr++] = pi2;
                     rgb[pixel_ptr++] = pi3;
@@ -149,13 +148,18 @@
                 }
             } else {
                 /* copy the same pixel directly to output 4 times */
-                rle_code *= 4;
-                CHECK_STREAM_PTR(rle_code);
-                CHECK_PIXEL_PTR(rle_code*2);
+                CHECK_STREAM_PTR(rle_code*4);
+                CHECK_PIXEL_PTR(rle_code*8);
 
                 while (rle_code--) {
                     rgb[pixel_ptr++] = ((s->buf[stream_ptr]) >> 4) & 0x0f;
                     rgb[pixel_ptr++] = (s->buf[stream_ptr++]) & 0x0f;
+                    rgb[pixel_ptr++] = ((s->buf[stream_ptr]) >> 4) & 0x0f;
+                    rgb[pixel_ptr++] = (s->buf[stream_ptr++]) & 0x0f;
+                    rgb[pixel_ptr++] = ((s->buf[stream_ptr]) >> 4) & 0x0f;
+                    rgb[pixel_ptr++] = (s->buf[stream_ptr++]) & 0x0f;
+                    rgb[pixel_ptr++] = ((s->buf[stream_ptr]) >> 4) & 0x0f;
+                    rgb[pixel_ptr++] = (s->buf[stream_ptr++]) & 0x0f;
                 }
             }
         }
@@ -169,7 +173,7 @@
     int header;
     int start_line;
     int lines_to_change;
-    int rle_code;
+    signed char rle_code;
     int row_ptr, pixel_ptr;
     int row_inc = s->frame.linesize[0];
     unsigned char pi1, pi2, pi3, pi4;  /* 4 palette indices */
@@ -213,7 +217,6 @@
                 CHECK_PIXEL_PTR(0);  /* make sure pixel_ptr is positive */
             } else if (rle_code < 0) {
                 /* decode the run length code */
-                rle_code = -rle_code;
                 /* get the next 4 bytes from the stream, treat them as palette
                  * indices, and output them rle_code times */
                 CHECK_STREAM_PTR(4);
@@ -222,9 +225,9 @@
                 pi3 = s->buf[stream_ptr++];
                 pi4 = s->buf[stream_ptr++];
 
-                CHECK_PIXEL_PTR(rle_code * 4);
+                CHECK_PIXEL_PTR(-rle_code * 4);
 
-                while (rle_code--) {
+                while (rle_code++) {
                     rgb[pixel_ptr++] = pi1;
                     rgb[pixel_ptr++] = pi2;
                     rgb[pixel_ptr++] = pi3;
@@ -232,12 +235,14 @@
                 }
             } else {
                 /* copy the same pixel directly to output 4 times */
-                rle_code *= 4;
-                CHECK_STREAM_PTR(rle_code);
-                CHECK_PIXEL_PTR(rle_code);
+                CHECK_STREAM_PTR(rle_code * 4);
+                CHECK_PIXEL_PTR(rle_code * 4);
 
                 while (rle_code--) {
                     rgb[pixel_ptr++] = s->buf[stream_ptr++];
+                    rgb[pixel_ptr++] = s->buf[stream_ptr++];
+                    rgb[pixel_ptr++] = s->buf[stream_ptr++];
+                    rgb[pixel_ptr++] = s->buf[stream_ptr++];
                 }
             }
         }
@@ -295,14 +300,13 @@
                 CHECK_PIXEL_PTR(0);  /* make sure pixel_ptr is positive */
             } else if (rle_code < 0) {
                 /* decode the run length code */
-                rle_code = -rle_code;
                 CHECK_STREAM_PTR(2);
                 rgb16 = BE_16(&s->buf[stream_ptr]);
                 stream_ptr += 2;
 
-                CHECK_PIXEL_PTR(rle_code * 2);
+                CHECK_PIXEL_PTR(-rle_code * 2);
 
-                while (rle_code--) {
+                while (rle_code++) {
                     *(unsigned short *)(&rgb[pixel_ptr]) = rgb16;
                     pixel_ptr += 2;
                 }
@@ -373,15 +377,14 @@
                 CHECK_PIXEL_PTR(0);  /* make sure pixel_ptr is positive */
             } else if (rle_code < 0) {
                 /* decode the run length code */
-                rle_code = -rle_code;
                 CHECK_STREAM_PTR(3);
                 r = s->buf[stream_ptr++];
                 g = s->buf[stream_ptr++];
                 b = s->buf[stream_ptr++];
 
-                CHECK_PIXEL_PTR(rle_code * 3);
+                CHECK_PIXEL_PTR(-rle_code * 3);
 
-                while (rle_code--) {
+                while (rle_code++) {
                     rgb[pixel_ptr++] = r;
                     rgb[pixel_ptr++] = g;
                     rgb[pixel_ptr++] = b;
@@ -453,7 +456,6 @@
                 CHECK_PIXEL_PTR(0);  /* make sure pixel_ptr is positive */
             } else if (rle_code < 0) {
                 /* decode the run length code */
-                rle_code = -rle_code;
                 CHECK_STREAM_PTR(4);
                 stream_ptr++;  /* skip the alpha (?) byte */
                 r = s->buf[stream_ptr++];
@@ -461,9 +463,9 @@
                 b = s->buf[stream_ptr++];
                 argb = (r << 16) | (g << 8) | (b << 0);
 
-                CHECK_PIXEL_PTR(rle_code * 4);
+                CHECK_PIXEL_PTR(-rle_code * 4);
 
-                while (rle_code--) {
+                while (rle_code++) {
                     *(unsigned int *)(&rgb[pixel_ptr]) = argb;
                     pixel_ptr += 4;
                 }



More information about the ffmpeg-devel mailing list