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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com