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

Reimar Döffinger Reimar.Doeffinger
Mon Jan 18 01:02:17 CET 2010


On Mon, Jan 18, 2010 at 12:31:35AM +0100, Maxim wrote:
> 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!

Yes, that happens when you don't test code. However I was close.
The sign_extend does the same as the -(val & 1), and whether the shift
is done before or after the xor does not matter (though your way is likely
to be faster).
However I didn't read properly and thought a set lowest bit gave the negative
result...

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

Not to you, I was talking about the sign_extend function.



More information about the ffmpeg-devel mailing list