Re: [FFmpeg-devel] [PATCH 2/3] build: rely on pkg-config for libx264 probing

2014-08-28 Thread Clément Bœsch
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

2014-08-28 Thread Carl Eugen Hoyos
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

2014-08-28 Thread Clément Bœsch
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

2014-08-21 Thread Clément Bœsch
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

2014-08-21 Thread Clément Bœsch
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

2014-08-21 Thread Clément Bœsch
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

2014-08-20 Thread Clément Bœsch
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

2014-08-20 Thread Clément Bœsch
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

2014-08-15 Thread Carl Eugen Hoyos
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

2014-08-14 Thread Carl Eugen Hoyos
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