[FFmpeg-devel] [PATCH] Fix 4XM decoding on big-endian and unaligned reads

[Reimar Döffinger]

On Thu, Nov 11, 2010 at 04:49:22PM +0100, Vitor Sessak wrote:
> Index: libavcodec/4xm.c
> ===================================================================
> --- libavcodec/4xm.c	(revision 25719)
> +++ libavcodec/4xm.c	(working copy)
> @@ -260,6 +260,23 @@
>      }
>  }
>  
> +#if HAVE_BIGENDIAN
> +#define LE_CENTRIC_MUL(dst, src, scale, dc) \
> +    { \
> +        unsigned tmpval = ((src)[1] << 16) + (src)[0];  \
> +        tmpval = tmpval * (scale) + (dc);               \
> +        (dst)[0] = tmpval & 0xFFFF;                     \
> +        (dst)[1] = tmpval >> 16;                        \
> +    }
> +#else
> +#define LE_CENTRIC_MUL(dst, src, scale, dc) \
> +    { \
> +        unsigned tmpval = ((src)[1] << 16) + (src)[0];  \
> +        tmpval = tmpval * (scale) + (dc);               \
> +        *((uint32_t *) (dst)) = tmpval;                 \
> +    }
> +#endif
> +
>  static inline void mcdc(uint16_t *dst, uint16_t *src, int log2w, int h, int stride, int scale, int dc){
>     int i;
>     dc*= 0x10001;
> @@ -274,25 +291,25 @@
>          break;
>      case 1:
>          for(i=0; i<h; i++){
> -            ((uint32_t*)dst)[0] = scale*((uint32_t*)src)[0] + dc;
> +            LE_CENTRIC_MUL(dst, src, scale, dc);

It seems very wrong to me that this should be necessary, if the order
matters that would mean that we have an overflow on LE as well.
Is there actually a visible difference?
And what if you replace your macro by
dst[0] = scale * src[0] + dc;
dst[1] = scale * src[1] + dc;
?
What if you in addition also clip the result (not you'll have to remove
the dc *= 0x10001; for that to work)?