[FFmpeg-devel] [PATCH 1/3] Indeo 5 decoder: common functions

[Maxim]

Reimar D?ffinger schrieb:
> [...]
>> static inline int ivi_pic_config_cmp(IVIPicConfig *str1, IVIPicConfig *str2)
>>     
>
> No documentation.
>
>   
>> /** convert unsigned values into signed ones (the sign is in the LSB) */
>> /* TODO: find a way to calculate this without the conditional using bit magic */
>> #define IVI_TOSIGNED(val) (((val) & 1) ? ((val) + 1) >> 1 : -(((val) + 1) >> 1))
>>     
>
> I'm sure this kind of code is already in other decoders.
> However the + 1 in the part after : sure is useless.
> Also I strongly suggest to make this a static inline function.
> And bit magic to do it should be
> ((sign_extend(val & 1, 1) ^ val) >> 1) + 1
>   

Ok, I suggest the following bitexact replacement:

-((val >> 1) ^ -(val & 1));

Reimar, I'm sorry but your code produces a different result compared to
the existing IVI_TOSIGNED!

> I think the +1 is necessary, but I can't say for sure since my request
> to document it when it was added was just ignored.
>   

Wait, when and where it was ignored? Do I misunterstand smth? I cannot
remember you ever requested something like that! Please point me to,
because I really tried to take each suggestion comment/seriously!

Regards
Maxim