A couple of minor build system adjustments

[qyot27]

Re-pushed with the new version using Cflags.private. This way, pkgconf uses -DLIBTWOLAME_STATIC only when the --static option is passed to it. Many distros (Fedora, notably) have either switched to pkgconf or have it in their repositories. Whether this is acceptable would be a matter of if old pkg-config just silently ignores metadata lines it doesn't recognize, although it's also a value judgement of whether supporting the old pkg-config is still worth it.

[qyot27]

At least on Ubuntu, freedesktop.org pkg-config does indeed silently ignore the Cflags.private entry, so adding that line for pkgconf's benefit doesn't cause any kind of regression or breakage if pkgconf isn't available.

[njh]

Libs.Private: @LIBS@ was added in b78d3c36a9d6f2f2ae93a2883f39070bb06b9f9a

[qyot27]

It's good that Libs.private was added, but what does that have to do with this pull request? Cflags.private is to fix the other part of building as static that Libs.private has no control over.

[njh]

Sorry, I got my .private options confused.

I still don't understand what the -DLIBTWOLAME_STATIC definition does. LIBTWOLAME_STATIC is only used by Win32 DLL builds.

Can you explain further what this is doing?

Thanks,

nick.

[qyot27]

When cross-compiling with MinGW-w64, programs that can link to twolame (FFmpeg in this case) require -DLIBTWOLAME_STATIC be added to their CFLAGS if you want it to link against libtwolame.a instead of twolame.dll. If -DLIBTWOLAME_STATIC doesn't do anything outside Windows builds, then adding it to the .pc file should probably be based on a configure check rather than hardcoded, but the basic point is still the same.

Generally, this is a case of something that pkgconf should be handling, since it's conditional on the library's build options, and it's easier/arguably more 'proper' for users to configure FFmpeg with --pkg-config-flags="--static" than it is to remember that in this case, you need to use --extra-cflags="-DLIBTWOLAME_STATIC" (especially since this can be the case for other libraries as well; libmodplug and libcaca both require similar measures, which means that the --extra-ldflags= list gets longer and longer).

[njh]

Sorry, I am still not clear what the problem is that you are trying to fix?

Are you able to outline the steps you are going through and the point where it fails?

[qyot27]

1) Build libtwolame as static-only:

PKG_CONFIG_PATH=/usr/i686-w64-mingw32/lib/pkgconfig \
./configure \
--prefix=/usr/i686-w64-mingw32 \
--disable-shared \
--enable-silent-rules \
--host=i686-w64-mingw32

2) Configure FFmpeg:

PKG_CONFIG_PATH=/usr/i686-w64-mingw32/lib/pkgconfig \
./configure \
--prefix=$HOME/ffmpeg_build/32bit \
--cross-prefix=i686-w64-mingw32- \
--enable-gpl \
--enable-version3 \
--disable-w32threads \
--enable-libtwolame \
--target-os=mingw32 \
--arch=x86 \
--pkg-config-flags="--static"

This fails (either the configure check itself or the link phase) because -DLIBTWOLAME_STATIC has not been passed to FFmpeg's CFLAGS. With Cflags.private, it works as-is, because --pkg-config-flags="--static" will tell pkgconf to add the contents of twolame.pc's .private fields to FFmpeg's CFLAGS and LIBS (currently it will just pass the LIBS, which requires the user to pass the CFLAGS themselves).

[njh]

Aha! This is starting to make sense now. So you are targeting mingw32. Hm.

[njh]

I am still very puzzled by this. I have looked through my /usr/local/lib/pkgconfig/ directory where there are 130 libraries installed. But none of them define Cflags.private and none of them mention special options for static.

Is this because TwoLAME is doing something weird elsewhere? Is the way it handles DLL exports unusual?

[qyot27]

Other .pc files lacking Cflags.private is more than likely just because they were written with freedesktop.org's pkg-config in mind, and Cflags.private was added in pkgconf (Fedora's Wiki page about why they switched their system pkg-config implementation to pkgconf in F26). fd.o pkg-config just ignores Cflags.private, and the feature request to add it hasn't been touched in four years.

If the majority of special static-related build defines is Windows-specific, there's no reason for Linux-specific builds to include them in their .pc files, so either the projects don't bother with it, or they intelligently add those options only when building for a Windows target (if those same projects have MSVC project files, the flags may be in there, but that doesn't help cross-compiling). I did add a commit which reflects this and only populates the Cflags.private field when MinGW is detected as the host.

[njh]

I have merged ad6ce6bdb5834095025b7777ced401f87ab498cb in to master. I think it is best to keep it simple - the define isn't doing any harm to the Linux / other builds.

It would be good to find a better solution at some point.