[FFmpeg-devel] [PATCH] Optimization of original IFF codec

[Sebastian Vater]

Ronald S. Bultje a ?crit :
>
>> +            uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
>>     
>
> No spaces.
>   
Fixed.
>> +            memset(row, 0, (avctx->width << 2));
>>     
>
> No brackets.
>   
Fixed.
>> -                decodeplane32(row, buf, FFMIN(s->planesize, buf_end - buf), avctx->bits_per_coded_sample, plane);
>>     
> [..]
>   
>> +                decodeplane32((uint32_t *) row, buf, FFMIN(s->planesize, buf_end - buf), avctx->bits_per_coded_sample, plane);
>>     
>
> Why did you add the cast? Is that necessary?
>   
Yes because I changed decodeplane from void * to uint8_t * and uint32_t
*, the cast removes a nasty GCC warning about incompatible
pointer-assignment ;)
>> -                    int length;
>> +                    unsigned length;
>>     
>
> Wrong patch.
>   
Oops, fixed!
>> -                    decodeplane32(row, s->planebuf, s->planesize, avctx->bits_per_coded_sample, plane);
>>     
> [..]
>   
>> +                    decodeplane32((uint32_t *) row, s->planebuf, s->planesize, avctx->bits_per_coded_sample, plane);
>>     
>
> Same as above (cast).
>   
Same reply as above :P
>> +                uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
>>     
> [..]
>   
>> +                uint8_t *row = &s->frame.data[0][ y*s->frame.linesize[0] ];
>>     
>
> Same as above (spaces).
>   
Fixed.
>> -                int length;
>> +                unsigned length;
>>     
>
> Same as above (unsigned/int conversion).
>   
Fixed.
> Btw, your patches are definitely getting better, I think you're
> getting the hang of it. We're just being anal for a little bit so you
> learn to submit great patches instead of just "patches". You'll notice
> it makes review of functional changes much easier, especially for
> complex code. You're well underway!
>   
Thank you very much!

Well, I trust you in this, you have years of experience regarding this
in advance!
So it's not my point on criticizing that, I'm sure that you're not doing
this to argue me! ;-)

-- 

Best regards,
                   :-) Basty/CDGS (-:

-------------- next part --------------
A non-text attachment was scrubbed...
Name: iff-opt-restruct-noindent.patch
Type: text/x-patch
Size: 4511 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100425/49d60721/attachment.bin>