| Message ID | 20170630180645.11192-1-wiiaboo@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 30.06.2017, at 20:06, Ricardo Constantino <wiiaboo@gmail.com> wrote: > libvorbis comes with pkg-config files since at least v1.0.1, way back > in 2003. > > The extra check is needed for shared builds, as the pkg-config file > for vorbisenc doesn't include vorbis and ogg if --static isn't used. If you still manually add -lvorbis that IMHO defeats the purpose of using pkg-config the first place. I don't know if our actual code using vorbisenc depend on libvorbis and libogg, but if it does then the flags for all 3 would have to be requested via pkg-config.
On 4 July 2017 at 07:54, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote: > On 30.06.2017, at 20:06, Ricardo Constantino <wiiaboo@gmail.com> wrote: > >> libvorbis comes with pkg-config files since at least v1.0.1, way back >> in 2003. >> >> The extra check is needed for shared builds, as the pkg-config file >> for vorbisenc doesn't include vorbis and ogg if --static isn't used. > > If you still manually add -lvorbis that IMHO defeats the purpose of using pkg-config the first place. > I don't know if our actual code using vorbisenc depend on libvorbis and libogg, but if it does then the flags for all 3 would have to be requested via pkg-config. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel If the first pkg-config test fails (in case of shared), I test with check_pkg_config with those extralibs added and then add them if it worked. It doesn't defeat the purpose because pkg-config is not just for the -l flags. In my usecase I need both -I and -L flags for the proper installation too but I didn't build with shared libraries so I never noticed this before. From what I could read from the code, we do need all three libraries as the code has references to symbols not just in vorbisenc. I don't know if that's a problem with the code, or with how the shared libraries are done, or if the pkg-config should just always contain vorbis and ogg on Requires instead of Requires.private. I can't help with any of these. The latter, even if fixed upstream would only be available in git master, since there haven't been libvorbis releases for years. My solution enables usage of pkg-config with shared libraries of libvorbis but does disable builds where pkg-config isn't available. Alternatively, I could add a use_pkg_config version that works with multiple .pc that would be used for libvorbis only.
On 04.07.2017, at 14:03, Ricardo Constantino <wiiaboo@gmail.com> wrote: > On 4 July 2017 at 07:54, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote: >> On 30.06.2017, at 20:06, Ricardo Constantino <wiiaboo@gmail.com> wrote: >> >>> libvorbis comes with pkg-config files since at least v1.0.1, way back >>> in 2003. >>> >>> The extra check is needed for shared builds, as the pkg-config file >>> for vorbisenc doesn't include vorbis and ogg if --static isn't used. >> >> If you still manually add -lvorbis that IMHO defeats the purpose of using pkg-config the first place. >> I don't know if our actual code using vorbisenc depend on libvorbis and libogg, but if it does then the flags for all 3 would have to be requested via pkg-config. >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > If the first pkg-config test fails (in case of shared), I test with > check_pkg_config with those extralibs added and then add them if it > worked. > It doesn't defeat the purpose because pkg-config is not just for the > -l flags. In my usecase I need both -I and -L flags for the proper > installation too but I didn't build with shared libraries so I never > noticed this before. Seems you found a solution, and "defeats the purpose" was exaggerated, but be point of pkg-config is to not make assumptions on where libraries are, and by just adding -lvorbis you hardcoded exactly such an assumption: that libvorbis is in the same place as libvorbisenc. Checking like your latest patch ensures that both get their proper -L options.
diff --git a/configure b/configure index 282114d268..54c1c93ecc 100755 --- a/configure +++ b/configure @@ -5902,7 +5902,10 @@ enabled libtwolame && require libtwolame twolame.h twolame_init -ltwolame enabled libv4l2 && require_pkg_config libv4l2 libv4l2.h v4l2_ioctl enabled libvidstab && require_pkg_config "vidstab >= 0.98" vid.stab/libvidstab.h vsMotionDetectInit enabled libvo_amrwbenc && require libvo_amrwbenc vo-amrwbenc/enc_if.h E_IF_init -lvo-amrwbenc -enabled libvorbis && require libvorbis vorbis/vorbisenc.h vorbis_info_init -lvorbisenc -lvorbis -logg +enabled libvorbis && { use_pkg_config vorbisenc vorbis/vorbisenc.h vorbis_info_init || + { check_pkg_config vorbisenc vorbis/vorbisenc.h vorbis_info_init -lvorbis -logg && + add_cflags $vorbisenc_cflags && add_extralibs $vorbisenc_extralibs -lvorbis -logg; } || + die "ERROR: libvorbis not found."; } enabled libvpx && { enabled libvpx_vp8_decoder && {