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

[Vitor Sessak]

On 11/11/2010 08:55 PM, Reimar D?ffinger wrote:
> 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;
> ?

Both your suggestion and current SVN in bigendian give the same visual 
artifacts (see attached). I'd say that the encoder use the same softsimd 
trick, and don't care if it overflows if the final distortion is lower.

> What if you in addition also clip the result (not you'll have to remove
> the dc *= 0x10001; for that to work)?

Even if it works, it would be slower...

-Vitor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: out_be5.jpg
Type: image/jpeg
Size: 57560 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101111/e1785e4e/attachment.jpg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: out_le5.jpg
Type: image/jpeg
Size: 58161 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101111/e1785e4e/attachment-0001.jpg>