Paths

Table of Contentst

CMake: use FFmpeg find module on Linux
ClosedPublic
Actions

Authored by Brecht Van Lommel (brecht) on Oct 12 2020, 2:54 PM.

Details

Reviewers

Bastien Montagne (mont29)
Ankit Meel (ankitm)
jim man (jimman2003)

Commits

rB75a1a578bd5d: CMake: use FFmpeg find module on Linux
rB62a0de167330: CMake: use FFmpeg find module on Linux

Summary

This is the last step that lets us get rid of LIBPATH variables and
link_directories() entirely, as recommended by the CMake docs.

It changes install_deps.sh to build shared (instead of static) FFMPEG libraries,
for consistency with other library dependencies and to simplify the logic. This
may require users of install_deps.sh to rebuild FFMPEG.

Some fixes were needed in the find FFMPEG module to make it actually work, this
code was unused up to now.

Followup to D8855.

Diff Detail

Repository

rB Blender

Build Status

Buildable 10684
Build 10684: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) created this revision.Oct 12 2020, 2:54 PM

Harbormaster completed remote builds in B10681: Diff 29838.Oct 12 2020, 2:54 PM

Brecht Van Lommel (brecht) requested review of this revision.Oct 12 2020, 2:54 PM

Thanks for fixing the mistakes in the find module!

build_files/cmake/platform/platform_apple.cmake
177	This is not strictly required, due to the `CMAKE_PREFIX_PATH` variable above. [1] [2]. Every Blender find module works off `CMAKE_PREFIX_PATH` itself, and doesn't use `*_ROOT_DIR`. Doesn't hurt to keep it though. [1] https://developer.blender.org/diffusion/B/browse/master/build_files/cmake/platform/platform_apple.cmake$47-53 [2] https://cmake.org/cmake/help/latest/command/find_library.html

I removed the code from install_deps.sh that finds ffmpeg dependencies for linking. I don't see why this would be needed if we are linking to shared libraries? And why only for FFmpeg?

IIRC this was mandatory in case we had to build our own FFMPEG when another one was also on the system from packages? I am not sure though, this is very old code. Will have to check again whether this is working properly now.

Yes, can confirm that with this patch you'll get linking error when building Blender, if you build your own ffmpeg (--build-ffmpeg option of install_deps).

Turns out this is because we build FFMPEG as a static lib (not sure why, it seems to be the default behavior…)

Tried with a dynamic build instead, and from quick looks it seems to work fine.

build_files/build_environment/install_deps.sh
3414	This can also be removed I think
3415	needs to be replaced by `--enable-shared`

This revision now requires changes to proceed.Oct 12 2020, 6:47 PM

Build shared ffmpeg library, consistent with other libraries.

Harbormaster completed remote builds in B10684: Diff 29851.Oct 12 2020, 7:03 PM

Bastien Montagne (mont29) accepted this revision.Oct 12 2020, 7:35 PM

This revision is now accepted and ready to land.Oct 12 2020, 7:35 PM

Ankit Meel (ankitm) awarded a token.Nov 23 2020, 4:10 PM

Keeping @Sybren A. Stüvel (sybren) in loop for the intended [removing link_directories] change.

jim man (jimman2003) accepted this revision.Jul 14 2021, 10:22 AM

jim man (jimman2003) awarded a token.Jul 22 2021, 11:21 AM

Sybren A. Stüvel (sybren) added inline comments.Jan 10 2022, 11:57 AM

build_files/cmake/Modules/FindFFmpeg.cmake
36–37	If this change is about using the "FFmpeg find module" on Linux, why the added components?
build_files/cmake/platform/platform_unix.cmake
181–184	If we list these components here, why also list them in `FindFFmpeg.cmake` again? This patch also adds some components here; why is that?

The ones in FindFFmpeg.cmake are generics/ defaults in case no input FFMPEG_FIND_COMPONENTS are given. I took them from https://ffmpeg.org/olddownload.html and wrote the common components without overextending the list with more niche components.

The ones in platform_unix.cmake are taken from libdir by listing each library present there instead of globbing as done previously.

For example avfilter is there in official release and not in libdir .

PS it was added in D8918: CMake/Find packages: add FindFFmpeg.cmake module. as part of D8855: CMake/macOS: Remove _LIBPATH, avoid link_directories.

I forgot about this patch, didn't commit it at the time because it wasn't the right time in the release cycle and this may require users of install_deps.sh to rebuild ffmpeg.

But I think we might as well do it now. I'll update the description.

Brecht Van Lommel (brecht) edited the summary of this revision. (Show Details)Jan 10 2022, 3:18 PM

Brecht Van Lommel (brecht) edited the summary of this revision. (Show Details)Jan 10 2022, 3:20 PM

Brecht Van Lommel (brecht) marked 2 inline comments as done.Jan 10 2022, 4:48 PM

In D9177#365103, @Ankit Meel (ankitm) wrote:

The ones in FindFFmpeg.cmake are generics/ defaults in case no input FFMPEG_FIND_COMPONENTS are given. I took them from https://ffmpeg.org/olddownload.html and wrote the common components without overextending the list with more niche components.

The ones in platform_unix.cmake are taken from libdir by listing each library present there instead of globbing as done previously.

I think this might be good to describe in a comment. It'll help future generations when they know the reasons why things are listed where they are.

Closed by commit rB62a0de167330: CMake: use FFmpeg find module on Linux (authored by Brecht Van Lommel (brecht)). · Explain WhyJan 12 2022, 6:25 PM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rB62a0de167330: CMake: use FFmpeg find module on Linux.

Brecht Van Lommel (brecht) added a reverting change: rBe594f23a27fe: Revert "CMake: use FFmpeg find module on Linux".Jan 12 2022, 7:14 PM

Brecht Van Lommel (brecht) added a commit: rB75a1a578bd5d: CMake: use FFmpeg find module on Linux.Jan 13 2022, 10:51 AM

Brecht Van Lommel (brecht) mentioned this in rB29450a2af3dd: Build: remove usage of link_directories.

Brecht Van Lommel (brecht) mentioned this in rCf72160777092: Build: remove usage of link_directories.Jan 27 2022, 11:38 PM