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

[Michael Niedermayer]

Hi

On Sun, Apr 01, 2007 at 10:40:14AM +0800, Xiaohui Sun wrote:
> -----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.

FABS(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?

yes


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

ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/b3d93b9b/attachment.pgp>