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

[Xiaohui Sun]

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

Michael Niedermayer wrote:
> Hi
> 
> On Mon, Apr 02, 2007 at 12:05:07AM +0800, Xiaohui Sun wrote:
>> 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.
> 
> optimization is certainly nice yes but you dont need that much complexity
> for it

ok, I would refactor that part of code.

> 
> 
>> [...]
>>> [...]
>>>  
>>>> +            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?
> 
> maybe though maybe i missunderstand you again but you definitly should
> check the thing so that it wont segfault when dereferenced

Would you please pointer out more specifically what the problem is,
since I did not meet problems when I test it.

> 
> 
>>>> +        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.
> 
> it is buggy (=not working)

it do work for me, could you send me the test file that fails, and thank
you again for your advice :)



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

iD8DBQFGELYf+BwsLu3sVWwRAr2UAJ9LW5AUVvbsu77nog4ZoAGMoCy41wCfbfKc
a52R0vQglt09FgWeZwchKqY=
=mz48
-----END PGP SIGNATURE-----