| Message ID | 20230905232630.2031-1-kasper93@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/2] configure: don't force specific C++ standard library linking | expand |
On 06/09/2023 01:26, Kacper Michajłow wrote: > Other C++ standard libraries exist. Also, this is not a proper way to > link the standard library anyway. Instead when a C++ dependency is > detected, switch to the C++ compiler driver to properly link everything. > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > --- > configure | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/configure b/configure > index bd7f7697c8..f3ff48586a 100755 > --- a/configure > +++ b/configure > @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr > caca_outdev_deps="libcaca" > decklink_deps_any="libdl LoadLibrary" > decklink_indev_deps="decklink threads" > -decklink_indev_extralibs="-lstdc++" > decklink_indev_suggest="libzvbi" > decklink_outdev_deps="decklink threads" > decklink_outdev_suggest="libklvanc" > -decklink_outdev_extralibs="-lstdc++" > dshow_indev_deps="IBaseFilter" > dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" > fbdev_indev_deps="linux_fb_h" > @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC > test -n "$cc_type" && enable $cc_type || > warn "Unknown C compiler $cc, unable to select optimal CFLAGS" > > +cxx_deps=( > + decklink > + libglslang > + libgme > + libopenmpt > + librubberband > + libsnappy > +) Can't say I'm a fan of maintaining a random list in the middle of configure, which manually contains all libs we think need libstdc++. This could for one change any time. But also, those dependenies might have other dependencies, which when linking statically might need libstdc++. But in general, this seems extremely brittle and a maintenance nightmare. > +for l in ${cxx_deps[@]}; do > + enabled $l && ld_default=$cxx > +done > + > : ${as_default:=$cc} > : ${objcc_default:=$cc} > : ${dep_cc_default:=$cc} > @@ -6706,12 +6716,12 @@ enabled libfribidi && require_pkg_config libfribidi fribidi fribidi.h fri > enabled libharfbuzz && require_pkg_config libharfbuzz harfbuzz hb.h hb_buffer_create > enabled libglslang && { check_lib spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ > -lglslang -lMachineIndependent -lOSDependent -lHLSL -lOGLCompiler -lGenericCodeGen \ > - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm || > + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm || > require spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ > -lglslang -lOSDependent -lHLSL -lOGLCompiler \ > - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm; } > + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm; } > enabled libgme && { check_pkg_config libgme libgme gme/gme.h gme_new_emu || > - require libgme gme/gme.h gme_new_emu -lgme -lstdc++; } > + require libgme gme/gme.h gme_new_emu -lgme; } > enabled libgsm && { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do > check_lib libgsm "${gsm_hdr}" gsm_create -lgsm && break; > done || die "ERROR: libgsm not found"; } > @@ -6770,7 +6780,7 @@ enabled libopencv && { check_headers opencv2/core/core_c.h && > enabled libopenh264 && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion > enabled libopenjpeg && { check_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version || > { require_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } } > -enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append libopenmpt_extralibs "-lstdc++" > +enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create > enabled libopenvino && { { check_pkg_config libopenvino openvino openvino/c/openvino.h ov_core_create && enable openvino2; } || > { check_pkg_config libopenvino openvino c_api/ie_c_api.h ie_c_api_version || > require libopenvino c_api/ie_c_api.h ie_c_api_version -linference_engine_c_api; } } > @@ -6789,12 +6799,12 @@ enabled librav1e && require_pkg_config librav1e "rav1e >= 0.5.0" rav1e. > enabled librist && require_pkg_config librist "librist >= 0.2.7" librist/librist.h rist_receiver_create > enabled librsvg && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_render_cairo > enabled librtmp && require_pkg_config librtmp librtmp librtmp/rtmp.h RTMP_Socket > -enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new -lstdc++ && append librubberband_extralibs "-lstdc++" > +enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new > enabled libshaderc && require_pkg_config spirv_compiler "shaderc >= 2019.1" shaderc/shaderc.h shaderc_compiler_initialize > enabled libshine && require_pkg_config libshine shine shine/layer3.h shine_encode_buffer > enabled libsmbclient && { check_pkg_config libsmbclient smbclient libsmbclient.h smbc_init || > require libsmbclient libsmbclient.h smbc_init -lsmbclient; } > -enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy -lstdc++ > +enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy > enabled libsoxr && require libsoxr soxr.h soxr_create -lsoxr > enabled libssh && require_pkg_config libssh libssh libssh/sftp.h sftp_init > enabled libspeex && require_pkg_config libspeex speex speex/speex.h speex_decoder_init The only correct way I see to deal with this would be to fully rely on pkg-config to contain the correct library.
On Wed, 6 Sept 2023 at 12:15, Timo Rothenpieler <timo@rothenpieler.org> wrote: > > On 06/09/2023 01:26, Kacper Michajłow wrote: > > Other C++ standard libraries exist. Also, this is not a proper way to > > link the standard library anyway. Instead when a C++ dependency is > > detected, switch to the C++ compiler driver to properly link everything. > > > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > > --- > > configure | 26 ++++++++++++++++++-------- > > 1 file changed, 18 insertions(+), 8 deletions(-) > > > > diff --git a/configure b/configure > > index bd7f7697c8..f3ff48586a 100755 > > --- a/configure > > +++ b/configure > > @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr > > caca_outdev_deps="libcaca" > > decklink_deps_any="libdl LoadLibrary" > > decklink_indev_deps="decklink threads" > > -decklink_indev_extralibs="-lstdc++" > > decklink_indev_suggest="libzvbi" > > decklink_outdev_deps="decklink threads" > > decklink_outdev_suggest="libklvanc" > > -decklink_outdev_extralibs="-lstdc++" > > dshow_indev_deps="IBaseFilter" > > dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" > > fbdev_indev_deps="linux_fb_h" > > @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC > > test -n "$cc_type" && enable $cc_type || > > warn "Unknown C compiler $cc, unable to select optimal CFLAGS" > > > > +cxx_deps=( > > + decklink > > + libglslang > > + libgme > > + libopenmpt > > + librubberband > > + libsnappy > > +) > > Can't say I'm a fan of maintaining a random list in the middle of > configure, which manually contains all libs we think need libstdc++. > > This could for one change any time. But also, those dependenies might > have other dependencies, which when linking statically might need libstdc++. > > But in general, this seems extremely brittle and a maintenance nightmare. Yes, that's the main problem. Linking static dependencies is an unsolved problem. We can discuss that, but wouldn't you agree that the proposed patch is an improvement over the current status quo? Forcing `-lstdc++` unconditionally, when some of the dependencies are enabled is way worse. The logic of maintaining the list does not change, it is just centralized in one place and properly switches linking language to C++ when needed, instead side-loading stdc++. Good solution that would not require any maintenance is to switch to CXX always for linking. > > +for l in ${cxx_deps[@]}; do > > + enabled $l && ld_default=$cxx > > +done > > + > > : ${as_default:=$cc} > > : ${objcc_default:=$cc} > > : ${dep_cc_default:=$cc} > > @@ -6706,12 +6716,12 @@ enabled libfribidi && require_pkg_config libfribidi fribidi fribidi.h fri > > enabled libharfbuzz && require_pkg_config libharfbuzz harfbuzz hb.h hb_buffer_create > > enabled libglslang && { check_lib spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ > > -lglslang -lMachineIndependent -lOSDependent -lHLSL -lOGLCompiler -lGenericCodeGen \ > > - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm || > > + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm || > > require spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ > > -lglslang -lOSDependent -lHLSL -lOGLCompiler \ > > - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm; } > > + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm; } > > enabled libgme && { check_pkg_config libgme libgme gme/gme.h gme_new_emu || > > - require libgme gme/gme.h gme_new_emu -lgme -lstdc++; } > > + require libgme gme/gme.h gme_new_emu -lgme; } > > enabled libgsm && { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do > > check_lib libgsm "${gsm_hdr}" gsm_create -lgsm && break; > > done || die "ERROR: libgsm not found"; } > > @@ -6770,7 +6780,7 @@ enabled libopencv && { check_headers opencv2/core/core_c.h && > > enabled libopenh264 && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion > > enabled libopenjpeg && { check_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version || > > { require_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } } > > -enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append libopenmpt_extralibs "-lstdc++" > > +enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create > > enabled libopenvino && { { check_pkg_config libopenvino openvino openvino/c/openvino.h ov_core_create && enable openvino2; } || > > { check_pkg_config libopenvino openvino c_api/ie_c_api.h ie_c_api_version || > > require libopenvino c_api/ie_c_api.h ie_c_api_version -linference_engine_c_api; } } > > @@ -6789,12 +6799,12 @@ enabled librav1e && require_pkg_config librav1e "rav1e >= 0.5.0" rav1e. > > enabled librist && require_pkg_config librist "librist >= 0.2.7" librist/librist.h rist_receiver_create > > enabled librsvg && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_render_cairo > > enabled librtmp && require_pkg_config librtmp librtmp librtmp/rtmp.h RTMP_Socket > > -enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new -lstdc++ && append librubberband_extralibs "-lstdc++" > > +enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new > > enabled libshaderc && require_pkg_config spirv_compiler "shaderc >= 2019.1" shaderc/shaderc.h shaderc_compiler_initialize > > enabled libshine && require_pkg_config libshine shine shine/layer3.h shine_encode_buffer > > enabled libsmbclient && { check_pkg_config libsmbclient smbclient libsmbclient.h smbc_init || > > require libsmbclient libsmbclient.h smbc_init -lsmbclient; } > > -enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy -lstdc++ > > +enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy > > enabled libsoxr && require libsoxr soxr.h soxr_create -lsoxr > > enabled libssh && require_pkg_config libssh libssh libssh/sftp.h sftp_init > > enabled libspeex && require_pkg_config libspeex speex speex/speex.h speex_decoder_init > > The only correct way I see to deal with this would be to fully rely on > pkg-config to contain the correct library. Using pkg-config is simply a workaround for an unresolved problem, and it "works" only because there is one blessed GNU C++ Library. Listing implicit stdlibs in .pc is not a good solution and in practice is not feasible to do so in a sensible way. Simply because your buildsystem doesn't know which implicit libraries the toolchain will link, making generating .pc files non-trivial and not portable. Of course, users of said library also lack this knowledge, which is why, when linking static libraries, it is generally recommended to use the same toolchain and the correct linking language mode. This is precisely what this patch accomplishes. Furthermore, the selection of stdlib is not done by `-l`, but rather by `-stdlib=` and the appropriate compiler driver (e.g., clang++ instead of clang). While -stdlib= can be included in the flags in .pc files, it's essential to use a C++ compiler explicitly. See https://libcxx.llvm.org/UsingLibcxx.html It is possible to use `-nostdinc++ -nodefaultlibs` and manually list everything, but this is not a task for pkg-config and could potentially disrupt the linking of other components. In general, pkg-config is better suited for shared library linking. If you don't have control over the build parameters of the static library, you shouldn't be using them anyway. A more appropriate solution would be to enhance pkg-config to include information about the required linking language for a given library. In summary, I don't believe pkg-config is suitable for listing implicit standard libraries. It's not straightforward to implement this on the library side, and it could likely cause issues for its users. - Kacper
On 06.09.2023 18:54, Kacper Michajlow wrote: > On Wed, 6 Sept 2023 at 12:15, Timo Rothenpieler <timo@rothenpieler.org> wrote: >> >> On 06/09/2023 01:26, Kacper Michajłow wrote: >>> Other C++ standard libraries exist. Also, this is not a proper way to >>> link the standard library anyway. Instead when a C++ dependency is >>> detected, switch to the C++ compiler driver to properly link everything. >>> >>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com> >>> --- >>> configure | 26 ++++++++++++++++++-------- >>> 1 file changed, 18 insertions(+), 8 deletions(-) >>> >>> diff --git a/configure b/configure >>> index bd7f7697c8..f3ff48586a 100755 >>> --- a/configure >>> +++ b/configure >>> @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr >>> caca_outdev_deps="libcaca" >>> decklink_deps_any="libdl LoadLibrary" >>> decklink_indev_deps="decklink threads" >>> -decklink_indev_extralibs="-lstdc++" >>> decklink_indev_suggest="libzvbi" >>> decklink_outdev_deps="decklink threads" >>> decklink_outdev_suggest="libklvanc" >>> -decklink_outdev_extralibs="-lstdc++" >>> dshow_indev_deps="IBaseFilter" >>> dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" >>> fbdev_indev_deps="linux_fb_h" >>> @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC >>> test -n "$cc_type" && enable $cc_type || >>> warn "Unknown C compiler $cc, unable to select optimal CFLAGS" >>> >>> +cxx_deps=( >>> + decklink >>> + libglslang >>> + libgme >>> + libopenmpt >>> + librubberband >>> + libsnappy >>> +) >> >> Can't say I'm a fan of maintaining a random list in the middle of >> configure, which manually contains all libs we think need libstdc++. >> >> This could for one change any time. But also, those dependenies might >> have other dependencies, which when linking statically might need libstdc++. >> >> But in general, this seems extremely brittle and a maintenance nightmare. > > Yes, that's the main problem. Linking static dependencies is an > unsolved problem. We can discuss that, but wouldn't you agree that the > proposed patch is an improvement over the current status quo? Forcing > `-lstdc++` unconditionally, when some of the dependencies are enabled > is way worse. The logic of maintaining the list does not change, it is > just centralized in one place and properly switches linking language > to C++ when needed, instead side-loading stdc++. It's not really unsolved. The dependencies need to properly advertise their dependencies via pkg-config. That only becomes an issue if we have some legacy manual detection that bypass pkg-config by default, or weird projects that don't supply pkg-config files. > Good solution that would not require any maintenance is to switch to > CXX always for linking. Just test if linking with CC/LD fails, and if it does, test if it works with CXX, and if so, use CXX? A bit crude, but at least does not cause a maintenance nightmare.
On Wed, 6 Sept 2023 at 19:09, Timo Rothenpieler <timo@rothenpieler.org> wrote: > > On 06.09.2023 18:54, Kacper Michajlow wrote: > > On Wed, 6 Sept 2023 at 12:15, Timo Rothenpieler <timo@rothenpieler.org> wrote: > >> > >> On 06/09/2023 01:26, Kacper Michajłow wrote: > >>> Other C++ standard libraries exist. Also, this is not a proper way to > >>> link the standard library anyway. Instead when a C++ dependency is > >>> detected, switch to the C++ compiler driver to properly link everything. > >>> > >>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > >>> --- > >>> configure | 26 ++++++++++++++++++-------- > >>> 1 file changed, 18 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/configure b/configure > >>> index bd7f7697c8..f3ff48586a 100755 > >>> --- a/configure > >>> +++ b/configure > >>> @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr > >>> caca_outdev_deps="libcaca" > >>> decklink_deps_any="libdl LoadLibrary" > >>> decklink_indev_deps="decklink threads" > >>> -decklink_indev_extralibs="-lstdc++" > >>> decklink_indev_suggest="libzvbi" > >>> decklink_outdev_deps="decklink threads" > >>> decklink_outdev_suggest="libklvanc" > >>> -decklink_outdev_extralibs="-lstdc++" > >>> dshow_indev_deps="IBaseFilter" > >>> dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" > >>> fbdev_indev_deps="linux_fb_h" > >>> @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC > >>> test -n "$cc_type" && enable $cc_type || > >>> warn "Unknown C compiler $cc, unable to select optimal CFLAGS" > >>> > >>> +cxx_deps=( > >>> + decklink > >>> + libglslang > >>> + libgme > >>> + libopenmpt > >>> + librubberband > >>> + libsnappy > >>> +) > >> > >> Can't say I'm a fan of maintaining a random list in the middle of > >> configure, which manually contains all libs we think need libstdc++. > >> > >> This could for one change any time. But also, those dependenies might > >> have other dependencies, which when linking statically might need libstdc++. > >> > >> But in general, this seems extremely brittle and a maintenance nightmare. > > > > Yes, that's the main problem. Linking static dependencies is an > > unsolved problem. We can discuss that, but wouldn't you agree that the > > proposed patch is an improvement over the current status quo? Forcing > > `-lstdc++` unconditionally, when some of the dependencies are enabled > > is way worse. The logic of maintaining the list does not change, it is > > just centralized in one place and properly switches linking language > > to C++ when needed, instead side-loading stdc++. > > It's not really unsolved. The dependencies need to properly advertise > their dependencies via pkg-config. > That only becomes an issue if we have some legacy manual detection that > bypass pkg-config by default, or weird projects that don't supply > pkg-config files. No, not really. I've explained why we shouldn't list standard libraries in pkg-config. No need to repeat myself, feel free to refer to my previous email. And in fact I found the exact same discussion on pkg-config ML from a few years ago. https://lists.freedesktop.org/archives/pkg-config/2016-August/001053.html The conclusion mostly matches mine argument, that we should not include `-lstdc++` in `Libs.private`. Or any other implicitly added standard lib for that matter. > > Good solution that would not require any maintenance is to switch to > > CXX always for linking. > > Just test if linking with CC/LD fails, and if it does, test if it works > with CXX, and if so, use CXX? > A bit crude, but at least does not cause a maintenance nightmare. What would be a downside of preferring CXX always if it exists? - Kacper
On 9/6/2023 6:31 PM, Kacper Michajlow wrote:
> What would be a downside of preferring CXX always if it exists?
FFmpeg runs in a multitude of environments with a multitude of portability
requirements. Needlessly linking a C++ runtime is not OK.
Further, we do not generally automatically change build output based on
what is available on the system.
- Derek
On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote: > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > > What would be a downside of preferring CXX always if it exists? > > FFmpeg runs in a multitude of environments with a multitude of portability > requirements. Needlessly linking a C++ runtime is not OK. This does not answer my question. Let me rephrase. Do we know the case where using C++ compiler driver rather than C would degrade the quality of the resulting build? Using C++ driver would indeed append the (correct) runtime library to the linker command, but if nothing references any symbols from it it would not be linked. It is also why the current way of forcing `lstdc++` kinda works, because it is silently ignored when not needed. Implementing logic to use C++ only when necessary is possible, but I'm not a big fan of such automation. And in practice not sure how well it would work, because it would require trying to link twice every dependency in configure. Also the fact that "FFmpeg runs in a multitude of environment" is precisely why I really don't like the current unconditional including `-lstdc++`. > Further, we do not generally automatically change build output based on > what is available on the system. I must kindly disagree. configure has 35 components marked as `[autodetect]` and they are certainly changing the build output based on what's available on the system. Whether it is intended or historical behaviour is another question. But I'm also not proposing to auto detect anything, quite the opposite with this silly manual list of libraries. - Kacper
On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow <kasper93@gmail.com> wrote: > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > <derek.buitenhuis@gmail.com> wrote: > > > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > > > What would be a downside of preferring CXX always if it exists? > > > > FFmpeg runs in a multitude of environments with a multitude of > portability > > requirements. Needlessly linking a C++ runtime is not OK. > > This does not answer my question. Let me rephrase. Do we know the case > where using C++ compiler driver rather than C would degrade the > quality of the resulting build? > The machine that ffmpeg is being compiled on is not necessarily the one that ffmpeg is going to be run on. Kieran
On 07.09.2023 23:38, Kacper Michajlow wrote: > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > <derek.buitenhuis@gmail.com> wrote: >> >> On 9/6/2023 6:31 PM, Kacper Michajlow wrote: >>> What would be a downside of preferring CXX always if it exists? >> >> FFmpeg runs in a multitude of environments with a multitude of portability >> requirements. Needlessly linking a C++ runtime is not OK. > > This does not answer my question. Let me rephrase. Do we know the case > where using C++ compiler driver rather than C would degrade the > quality of the resulting build? > > Using C++ driver would indeed append the (correct) runtime library to > the linker command, but if nothing references any symbols from it it > would not be linked. It is also why the current way of forcing > `lstdc++` kinda works, because it is silently ignored when not needed. > > Implementing logic to use C++ only when necessary is possible, but I'm > not a big fan of such automation. And in practice not sure how well it > would work, because it would require trying to link twice every > dependency in configure. > > Also the fact that "FFmpeg runs in a multitude of environment" is > precisely why I really don't like the current unconditional including > `-lstdc++`. Couldn't you just check if stdc++ is in the ldflags/extralibs, and if so, remove it, and use g++ to link?
On Fri, 8 Sept 2023 at 00:11, Kieran Kunhya <kierank@obe.tv> wrote: > > On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow <kasper93@gmail.com> wrote: > > > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > > <derek.buitenhuis@gmail.com> wrote: > > > > > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > > > > What would be a downside of preferring CXX always if it exists? > > > > > > FFmpeg runs in a multitude of environments with a multitude of > > portability > > > requirements. Needlessly linking a C++ runtime is not OK. > > > > This does not answer my question. Let me rephrase. Do we know the case > > where using C++ compiler driver rather than C would degrade the > > quality of the resulting build? > > > > The machine that ffmpeg is being compiled on is not necessarily the one > that ffmpeg is going to be run on. Not sure how this is relevant to the discussion? - Kacper
On Fri, 8 Sept 2023 at 19:42, Kacper Michajlow <kasper93@gmail.com> wrote: > On Fri, 8 Sept 2023 at 00:11, Kieran Kunhya <kierank@obe.tv> wrote: > > > > On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow <kasper93@gmail.com> > wrote: > > > > > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > > > <derek.buitenhuis@gmail.com> wrote: > > > > > > > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > > > > > What would be a downside of preferring CXX always if it exists? > > > > > > > > FFmpeg runs in a multitude of environments with a multitude of > > > portability > > > > requirements. Needlessly linking a C++ runtime is not OK. > > > > > > This does not answer my question. Let me rephrase. Do we know the case > > > where using C++ compiler driver rather than C would degrade the > > > quality of the resulting build? > > > > > > > The machine that ffmpeg is being compiled on is not necessarily the one > > that ffmpeg is going to be run on. > > Not sure how this is relevant to the discussion? > > > > > What would be a downside of preferring CXX always if it exists? Because it forces a dependency on libstdc++ even if another machine does not have it. Kieran
On Fri, 8 Sept 2023 at 01:35, Timo Rothenpieler via ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote: > > On 07.09.2023 23:38, Kacper Michajlow wrote: > > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > > <derek.buitenhuis@gmail.com> wrote: > >> > >> On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > >>> What would be a downside of preferring CXX always if it exists? > >> > >> FFmpeg runs in a multitude of environments with a multitude of portability > >> requirements. Needlessly linking a C++ runtime is not OK. > > > > This does not answer my question. Let me rephrase. Do we know the case > > where using C++ compiler driver rather than C would degrade the > > quality of the resulting build? > > > > Using C++ driver would indeed append the (correct) runtime library to > > the linker command, but if nothing references any symbols from it it > > would not be linked. It is also why the current way of forcing > > `lstdc++` kinda works, because it is silently ignored when not needed. > > > > Implementing logic to use C++ only when necessary is possible, but I'm > > not a big fan of such automation. And in practice not sure how well it > > would work, because it would require trying to link twice every > > dependency in configure. > > > > Also the fact that "FFmpeg runs in a multitude of environment" is > > precisely why I really don't like the current unconditional including > > `-lstdc++`. > > Couldn't you just check if stdc++ is in the ldflags/extralibs, and if > so, remove it, and use g++ to link? Well, I'm lost now. Are you suggesting building on top of existing hacks is a better solution than the proposed patch? I refuse supporting any kind of random `-lstdc++` adding in configure and then removing it in the end. - Kacper
On 08.09.2023 20:55, Kacper Michajlow wrote: > On Fri, 8 Sept 2023 at 01:35, Timo Rothenpieler via ffmpeg-devel > <ffmpeg-devel@ffmpeg.org> wrote: >> >> On 07.09.2023 23:38, Kacper Michajlow wrote: >>> On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis >>> <derek.buitenhuis@gmail.com> wrote: >>>> >>>> On 9/6/2023 6:31 PM, Kacper Michajlow wrote: >>>>> What would be a downside of preferring CXX always if it exists? >>>> >>>> FFmpeg runs in a multitude of environments with a multitude of portability >>>> requirements. Needlessly linking a C++ runtime is not OK. >>> >>> This does not answer my question. Let me rephrase. Do we know the case >>> where using C++ compiler driver rather than C would degrade the >>> quality of the resulting build? >>> >>> Using C++ driver would indeed append the (correct) runtime library to >>> the linker command, but if nothing references any symbols from it it >>> would not be linked. It is also why the current way of forcing >>> `lstdc++` kinda works, because it is silently ignored when not needed. >>> >>> Implementing logic to use C++ only when necessary is possible, but I'm >>> not a big fan of such automation. And in practice not sure how well it >>> would work, because it would require trying to link twice every >>> dependency in configure. >>> >>> Also the fact that "FFmpeg runs in a multitude of environment" is >>> precisely why I really don't like the current unconditional including >>> `-lstdc++`. >> >> Couldn't you just check if stdc++ is in the ldflags/extralibs, and if >> so, remove it, and use g++ to link? > > Well, I'm lost now. Are you suggesting building on top of existing > hacks is a better solution than the proposed patch? The proposed patch looks like a near unmaintainable hack to me as it is. Anything that's less brittle is highly preferred. There is no good solution here, but I'm very much against any solution that has high potential for random breakages and hard to predict side-effects. > I refuse supporting any kind of random `-lstdc++` adding in configure > and then removing it in the end. > > - Kacper
On Fri, 8 Sept 2023 at 20:46, Kieran Kunhya <kierank@obe.tv> wrote: > > On Fri, 8 Sept 2023 at 19:42, Kacper Michajlow <kasper93@gmail.com> wrote: > > > On Fri, 8 Sept 2023 at 00:11, Kieran Kunhya <kierank@obe.tv> wrote: > > > > > > On Thu, 7 Sept 2023 at 22:39, Kacper Michajlow <kasper93@gmail.com> > > wrote: > > > > > > > On Thu, 7 Sept 2023 at 15:12, Derek Buitenhuis > > > > <derek.buitenhuis@gmail.com> wrote: > > > > > > > > > > On 9/6/2023 6:31 PM, Kacper Michajlow wrote: > > > > > > What would be a downside of preferring CXX always if it exists? > > > > > > > > > > FFmpeg runs in a multitude of environments with a multitude of > > > > portability > > > > > requirements. Needlessly linking a C++ runtime is not OK. > > > > > > > > This does not answer my question. Let me rephrase. Do we know the case > > > > where using C++ compiler driver rather than C would degrade the > > > > quality of the resulting build? > > > > > > > > > > The machine that ffmpeg is being compiled on is not necessarily the one > > > that ffmpeg is going to be run on. > > > > Not sure how this is relevant to the discussion? > > > > > > > > What would be a downside of preferring CXX always if it exists? > > Because it forces a dependency on libstdc++ even if another machine does > not have it. On all "affected" platforms, configure already adds -Wl,--as-needed, so the resulting library has exactly the same dependencies. Unless something references symbols from stdlib, but then it would just not link with clang anyway. ~ clang++ m.o -Wl,--as-needed && ldd a.out linux-vdso.so.1 (0x00007ffeeb912000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f64050a7000) /lib64/ld-linux-x86-64.so.2 (0x00007f64052ad000) ~ clang m.o -Wl,--as-needed && ldd a.out linux-vdso.so.1 (0x00007ffe1fb36000) libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f5e7a3c2000) /lib64/ld-linux-x86-64.so.2 (0x00007f5e7a5c8000) - Kacper
Le keskiviikkona 6. syyskuuta 2023, 2.26.29 EEST Kacper Michajłow a écrit : > Other C++ standard libraries exist. Also, this is not a proper way to > link the standard library anyway. Instead when a C++ dependency is > detected, switch to the C++ compiler driver to properly link everything. > > Signed-off-by: Kacper Michajłow <kasper93@gmail.com> > --- > configure | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/configure b/configure > index bd7f7697c8..f3ff48586a 100755 > --- a/configure > +++ b/configure > @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h > machine_ioctl_bt848_h dev_video_bktr caca_outdev_deps="libcaca" > decklink_deps_any="libdl LoadLibrary" > decklink_indev_deps="decklink threads" > -decklink_indev_extralibs="-lstdc++" > decklink_indev_suggest="libzvbi" > decklink_outdev_deps="decklink threads" > decklink_outdev_suggest="libklvanc" > -decklink_outdev_extralibs="-lstdc++" > dshow_indev_deps="IBaseFilter" > dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 > -lshlwapi" fbdev_indev_deps="linux_fb_h" > @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC > test -n "$cc_type" && enable $cc_type || > warn "Unknown C compiler $cc, unable to select optimal CFLAGS" > > +cxx_deps=( > + decklink > + libglslang > + libgme > + libopenmpt > + librubberband > + libsnappy > +) Hard-coding "-lstdc++" is admittedly a brittle kludge. On some systems, the C++ runtime has a different name for one. But hard-coding the set of library that would require the C++ linker is just as much of a hack. I would argue it's even more of a kludge. If a certain runtime (C++ or other) is necessary for a library then it should be in Libs.static in the pkg-config manifest. From there, FFmpeg should pick it up if and *only* if the libarry is linked statically. Indeed, there are no reasons to even force C++ linking to use a C++ *shared* library exposing a pure C interface. Thus, I don't think that forcing the C++ linker is correct. (Also I more or less agree with Timo here.)
diff --git a/configure b/configure index bd7f7697c8..f3ff48586a 100755 --- a/configure +++ b/configure @@ -3584,11 +3584,9 @@ bktr_indev_deps_any="dev_bktr_ioctl_bt848_h machine_ioctl_bt848_h dev_video_bktr caca_outdev_deps="libcaca" decklink_deps_any="libdl LoadLibrary" decklink_indev_deps="decklink threads" -decklink_indev_extralibs="-lstdc++" decklink_indev_suggest="libzvbi" decklink_outdev_deps="decklink threads" decklink_outdev_suggest="libklvanc" -decklink_outdev_extralibs="-lstdc++" dshow_indev_deps="IBaseFilter" dshow_indev_extralibs="-lpsapi -lole32 -lstrmiids -luuid -loleaut32 -lshlwapi" fbdev_indev_deps="linux_fb_h" @@ -4984,6 +4982,18 @@ set_ccvars HOSTCC test -n "$cc_type" && enable $cc_type || warn "Unknown C compiler $cc, unable to select optimal CFLAGS" +cxx_deps=( + decklink + libglslang + libgme + libopenmpt + librubberband + libsnappy +) +for l in ${cxx_deps[@]}; do + enabled $l && ld_default=$cxx +done + : ${as_default:=$cc} : ${objcc_default:=$cc} : ${dep_cc_default:=$cc} @@ -6706,12 +6716,12 @@ enabled libfribidi && require_pkg_config libfribidi fribidi fribidi.h fri enabled libharfbuzz && require_pkg_config libharfbuzz harfbuzz hb.h hb_buffer_create enabled libglslang && { check_lib spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ -lglslang -lMachineIndependent -lOSDependent -lHLSL -lOGLCompiler -lGenericCodeGen \ - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm || + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm || require spirv_compiler glslang/Include/glslang_c_interface.h glslang_initialize_process \ -lglslang -lOSDependent -lHLSL -lOGLCompiler \ - -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lstdc++ -lm; } + -lSPVRemapper -lSPIRV -lSPIRV-Tools-opt -lSPIRV-Tools -lpthread -lm; } enabled libgme && { check_pkg_config libgme libgme gme/gme.h gme_new_emu || - require libgme gme/gme.h gme_new_emu -lgme -lstdc++; } + require libgme gme/gme.h gme_new_emu -lgme; } enabled libgsm && { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do check_lib libgsm "${gsm_hdr}" gsm_create -lgsm && break; done || die "ERROR: libgsm not found"; } @@ -6770,7 +6780,7 @@ enabled libopencv && { check_headers opencv2/core/core_c.h && enabled libopenh264 && require_pkg_config libopenh264 openh264 wels/codec_api.h WelsGetCodecVersion enabled libopenjpeg && { check_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version || { require_pkg_config libopenjpeg "libopenjp2 >= 2.1.0" openjpeg.h opj_version -DOPJ_STATIC && add_cppflags -DOPJ_STATIC; } } -enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create -lstdc++ && append libopenmpt_extralibs "-lstdc++" +enabled libopenmpt && require_pkg_config libopenmpt "libopenmpt >= 0.2.6557" libopenmpt/libopenmpt.h openmpt_module_create enabled libopenvino && { { check_pkg_config libopenvino openvino openvino/c/openvino.h ov_core_create && enable openvino2; } || { check_pkg_config libopenvino openvino c_api/ie_c_api.h ie_c_api_version || require libopenvino c_api/ie_c_api.h ie_c_api_version -linference_engine_c_api; } } @@ -6789,12 +6799,12 @@ enabled librav1e && require_pkg_config librav1e "rav1e >= 0.5.0" rav1e. enabled librist && require_pkg_config librist "librist >= 0.2.7" librist/librist.h rist_receiver_create enabled librsvg && require_pkg_config librsvg librsvg-2.0 librsvg-2.0/librsvg/rsvg.h rsvg_handle_render_cairo enabled librtmp && require_pkg_config librtmp librtmp librtmp/rtmp.h RTMP_Socket -enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new -lstdc++ && append librubberband_extralibs "-lstdc++" +enabled librubberband && require_pkg_config librubberband "rubberband >= 1.8.1" rubberband/rubberband-c.h rubberband_new enabled libshaderc && require_pkg_config spirv_compiler "shaderc >= 2019.1" shaderc/shaderc.h shaderc_compiler_initialize enabled libshine && require_pkg_config libshine shine shine/layer3.h shine_encode_buffer enabled libsmbclient && { check_pkg_config libsmbclient smbclient libsmbclient.h smbc_init || require libsmbclient libsmbclient.h smbc_init -lsmbclient; } -enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy -lstdc++ +enabled libsnappy && require libsnappy snappy-c.h snappy_compress -lsnappy enabled libsoxr && require libsoxr soxr.h soxr_create -lsoxr enabled libssh && require_pkg_config libssh libssh libssh/sftp.h sftp_init enabled libspeex && require_pkg_config libspeex speex speex/speex.h speex_decoder_init
Other C++ standard libraries exist. Also, this is not a proper way to link the standard library anyway. Instead when a C++ dependency is detected, switch to the C++ compiler driver to properly link everything. Signed-off-by: Kacper Michajłow <kasper93@gmail.com> --- configure | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)