[Ffmpeg-devel] [PATCH] ported SGI decoder to the new API

[Xiaohui Sun]

I have updated the patch.

Xiaohui Sun wrote:
> 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

I have simplified the rle.c and only put the count_pixel function in it. 
It looks that is the common part of different RLE encoders.

>
> [...]
>
> > +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

changed

>
>
> [...]
> > +            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
>
fixed

>
> [...]
> > +        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
fixed

>
> [...]
> > +    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
>

fixed

>
> [...]
> > +        case PIX_FMT_RGB32:
> > +            dimension = SGI_MULTI_CHAN;
> > +            depth = SGI_RGBA;
> > +            break;
>
> i think this is broken on either little or big endian
I have changed that to PIX_FMT_RGBA.

>
> [...]
> > +    return (buf - orig_buf);
>
> superfluous ()
>
done

>
> [...]
>

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: sgi_port.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070403/74d88e5c/attachment.asc>