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

[Måns Rullgård]

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.

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