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

[Xiaohui Sun]

Michael Niedermayer 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 think I need to optimize for 1bpp and for step encoding situation , 
which is needed for SGI encoding.

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

you means I should check if the start_offset is overflow, and check 
against INT32_MAX?

>> +        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
>   

yes, it seems quite ugly, since I want to do a discrete read and a 
sequential write, so I move the pointer each time.

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

 should I use PIX_FMT_RGBA instead,  which is endian independant.

>> +    return (buf - orig_buf);
>>     
>
> superfluous ()
>   
I need to calculate the encoded size here.