[FFmpeg-devel] pixfmt.h installed header using HAVE_BIGENDIAN?
Sun Jan 17 15:24:30 CET 2010
On Sun, Jan 17, 2010 at 03:19:35PM +0100, Reimar D?ffinger wrote:
> On Sun, Jan 17, 2010 at 03:14:57PM +0100, Reimar D?ffinger wrote:
> > On Sun, Jan 17, 2010 at 02:11:23PM +0000, M?ns Rullg?rd wrote:
> > > Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> > >
> > > > On Sun, Jan 17, 2010 at 01:48:23PM +0000, M?ns Rullg?rd wrote:
> > > >> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> > > >>
> > > >> > Hello,
> > > >> > am I missing something or is this a double-WTF?
> > > >> > First, pixfmt.h uses HAVE_BIGENDIAN which is a big no-no for an installed header,
> > > >> > but in addition it does not include config.h, so even pixdesc.c ends up saying
> > > >> > In file included from pixdesc.c:22:
> > > >> > pixfmt.h:130:5: warning: "HAVE_BIGENDIAN" is not defined
> > > >>
> > > >> I know, and it's a problem. Some discussion here:
> > > >> http://thread.gmane.org/gmane.comp.video.ffmpeg.devel/81522
> > > >
> > > > That one I though was fixed. Any objections if I just apply patch
> > > > below for now, in the worst case it breaks building of some
> > > > applications, but that's still better than breaking them
> > > > at runtime on bigendian which would happen right now.
> > > > Index: pixfmt.h
> > > > ===================================================================
> > > > --- pixfmt.h (revision 21108)
> > > > +++ pixfmt.h (working copy)
> > > > @@ -127,6 +127,8 @@
> > > > PIX_FMT_NB, ///< number of pixel formats, DO NOT USE THIS if you want to link with shared libav* because the number of formats might differ between versions
> > > > };
> > > >
> > > > +#ifdef HAVE_AV_CONFIG_H
> > > > +#include "config.h"
> > > > #if HAVE_BIGENDIAN
> > > > # define PIX_FMT_NE(be, le) PIX_FMT_##be
> > > > #else
> > > > @@ -148,5 +150,6 @@
> > > > #define PIX_FMT_YUV420P16 PIX_FMT_NE(YUV420P16BE, YUV420P16LE)
> > > > #define PIX_FMT_YUV422P16 PIX_FMT_NE(YUV422P16BE, YUV422P16LE)
> > > > #define PIX_FMT_YUV444P16 PIX_FMT_NE(YUV444P16BE, YUV444P16LE)
> > > > +#endif
> > > >
> > > > #endif /* AVUTIL_PIXFMT_H */
> > >
> > > This is the type of temporary hack that will stay forever. If we
> > > intend to ever fix it, we might as well do so now. Having a broken
> > > API serves nobody.
> > Uh, that actually is my fix. What more do you want?
> > We could make a pixfmt-internal header or move this stuff to internal.h or
> > what it is called though...
> Or to put it differently, removing the _NE formats from the public API is
> my idea of fixing the API, thus after this we actually don't have a broken
> API anymore (just an inconvenient one).
i wont approve a patch that slows down or bloats code in swscale & lavc
because of some philosophical thing about config.h
config.h should be installed, or at the very very least a file that contains
the big_endian define, none of the arguments against config.h installation
apply to this define
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 189 bytes
Desc: Digital signature
More information about the ffmpeg-devel