[MPlayer-cvslog] r26219 - trunk/mpbswap.h

[Aurelien Jacobs]

Ivan Kalvachev wrote:

> On Tue, Mar 11, 2008 at 1:29 PM, Reimar Döffinger
> <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> >
> > On Tue, Mar 11, 2008 at 01:20:31PM +0200, Ivan Kalvachev wrote:
> >  > On Tue, Mar 11, 2008 at 12:42 PM, Reimar Döffinger
> >  > <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> >  > > On Mon, Mar 10, 2008 at 09:20:08PM +0100, diego wrote:
> >  > >  > Author: diego
> >  > >  > Date: Mon Mar 10 21:20:08 2008
> >  > >  > New Revision: 26219
> >  > >  >
> >  > >  > Log:
> >  > >  > Add missing header #include.
> >  > >  >
> >  > >  >
> >  > >  > Modified:
> >  > >  >    trunk/mpbswap.h
> >  > >  >
> >  > >  > Modified: trunk/mpbswap.h
> >  > >  > ==============================================================================
> >  > >  > --- trunk/mpbswap.h   (original)
> >  > >  > +++ trunk/mpbswap.h   Mon Mar 10 21:20:08 2008
> >  > >  > @@ -3,6 +3,8 @@
> >  > >  >
> >  > >  >  #include <sys/types.h>
> >  > >  >  #include "libavutil/bswap.h"
> >  > >  > +#include "config.h"
> >  > >  > +
> >  > >
> >  > >  Uh, libavutil/bswap.h almost certainly requires config.h,
> >  > > this seems like a stupid place to put it?

Uh, libavutil/bswap.h almost certainly doesn't require anything as
it is supposed to be self-contained.

> >  > I think I should save you some time and give you the previous
> >  > answer we got from diego about same kind of bug:
> >  >
> >  > On Sat, Mar 8, 2008 at 4:36 PM, Diego Biurrun <diego at biurrun.de>
> >  > wrote:
> >  > >  Nonsense.
> >  > >  Diego
> >
> >  I don't claim it to be a bug, I just say it does not make much
> > sense to me. Also previously the policy I think was if config.h is
> > included it should be included directly after the system headers.
>  
> Yes, and there is very good reason for that rule.
> 
> This is not bug only because remaining proper config.h inclusions are
> still in the .c files, so this include have no actual meaning.
> 
> If the some .c file for some reason doesn't do that, this commit would
> ensure very rare and obscure breaking (for example on fedora ).

Whether .c files include config.h or not won't change anything.
All ffmpeg headers are supposed to be self-contained !
Feel free to show us how is this supposed to fail on fedora (or whatever).

BTW: The real bug here is to include libavutil/bswap.h which is *not*
part of ffmpeg public API. But that's another story. And it seems
everyone agree it's OK for MPlayer to deal with ffmpeg internals.

Aurel