[MPlayer-dev-eng] [PATCH] support for shared libavformat

[Diego Biurrun]

On Sun, Jan 08, 2006 at 10:16:08PM +0100, Nico Sabbi wrote:
> Diego Biurrun wrote:
> 
> >On Sun, Jan 08, 2006 at 06:23:02PM +0100, Nico Sabbi wrote:
> >
> >>The order of preference is static, shared.
> >>If there are no objections I'll commit soon.
> >
> >Not yet, I still have some issues for you to address.
> >
> >>--- configure	26 Dec 2005 03:16:47 -0000	1.1114
> >>+++ configure	8 Jan 2006 17:17:12 -0000
> >>@@ -1436,6 +1436,8 @@
> >>_libavcodecs=`grep 'register_avcodec(&[a-z]' libavcodec/allcodecs.c  | 
> >>sed 's/.*&\(.*\)).*/\1/'`
> >>_libavcodecso=auto 
> >>_libavformat=auto
> >>+_libavformat_so=no
> >>+_conf_libavformat=no
> >
> >I think _conf_libavformat is unnecessary complication.  Do it the same
> >way, libavcodec does, see below.
> 
> I don't like it: you have no way to know if you are using the internal 
> or the external one

I disagree, only one gets enabled...

> >>@@ -7158,7 +7178,7 @@
> >>CONFIG_LIBAVUTIL = $_libavutil
> >>CONFIG_LIBAVCODEC = $_libavcodec
> >>CONFIG_LIBAVCODECSO = $_libavcodecso
> >>-CONFIG_LIBAVFORMAT = $_libavformat
> >>+CONFIG_LIBAVFORMAT = $_conf_libavformat
> >
> >.. you add CONFIG_LIBAVFORMATSO = $_libavformatso, much simpler IMO.
> 
> but in that case you have 2 different CONFIG_LIBAVFORMAT in config.mak,
> that is bad, IMO

What's the problem?  One is set to yes, the other to no, just like
libavcodec...

> >>--- libmpdemux/demux_lavf.c	10 Oct 2005 05:45:38 -0000	1.22
> >>+++ libmpdemux/demux_lavf.c	8 Jan 2006 17:17:13 -0000
> >>@@ -29,9 +29,12 @@
> >>#include "stheader.h"
> >>
> >>#ifdef USE_LIBAVFORMAT
> >>-
> >>#include "avformat.h"
> >>#include "avi.h"
> >>+#elif defined(USE_LIBAVFORMAT_SO)
> >>+#include <ffmpeg/avformat.h>
> >>+#endif
> >>
> >>#define PROBE_BUF_SIZE 2048
> >>
> >>@@ -451,4 +454,3 @@
> >>  demux_lavf_control
> >>};
> >>
> >>-#endif // USE_LIBAVFORMAT
> >
> >I think you can remove the #ifdef USE_LIBAVFORMAT altogether, the file
> >is compiled in under that condition in the Makefile.
> 
> it's needed to choose the right way to include headers

OK, yes, but maybe you can just invert the logic:

#ifdef USE_LIBAVFORMAT_SO
#include <ffmpeg/avformat.h>
#else
#include "avformat.h"
#include "avi.h"
#endif

> All in all I still think that my original patch, where we can keep
> track of internal or external libafvormat, is better (to say the
> truth, I consider the current USE_LIBVAVCODEC system a serious mess,
> because the macro is defined even when USE_LIBVAVCODEC is set).
> Opinions?

In any case it should be consistent.  Maybe you can commit my suggestion
first and then we can discuss changing lavc/lavf together.  How about
that?

Diego