Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Thu, Aug 21, 2014 at 02:34:42PM +0200, Clément Bœsch wrote: On Thu, Aug 21, 2014 at 02:24:40PM +0200, Clément Bœsch wrote: On Thu, Aug 21, 2014 at 12:16:35PM +, Carl Eugen Hoyos wrote: Clément Bœsch u at pkh.me writes: [...] Sorry, I seem to have lost track of what you fear will not work if pkg-config is used for x264 detection but will fallback to the current system if pkg-config does not find the right version. It's simple really. You said earlier: or do you actually want a real fallback when it is present but didn't succeed? This is the preferred solution imo. In this case, if pkg-config finds the system install, it will not honor the user c/ld flags but the one from pkg-config. BTW, now that I think of it, depending on where the --extra-{ld,cflags} are added in the compilation and linker flag, they *might* allow you to trick the detection. Did you try the patch with pkg-config only detection? And try to add --pkg-config=true to trick the detect function. So, I did try myself, and it does the trick. Here is a new proposed diff (will submit patches from it): diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 3fbf556..3f7a432 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -51,3 +51,4 @@ • dctdnoiz filter now uses a block size of 8x8 instead of 16x16 by default • -vismv option is deprecated in favor of the codecview filter + • libx264 is now detected through pkg-config diff --git a/configure b/configure index 8e5f49b..03e9108 100755 --- a/configure +++ b/configure @@ -408,6 +408,7 @@ warn(){ } die(){ +test -n $WARNINGS printf \n$WARNINGS echolog $@ cat EOF @@ -1091,6 +1092,10 @@ check_pkg_config(){ headers=$2 funcs=$3 shift 3 +if ! $pkg_config --version /dev/null 21; then +warn $pkg_config not found but $pkg library detection relies on it +return 1 +fi check_cmd $pkg_config --exists --print-errors $pkgandversion || return pkg_cflags=$($pkg_config --cflags $pkg_config_flags $pkg) pkg_libs=$($pkg_config --libs $pkg_config_flags $pkg) @@ -1198,8 +1203,18 @@ require_cpp(){ } require_pkg_config(){ -pkg=$1 -check_pkg_config $@ || die ERROR: $pkg not found +pkgandversion=$1 +pkg=${1%% *} +if ! $pkg_config --version /dev/null 21; then +cat EOF +ERROR: $pkg_config not found but $pkg detection relies on it. + You should either install pkg-config (recommended), or use + --pkg-config=true with the $pkg linker flags specified through + --extra-ldflags. +EOF +die +fi +check_pkg_config $@ || die ERROR: $pkgandversion not found add_cflags$(get_safe ${pkg}_cflags) add_extralibs $(get_safe ${pkg}_libs) } @@ -3038,11 +3053,6 @@ set_default arch cc cxx doxygen pkg_config ranlib strip sysinclude \ enabled cross_compile || host_cc_default=$cc set_default host_cc -if ! $pkg_config --version /dev/null 21; then -warn $pkg_config not found, library detection may fail. -pkg_config=false -fi - if test $doxygen != $doxygen_default \ ! $doxygen --version /dev/null 21; then warn Specified doxygen \$doxygen\ not found, API documentation will fail to build. @@ -4851,9 +4861,7 @@ enabled libwavpack require libwavpack wavpack/wavpack.h WavpackOpenFil enabled libwebprequire_pkg_config libwebp webp/encode.h WebPGetEncoderVersion { check_code cc webp/encode.h WebPPicture wp; wp.use_argb++ || die ERROR: libwebp too old.; } -enabled libx264require libx264 x264.h x264_encoder_encode -lx264 - { check_cpp_condition x264.h X264_BUILD = 118 || - die ERROR: libx264 must be installed and version must be = 0.118.; } +enabled libx264require_pkg_config x264 = 0.118 stdint.h x264.h x264_encoder_encode enabled libx265require_pkg_config x265 x265.h x265_encoder_encode { check_cpp_condition x265.h X265_BUILD = 17 || die ERROR: libx265 version must be = 17.; } And basically, this is what happens: • If you don't have the required x264 (but pkg-config): [~/src/ffmpeg]☭ dash ./configure --disable-everything --enable-gpl --enable-libx264 ERROR: x264 = 999.118 not found If you think configure made a mistake, make sure you are using the latest version from Git. If the latest version fails, report the problem to the ffmpeg-u...@ffmpeg.org mailing list or IRC #ffmpeg on irc.freenode.net. Include the log file config.log produced by configure as this will help solve the problem. • If you don't have pkg-config: [~/src/ffmpeg]☭ dash ./configure --disable-everything --enable-gpl --enable-libx264 --pkg-config=pkg-config-nope ERROR: pkg-config-nope not found but x264 detection relies on it. You should either install pkg-config
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
Clément Bœsch u at pkh.me writes: Did you try the patch with pkg-config only detection? And try to add --pkg-config=true to trick the detect function. So, I did try myself Sorry, I was away and still didn't test all new tickets. [~/src/ffmpeg]☭ dash ./configure --disable-everything --enable-gpl --enable-libx264 --pkg-config=/bin/true --extra-ldflags=-lx264 The extra-ldflags should not be necessary. I still wonder why --pkg-config=/bin/true is necessary: If another version (than the one intended by the user) is found then he has a bad system installation (and has to use above work-around). But if no installation can be found, the work-around should be tested by configure. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Thu, Aug 28, 2014 at 01:18:55PM +, Carl Eugen Hoyos wrote: Clément Bœsch u at pkh.me writes: Did you try the patch with pkg-config only detection? And try to add --pkg-config=true to trick the detect function. So, I did try myself Sorry, I was away and still didn't test all new tickets. [~/src/ffmpeg]☭ dash ./configure --disable-everything --enable-gpl --enable-libx264 --pkg-config=/bin/true --extra-ldflags=-lx264 The extra-ldflags should not be necessary. Well, it's incomplete depending on the situation so we don't want it in the configure (Cf my mail about testing such things) and you're probably setting --extra-ldflags already for your custom paths. Also, you need to add --pkg-config=true anyway, so you can do the effort to add -lx264 as well. I still wonder why --pkg-config=/bin/true is necessary: If another version (than the one intended by the user) is found then he has a bad system installation (and has to use above work-around). But if no installation can be found, the work-around should be tested by configure. Unless I misunderstood the question, it will; --pkg-config=true will just make the fake pkg-config raise no flags, and then compilation will be tried with those empty flags (and the --extra-{c,ld}flags). -- Clément B. pgpB4QHn3Keyq.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Thu, Aug 21, 2014 at 11:23:41AM +, Carl Eugen Hoyos wrote: Clément Bœsch u at pkh.me writes: I thought the purpose is to allow educated developers to use pkg-config while (uneducated) users (like me) will not understand how this is easier than using configure parameters. Yes, it's also simpler for the users. Please understand that we disagree on this point. (It makes no difference though, I already accepted that some developers need pkg-config for x264, opus and hevc.) They will also almost never be tested, I will care about the testing. Well, you probably won't test static linking of random libs on various platforms typically. If you don't allow testing this on your fate client, I will have to take care. It's a generic linux, it would only cover a very limited usecase. If you want to test this properly you'd have to: - create multiple builds of the target library with different compilation flags (for x264 that would mean with or without opencl for example) - follow that project to look for any potential new library dependency addition - test with static and shared - test with minimal other dependencies in the build (because other places could add a linker flag or something that happen to actually be a dep you forgot) ; this mean an instance per library check - test on different plateform, where the linkers behave differently Are you willing to do that or... pkg-config makes possible to completely ignore that part since it moves the responsibility away from us. Completely apart from the fact that we already know this doesn't work, I still consider it an undesired change of behaviour. ...simply trust the project delivering the .pc? That said, if you want to support a fallback as I suggested above, it won't work as you expect: I feel that all this only supports my point that we should not rely on pkg-config. No, it shows that the hack is not reliable and never will. But since you insist, let's ignore the unavoidable problems, just make sure that a user who (already) knows about --extra-cflags and --extra-ldflags and remembers how he custom-compiled his library still has a chance to compile FFmpeg with the library even if he does not want to rely on pkg-config. We can't keep the current behaviour. We will need the users to add yet another option flag for this, probably in addition to a --pkg-config=/bin/false Basically, it will require them to change: --extra-cflags=-I/usr/local/include --extra-ldflags=-L/usr/local/lib Into: --extra-cflags=-I/usr/local/include --extra-ldflags=-L/usr/locallib --disable-pkg-config-detection (Yes, we want to make a distinction with --pkg-config=false) Or if they're going to change their command line anyway, they could just do: PKG_CONFIG_PATH=/usr/local/lib/pkgconfig ./configure ...and we wouldn't have to deal with such stupid hack. If the build system chooses the wrong one of two working libraries, this is the user's problem. What I try to avoid is just that the user installs a second version of the library because the system library doesn't work but cannot select it because the configure script blindly trusts the pkg-config return value. He can select it through PKG_CONFIG_PATH instead of --extra-* options. No difference here. As I said several times, we can be more verbose about the pkg-config process; I don't mind sending a patch to print debug the detection process, like which libs with which flags is getting selected. [...] -- Clément B. pgpKFtKhB0rj9.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Thu, Aug 21, 2014 at 12:16:35PM +, Carl Eugen Hoyos wrote: Clément Bœsch u at pkh.me writes: [...] Sorry, I seem to have lost track of what you fear will not work if pkg-config is used for x264 detection but will fallback to the current system if pkg-config does not find the right version. It's simple really. You said earlier: or do you actually want a real fallback when it is present but didn't succeed? This is the preferred solution imo. In this case, if pkg-config finds the system install, it will not honor the user c/ld flags but the one from pkg-config. [...] -- Clément B. pgpd_s6qnlY2C.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Thu, Aug 21, 2014 at 02:24:40PM +0200, Clément Bœsch wrote: On Thu, Aug 21, 2014 at 12:16:35PM +, Carl Eugen Hoyos wrote: Clément Bœsch u at pkh.me writes: [...] Sorry, I seem to have lost track of what you fear will not work if pkg-config is used for x264 detection but will fallback to the current system if pkg-config does not find the right version. It's simple really. You said earlier: or do you actually want a real fallback when it is present but didn't succeed? This is the preferred solution imo. In this case, if pkg-config finds the system install, it will not honor the user c/ld flags but the one from pkg-config. BTW, now that I think of it, depending on where the --extra-{ld,cflags} are added in the compilation and linker flag, they *might* allow you to trick the detection. Did you try the patch with pkg-config only detection? And try to add --pkg-config=true to trick the detect function. -- Clément B. pgpbnBlHSU4vx.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Tue, Jun 17, 2014 at 11:05:39PM +0200, Clément Bœsch wrote: --- Changelog | 2 ++ configure | 4 +--- doc/encoders.texi | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Changelog b/Changelog index 40a7751..268205c 100644 --- a/Changelog +++ b/Changelog @@ -54,3 +54,5 @@ │ ⚠ Behaviour changes │ └┘ + • libx264 is now detected through pkg-config. + diff --git a/configure b/configure index 4f909a0..6946133 100755 --- a/configure +++ b/configure @@ -4731,9 +4731,7 @@ enabled libvpx { enabled libvpx_vp9_encoder { check_lib2 vpx/vpx_encoder.h vpx/vp8cx.h vpx_codec_vp9_cx VP9E_SET_SVC -lvpx || disable libvpx_vp9_encoder; } } enabled libwavpack require libwavpack wavpack/wavpack.h WavpackOpenFileOutput -lwavpack enabled libwebprequire_pkg_config libwebp webp/encode.h WebPGetEncoderVersion -enabled libx264require libx264 x264.h x264_encoder_encode -lx264 - { check_cpp_condition x264.h X264_BUILD = 118 || - die ERROR: libx264 must be installed and version must be = 0.118.; } +enabled libx264require_pkg_config x264 = 0.118 stdint.h x264.h x264_encoder_encode enabled libx265require_pkg_config x265 x265.h x265_encoder_encode { check_cpp_condition x265.h X265_BUILD = 17 || die ERROR: libx265 version must be = 17.; } diff --git a/doc/encoders.texi b/doc/encoders.texi index 2781574..3183027 100644 --- a/doc/encoders.texi +++ b/doc/encoders.texi @@ -1515,7 +1515,7 @@ x264 H.264/MPEG-4 AVC encoder wrapper. This encoder requires the presence of the libx264 headers and library during configuration. You need to explicitly configure the build with -@code{--enable-libx264}. +@code{--enable-libx264}. This library is detected through @code{pkg-config}. libx264 supports an impressive number of features, including 8x8 and 4x4 adaptive spatial transform, adaptive B-frame placement, CAVLC/CABAC Note: will fix Ticket #3876: [~]☭ pkg-config --libs x264 -lx264 [~]☭ pkg-config --static --libs x264 -lx264 -lpthread -lm -ldl -- Clément B. pgpcC3FQv6zj3.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
On Sat, Aug 16, 2014 at 03:27:17PM +0200, Clément Bœsch wrote: On Fri, Aug 15, 2014 at 06:49:39AM +, Carl Eugen Hoyos wrote: [...] But really that's insane, because I know you will end-up hardcoding all kind of linker flags to these fallback calls, Yes, some external libraries that never have additional dependencies currently depend on pkg-config. and this defeat completely the purpose of such simplified code. I thought the purpose is to allow educated developers to use pkg-config while (uneducated) users (like me) will not understand how this is easier than using configure parameters. Yes, it's also simpler for the users. They will also almost never be tested, I will care about the testing. Well, you probably won't test static linking of random libs on various platforms typically. pkg-config makes possible to completely ignore that part since it moves the responsibility away from us. That said, if you want to support a fallback as I suggested above, it won't work as you expect:if you want to link with your custom flags and be sure not to have pkg-config taking over your parameters (basically linking to the system package instead of the one you specified through custom link flags). Carl, do you see a solution? [...] -- Clément B. pgp2zc5Kjm1Wb.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
Clément Bœsch u at pkh.me writes: No it won't work because require_pkg_config dies if it fails, so you can fallback after it. True. Of course we can add another require_pkg_config2() that takes some explicit linking flags in addition to the user ones as a fallback. Sounds useful to me. But really that's insane, because I know you will end-up hardcoding all kind of linker flags to these fallback calls, Yes, some external libraries that never have additional dependencies currently depend on pkg-config. and this defeat completely the purpose of such simplified code. I thought the purpose is to allow educated developers to use pkg-config while (uneducated) users (like me) will not understand how this is easier than using configure parameters. They will also almost never be tested, I will care about the testing. (I hope with your help but if necessary I can do it alone.) Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing
Clément Bœsch u at pkh.me writes: Carl, do you want the fallback to work just when pkg-config is not available I believe this would still leave some use-cases unsolved. or do you actually want a real fallback when it is present but didn't succeed? This is the preferred solution imo. (the second solution is way uglier to solve) You had already sent a patch iirc. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel