[Ffmpeg-devel] [PATCH] Cygwin vhook, always static avformat

[wzrlpy at arsystel.com]

Hi,

>> @@ -582,10 +583,14 @@
>>  v4l2="no"
>>  audio_oss="yes"
>>  dv1394="no"
>> -vhook="no"
>
> Somewhat unrelated.
A bit related, one of the goals of the patch is to allow vhook under Cygwin.


>>  extralibs=""
>>  cygwin="yes"
>>  EXESUF=".exe"
>> +SLIBPREF="cyg"
>
> Why?  I'm not saying it's wrong, I just don't know.
It is not very widely published, the command "info ld", in the section
"Machine Dependent features" says for cygwin/mingw:

(*) Actually, this is not `cygxxx.dll' but in fact is
     `<prefix>xxx.dll', where `<prefix>' is set by the `ld' option
     `--dll-search-prefix=<prefix>'. In the case of cygwin, the
     standard gcc spec file includes `--dll-search-prefix=cyg', so in
     effect we actually search for `cygxxx.dll'.

     Other win32-based unix environments, such as mingw or pw32, may
     use other `<prefix>'es, although at present only cygwin makes use
     of this feature.  It was originally intended to help avoid name
     conflicts among dll's built for the various win32/un*x
     environments, so that (for example) two versions of a zlib dll
     could coexist on the same machine.



>> +SLIBSUF=".dll"
>> +SLIBNAME_WITH_VERSION='$(SLIBPREF)$(NAME)-$(LIBVERSION)$(SLIBSUF)'
>> +SLIBNAME_WITH_MAJOR='$(SLIBPREF)$(NAME)-$(LIBMAJOR)$(SLIBSUF)'
>> +SHFLAGS='-shared
>> -Wl,--out-implib=lib$(NAME).dll.a,--export-all-symbols,--enable-auto-import'
>
> OK, assuming the odd-looking (in my Unix eyes) SHFLAGS is correct.
Yes, it is correct, in the sense that the build succeeds. But I'am afraid it
is yet incomplete, I forgot the --no-whole-archive option, so that
symbols in dependency libraries are not exported multiple times.



>> @@ -1462,6 +1467,12 @@
>>      libdir="${prefix}/lib"
>>  fi
>>
>> +if test "$TARGET_OS" = "CYGWIN" ; then
>> +    shlibdir="${bindir}"
>> +else
>> +    shlibdir="${libdir}"
>> +fi
>
> This should be done along with other platform specific setup.
OK, I'll move it.


>> @@ -1930,6 +1942,15 @@
>>    echo "#endif" >> $TMPH
>>  fi
>>
>> +if test "$TARGET_OS" = "CYGWIN" ; then
>> +  echo 'ifeq ($(MAKEFILE_NAME),MAKEFILE_VHOOK)' >> config.mak
>> +  echo '    EXTRALIBS := -L../libavformat -lavformat$(BUILDSUF) \' >>
>> config.mak
>> +  echo '                 -L../libavcodec  -lavcodec$(BUILDSUF)  \' >>
>> config.mak
>> +  echo '                 -L../libavutil   -lavutil$(BUILDSUF)
>> $(EXTRALIBS) ' >> config.mak
>> +  echo '    SHFLAGS=-shared
>> -Wl,--export-all-symbols,--enable-auto-import' >> config.mak
>> +  echo 'endif' >> config.mak
>> +fi
>
> Rejected.  This belongs in vhook/Makefile, except the SHFLAGS part,
> which should be combined with the other cygwin SHFLAGS somehow.
OK, I'll move it to vhook/Makefile.
I am not so sure about if SHFLAGS should be combined, the import
library (--out-implib=lib$(NAME).dll.a) is not needed for vhook, because
they are being called explicitly via dlopen.


>>  include ../config.mak
>>
>>  VPATH=$(SRC_PATH)/vhook
>> @@ -14,13 +16,13 @@
>>
>>  ifeq ($(HAVE_IMLIB2),yes)
>>      HOOKS += imlib2$(SLIBSUF)
>> -    LDFLAGS += -lImlib2
>> +    EXTRALIBS += -lImlib2
>>  endif
>>
>>  ifeq ($(HAVE_FREETYPE2),yes)
>>      HOOKS += drawtext$(SLIBSUF)
>>      CFLAGS += `freetype-config --cflags`
>> -    LDFLAGS += `freetype-config --libs`
>> +    EXTRALIBS += `freetype-config --libs`
>>  endif
>
> Unrelated, but OK.
A bit related, under Cygwin the order of libraries is relevant, I needed to
move the dependencies to the right of the command line.

> I think this can be made to work.
Thanks for your time reviewing the patch, I will rework it following your
comments.


The patch on loop input got no comments, should I open another mail thread
for it?
http://usuarios.arsystel.com/wzrlpy/ffmpeg/ffmpeg.cygwin.loop_input.2.patch


Regards,
V?ctor Paesa