[PATCH] build: rely on pkg-config for libx264 probing.
From: Clément Bœsch <clement@stupeflix.com> This helps populating {c,ld}flags properly without manual user intervention. x264 needs to be built with either --bit-depth=8 or --bit-depth=10. Most distro package the former, so users often have to build their own and install it in a separated place. Instead of having them to require --extra-ldflags and --extra-cflags, we can now use pkg-config utils to append them automatically. before: ./configure --enable-gpl --enable-libx264 --extra-cflags="-I/usr/local/include" --extra-ldflags="-L/usr/local/lib" after: PKG_CONFIG_PATH=/usr/local/lib/pkgconfig ./configure --enable-gpl --enable-libx264 Trolled-on Ticket #3218 --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 3c6d766..0f09310 100755 --- a/configure +++ b/configure @@ -4434,7 +4434,7 @@ enabled libvpx && { enabled libvpx_vp9_decoder && { check_lib2 "vpx/vpx_decoder.h vpx/vp8dx.h" "vpx_codec_vp9_dx" -lvpx || disable libvpx_vp9_decoder; } 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 libx264 && require libx264 x264.h x264_encoder_encode -lx264 && +enabled libx264 && require_pkg_config x264 "stdint.h x264.h" x264_encoder_encode && { check_cpp_condition x264.h "X264_BUILD >= 118" || die "ERROR: libx264 must be installed and version must be >= 0.118."; } enabled libxavs && require libxavs xavs.h xavs_encoder_encode -lxavs -- 1.8.5.1
Clément Bœsch <u <at> pkh.me> writes:
before: ./configure --enable-gpl --enable-libx264 --extra-cflags="-I/usr/local/include" --extra-ldflags="-L/usr/local/lib" after: PKG_CONFIG_PATH=/usr/local/lib/pkgconfig ./configure --enable-gpl --enable-libx264
As said, I am mildly against this because I don't see what it fixes (you still need a specific configure line) but at the same time breaks existing configure lines. Or doesn't it break them? Carl Eugen
On Tue, Dec 17, 2013 at 02:00:52PM +0000, Carl Eugen Hoyos wrote:
Clément Bœsch <u <at> pkh.me> writes:
before: ./configure --enable-gpl --enable-libx264 --extra-cflags="-I/usr/local/include" --extra-ldflags="-L/usr/local/lib" after: PKG_CONFIG_PATH=/usr/local/lib/pkgconfig ./configure --enable-gpl --enable-libx264
As said, I am mildly against this because I don't see what it fixes (you still need a specific configure line)
It fixes the need to add -I and -L flags. It also ease the creation of static build: typically, with the help of --pkg-config-flags="--static" it will allow these flags to be automatically added: ☭ pkg-config --libs --static x264 -lx264 -lpthread -lm -ldl Note that when this kind of option is enabled, it is automatically applied to any library where we use pkg-config. Since we use pkg-config for quite a bunch of libraries, this should be expected (maybe the detection mechanism used for each library could be documented). Basically, as said in the original mail, I see pkg-config as a more reliable, simpler and expected way for handling all the dependency needs and ease multiple installations. BTW, the following diff is actually simpler: -enabled libx264 && require 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 libx264 && require_pkg_config "x264 >= 0.118" "stdint.h x264.h" x264_encoder_encode
but at the same time breaks existing configure lines.
Or doesn't it break them?
Yes, probably. Can't do much about it, these command lines are really workarounds for a bad detection. Now I think this is part of a real communication problem we have with our downstreams. Typically, to overcome that kind of issue, I would suggest to change our Changelog layout: we *NEED* a release note. We need to split features, bugfixes (important ones only probably), and behaviour changes. Suggestion: we wipe Changelog after every release, and replace it with something like that: Example inspired from i3 release notes (because it's pretty ;)): ----8<---------------------------------------------- ┌──────────────────────────────┐ │ Release notes for FFmpeg 2.3 │ └──────────────────────────────┘ 2.3 was released blablabla It includes the following library versions: • libavutil xx.yy.1zz • libavcodec xx.yy.1zz • libavformat xx.yy.1zz • libavdevice xx.yy.1zz • libavfilter xx.yy.1zz • libswscale xx.yy.1zz • libswresample xx.yy.1zz • libpostproc xx.yy.1zz Please refer to the doc/APIChanges file for more information. ┌────────────────────────────┐ │ Features │ └────────────────────────────┘ • AC3 fixed-point decoding • shuffleplanes filter • subfile protocol • Phantom Cine demuxer • replaygain data export • VP7 video decoder • Alias PIX image encoder and decoder • Improvments to the BRender PIX image decoder • Improvments to the XBM decoder • QTKit input device • improvments to OpenEXR image decoder • support decoding 16-bit RLE SGI images • GDI screen grabbing for Windows • alternative rendition support for HTTP Live Streaming • AVFoundation input device • Direct Stream Digital (DSD) decoder • Magic Lantern Video (MLV) demuxer • On2 AVC (Audio for Video) decoder • support for decoding through DXVA2 in ffmpeg • libbs2b-based stereo-to-binaural audio filter ┌────────────────────────────┐ │ Bugfixes │ └────────────────────────────┘ • issue3327-libc-2.17.so is not a MP3 anymore ┌────────────────────────────┐ │ ⚠ Behaviour changes ⚠ │ └────────────────────────────┘ • libx264 is now detected through pkg-config ----8<---------------------------------------------- I'm not sure for the Bugfixes section since it will probably be a pain to maintain, and maybe irrelevant since it might be backported. Anyway, you get the idea. I realize I should have started a new thread to discuss this but well, deal with it. Regards, -- Clément B.
On Wed, May 14, 2014 at 06:51:52PM +0200, Clément Bœsch wrote:
On Tue, Dec 17, 2013 at 02:00:52PM +0000, Carl Eugen Hoyos wrote:
Clément Bœsch <u <at> pkh.me> writes:
before: ./configure --enable-gpl --enable-libx264 --extra-cflags="-I/usr/local/include" --extra-ldflags="-L/usr/local/lib" after: PKG_CONFIG_PATH=/usr/local/lib/pkgconfig ./configure --enable-gpl --enable-libx264
As said, I am mildly against this because I don't see what it fixes (you still need a specific configure line)
It fixes the need to add -I and -L flags. It also ease the creation of static build: typically, with the help of --pkg-config-flags="--static" it will allow these flags to be automatically added:
☭ pkg-config --libs --static x264 -lx264 -lpthread -lm -ldl
Note that when this kind of option is enabled, it is automatically applied to any library where we use pkg-config. Since we use pkg-config for quite a bunch of libraries, this should be expected (maybe the detection mechanism used for each library could be documented).
Basically, as said in the original mail, I see pkg-config as a more reliable, simpler and expected way for handling all the dependency needs and ease multiple installations.
BTW, the following diff is actually simpler:
-enabled libx264 && require 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 libx264 && require_pkg_config "x264 >= 0.118" "stdint.h x264.h" x264_encoder_encode
but at the same time breaks existing configure lines.
Or doesn't it break them?
Yes, probably. Can't do much about it, these command lines are really workarounds for a bad detection.
Now I think this is part of a real communication problem we have with our downstreams. Typically, to overcome that kind of issue, I would suggest to change our Changelog layout: we *NEED* a release note. We need to split features, bugfixes (important ones only probably), and behaviour changes.
Suggestion: we wipe Changelog after every release, and replace it with something like that:
Example inspired from i3 release notes (because it's pretty ;)):
----8<----------------------------------------------
┌──────────────────────────────┐ │ Release notes for FFmpeg 2.3 │ └──────────────────────────────┘
2.3 was released blablabla It includes the following library versions:
• libavutil xx.yy.1zz • libavcodec xx.yy.1zz • libavformat xx.yy.1zz • libavdevice xx.yy.1zz • libavfilter xx.yy.1zz • libswscale xx.yy.1zz • libswresample xx.yy.1zz • libpostproc xx.yy.1zz
Please refer to the doc/APIChanges file for more information.
┌────────────────────────────┐ │ Features │ └────────────────────────────┘
• AC3 fixed-point decoding • shuffleplanes filter • subfile protocol • Phantom Cine demuxer • replaygain data export • VP7 video decoder • Alias PIX image encoder and decoder • Improvments to the BRender PIX image decoder • Improvments to the XBM decoder • QTKit input device • improvments to OpenEXR image decoder • support decoding 16-bit RLE SGI images • GDI screen grabbing for Windows • alternative rendition support for HTTP Live Streaming • AVFoundation input device • Direct Stream Digital (DSD) decoder • Magic Lantern Video (MLV) demuxer • On2 AVC (Audio for Video) decoder • support for decoding through DXVA2 in ffmpeg • libbs2b-based stereo-to-binaural audio filter
┌────────────────────────────┐ │ Bugfixes │ └────────────────────────────┘
• issue3327-libc-2.17.so is not a MP3 anymore
┌────────────────────────────┐ │ ⚠ Behaviour changes ⚠ │ └────────────────────────────┘
• libx264 is now detected through pkg-config
----8<----------------------------------------------
looks nice
I'm not sure for the Bugfixes section since it will probably be a pain to maintain, and maybe irrelevant since it might be backported.
Its maybe not so hard to maintain, git can be used to autogenerate this: git log n2.2.1..n2.2.2 --oneline --grep 'ixes .icket' 9d0ff64 ffmpeg: print an error at the end if conversion failed c588316 avfilter/filtfmts: Support dynamically allocated in/outputs e3b08b3 avformat/rtmpproto: Added handling of an initial RTMP chunk size packet. 09abca6 sws: dont use the optimized 410->420 unscaled conversion when height%4 43d6482 avcodec/vorbisdec: try to workaround libvorbisenc bug 4a479fd swresample: fix AV_CH_LAYOUT_STEREO_DOWNMIX input 4f41717 avformat/mux: Check for and remove invalid packet durations 6896dcb avformat/avidec: Speed up keyframe detection code e9e42be avformat/asfenc: dont allow non interleaved packets abd6dec avformat/pmpdec: remove cur_dts timestamp hack 0385c82 avformat: Fix decoder search in find stream info 30cf47c ff_id3v2_read: add option to limit ID3 magic number search b45cd17 ff_id3v2_free_extra_meta: set the pointer pointing to extra_meta to NULL 32919db Fix compilation with --disable-everything --enable-muxer=avi --disable-network. 56f44c2 avutil/avstring: do not lose ascii characters when decoding non utf-8 with av_utf8_decode() fe87a40 ffprobe: do not lose non ascii characters in non utf8 strings [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I do not agree with what you have to say, but I'll defend to the death your right to say it. -- Voltaire
On May 14, 2014 4:30 PM, "Michael Niedermayer" <michaelni@gmx.at> wrote:
Its maybe not so hard to maintain, git can be used to autogenerate this:
git log n2.2.1..n2.2.2 --oneline --grep 'ixes .icket'
+1 This would require a uniform patch format, which we don't require right now. Some people write "fix Trac ticket blah", some "fix ticket" (without es). Timothy
On Wed, May 14, 2014 at 05:50:56PM -0700, Timothy Gu wrote:
On May 14, 2014 4:30 PM, "Michael Niedermayer" <michaelni@gmx.at> wrote:
Its maybe not so hard to maintain, git can be used to autogenerate this:
git log n2.2.1..n2.2.2 --oneline --grep 'ixes .icket'
+1
This would require a uniform patch format, which we don't require right now. Some people write "fix Trac ticket blah", some "fix ticket" (without es).
git log --grep '[fF]ix\(\|es\) *\(\|[tT]rac\) *[tT]ick' anything this misses ? -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle
On Thu, May 15, 2014 at 05:19:04AM +0200, Michael Niedermayer wrote:
On Wed, May 14, 2014 at 05:50:56PM -0700, Timothy Gu wrote:
On May 14, 2014 4:30 PM, "Michael Niedermayer" <michaelni@gmx.at> wrote:
Its maybe not so hard to maintain, git can be used to autogenerate this:
git log n2.2.1..n2.2.2 --oneline --grep 'ixes .icket'
+1
This would require a uniform patch format, which we don't require right now. Some people write "fix Trac ticket blah", some "fix ticket" (without es).
git log --grep '[fF]ix\(\|es\) *\(\|[tT]rac\) *[tT]ick'
I still think having Bugfixes is not what we really want (I shouldn't have mentioned it in the first place…). I see a few problems that can't be fixed with a regex: - it does not mean the bug fixes are fixed in comparison to the previous release; it could be a bug introduced post-release - it suggests the bug is fixed only in this release while it could have been backported, and might be in the future - the list might get pretty long between releases, and while people care about having their bugs fixed, they don't care about the complete list and will probably never read it -- Clément B.
On May 14, 2014 9:52 AM, "Clément Bœsch" <u@pkh.me> wrote:
Now I think this is part of a real communication problem we have with our downstreams. Typically, to overcome that kind of issue, I would suggest to change our Changelog layout: we *NEED* a release note. We need to split features, bugfixes (important ones only probably), and behaviour changes.
Suggestion: we wipe Changelog after every release, and replace it with something like that:
Example inspired from i3 release notes (because it's pretty ;)):
That is one of the things Libav does a really good job on, but FFmpeg not so. Therefore, +inf. Timothy
participants (4)
-
Carl Eugen Hoyos -
Clément Bœsch -
Michael Niedermayer -
Timothy Gu