On Sat, 13 Oct 2012, Måns Rullgård wrote:

Martin Storsjö <[email protected]> writes:

diff --git a/configure b/configure
index 667089b..362f8d1 100755
--- a/configure
+++ b/configure
@@ -2794,6 +2794,29 @@ case $target_os in
         add_cppflags -U__STRICT_ANSI__
         ;;
     msvc)
+        if enabled shared; then
+            # Link to the import library instead of the
+            # normal static library.
+            LD_LIB='%.lib'
+            enabled static &&
+                die "Cannot build shared and static libraries at the same time with 
MSVC."
+        fi

The static+shared error feels misplaced here.

Yeah, it's a bit weird here. What section would be a better match?

+        LIBTARGET=i386
+        if enabled x86_64; then
+            LIBTARGET=x64
+        fi

enabled x86_64 && LIBTARGET=i386 || LIBTARGET=x64

+        shlibdir_default="$bindir_default"
+        SLIBPREF=""
+        SLIBSUF=".dll"
+        SLIBNAME_WITH_VERSION='$(SLIBPREF)$(FULLNAME)-$(LIBVERSION)$(SLIBSUF)'
+        SLIBNAME_WITH_MAJOR='$(SLIBPREF)$(FULLNAME)-$(LIBMAJOR)$(SLIBSUF)'
+        SLIB_CREATE_DEF_CMD='makedef $(SUBDIR)lib$(NAME).ver $(OBJS) > 
$$(@:$(SLIBSUF)=.def)'

Is makedef a friend of c99wrap?

Yeah, we'll put it into the same repo.

+        SLIB_EXTRA_CMD='-lib.exe -machine:$(LIBTARGET) 
-def:$$(@:$(SLIBSUF)=.def) -out:$(SUBDIR)$(SLIBNAME:$(SLIBSUF)=.lib)'
+        SLIB_INSTALL_NAME='$(SLIBNAME_WITH_MAJOR)'
+        SLIB_INSTALL_LINKS=
+        SLIB_INSTALL_EXTRA_SHLIB='$(SLIBNAME:$(SLIBSUF)=.lib)'

Should the .lib file really end up in $bindir?  That doesn't feel right
at all.

It's pretty weird, but this is the convention - we do it the same way for mingw.

+        SLIB_INSTALL_EXTRA_LIB='$(SLIBNAME_WITH_MAJOR:$(SLIBSUF)=.def)'
+        SHFLAGS='-dll -def:$$(@:$(SLIBSUF)=.def) 
-implib:$(SUBDIR)lib$(SLIBNAME:$(SLIBSUF)=.dll.a)'

The mentino of .dll.a there is confusing.  Can you please tell me which
command produce/consume what files?  I have the distinct feeling all
this can be cleaned up considerably.

Indeed, this turned out to have some duplication I missed before. link.exe creates one import library, then we created a separate one using lib.exe. I'll change this to not use lib.exe at all and make link.exe produce the .lib named import library straight away.

After my local changes, link.exe produces e.g. avutil.lib, which is used by LD_LIB when linking the avcodec DLL and all other DLLs/executables.

+extern AVCODEC_SYMBOL const uint16_t avpriv_ac3_channel_layout_tab[8];

If this annotation is really required, please do two things:

1. Call it something at least halfway descriptive.
2. Make it less of an eyesore (i.e. not uppercase).

Right, like avcodec_exportdata or so? It needs to have the lib name included, since it should be enabled/disabled per library.

diff --git a/libavcodec/symbols.h b/libavcodec/symbols.h
new file mode 100644
index 0000000..3c98d46
--- /dev/null
+++ b/libavcodec/symbols.h

As nondescriptive filenames go, this one ranks pretty darn high.

Indeed. Since this doesn't need to be installed, perhaps we could just put it into internal.h instead?

[...]

+#if CONFIG_SHARED && defined(_MSC_VER) && !defined(COMPILING_avcodec)

This condition seems a bit too specific.  There are other compilers.

That might be good for consistency, even if mingw can do without it. So something like this then:

#if CONFIG_SHARED && defined(_WIN32) && !defined(COMPILING_avcodec)

+#define AVCODEC_SYMBOL __declspec(dllimport)
+#else
+#define AVCODEC_SYMBOL
+#endif
+
+#endif /* AVCODEC_SYMBOLS_H */
diff --git a/library.mak b/library.mak
index b365935..318fe86 100644
--- a/library.mak
+++ b/library.mak
@@ -19,6 +19,7 @@ $(SUBDIR)x86/%.o: $(SUBDIR)x86/%.asm
        $(DEPYASM) $(YASMFLAGS) -I $(<D)/ -M -o $@ $< > $(@:.o=.d)
        $(YASM) $(YASMFLAGS) -I $(<D)/ -o $@ $<

+$(OBJS): CPPFLAGS := -DCOMPILING_$(NAME)=1 $(CPPFLAGS)

This should use +=

Ok, will change.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to