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

[Clément Bœsch]

On Wed, Jun 18, 2014 at 11:24:38AM +0000, Carl Eugen Hoyos wrote:
> Clément Bœsch <u <at> pkh.me> writes:
> 
> > > > +enabled libx264           && require_pkg_config 
> > > > "x264 >= 0.118" "stdint.h x264.h" x264_encoder_encode
> > > 
> > > Since this needlessly breaks scripts, I am against this.
> > > 
> > 
> > It's not needlessly since it fixes builds when libx264 is 
> > built against external libs to the least, without a hack 
> > of trying to guess what library could be needed. It 
> > simplifies the creation of static builds.
> 
> This sounds as if these were two different issues, 
> do I misunderstand?
> 

You don't misunderstand, I was enumerating a bunch of problems with the
current hack.

> I agree that there theoretically could be an issue for 
> a user, I sent a patch to solve this.

Yeah, and the patch is "libx264 will be detected with pkg-config if you're
lucky enough that the guess-hack is not triggered", which is definitely
worse than ever.

> (I do not remember a report.)
> 

I believe that kind of clashes:
https://trac.ffmpeg.org/ticket/3218

...could be avoided with pkg-config, because pkg-config will be
responsible from generating the correct flags and paths.

> > And also it simplifies specifying a custom x264 build.
> 
> It definitely doesn't simplify things, I honestly wonder 
> if everybody on this list knows how to use the future 
> variant, while I consider the current solution very 
> user-friendly.

Very user friendly? To specify compiler and linker flags manually? Yeah
well, I don't think we can agree here. The configure is here to provide a
layer of abstraction to the users, otherwise we would ask them to do gcc
*.c libav*/*.c -o ffmpeg, that's definitely simpler after all...

> 
> > It indeed might break some scripts that used to 
> > workaround the previous issues with --extra-{c,ld}flags. 
> > But that's the reason we are documenting the change, and 
> > also displays a very obvious error in case it fails.
> 
> But your patch doesn't tell a user how he can actually 
> fix compilation.

It says to install pkg-config?

I could even add more debug to the library lookup. Typically displaying
which method is used and give even more insight on what include and lib
dir are actually tested, at least in the case of pkg-config.

> And this is exactly the problem:
> Currently, FFmpeg compilation with libx264 works for 
> everyone: For you, because you know both the incantations 
> for pkg-config and FFmpeg configure, and for the users 
> because they only know the (simple) FFmpeg configure 
> syntax.

Well, every time I wanted a custom build with special paths or options, I
was always ended up reading or hacking the configure with echo to figure
out what was going on. Most users just install the libs on their system or
copy/paste random tutorial with an insanely long complex configure line.
If it works, it's merely by chance.

> 
> This is my main concern, and this is why this patch 
> shouldn't get committed.
> 
> My other concern is more personal: I hate to install, 
> and particularly if it is not necessary.
> I would expect that this would also hit a few users, 
> possibly less than the pkg-config issue.

pkg-config is a 100kB standalone portable C program, and it is used by
most library projects nowadays. If you don't have it, you're probably
asking for trouble.

You know, I would be even more compliant to wget the pkg-config light
version (the one with no glib dep) in the configure if it's not found on
the system, and built it within the tree to probe the libraries.

I'm gonna get killed for what I just said, but that's probably a more
reliable method than perpetuating the archaic broken library probing we
are currently doing.

> Carl Eugen
> 

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140618/67e0da16/attachment.asc>