[MPlayer-dev-eng] [FFmpeg-devel] [PATCH] remove useless math.h include in libavutil/mathematics.h

[Reimar Döffinger]

On Mon, Aug 25, 2008 at 09:15:29PM +0200, Guillaume POIRIER wrote:
> Hello,
> 
> On Mon, Aug 25, 2008 at 8:47 PM, David Conrad <lessen42 at gmail.com> wrote:
> > On Aug 25, 2008, at 12:07 PM, Måns Rullgård wrote:
> >
> >> Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:
> >>
> >>> On Mon, Aug 25, 2008 at 05:47:49PM +0200, Guillaume Poirier wrote:
> >>>>
> >>>> I noticed that the include of math.h in libavutil/mathematics.h doesn't
> >>>> seem to be usefull.
> >>>
> >>> The point is that it can provide M_E etc. If you remove it and some code
> >>> (possibly indirectly) includes <math.h> after mathematics.h those will
> >>> be defined twice. I.e. removing it IMHO makes a mess.
> >>
> >> Yes, the #include stays.
> >>
> >>>> In file included from /usr/include/math.h:26,
> >>>>              from ./libavutil/mathematics.h:25,
> >>>>              from ./libavutil/avutil.h:57,
> >>>>              from liba52/parse.c:40:
> >>>> /usr/include/architecture/ppc/math.h:179: error: parse error before
> >>>> '__attribute__'
> >>>
> >>> Well, wtf is in math.h that it stumbles over?
> >>
> >> This is getting better and better.  First macosx can't link with its
> >> own libs, and now it can't even compile its own headers.  If this
> >> keeps up, it won't be long before it stops booting at all, and we'll
> >> finally see an end to these problems.
> >
> > It uses __attribute__((always_inline)), and mplayer's liba52/bitstream.h
> > defines  always_inline to __attribute__((always_inline)) inline, so it's
> > expanded to __attribute__((__attribute__((always_inline)) inline)) and
> > pukes.
> >
> > Attached mplayer patch should fix the root of the problem. (untested)
> 
> Good catch.
> This simpler change:
> 
> Index: liba52/parse.c
> ===================================================================
> --- liba52/parse.c	(revision 27478)
> +++ liba52/parse.c	(working copy)
> @@ -32,12 +32,12 @@
>  #include <string.h>
>  #include <inttypes.h>
> 
> +#include "libavutil/avutil.h"
>  #include "a52.h"
>  #include "a52_internal.h"
>  #include "bitstream.h"
>  #include "tables.h"
>  #include "mm_accel.h"
> -#include "libavutil/avutil.h"
> 
>  #ifdef HAVE_MEMALIGN
>  /* some systems have memalign() but no declaration for it */
> 
> 
> did the trick as well.
> Reimar, what do you think about that change?

Bad idea, if there is code duplicated with libavutil it should at least
be kept in sync.
IMO the always_inline define should be completely removed and replaced
with a libavutil/common.h include if possible.