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

[Xiaohui Sun]

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

Hi,
   patch updated


Michael Niedermayer wrote:
> Hi
> 
> On Wed, Apr 04, 2007 at 12:11:08AM +0800, Xiaohui Sun wrote:
>> Hi,
>>   I updated the patch.

[...]

> 
>>> [...]
>>>> +            start_offset = bytestream_get_be32(&start_table);
>>>> +            if(start_offset > end_buf - in_buf) {
>>>> +                return AVERROR_INVALIDDATA;
>>>> +            }
>>> excelent, finally the check looks good
>>>
>>>
>>>> +            if (expand_rle_row(in_buf + start_offset, end_buf, 
>>> dest_row,
>>>> +                dest_row + s->linesize, z, s->depth) != s->width)
>>>> +                return AVERROR_INVALIDDATA;
>>> linesize can be negative
>>>
>> I modified, but not sure.
> 
> it should have been dest_row + width*pixelsize or dest_row + FFABS(s->linesize)

done

> 
> 
> [...]
>> +/**
>> + * expand an RLE row into a channel
>> + * @param in_buf input buffer
>> + * @param out_ptr output buffer
>> + * @param end_ptr end of line in output 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_ptr, uint8_t* end_ptr, int pixelstride)
>> +{
>> +    unsigned char pixel, count;
>> +    unsigned char *orig = out_ptr;
>> +
>> +    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;
>> +        }
>> +
>> +        /* check for buffer overflow */
>> +        if(out_ptr + pixelstride * count >= end_ptr) return -1;
> 
> please rename end_ptr to out_end and end_buf to in_end

done

> 
> 
> [...]
>> +    out_ptr = p->data[0];
>> +
>> +    end_ptr = out_ptr + p->linesize[0] * s->height;
>> +
>> +    if(p->linesize[0] < 0) {
>> +        s->linesize = - p->linesize[0];
>> +        FFSWAP(uint8_t*, end_ptr, out_ptr);
>> +    } else {
>> +        s->linesize = p->linesize[0];
>> +    }
> 
> this is wrong
> first, it simply wont work as the pointers are off by one line
> second, if the pointers where not off by one line it would store a
> vertically fliped image

removed

> 
> 
> [...]
>> +    avcodec_get_frame_defaults((AVFrame*)&s->picture);
>> +    avctx->coded_frame = (AVFrame*)&s->picture;
> 
> unneeded casts

removed

> 
> 
> [...]
>> +    .pix_fmts= (enum PixelFormat[]){PIX_FMT_RGB24, PIX_FMT_RGB32, PIX_FMT_PAL8, PIX_FMT_GRAY8, -1},
> 
> PIX_FMT_RGB32 is wrong, i think this was already correct in a previous
> patch ...

done, sorry for my false code merging.

> 
> PIX_FMT_RGB32 store native endian 32bit integers with rgb in them so
> it differs on little and big endian, the bytewise readeing and writing
> code in your codec though writes the same format on both little and
> big endian so PIX_FMT_RGB32 cannot be correct
> 
> [...]
> 

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

iD8DBQFGFFQj+BwsLu3sVWwRAqGRAJ9sj8/1DKPdIPUTrI8ENA9R9ZmKIACfbhJl
zX6SHnLRwIb2Y1xG85nGJPw=
=MxSb
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: sgi_port.patch
Type: text/x-diff
Size: 16576 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070405/74f399e4/attachment.patch>