[FFmpeg-devel] [PATCH] VP8 decoder

[Ronald S. Bultje]

Hi,

2010/6/22 M?ns Rullg?rd <mans at mansr.com>:
> "Ronald S. Bultje" <rsbultje at gmail.com> writes:
>> 2010/6/22 M?ns Rullg?rd <mans at mansr.com>:
>>> "Ronald S. Bultje" <rsbultje at gmail.com> writes:
>>>> 2010/6/22 M?ns Rullg?rd <mans at mansr.com>:
>>>>> "Ronald S. Bultje" <rsbultje at gmail.com> writes:
>>>>>> +#ifndef PACK4x8
>>>>>> +#define PACK4x8(a,b,c,d) \
>>>>>> + ? ?HAVE_BIGENDIAN ? (a << 24) | (b << 16) | (c << 8) | d : \
>>>>>> + ? ? ? ? ? ? ? ? ? ? (d << 24) | (c << 16) | (b << 8) | a
>>>>>> +#endif
>>>>>
>>>>> This is missing parens everywhere. ?I'm also uncertain whether it
>>>>> might not be better to use #if HAVE_BIGENDIAN instead. ?You never know
>>>>> with compilers...
>>>>
>>>> That one I'm not too worried about, don't we require it to do that
>>>> correctly (symbol elimination) in allcodecs.c?
>>>
>>> We do, but I still don't trust them. ?This one would go unnoticed if
>>> it didn't do the right thing.
>>>
>>>> Brackets around the whole operation added.
>>>>
>>>> +#ifndef PACK4x8
>>>> +#define PACK4x8(a,b,c,d) \
>>>> + ? ?HAVE_BIGENDIAN ? ((a << 24) | (b << 16) | (c << 8) | d) : \
>>>> + ? ? ? ? ? ? ? ? ? ? ((d << 24) | (c << 16) | (b << 8) | a)
>>>> +#endif
>>>
>>> Congratulations, you added the only parens you don't need. ?You need
>>> parens around each use of the args and around the whole thing.
>>
>> OK, both changed now. I kept the macros around the whole thing also.
>>
>> Ronald
>>
>> Index: ffmpeg-svn/libavcodec/mathops.h
>> ===================================================================
>> --- ffmpeg-svn.orig/libavcodec/mathops.h ? ? ?2010-06-22 11:10:24.000000000 -0400
>> +++ ffmpeg-svn/libavcodec/mathops.h ? 2010-06-22 11:13:15.000000000 -0400
>> @@ -146,5 +146,15 @@
>> ?# ? define NEG_USR32(a,s) (((uint32_t)(a))>>(32-(s)))
>> ?#endif
>>
>> +#ifndef PACK4x8
>> +#if HAVE_BIGENDIAN
>> +#define PACK4x8(a,b,c,d) \
>> + ? ?(((a) << 24) | ((b) << 16) | ((c) << 8) | (d))
>> +#else
>> +#define PACK4x8(a,b,c,d) \
>> + ? ?(((d) << 24) | ((c) << 16) | ((b) << 8) | (a))
>> +#endif
>> +#endif
>> +
>> ?#endif /* AVCODEC_MATHOPS_H */
>
> #ifndef PACK4x8
> # if HAVE_BIGENDIAN
> # ? define PACK4x8(a,b,c,d) (((a) << 24) | ((b) << 16) | ((c) << 8) | (d))
> # else
> # ? define PACK4x8(a,b,c,d) (((d) << 24) | ((c) << 16) | ((b) << 8) | (a))
> # endif
> #endif

Fine with me also. :-). Any other comments before I apply?

Ronald