[Ffmpeg-devel] [PATCH] ported SGI decoder to the new API
Michael Niedermayer
michaelni
Sun Apr 1 17:18:30 CEST 2007
Hi
On Sun, Apr 01, 2007 at 08:38:24PM +0800, Xiaohui Sun wrote:
[...]
> Index: libavcodec/rle.c
> ===================================================================
> --- libavcodec/rle.c (revision 0)
> +++ libavcodec/rle.c (revision 0)
> @@ -0,0 +1,253 @@
your rle encoder has as many lines of code as the whole targa encoder
this can be done simpler
[...]
> +static int expand_rle_row(uint8_t *in_buf, uint8_t* end_buf,
> + unsigned char *out_ptr, uint8_t* end_ptr,
> + int chan_offset, int pixelstride)
> +{
> + unsigned char pixel, count;
> + unsigned char *orig = out_ptr;
> +
> + out_ptr += chan_offset;
> +
> + while (1) {
> + if(in_buf + 1 > end_buf) return -1;
> + pixel = bytestream_get_byte(&in_buf);
> + if (!(count = (pixel & 0x7f))) {
> + return (out_ptr - orig) / pixelstride;
> + }
> +
> + if (pixel & 0x80) {
> + if ((in_buf + count >= end_buf) || (out_ptr + count > end_ptr)) return -1;
> + while (count--) {
> + *out_ptr = bytestream_get_byte(&in_buf);
> + out_ptr += pixelstride;
> + }
> + } else {
> + if ((in_buf + 1 >= end_buf) || (out_ptr + count > end_ptr)) return -1;
> + pixel = bytestream_get_byte(&in_buf);
> +
> + while (count--) {
> + *out_ptr = pixel;
> + out_ptr += pixelstride;
> + }
> + }
> + }
you check for buffer overflows now, sadly the checks are buggy
also they can be simplified
[...]
> + start_offset = bytestream_get_be32(&start_table);
> + len = bytestream_get_be32(&length_table);
> + if(in_buf + start_offset + len > end_buf) {
> + return AVERROR_INVALIDDATA;
> + }
this check still is not catching all cases, also it seems you missunderstood
me, i didnt mean that you check the values in an otherwise unused table but
rather that you properly check the values you do use for overflow
[...]
> + for (x = s->width; x > 0; x--) {
> + ptr = in_buf;
> + offset = 0;
> + for(z = 0; z < s->depth; z ++) {
> + ptr += offset;
> + bytestream_put_byte(&dest_row, *ptr);
> + }
> + in_buf ++;
> + }
this is buggy
[...]
> + s->linesize = p->linesize[0];
> + p->pict_type = FF_I_TYPE;
> + p->key_frame = 1;
> + ptr = p->data[0];
> +
> + end_ptr = ptr + FFABS(s->linesize * s->height);
this is buggy
[...]
> + case PIX_FMT_RGB32:
> + dimension = SGI_MULTI_CHAN;
> + depth = SGI_RGBA;
> + break;
i think this is broken on either little or big endian
[...]
> + return (buf - orig_buf);
superfluous ()
[...]
--
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/20070401/776be5a4/attachment.pgp>
More information about the ffmpeg-devel
mailing list