-
Notifications
You must be signed in to change notification settings - Fork 107
Always use -Wl,-Bsymbolic flag in shared build #460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
69080a4 to
ff54b49
Compare
Without this, when I was building ffms2 and ffmpeg in a shared library AND ffmpeg wasn't installed on the system on ubuntu (in /usr/lib) (it was installed to a path I specified with --prefix), ffms2 was failing to build. I was getting this error: ``` libtool: link: g++ -std=c++11 -fvisibility=hidden -Werror -Wno-error=deprecated-declarations -o src/index/.libs/ffmsindex src/index/ffmsindex.o -lavutil src/core/.libs/libffms2.so -Wl,-rpath -Wl,/home/runner/work/ffms2/ffms2/ffmpeg-8.0/ffmpeg_build/lib /usr/bin/ld: cannot find -lavutil: No such file or directory ``` After this change, it works fine and this is now how the libtool logs looks: ``` libtool: link: g++ -std=c++11 -fvisibility=hidden -Werror -Wno-error=deprecated-declarations -o src/index/.libs/ffmsindex src/index/ffmsindex.o -L/home/runner/work/ffms2/ffms2/ffmpeg-8.0/ffmpeg_build/lib -lavutil src/core/.libs/libffms2.so -Wl,-rpath -Wl,/home/runner/work/ffms2/ffms2/ffmpeg-8.0/ffmpeg_build/lib ```
The file m4/ax_check_link_flag.m4 is a copy of https://www.gnu.org/software/autoconf-archive/ax_check_link_flag.html Fix FFMS#458 and also fix when building ffms2 as a shared library and link with a static ffmpeg on ubuntu (it was building properly on macOS + windows (msys2)).
|
@dwbuiten What do you think about this PR? |
dwbuiten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment on the first commit.
For the second commit: This is not a supported use case of the *nix configure as far as I am concerned.
| AC_SUBST(pkgconfigdir) | ||
|
|
||
| PKG_CHECK_MODULES(FFMPEG, [libavformat >= 61.7.0 libavcodec >= 61.19.0 libswscale >= 8.3.0 libavutil >= 59.39.0 libswresample >= 5.3.0]) | ||
| PKG_CHECK_MODULES([AVUTIL], [libavutil >= 59.39.0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did this version come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's from the precedent line.
I guess you are referring to But, as we can see in this pr that test the build (moi15moi#3), on ubuntu, without this fix, when ffmpeg is shared and ffms2 is static, without this fix, the build fail. |
No, I'm talking about building a shared library linking a static ffmpeg. I don't consider this something autotools should support, or any distro should support. It's not a done thing. If people insist on using msys+mingw instead of msvc or clang, they should just pass extra cflags. |
|
In this case, why this code is present? As specified in the doc (https://ffmpeg.org/platform.html#Advanced-linking-configuration), If you don't want to support this case, this should just be removed. |
|
That is FFmpeg documentation, not FFMS2's. |
|
Yes, it is ffmpeg doc. It explain the reason when we need to use the linking flag |
Fix #458
I created another PR to test all the possibility (ex: ffmpeg static with ffms2 shared, ffmpeg shared with ffms2 shared, etc) to properly test this PR.
See: moi15moi#3
As we can see, without this PR, we couldn't even build ffms2 as a shared library on ubuntu when ffmpeg was installed with a specific prefix (thought it was working properly on macOS and Windows (msys2)).