Netflix / vmaf

Perceptual video quality assessment based on multi-method fusion.
Other
4.65k stars 755 forks source link

libvmaf: don't set _XOPEN_SOURCE #1390

Closed alyssais closed 1 month ago

alyssais commented 1 month ago

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.

kylophone commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Thanks, will merge your PR.