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

[Maxim]

Reimar D?ffinger schrieb:
> [...]
>> /** divide the motion vector mv by 4 */
>> #define IVI_MV_DIV4(mv) (((mv) + 1 + ((mv) > 0))>>2)
>>
>> /** divide the motion vector mv by 2 */
>> #define IVI_MV_DIV2(mv) (((mv) + ((mv) > 0))>>1)
>>     
>
> Since the mv == 0 case does not matter, you should be able to avoid
> the comparison by
> static inline int32_t ivi_mv_div4(int32_t mv)
> {
>     return (mv + 2 - ((uint32_t)mv >> 31)) >> 2;
> }
> And yes, use static inline functions if you can.
>   


It just came into my mind that those both macros can be merged together.
This would simplify the code and help to drop switch statements:

if (band->inherit_mv){
    /* motion vector inheritance */
    switch (mv_scale) {
    case 0:
        mb->mv_x = ref_mb->mv_x;
        mb->mv_y = ref_mb->mv_y;
        break;
    case 1:
        mb->mv_x = IVI_MV_DIV2(ref_mb->mv_x);
        mb->mv_y = IVI_MV_DIV2(ref_mb->mv_y);
        break;
    case 2:
        mb->mv_x = IVI_MV_DIV4(ref_mb->mv_x);
        mb->mv_y = IVI_MV_DIV4(ref_mb->mv_y);
        break;
    }
}

So the mv_scale == amount of shift in the macros above. I suggest to
simplify the whole stuff like this (untested):

/** divide the motion vector mv by n */
#define IVI_MV_DIV(mv, n) (((mv) + ((n)-1) + ((mv) > 0))>>(n))

if (mv_scale) {
    mb->mv_x = IVI_MV_DIV(ref_mb->mv_x, mv_scale);
    mb->mv_y = IVI_MV_DIV(ref_mb->mv_y, mv_scale);
} else {
    mb->mv_x = ref_mb->mv_x;
    mb->mv_y = ref_mb->mv_y;
}


Regards
Maxim