[FFmpeg-devel] Broken compile with latest libavutil/common.h

[Aurelien Jacobs]

Michael Niedermayer wrote:

> On Sun, Jan 18, 2009 at 07:05:52PM +0000, M?ns Rullg?rd wrote:
> > Aurelien Jacobs <aurel at gnuage.org> writes:
> > 
> > > On Sun, 18 Jan 2009 00:24:30 +0000
> > > M?ns Rullg?rd <mans at mansr.com> wrote:
> > >
> > >> Aurelien Jacobs <aurel at gnuage.org> writes:
> > >> 
> > >> > On Sat, 17 Jan 2009 21:52:16 +0100
> > >> > Diego Biurrun <diego at biurrun.de> wrote:
> > >> >
> > >> >> On Sat, Jan 17, 2009 at 08:30:06PM +0100, Aurelien Jacobs wrote:
> > >> >> > On Sat, 17 Jan 2009 16:19:17 +0000
> > >> >> > M?ns Rullg?rd <mans at mansr.com> wrote:
> > >> >> > > 
> > >> >> > > If it's going to libavcodec, I'd prefer it in mathops.h.  That file
> > >> >> > > has per-arch overrides already, and separating the assembler in that
> > >> >> > > function as well would be nice.
> > >> >> > 
> > >> >> > That sounds like a good idea. Attached patch moves mid_pred() in mathops.h
> > >> >> > and the x86 specific implementation in x86/mathops.h.
> > >> >> > 
> > >> >> > --- libavcodec/mathops.h	(revision 16652)
> > >> >> > +++ libavcodec/mathops.h	(working copy)
> > >> >> > @@ -83,5 +83,35 @@
> > >> >> >  
> > >> >> > +/* median of 3 */
> > >> >> > +#ifndef mid_pred
> > >> >> > +#define mid_pred mid_pred
> > >> >> > +static inline av_const int mid_pred(int a, int b, int c)
> > >> >> > +{
> > >> >> > +#if 0
> > >> >> 
> > >> >> Maybe this would be a good point to get rid of the disabled code..
> > >> >
> > >> > I agree, but it would probably be better in a separate patch.
> > >> >
> > >> >> > --- libavcodec/x86/mathops.h	(revision 16652)
> > >> >> > +++ libavcodec/x86/mathops.h	(working copy)
> > >> >> > @@ -22,6 +22,8 @@
> > >> >> >  #ifndef AVCODEC_X86_MATHOPS_H
> > >> >> >  #define AVCODEC_X86_MATHOPS_H
> > >> >> >  
> > >> >> > +#include "libavutil/avutil.h"
> > >> >> 
> > >> >> What is this header needed for?  It looks rather as if you should
> > >> >> #include config.h and common.h...
> > >> >
> > >> > AFAIK, avutil.h is granting inclusion of config.h and common.h.
> > >> > So I think it is simpler, cleaner and more future proof to
> > >> > include avutil.h. Not that it is important...
> > >> 
> > >> avutil.h also contains/includes other stuff.  In general, dependencies
> > >> should be kept minimal.  If you need avutil.h for some other reason,
> > >> you may omit the things it promises to include.  Do not use it merely
> > >> as a shorthand for what you actually need.
> > >
> > > Well, OK. Changed #include in attached patch.
> > 
> > This looks OK to be, but I'm not maintainer for those files.
> 
> iam ok with the patch though have to note that there are quite a few
> #includes added

Applied.

Aurel