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

[Nico Sabbi]

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

>  
>
>>@@ -5999,15 +6001,21 @@
>> fi
>> echores "$_libavcodec"
>> 
>>-echocheck "FFmpeg libavformat (static)"
>>+echocheck "FFmpeg libavformat "
>> if test "$_libavformat" = auto ; then
>>   # Note: static linking is preferred to dynamic linking
>>   _libavformat=no
>>   if test -d libavformat && test -f libavformat/utils.c ; then
>>     _libavformat=yes
>>+  else
>>+    cat > $TMPC <<EOF
>>+#include <ffmpeg/avformat.h>
>>+int main(void) { static URLProtocol mp_protocol = {"mp",NULL,NULL,NULL,NULL,NULL,}; register_protocol(&mp_protocol); return 0; }
>>+EOF
>>    
>>
>
>Hmm, will this always work if I commit the pending --disable-protocols
>patch for libavformat?  I doubt it.
>  
>

indeed. I didn't see that patch. In this case the safest function to 
call should be av_alloc_format_context()

>  
>
>>+    cc_check $_ld_lm -lavformat && _libavformat_so=yes
>>   fi
>> fi
>>-echores "$_libavformat"
>>+echores "static: $_libavformat, shared: $_libavformat_so"
>>    
>>
>
>I think I'd prefer if you could copy the way libavcodec is detected with
>two separate tests.
>
>  
>
>>@@ -6067,6 +6076,17 @@
>>   if win32 ; then
>>     _def_libavformat_win32='#define CONFIG_WIN32 1'
>>   fi
>>+else
>>+  if test "$_libavformat_so" = yes ; then
>>+    _def_libavformat_so='#define USE_LIBAVFORMAT_SO 1'
>>+    _ld_libavformat='-lavformat'
>>+    if win32 ; then
>>+      _def_libavformat_win32='#define CONFIG_WIN32 1'
>>+    fi
>>+  fi
>>+fi
>>+if test "$_libavformat" = yes -o "$_libavformat_so" = yes; then
>>+_conf_libavformat=yes
>>    
>>
>
>You can delete the last two lines if..
>  
>

same as above

>  
>
>>@@ -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

>  
>
>>--- 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

>  
>
>>--- libmpdemux/demuxer.c	17 Dec 2005 10:46:48 -0000	1.211
>>+++ libmpdemux/demuxer.c	8 Jan 2006 17:17:13 -0000
>>@@ -62,7 +62,9 @@
>> extern demuxer_desc_t demuxer_desc_xmms;
>> extern demuxer_desc_t demuxer_desc_mpeg_ty;
>> extern demuxer_desc_t demuxer_desc_rtp;
>>+#if defined(USE_LIBAVFORMAT) ||  defined(USE_LIBAVFORMAT_SO)
>> extern demuxer_desc_t demuxer_desc_lavf;
>>+#endif
>>    
>>
>
>Oh, this worked before?  Or does everybody compile with libavformat
>nowadays?
>
>Diego
>
>  
>

indeed!
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?