[ffmpeg] use pkgconfig to get system dependencies for cmake #17985

mcmtroffaes · 2021-05-18T07:43:30Z

What does your PR fix?

This PR uses the config.mak file generated during the build process to get all system dependencies. This ensures that only the required system dependencies are included, and future-proofs the port against changes in such dependencies, which are a pain to track. This substantially simplifies the FindFFMPEG.cmake script and makes it more robust and easier to maintain. It also ensures that FindFFMPEG.cmake lists the same dependencies as the generated pkg-config file. I've noticed that for many builds the dependencies are slightly different (sometimes more, sometimes less) from what the old FindFFMPEG.cmake did, so presumably this PR also might fix some subtle bugs with less often used functions that need those libraries.

One change I do point out concerns non-static builds: some dependent libraries were only used in the FindFFMPEG.cmake script in static builds: see the set(ENABLE_* ${STATIC_LINKAGE}) lines in the original portfile. This is not what the upstream pkgconfig file does, however. With this PR, dependencies will be listed exactly as in the pkgconfig file. I think it's a price worth paying for keeping things simple.

Finally, I note that many past pull requests for the ffmpeg port have been concerned with fixing dependencies across platforms. My hope is that this PR will result in a more stable port in the long term, and less time wasted on chasing dependency fixes.

I've thoroughly tested the PR on linux, osx, and windows triplets (see https://travis-ci.com/github/mcmtroffaes/ffmpeg-msvc-build/builds/225915669 - this has one failure but that's addressed in [openh264] only build shared or static library #17592 and also https://ci.appveyor.com/project/mcmtroffaes/ffmpeg-msvc-build/builds/39162659).
Which triplets are supported/not supported? Have you updated the CI baseline?

No changes to baseline. No changes to supported triplets.
Does your PR follow the maintainer guide?

Yes, to the best of my knowledge.
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes.

cenit · 2021-05-19T20:47:31Z

Just a side note, not against your work of course

FindFFMPEG.cmake is slowly transforming irreversibly from a CMake module that is able to find FFMPEG, independently from how it's built, to a CMake config file tailored around the specific options.
This should be taken into consideration too: because at the beginning I was hoping to send the module upstream to Kitware, now it looks like the only way to export it from vcpkg will be sending it to ffmpeg (with a much lower possibilities of acceptance).

cenit · 2021-05-19T20:48:31Z

but again, this is a route that the module took some time ago, and this is just the final step.
I'd rename it into FFMPEGConfig.cmake, and remove the unnecessary wrapper

mcmtroffaes · 2021-05-19T22:19:13Z

Thanks for your useful and considerate feedback @cenit - I'm sorry that I didn't think of that, and in retrospect I definitely should have mentioned this in my PR as an important consideration (and maybe discussed it first via an issue). Definitely with this patch there's little reason indeed then not to go for a cmake config file. IMO, having looked for quite some time at the complexity of the configure script, I don't think it is feasible to have a fully functional FindFFMPEG script that would work reliably for all possible platforms and variations of ffmpeg builds. Anyway, I'm happy to have a discussion and if there are good reasons not to this route and thus reject this PR, I'd be interested to hear them.

I note that a cmake config could be generated as part of the configure process, which could be submitted to upstream ffmpeg after some testing here in vcpkg. Dealing with upstream ffmpeg is slow but I've managed to get some small patches applied. But I'm not sure to what extent support for generating a cmake config would be welcomed by upstream ffmpeg.

Another route might be for the vcpkg cmake config to be able to import the pkg-config file. I don't know how to do that however. If someone knows this is possible and can point me to some examples, I'd be happy to investigate.

One question though, if folks agree we should go for the config route: should this then be turned into an unofficial cmake config (with the unofficial prefix), as in for instance https://github.com/microsoft/vcpkg/blob/master/ports/libvpx/unofficial-libvpx-config.cmake.in and other ports that provide non-upstream cmake configs? Or should we stick with FFMPEGConfig.cmake so find_package(FFMPEG) keeps working? I assume FFMPEGConfig.cmake would be preferred to keep compatibility?

PhoebeHui · 2021-05-20T01:52:30Z

/azp run

azure-pipelines · 2021-05-20T01:52:48Z

Azure Pipelines successfully started running 1 pipeline(s).

cenit · 2021-05-20T04:38:42Z

to be totally honest, i never understood the unofficial prefix. it's just a way to ask for more patches and future work. Let's keep it this way so that you don't have to fix dozen of ports that would become broken (do not think only about ports here, but also private ports that expects to work with find_package(FFMPEG))

otoh, if you want to try to upstream the config after some testing here, for me that would be wonderful. I know it will be slow and difficult and maybe undesired by ffmpeg maintainers, but at least you had a chance of success and so you are in a better position :)

mcmtroffaes · 2021-05-20T08:28:15Z

Sounds like there's a plan then. I'd prefer this to be merged first, and then work towards a proper config at a later time, if that's ok.

And thanks for rerunning CI. Looks like there's a failure of opencv4, fortunately only on linux. I tried to reproduce locally and this looks related to an issue with png detection, unrelated to this patch? Here's my local build log:

$ ./vcpkg install ffmpeg[core]:x64-linux opencv[core,ffmpeg]:x64-linux
...
-- Configuring x64-linux-dbg
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:105 (message):
    Command failed: /home/xxx/vcpkg/downloads/tools/cmake-3.20.2-linux/cmake-3.20.2-linux-x86_64/bin/cmake /home/xxx/vcpkg/buildtrees/opencv4/src/4.5.1-e92f755db3.clean [...]
    Working Directory: /home/xxx/vcpkg/buildtrees/opencv4/x64-linux-dbg
    Error code: 1
    See logs for more information:
      /home/xxx/vcpkg/buildtrees/opencv4/config-x64-linux-dbg-out.log
      /home/xxx/vcpkg/buildtrees/opencv4/config-x64-linux-dbg-err.log

Call Stack (most recent call first):
  scripts/cmake/vcpkg_configure_cmake.cmake:332 (vcpkg_execute_required_process)
  ports/opencv4/portfile.cmake:290 (vcpkg_configure_cmake)
  scripts/ports.cmake:141 (include)

Error: Building package opencv4:x64-linux failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `./vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: opencv4:x64-linux
  Vcpkg version: 2021-01-13-unknownhash

Additionally, attach any relevant sections from the log files above.
$ tail /home/xxx/vcpkg/buildtrees/opencv4/config-x64-linux-dbg-out.log
-- Found Threads: TRUE
-- Found ZLIB: /home/xxx/vcpkg/installed/x64-linux/debug/lib/libz.a (found version "1.2.11")
-- Found ZLIB: /home/xxx/vcpkg/installed/x64-linux/debug/lib/libz.a (found version "1.2.11")
-- Found PNG: optimized;/home/xxx/vcpkg/installed/x64-linux/debug/lib/libpng.a;debug;/home/xxx/vcpkg/installed/x64-linux/debug/lib/libpng16d.a (found version "1.6.37")
-- Looking for /home/xxx/vcpkg/installed/x64-linux/include/libpng/png.h
-- Looking for /home/xxx/vcpkg/installed/x64-linux/include/libpng/png.h - not found
-- OpenCV Python: during development append to PYTHONPATH: /home/xxx/vcpkg/buildtrees/opencv4/x64-linux-dbg/python_loader
-- Configuring incomplete, errors occurred!
See also "/home/xxx/vcpkg/buildtrees/opencv4/x64-linux-dbg/CMakeFiles/CMakeOutput.log".
See also "/home/xxx/vcpkg/buildtrees/opencv4/x64-linux-dbg/CMakeFiles/CMakeError.log".

PhoebeHui · 2021-05-20T08:41:35Z

Opencv:x64-linux failed on CI testing:

CMake Error at /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/FindFFMPEG.cmake:66 (find_library):
  Could not find FFMPEG_DEPENDENCY_-Wl,--no-undefined_RELEASE using the
  following names: -Wl,--no-undefined
Call Stack (most recent call first):
  /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/FindFFMPEG.cmake:146 (append_dependencies)
  /mnt/vcpkg-ci/installed/x64-linux/share/ffmpeg/vcpkg-cmake-wrapper.cmake:6 (_find_package)
  /agent/_work/1/s/scripts/buildsystems/vcpkg.cmake:815 (include)
  modules/videoio/cmake/detect_ffmpeg.cmake:6 (find_package)
  modules/videoio/cmake/init.cmake:3 (include)
  modules/videoio/cmake/init.cmake:30 (add_backend)
  cmake/OpenCVModule.cmake:298 (include)
  cmake/OpenCVModule.cmake:361 (_add_modules_1)
  modules/CMakeLists.txt:7 (ocv_glob_modules)

config-x64-linux-dbg-err[1].log
config-x64-linux-dbg-out[1].log

mcmtroffaes · 2021-05-20T08:44:16Z

Many thanks for the logs. Somehow I'm hitting some other errors and not this one, but I think I should be able to fix this error from the log message.

mcmtroffaes · 2021-05-20T09:24:41Z

I still cannot build opencv4 locally on linux with vcpkg but I've found a way to reproduce some link errors on linux using a test application. I think it's related to the link order when linking against a large number of libraries; I'll put this back to draft status for the time being until I'm sure it's all ironed out.

cenit · 2021-05-20T12:02:23Z

I still cannot build opencv4 locally on linux with vcpkg

[off topic]
what's your error?
i don't think that the png message above is an error. It's just too verbose, because header has a different name and is verified after that one
I am interested because it should build ;)
[/off topic]

mcmtroffaes · 2021-05-20T12:33:04Z

Absolutely, I plan have a separate detailed look at opencv4 on x64-linux when I have some time. It's a useful port for testing the vcpkg ecosystem, especially ffmpeg, so I really would like to be able to build it myself on x64-linux for local testing. I'll do some more investigation (probably next week) and will either post an issue or PR about it.

Anyway, my own test succeeds for me locally now (indeed library ordering was the problem), but azure opencv4 still isn't happy. 😕 Any chance to look again at the build logs @PhoebeHui ?

cenit · 2021-05-20T17:39:17Z

are you sure you are building the same opencv4 as CI?
please look at vcpkg/scripts/test_ports/vcpkg-ci-opencv to see the many features that are tested in CI

cenit · 2021-05-20T17:40:45Z

I did that special port to test as many features as possible because otherwise they were getting rotten extremely quickly due to other updates.
Even now some triplets are not building opencv4 anymore due to cascaded failures, which means that a dependency has been accepted in master with a broken status in that triplet... unfortunately!

…ld-config

mcmtroffaes · 2021-05-20T19:06:40Z

Yes, I tried precisely those features. However, I just found that after merging the latest master, opencv4[ffmpeg] locally builds successfully. 🤦 I've pushed the merge just now, hopefully that makes azure happy again. 🤞

PhoebeHui · 2021-05-21T07:24:31Z

@mcmtroffaes, the opencv4:x64-linux still failed with same error as before in CI testing. I will try to build it in my VM.

mcmtroffaes · 2021-05-21T07:34:38Z

Many thanks @PhoebeHui - it's odd that it gives the exact same error... I got exactly the same error compiling a test application against ffmpeg (with the same feature set as in vcpkg CI), and 4d68c69 solved that error for me. Then I ran into some linking errors which were fixed in 2049bd6.

Anyway, I'm getting closer to building opencv4 locally with the same feature set as in vcpkg CI on linux. There are many system dependencies on linux for some of opencv4's requirements, which is the main thing slowing me down (and which might also explain my earlier problems). But hopefully that will tease out the error eventually. I will report once it's done.

cenit · 2021-05-21T08:43:10Z

There are many system dependencies on linux for some of opencv4's requirements, which is the main thing slowing me down (and which might also explain my earlier problems). But hopefully that will tease out the error eventually. I will report once it's done.

you can see here https://github.com/microsoft/vcpkg/blob/master/scripts/azure-pipelines/linux/provision-image.sh how linux image is provisioned in azure for the CI system (all system dependencies that are installed)

PhoebeHui · 2021-05-25T07:16:41Z

Opencv4 cascade due to the dependency ports cascade or skip in CI testing. Some of them are feature dependent ports, so this should not affect local test, I think test the core and feature 'ffmpeg' is fine, @cenit, do you think so?

The dependency ports with x64-uwp in CI testing:
Cascade: ceres, ffmpeg, glog, jasper, ogre, qt5-base, tesseract, vtk
Skip: cudnn, gflags,hdf5, openexr, tbb,
Doesn't support uwp: halide, gdcm

mcmtroffaes · 2021-05-25T07:25:33Z

I found the culprit, it's openh264:

..\src\33cf500b7b-c619a10b3a.clean\meson.build:127:2: ERROR: Problem encountered: FIXME: Unhandled system windowsstore

cenit · 2021-05-25T07:31:49Z

:(
better to disable that feature.
If a port is marked as "cascade failure", not even its core features are tested! And in fact in the past I saw opencv getting in a very bad situation if left untested.
I know it's a little bit out of scope for this PR, but would it make sense to "unlock CI" with minimal fixes here so that we are sure the PR is working as expected in all triplets?

@mcmtroffaes I know it's misleading, but green badge sometimes just means "I had no opportunity to test so I won't tell you it's broken". With @BillyONeal I have an open issue on exactly this problem, because any cascade failure is not marked as a problem in the PR first triggering it

mcmtroffaes · 2021-05-25T07:51:53Z

I noticed that openh264 is marked as "supports": "!uwp", but vcpkg install openh264:x64_uwp will go ahead and install openh264 anyway. That's... unexpected.

Anyway, I've added a commit to remove the feature from ffmpeg[all] under uwp, that should remove it from the CI test on the uwp platforms as well. 🤞

PhoebeHui · 2021-05-25T07:58:49Z

The 'supports' field only works in CI testing.

mcmtroffaes · 2021-05-25T08:17:25Z

Ah, thanks for clarifying that!

PhoebeHui · 2021-05-25T10:07:52Z

avcpp with x64-uwp and arm-uwp failed with same error:

  D:\buildtrees\avcpp\src\7f1e65bea1-0345499867.clean\src\packet.cpp(97,5): error C4996: 'av_init_packet': was declared deprecated

failure logs for x64-uwp.zip

cenit · 2021-05-25T10:18:27Z

@mcmtroffaes happy that you found out that opencv was cascaded by your own modifications and that now is verified (and working) in CI

not so happy that unblocking ffmpeg on uwp you now uncovered other ports that were dormant but broken

cenit · 2021-05-25T10:20:27Z

is ffmpeg or one of those ports declarating /sdl or /we4996 ? If yes, it might just require removing those flags from the compiler flags

mcmtroffaes · 2021-05-25T12:35:23Z

Not that I know. It looks like avcpp might be the cause; see https://github.com/h4tr3d/avcpp/blob/master/src/avutils.h#L27 I'm not certain yet this is the cause but I'm working on a patch.

mcmtroffaes · 2021-05-25T13:34:10Z

Thanks so much @PhoebeHui for your patience and helping with the error logs: I appreciate your diligence. I've added a quick patch for the uwp issue, pleased to see it all green again on uwp.

PhoebeHui · 2021-05-26T05:42:39Z

@mcmtroffaes @cenit, opencv4 with arm-uwp and x64-uwp passed in CI testing, appreciate your effort very much!

@strega-nil-ms, could you please help further review?

…ld-config

mcmtroffaes · 2021-06-04T12:12:18Z

Merged master & bumped port version.

strega-nil-ms · 2021-06-09T22:25:06Z

LGTM thanks @mcmtroffaes :)

mcmtroffaes · 2021-06-10T06:49:40Z

Thanks so much for the merge!

mcmtroffaes added 3 commits May 18, 2021 08:27

[ffmpeg] use ffbuild/config.mak to get dependencies

6828d80

[ffmpeg] bump port version

af8ae0a

[ffmpeg] x-add-version

b05930b

PhoebeHui self-assigned this May 18, 2021

PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label May 18, 2021

PhoebeHui added the requires:discussion label May 20, 2021

mcmtroffaes added 2 commits May 20, 2021 09:47

[ffmpeg] fix dependency issue on x64-linux

4d68c69

[ffmpeg] x-add-version

552789c

mcmtroffaes marked this pull request as draft May 20, 2021 09:25

mcmtroffaes added 2 commits May 20, 2021 10:40

[ffmpeg] link order fix on linux

2049bd6

[ffmpeg] x-add-version

d0f77d8

Merge remote-tracking branch 'upstream/master' into feature/use-ffbui…

1a109f4

…ld-config

mcmtroffaes added 2 commits May 25, 2021 08:42

[ffmpeg] disable openh264 on uwp when building all

b473dd9

[ffmpeg] x-add-version

a5fee83

mcmtroffaes added 5 commits May 25, 2021 14:19

[avcpp] fix uwp build failure due to deprecations treated as errors

377c4c9

[avcpp] bump port version

7a3154d

[avcpp] x-add-version

a4cf8f2

[avcpp] smaller patch

9a8d0ad

[avcpp] x-add-version

d6b8901

mcmtroffaes added 2 commits June 4, 2021 13:08

Merge remote-tracking branch 'upstream/master' into feature/use-ffbui…

115ff68

…ld-config

[ffmpeg] x-add-version

0713274

PhoebeHui removed the requires:discussion label Jun 7, 2021

PhoebeHui approved these changes Jun 7, 2021

View reviewed changes

PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Jun 7, 2021

strega-nil-ms merged commit 1d6e1be into microsoft:master Jun 9, 2021

mcmtroffaes deleted the feature/use-ffbuild-config branch June 10, 2021 06:49

mcmtroffaes mentioned this pull request Jun 10, 2021

ffmpeg on Windows is currently exporting malformed .pc files. #12376

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ffmpeg] use pkgconfig to get system dependencies for cmake #17985

[ffmpeg] use pkgconfig to get system dependencies for cmake #17985

mcmtroffaes commented May 18, 2021

cenit commented May 19, 2021

cenit commented May 19, 2021

mcmtroffaes commented May 19, 2021

PhoebeHui commented May 20, 2021

azure-pipelines bot commented May 20, 2021

cenit commented May 20, 2021

mcmtroffaes commented May 20, 2021

PhoebeHui commented May 20, 2021

mcmtroffaes commented May 20, 2021

mcmtroffaes commented May 20, 2021

cenit commented May 20, 2021 •

edited

Loading

mcmtroffaes commented May 20, 2021

cenit commented May 20, 2021

cenit commented May 20, 2021

mcmtroffaes commented May 20, 2021

PhoebeHui commented May 21, 2021

mcmtroffaes commented May 21, 2021

cenit commented May 21, 2021

PhoebeHui commented May 25, 2021 •

edited

Loading

mcmtroffaes commented May 25, 2021

cenit commented May 25, 2021 •

edited

Loading

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 25, 2021 •

edited

Loading

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 25, 2021

cenit commented May 25, 2021

cenit commented May 25, 2021

mcmtroffaes commented May 25, 2021

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 26, 2021

mcmtroffaes commented Jun 4, 2021

strega-nil-ms commented Jun 9, 2021

mcmtroffaes commented Jun 10, 2021

[ffmpeg] use pkgconfig to get system dependencies for cmake #17985

[ffmpeg] use pkgconfig to get system dependencies for cmake #17985

Conversation

mcmtroffaes commented May 18, 2021

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

cenit commented May 19, 2021

cenit commented May 19, 2021

mcmtroffaes commented May 19, 2021

PhoebeHui commented May 20, 2021

azure-pipelines bot commented May 20, 2021

cenit commented May 20, 2021

mcmtroffaes commented May 20, 2021

PhoebeHui commented May 20, 2021

mcmtroffaes commented May 20, 2021

mcmtroffaes commented May 20, 2021

cenit commented May 20, 2021 • edited Loading

mcmtroffaes commented May 20, 2021

cenit commented May 20, 2021

cenit commented May 20, 2021

mcmtroffaes commented May 20, 2021

PhoebeHui commented May 21, 2021

mcmtroffaes commented May 21, 2021

cenit commented May 21, 2021

PhoebeHui commented May 25, 2021 • edited Loading

mcmtroffaes commented May 25, 2021

cenit commented May 25, 2021 • edited Loading

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 25, 2021 • edited Loading

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 25, 2021

cenit commented May 25, 2021

cenit commented May 25, 2021

mcmtroffaes commented May 25, 2021

mcmtroffaes commented May 25, 2021

PhoebeHui commented May 26, 2021

mcmtroffaes commented Jun 4, 2021

strega-nil-ms commented Jun 9, 2021

mcmtroffaes commented Jun 10, 2021

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

cenit commented May 20, 2021 •

edited

Loading

PhoebeHui commented May 25, 2021 •

edited

Loading

cenit commented May 25, 2021 •

edited

Loading

PhoebeHui commented May 25, 2021 •

edited

Loading