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

libvmaf: don't set _XOPEN_SOURCE #1390

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Conversation

alyssais
Copy link
Contributor

@alyssais alyssais commented Oct 10, 2024

This used to set _POSIX_C_SOURCE. I tried to remove it, because it caused some functions to be unavailable on BSD, but this broke Windows, so I settled for setting _XOPEN_SOURCE=600, which exposed the necessary functions on BSDs.

This no longer works. libvmaf now uses the non-standard strsep() function, and FreeBSD only exposes non-standard functions if no standards macros are defined — they're purely subtractive. So if setting _POSIX_C_SOURCE or _XOPEN_SOURCE isn't the right thing to do, what is? The build error for Windows is due to a missing M_PI macro. mingw provides this macro if any of the standards macros defined, or _BSD_SOURCE, _GNU_SOURCE, or _USE_MATH_DEFINES. Since we already set _GNU_SOURCE for Linux, using that for mingw as well is probably the best thing to do. That way, we don't have to set a macro thta causes FreeBSD to restrict which functions are available.

This used to set _POSIX_C_SOURCE.  I tried to remove it, because it
caused some functions to be unavailable on BSD, but this broke
Windows, so I settled for setting _XOPEN_SOURCE=600, which exposed the
necessary functions on BSDs.

This no longer works.  libvmaf now uses the non-standard strsep()
function, and FreeBSD only exposes non-standard functions if no
standards macros are defined — they're purely subtractive.  So if
setting _POSIX_C_SOURCE or _XOPEN_SOURCE isn't the right thing to do,
what is?  The build error for Windows is due to a missing M_PI
macro.  mingw provides this macro if any of the standards macros
defined, or _BSD_SOURCE, _GNU_SOURCE, or _USE_MATH_DEFINES.  Since we
already set _GNU_SOURCE for Linux, using that for mingw as well is
probably the best thing to do.  That way, we don't have to set a macro
thta causes FreeBSD to restrict which functions are available.
@nilfm99 nilfm99 requested a review from kylophone October 10, 2024 16:55
@kylophone
Copy link
Collaborator

kylophone commented Oct 10, 2024

We have a fallback for strsep() here, I'm curious if that is not working for you? I would like to improve compatibility with FreeBSD, so please let me know if you have any other suggestions.

@alyssais
Copy link
Contributor Author

Hmm, no, it didn't work. I can look into why, but I still think that this is a good change to make, because it doesn't make sense to explicitly opt in to non-standard functions on Linux (via _GNU_SOURCE), while at the same time explicitly opting out on BSD.

@kylophone
Copy link
Collaborator

Thanks, will merge your PR.

@kylophone kylophone closed this Oct 10, 2024
@kylophone kylophone reopened this Oct 10, 2024
@kylophone kylophone merged commit 7bd1663 into Netflix:master Oct 10, 2024
12 checks passed
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.

2 participants