[FFmpeg-devel] [PATCH] Add AV_CONFIG_* for libav* libs to libavutil/avconfig.h

Måns Rullgård mans
Sat Mar 6 01:07:16 CET 2010


Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:

> On date Wednesday 2010-03-03 00:30:01 +0000, M?ns Rullg?rd encoded:
>> Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> 
>> > On date Thursday 2010-02-25 00:54:57 +0100, Stefano Sabatini encoded:
>> >> On date Sunday 2010-02-21 18:41:32 +0000, M?ns Rullg?rd encoded:
>> >> > Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> >> > 
>> >> > > On date Sunday 2010-02-21 15:46:37 +0100, Reimar D?ffinger encoded:
>> > [...]
>> >> > >> So to say it without the flaming:
>> >> > >> Please include avconfig.h and if it doesn't contain what you need
>> >> > >> change it to include it.
>> >> > >
>> >> > > Check attached.
>> >> > 
>> >> > Let me think about it for a minute.
>> >> 
>> >> The minute passed on... ;-)
>> >
>> > Mans-ping.
>> > -- 
>> > FFmpeg = Freak and Fierce Magic Powered Empowered God
>> >
>> > From 0abab5186d10f9bb48c1f8387a1e8403f658d1f5 Mon Sep 17 00:00:00 2001
>> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
>> > Date: Wed, 3 Mar 2010 00:53:47 +0100
>> > Subject: [PATCH] Add AV_CONFIG_* symbols for libav* libs to libavutil/avconfig.h,
>> >  and make ffmpeg.c and ffplay.c include avconfig.h rather than config.h.
>> >
>> > config.h should be never directly included by an application, not even
>> > by an internal ff* tool.
>> 
>> Those files depend on many more configure-detected values, some of
>> which should definitely not go into any installed header.
>> 
>> If we insist on not including config.h in ff*.c for some obscure
>> purist reasons, we will have to make configure output yet another
>> header just for these.  I really don't see any advantage in doing
>> that.
>
> Patch updated, and yes the previous patch was just wrong, this should
> be enough for supporting indevs in ffprobe.

I've thought about this, and I've come to the conclusion that it's the
wrong approach.

libavutil/avconfig.h is meant to contain system invariants determined
by configure, e.g. machine byte order.  It is not the proper place to
indicate whether or not the other libs were installed.

Consider a completely separate application.  To check for a library,
one naturally runs a test in a configure script.  Expecting a
different library to provide the answer feels strange to me.  To find
out whether libvorbis is installed, you do not include libogg.h.

The ff* apps are in a special situation in that they are considered
external to the libav* libraries, and thus must use only public the
API, while still sharing configure script and makefiles.  This is not
a problem at all in my opinion.  The config.h file we create does in
no way depend on internals of the libraries.  It is therefor not wrong
of ffmpeg.c et al to #include config.h.  If they were built separately
from the libs, there would still be a configure script creating a very
similar config.h.

What we should _not_ be doing is defining HAVE_AV_CONFIG_H when
compiling ffmpeg.c.  That would expose internals which we should not
be touching.

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



More information about the ffmpeg-devel mailing list