[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