[FFmpeg-devel] [PATCH] IFF: Add grayscale support to decoder

[Sebastian Vater]

Ronald S. Bultje a ?crit :
> Hi,
>
> On Mon, May 10, 2010 at 1:58 PM, Sebastian Vater
> <cdgs.basty at googlemail.com> wrote:
>   
>> Of course, this could be solved other ways, too. But this will make the
>> code more complex.
>>     
>
> You probably mean:
>
> count = av_clip(extradata_size / 3, 0, 1 << bps);
>
> That is a lot shorter and simpler than anything I've seen so far.
>   

If it does exactly the same as:
count = FFMAX(FFMIN(avctx->extradata_size / 3, count), 0);

Then, yes...as I understand it, it clips simply between, but please note
that including 1 << bps directly in the line will later require a change
when HAM comes (because for ham it's not 1 << bps but 1 << hbits instead.

But remember, this will make the code larger in size and slower in speed
(the if check if it's 0 has to be done anyway).
So unless there aren't other reasons for take the above stuff, I'ld
prefer if (count > 0)

There's no reason to clip to 0 first and then check if it's 0 (esp. when
I can do neg. or null checks with one if and one asm instruction, as
said it's just a change from jne to jge or je to jle...

So for now, just tell me, should I change that line right now to if
(count) instead of if (count > 0)?

The final word is upon you!

-- 

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