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

uvgRTP doesn't compile on CentOS7 #14

Closed
lhp-git opened this issue May 13, 2021 · 6 comments
Closed

uvgRTP doesn't compile on CentOS7 #14

lhp-git opened this issue May 13, 2021 · 6 comments

Comments

@lhp-git
Copy link
Contributor

lhp-git commented May 13, 2021

I am using uvgRTP for a streaming application developped on CentOS7.9. Upgrading the OS is not an option.
Until now, I used the 1.0.0 release. I had to modify the random.cc file (more on this later) but the build succedeed.
I now want to upgrade to the 2.0.0 release. Unfortunately, the Makefile generated by cmake3 hardcodes the C++ standard version to c++17. The gcc compiler of CentOS7.9 is 4.8.5, which supports only c++11.

Wouldn't it be possible to ensure uvgrtp compiles with c++11 ?

The only thing that is specific to c++17 is located in the include/crypto.hh file.

I suggest to replace :
#if __has_include(<cryptopp/aes.h>) &&
...
!defined(RTP_NO_CRYPTO)

#define RTP_CRYPTO

#include <cryptopp/aes.h>
...
#endif

with :
#ifndef RTP_NO_CRYPTO
#include <cryptopp/aes.h>
...
#endif

The second point that is not available on CentOS7 is the getrandom() function.
I suggest to use the SYS_getrandom system call if the getrandom() function isn't available (HAVE_GETRANDOM undefined).
I have attached the modified file random.cc
random.txt

@altonen
Copy link
Collaborator

altonen commented May 13, 2021

Hi,

unfortunately it was decided that for the sake of user experience, Crypto++ should be checked and enabled automatically. There was internal debate about this but this was the direction that was chosen for uvgRTP and thus, unfortunately for you, I don't see this change being merged to master. On the other hand, I'm no longer in a position to make any decisions regarding uvgRTP (that's now Joni's or Marko's job) so you may also try your luck and open a pull request for the Crypto++ detection update and see what happens.

The updates regarding crypto.cc look good to me and if you want them to be merged to master, please open a pull request.

@lhp-git
Copy link
Contributor Author

lhp-git commented May 13, 2021

Thank you for your quick answer.

I will guard my modifications to the crypto.hh file with a check to the GCC version (< 5), hence the "official" Crypto++ remains unchanged if GCC version is > 5. I will try my luck with a pull request for this, and another for the getrandom() function.

What about the hardcoding of the standard c++17 in the CMakeLists.txt ? This requirement isn't mentioned in the BUILDING.md, and is new with the 2.0.0 version. This is disapointing for users of the 1.0.0 who wants to upgrade.

@altonen
Copy link
Collaborator

altonen commented May 13, 2021

Checking against GCC's version sounds reasonable to me. As you mentioned, uvgRTP only uses the __has_include from C++17 so you could modify the CMake file to detect if a C++17-compatible compiler is present and enable -std=c++17 if so. And if not, it could default to C++11 and use your GCC version check code to enable/disable crypto (if merged to master).

@lhp-git
Copy link
Contributor Author

lhp-git commented May 13, 2021

I will prepare a pull request, but for that I need to learn how to do it. I think there are tutorials for that.

@lhp-git
Copy link
Contributor Author

lhp-git commented May 26, 2021

PR "build: Support for CentOS7 #18" submitted

@lhp-git
Copy link
Contributor Author

lhp-git commented May 28, 2021

The PR #18 has been merged. I can close this issue now.

@lhp-git lhp-git closed this as completed May 28, 2021
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

No branches or pull requests

2 participants