ffmpeg: Add missing dependency on OpenSSL for avutil #22990
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🤖 Beep Boop! This pull request is making changes to 'recipes/ffmpeg//'. 👋 @MartinDelille you might be interested. 😉 |
This was referenced Mar 5, 2024
jwillikers
force-pushed
the
ffmpeg-fix-avutil-ssl
branch
2 times, most recently
from
56e46cf to
de82ca8
Compare
jwillikers
force-pushed
the
ffmpeg-fix-avutil-ssl
branch
from
de82ca8 to
67d9618
Compare
conan-center-bot
added
Failed
Missing dependencies
This comment has been minimized.
This comment has been minimized.
jwillikers
force-pushed
the
ffmpeg-fix-avutil-ssl
branch
from
bd11ef8 to
f4179cd
Compare
jwillikers
force-pushed
the
ffmpeg-fix-avutil-ssl
branch
from
f4179cd to
0365d23
Compare
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 0365d23ffmpeg/6.0.1@#368542a392d718d2720685a3dccd3bd7ffmpeg/6.1@#3566c06a1d67fc107ced834742f6c1acffmpeg/5.1.3@#190dd8bd0d302732786abc773c002cbbffmpeg/5.0.3@#b9ae8ce01691c84ca2e8b349ec1fdf58ffmpeg/6.0@#f104c8cae411d21c3986f00107798fc0ffmpeg/5.0@#abf1e507aa01e9498ef2014c5ee8156fffmpeg/4.2.1@#42c4f167b6ee02b5aaf66bb01a882f69ffmpeg/5.1@#c64bf5bcf22284422ec9f30ac416aca2ffmpeg/4.4.4@#4cf05c7f3aa77e8174d8e0217cec1f65ffmpeg/4.4.3@#36f429d3d04bb99f77c8a317ac04af59ffmpeg/4.4@#eb190a523fae05d741bc7b8e204f9546ffmpeg/4.3.2@#51ab6a4a9e2f5a7d85a1e512afec445e |
|
Just started an internal build for libva/2.20.0 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit d22a0e2ffmpeg/6.0.1@#368542a392d718d2720685a3dccd3bd7 |
|
Well, I'm really not sure how I would have cause this error that is showing up on Windows: The link line is indeed super long, but my changes shouldn't have any effect on Windows. |
ghost
mentioned this pull request
3 tasks
…ffmpeg-fix-avutil-ssl
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
This comment has been minimized.
This comment has been minimized.
|
I rebased this PR over the master |
uilianries
reviewed
Aug 7, 2024
| avutil.requires.append("vdpau::vdpau") | ||
| avutil.requires.append("libvdpau::libvdpau") | ||
|
|
||
| if self.options.with_ssl == "openssl": |
There was a problem hiding this comment.
I can confirm this fix working, I tested locally on Linux. I would ask to split this PR. Those extra dependencies will require extra steps to be validated, and replacing packages is even more fragile as ffmpeg is a critical package.
There was a problem hiding this comment.
I've reverted those changes to use the libva and libvdpau.
uilianries
reviewed
Aug 8, 2024
recipes/ffmpeg/all/conanfile.py
Outdated
| self.requires("xorg/system") | ||
| if self.options.get_safe("with_pulse"): | ||
| self.requires("pulseaudio/14.2") | ||
| if self.options.get_safe("with_vaapi"): | ||
| self.requires("vaapi/system") | ||
| self.requires("libva/2.20.0") |
There was a problem hiding this comment.
libva looks a good replacement. 👍
I just compared both packages and they look equivalent. The only possible problem that may cause is breakage for few users, as it is closer to hardware and the package may be incompatible. Of course, rebuilding from source should be enough.
About libva distributed from AUR (Arch Linux):
- https://archlinux.org/packages/extra/x86_64/libva/
- https://gitlab.archlinux.org/archlinux/packaging/packages/libva/-/blob/main/PKGBUILD?ref_type=heads
- https://archlinux.org/packages/extra/x86_64/libva/files/
About libva distributed from APT (Ubuntu):
- https://packages.ubuntu.com/focal/amd64/libva-dev/filelist
- https://git.launchpad.net/ubuntu/+source/libva/tree/debian/control?h=applied/ubuntu/devel
Both packages look be very similar with the Conan libva package, in terms of headers and libraries.
uilianries
reviewed
Aug 8, 2024
recipes/ffmpeg/all/conanfile.py
Outdated
| if self.options.get_safe("with_vdpau"): | ||
| self.requires("vdpau/system") | ||
| self.requires("libvdpau/1.5") |
There was a problem hiding this comment.
Same for libvdpau, close to hardware, may cause incompatibility, but still looks following the same implementation as from distros:
AUR:
- https://archlinux.org/packages/extra/x86_64/libvdpau/
- https://archlinux.org/packages/extra/x86_64/libvdpau/files/
- https://gitlab.archlinux.org/archlinux/packaging/packages/libvdpau/-/blob/main/PKGBUILD?ref_type=heads
APT:
uilianries
reviewed
Aug 8, 2024
| avutil.requires.extend(["vaapi::vaapi", "xorg::x11"]) | ||
| avutil.requires.append("vaapi::vaapi") | ||
| if self.options.get_safe("with_xcb"): | ||
| avutil.requires.append("xorg::x11") |
There was a problem hiding this comment.
Are you sure about this change? What evidence could be used? At line 317 there is no dependency between vaapi and xcb
There was a problem hiding this comment.
It's been a while now, so I don't quite remember the exact details, but I remember this being an important dependency when enabling libva. It should be able to track down the dependency in the code for avutil.
There was a problem hiding this comment.
I mean, they were independent before. In case not having evidence, please, revert it. The FFMpeg is really critical, so you can open a new PR in the future, in case finding the reason with proper details.
There was a problem hiding this comment.
I remember now. This is necessary because otherwise the package breaks when with_vaapi=True and with_xcb=False.
conan create . --build missing --settings:host 'build_type=Release' --options:host 'ffmpeg/*:with_xcb=False' --options:host 'ffmpeg/*:with_vaapi=True' --version 7.0.1
This results in the following error:
ERROR: ffmpeg/7.0.1: required component package 'xorg::' not in dependencies
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The new CI result shows an error when libdrm is enabled: https://c3i.jfrog.io/c3i/misc-v2/logs/pr/22990/14-linux-gcc/ffmpeg/5.0.3//9e4a072e6a84bebd211fbe2e2d40a0d437ccab5d-build.txt This occurs because the Conan package The required define I would recommend disabling As alternative, we could use BTW, I can build the current commit cf8cc9a without error in Ubuntu 22.04 + Conan 2.5.0 + ffmpeg/5.0.3:with_libdrm=True: |
Conan v1 pipeline ✔️All green in build 13 (
Conan v2 pipeline ✔️
All green in build 15 (
|
Hooks produced the following warnings for commit c8d3cc7ffmpeg/6.1.1@#260a95e53eb63cce98e42ff8ece2083cffmpeg/6.1@#3f0bec8d31f809ee23fcd496024768feffmpeg/7.0.1@#549af51c795b459e0bbe1c919e0cf100ffmpeg/6.0.1@#13de18f0c13f833d00d409f76bef4753ffmpeg/5.0.3@#0a10f5569cf38731812b71fa04f7b424ffmpeg/5.1.3@#7521e1cc46a5c2df0e8ffe663f807314ffmpeg/4.4.4@#7d252ea17f74ceb50ac858b5309f6c1b |
uilianries
approved these changes
Aug 14, 2024
There was a problem hiding this comment.
LGTM.
- with_libdrm requires libva >=2.1 due new defines: ffmpeg: Add missing dependency on OpenSSL for avutil #22990 (comment)
- Conan has libva packaged already, but it depends on the OS and may break consumers: ffmpeg: Add missing dependency on OpenSSL for avutil #22990 (comment)
- with_vaapi is not directly connected to with_xcb, it was libva_x11 dependendy actually. Addressed by the PR [ffmpeg] Add option for xlib #24879
- The OpenSSL is indeed missing for avutil, tested locally. The PR pipewire: Add recipe #22991 express the bug.
@jwillikers Thank you for your PR. Please, for future PRs related to ffmpeg, try not mixing things at same PR, try only one change per PR instead, because ffmpeg is not only complex but largely used by the community and any bug may affect several users.
AbrilRBS
approved these changes
Aug 14, 2024
OMGtechy
pushed a commit
to OMGtechy/conan-center-index
that referenced
this pull request
Dec 31, 2024
* ffmpeg: Add missing dependency on OpenSSL for avutil * Use libva and libvdpau dependencies * Add with_libdrm option * Fix vulkan in cpp_info Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix options Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix options Signed-off-by: Uilian Ries <uilianries@gmail.com> * Remove changes to use non-system libva and libvdpau packages * Default libdrm to False --------- Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: Rubén Rincón Blanco <git@rinconblanco.es> Co-authored-by: Uilian Ries <uilianries@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes missing symbols errors at link time when using the component dependency
ffmpeg::avutil.Also, use
libvaandlibvdpauinstead of the system packages since they are now available as proper Conan package. This prevents conflicts in consumers.I guess I had to get one more thing in. I've added a
with_libdrmoption as well to includelibdrm.