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

[Xiaohui Sun]

Xiaohui Sun wrote:
patch updated.
> Hi
>
> On Thu, Apr 05, 2007 at 09:42:59AM +0800, Xiaohui Sun wrote:
> [...]
>
> > + * @param out_ptr output buffer
>
> this pointer does not point to the output buffer but rather to the line
> after the output buffer

done

>
>
> > + * @param in_buf input buffer
> > + * @param end_buf end of input buffer
> > + * @param s the current image state
> > + * @return 0 if no error, else return error number
> > + */
> > +static int read_rle_sgi(unsigned char* out_ptr, uint8_t *in_buf,
> > +                        uint8_t *end_buf, SgiState* s)
>
> s/end_buf/in_end/

done

>
>
> > +{
> > +    uint8_t *dest_row = out_ptr;
> > +    unsigned int len = s->height * s->depth * 4;
> > +    uint8_t *start_table = in_buf;
> > +    unsigned int y, z;
> > +    unsigned int start_offset;
> > +
> > +    /* size of  RLE offset and length tables */
> > +    if(len * 2  > end_buf - in_buf) {
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> > +    in_buf -= SGI_HEADER_SIZE;
> > +    for (z = 0; z < s->depth; z++) {
> > +        for (y = 0; y < s->height; y++) {
> > +            dest_row -= s->linesize;
> > +            start_offset = bytestream_get_be32(&start_table);
> > +            if(start_offset > end_buf - in_buf) {
> > +                return AVERROR_INVALIDDATA;
> > +            }
> > +            if (expand_rle_row(in_buf + start_offset, end_buf, 
> dest_row + z,
> > +                dest_row + FFABS(s->linesize), s->depth) != s->width)
> > +                return AVERROR_INVALIDDATA;
> > +        }
>
> > +        dest_row = out_ptr;
>
> if you move this line before the for() then you avoid the second such
> assignment above

done

>
>
> > +    }
> > +    return 0;
> > +}
> > +
> > +/**
> > + * read an uncompressed SGI image
> > + * @param out_ptr output buffer
>
> > + * @param end_ptr end ofoutput buffer
>
> typo also s/end_ptr/out_end/

done

>
>
> > + * @param in_buf input buffer
>
> > + * @param end_buf end of input buffer
>
> s/end_buf/in_end/
>

done

>
> [...]
> > +    uint8_t *dest_row = end_ptr;
>
> this is uneeded
>

done

>
> > +    uint8_t *ptr = in_buf;
> > +    unsigned int offset = s->height * s->width;
> > +
> > +    /* test buffer size */
> > +    if (offset * s->depth > end_buf - in_buf) {
> > +       return -1;
> > +    }
> > +
> > +    for (y = s->height - 1; y >= 0; y--) {
>
> > +        dest_row = out_ptr + (y * FFABS(s->linesize));
>
> this looks wrong

changed

>
>
> > +        for (x = s->width; x > 0; x--) {
> > +            for(z = 0; z < s->depth; z ++) {
> > +                bytestream_put_byte(&dest_row, *ptr);
> > +                ptr += offset;
> > +            }
>
> > +            ptr = ++in_buf;
>
> if you move this assignment before the for loop then you avoid the second
> redundant assignment farther above

done

>
>
> [...]
> > +    dimension = bytestream_get_be16(&in_buf);
> > +    s->width = bytestream_get_be16(&in_buf);
> > +    s->height = bytestream_get_be16(&in_buf);
> > +    s->depth = bytestream_get_be16(&in_buf);
>
> these could be verticall aligned like
>
> dimension = bytestream_get_be16(&in_buf);
> s->width  = bytestream_get_be16(&in_buf);
> s->height = bytestream_get_be16(&in_buf);
> s->depth  = bytestream_get_be16(&in_buf);
>
> looks nicer ... (not really important though)

changed

>
>
> [...]
> > +    /* name */
> > +    memset(buf, 0, 80);
> > +    buf += 80;
> > +
> > +     /* colormap */
> > +    bytestream_put_be32(&buf, 0L);
> > +
> > +    /* the rest of the 512 byte header is unused */
> > +    memset(buf, 0, 404);
> > +    buf += 404;
>
> thats just a single memset() ...
>
>
> [...]
> > +            if((length = ff_rle_encode(buf, end_buf - buf - 1, 
> encode_buf, 1, width, 0, 0, 0x80, 0)) == -1) {
>
> if(... < 1)
>

done

>
> [...]

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