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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20090118/04d5c025/attachment.pgp>