Skip to content
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

[ffmpeg] Fix static linking on Windows, FindFFMPEG #7739

Merged
merged 1 commit into from
Aug 19, 2019

Conversation

mkroening
Copy link
Contributor

@mkroening mkroening commented Aug 17, 2019

This adds zlib as dependency to make it possible to find in FindFFMPEG.cmake.
Also FFmpeg likes to autodetect zlib for additional features on compilation.
This removes the check for stdint.h, as it is included since MSVC 2010 and does not always find it.
To enable static linking on windows, bcript had to be added:

avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptOpenAlgorithmProvider@16 referenced in function _av_get_random_seed
avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptCloseAlgorithmProvider@8 referenced in function _av_get_random_seed
avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptGenRandom@16 referenced in function _av_get_random_seed

This adds zlib as dependency to make it possible to find in FindFFMPEG.cmake.
Also FFmpeg likes to autodetect zlib for additional features on compilation.
This removes the check for stdint.h, as it is included since MSVC 2010 and does not always find it.
To enable static linking on windows, bcript had to be added:

avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptOpenAlgorithmProvider@16 referenced in function _av_get_random_seed
avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptCloseAlgorithmProvider@8 referenced in function _av_get_random_seed
avutil.lib(random_seed.o) : error LNK2019: unresolved external symbol _BCryptGenRandom@16 referenced in function _av_get_random_seed
@mkroening
Copy link
Contributor Author

First I tried to upgrade to 4.2 as well but that failed on Windows on ARM: old vcpkg-Linux-PR

This should now succeed and then be ready for review.

@mkroening mkroening changed the title [ffmpeg] Upgrade, static linking on Windows [ffmpeg] Fix static linking on Windows, FindFFMPEG Aug 17, 2019
@mkroening mkroening marked this pull request as ready for review August 17, 2019 21:08
Copy link
Contributor

@cbezault cbezault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine since the ffmpeg port already provides a cmake module but just as a note ports should be providing cmake config files not modules.

@cbezault cbezault merged commit 9a6f380 into microsoft:master Aug 19, 2019
@mkroening
Copy link
Contributor Author

Thanks for the quick reply!

So you mean a FFMPEGConfig.cmake.in (like in CMake docs - Package Configuration File using CMakePackageConfigHelpers) would be better than the FindFFMPEG.cmake?

Is there any documentation about this regarding vcpkg? What are the benefits? Would it be worth to switch? Is there a particular port worth looking at regarding best practices?

@cenit
Copy link
Contributor

cenit commented Aug 20, 2019

ports should be providing cmake config files not modules.

? But ffmpeg is not built with CMake... like many other ports! In this case providing a config file would be just hard coding values with no real benefits, while in this case we can just enhance the module step by step and maybe push for it upstream... (to ffmpeg or to cmake)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants