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

[Xiaohui Sun]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Michael Niedermayer wrote:
> Hi

[...]
>> +} SgiState;
>> +
>> +/**
>> + * expand an RLE row into a channel
>> + * @param in_buf input buffer
>> + * @param out_buf output buffer
>> + * @param chan_offset offsets into input buffer
>> + * @param pixelstride pixel stride of input buffer
>> + * @return Size of output in bytes, -1 if buffer overflows
>> + */
>> +static int expand_rle_row(uint8_t *in_buf, uint8_t* end_buf,
>> +            unsigned char *out_buf, int chan_offset, int pixelstride)
>> +{
>> +    unsigned char pixel, count;
>> +    unsigned char *orig = out_buf;
>> +
>> +    out_buf += chan_offset;
>> +
>> +    while (1) {
>> +        if(in_buf + 1 > end_buf) return -1;
>> +        pixel = bytestream_get_byte(&in_buf);
>> +        if (!(count = (pixel & 0x7f))) {
>> +            return (out_buf - orig) / pixelstride;
>> +        }
>> +
>> +        if (pixel & 0x80) {
>> +            if(in_buf + count >= end_buf) return -1;
>> +            while (count--) {
>> +                *out_buf = bytestream_get_byte(&in_buf);
>> +                out_buf += pixelstride;
>> +            }
>> +        } else {
>> +            if(in_buf + 1 >= end_buf) return -1;
>> +            pixel = bytestream_get_byte(&in_buf);
>> +
>> +            while (count--) {
>> +                *out_buf = pixel;
>> +                out_buf += pixelstride;
>> +            }
>> +        }
>> +    }
>> +}
> 
> this code still lacks checks against buffer overflows

I find nowhere to get the out_buf size, I think it is linesize * s->height.

[...]
>> +
>> +    for (z = 0; z < s->depth; z++) {
>> +        dest_row = out_buf + s->height * s->linesize;
>> +        for (y = 0; y < s->height; y++) {
>> +            dest_row -= s->linesize;
> 
>> +            start_offset = bytestream_get_be32(&start_table);
>> +            if(in_buf + start_offset > end_buf) {
>> +                return AVERROR_INVALIDDATA;
>> +            }
> 
> this is better but it still does not catch all cases

Do you mean I should read the length_table and check if the encoded
length is overflow?

[...]
> 
> 
> [...]
>> +/**
>> + * Count up to 127 consecutive pixels which are either all the same or
>> + * all differ from the previous and next pixels.
>> + * @param start pointer to the first pixel
>> + * @param len maximum number of pixels
>> + * @param same 1 if searching for identical pixel values, 0 for differing
>> + * @return number of matching consecutive pixels found
>> + */
>> +static av_always_inline int count_pixels_1bpp(uint8_t *start, int len, int same)
>> +{
>> +    uint8_t *pos;
>> +    int count = 1;
>> +
>> +    for (pos = start + 1; count < FFMIN(127, len); pos ++, count ++) {
>> +        if (same != !(*(pos - 1) != * pos)) {
>> +            if (!same) {
>> +                /* if bpp == 1, then 0 1 1 0 is more efficiently encoded as a single
>> +                 * raw block of pixels.  for larger bpp, RLE is as good or better */
>> +                if (count + 1 < FFMIN(127, len) && *pos != *(pos + 1))
>> +                    continue;
>> +
>> +                /* if RLE can encode the next block better than as a raw block,
>> +                 * back up and leave _all_ the identical pixels for RLE */
>> +                count --;
>> +            }
>> +            break;
>> +        }
>> +    }
>> +    return count;
>> +}
>> +
>> +/**
>> + * Count up to 127 consecutive pixels which are either all the same or
>> + * all differ from the previous and next pixels.
>> + * @param start Pointer to the first pixel
>> + * @param len Maximum number of pixels
>> + * @param bpp Bytes per pixel
>> + * @param same 1 if searching for identical pixel values, 0 for differing
>> + * @return number of matching consecutive pixels found
>> + */
>> +static av_always_inline int count_pixels_nbpp(uint8_t *start, int len, int bpp, int same)
>> +{
>> +    uint8_t *pos;
>> +    int count = 1;
>> +
>> +    for (pos = start + bpp; count < FFMIN(127, len); pos += bpp, count ++) {
>> +        if (same != !memcmp(pos - bpp, pos, bpp)) {
>> +            if (!same) {
>> +                /* if RLE can encode the next block better than as a raw block,
>> +                 * back up and leave _all_ the identical pixels for RLE */
>> +                count --;
>> +            }
>> +            break;
>> +        }
>> +    }
>> +    return count;
>> +}
> 
> this is still duplicated relative to targaenc.c

I will put them in a separeate rle.c/rle.h and let targaenc.c call it.




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGDxuN+BwsLu3sVWwRAmLbAJ0St5C8+euWIHbH3WDI+Cvc+g1dyACgwWLa
3lOs6ZxtzvieWpNK73wE9eY=
=3RNu
-----END PGP SIGNATURE-----