[FFmpeg-devel] [PATCH] VP8 decoder

[Michael Niedermayer]

On Tue, Jun 22, 2010 at 11:27:14AM -0400, Ronald S. Bultje wrote:
> 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?

call it PACK4UINT8 please

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100622/07649a65/attachment.pgp>