[FFmpeg-devel] [PATCH] VP8 decoder

[Ronald S. Bultje]

Hi,

On Tue, Jun 22, 2010 at 4:11 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Jun 22, 2010 at 11:27:14AM -0400, Ronald S. Bultje wrote:
>> 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?
>
> call it PACK4UINT8 please

Changed.

Ronald