-
Notifications
You must be signed in to change notification settings - Fork 181
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
upgrade vendored libavif to 1.0.1 release (and dav1d to 1.2.1 release) #383
Comments
Btw, this also requires changes to |
@kmilos: libavif cmake'ry outputs |
No, I mean e.g.
So you either have to This has nothing to do w/ DLL names. |
Ah, you are right. I had tested on the SDL2 branch using autotools, but not with cmake: @slouken, @icculus: What should be our policy here? There are distros out there with libavif-0.10.x, libavif-0.11.x (maybe even libavif-0.9.x). Check 1.0.0 first, and if it failed then check 0.9.2??
Right. That one is another problem, and it actually is a problem but at libavif side. |
Yes, autotools go through pkg-conf files where the version check is a simple >= so it works.
More of a problem on CMake side, don't know why they turned that off (or rather never had it on) by default... Anyway, just build libavif (and any other shared lib) w/ |
Thanks. P.S.: Did you rebuild your SDL2_image against it? |
Yes, we just temporarily changed the version check as in the PKGBUILD recipe using |
Maybe something like the following? (applies to both SDL2 and SDL3. Kde does something similar) diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9d3d4fe..8a7490f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -371 +371,7 @@ if(SDL2IMAGE_AVIF)
- find_package(libavif ${LIBAVIF_MINIMUM_VERSION} REQUIRED)
+ find_package(libavif 1.0 QUIET)
+ if(NOT libavif_FOUND)
+ find_package(libavif ${LIBAVIF_MINIMUM_VERSION} QUIET)
+ endif()
+ if(NOT libavif_FOUND)
+ message(FATAL_ERROR "libavif NOT found")
+ endif() CC: @madebr |
I don't think it needs to be marked Otherwise, I think this is fine, but I always defer to @madebr's opinion on CMake issues. |
Well, cmake output is too verbose in not-found cases of find_package:
Maybe something like this? diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9d3d4fe..e13cb4b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -371 +371,11 @@ if(SDL2IMAGE_AVIF)
- find_package(libavif ${LIBAVIF_MINIMUM_VERSION} REQUIRED)
+ find_package(libavif 1.0 QUIET)
+ if(NOT libavif_FOUND)
+ message(STATUS "libavif-1.0 or compatible not found")
+ find_package(libavif ${LIBAVIF_MINIMUM_VERSION} QUIET)
+ endif()
+ if(libavif_FOUND)
+ message(STATUS "libavif-${libavif_VERSION} found")
+ else()
+ message(STATUS "libavif-${LIBAVIF_MINIMUM_VERSION} or compatible not found")
+ message(FATAL_ERROR "libavif NOT found")
+ endif() |
Too bad version ranges don't work. sezero's suggestion looks good. |
May I leave this cmake'ry to you? P.S.: Compared avif.h across versions:
To avoid any confusion, I wanted to bump our minimum required version from 0.9.1 |
No problem.
The oldest Ubuntu version in the list is 22.04 LTS with 0.9.3. |
No, it looks like I misread that: ubuntu-22.04 does seem to have 0.9.3 Therefore, I suggest that we bump our minimum required libavif to 0.9.3 diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9d3d4fe..ef5062f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -97 +96,0 @@
-set(LIBAVIF_MINIMUM_VERSION "0.9.1")
@@ -371 +370,19 @@
- find_package(libavif ${LIBAVIF_MINIMUM_VERSION} REQUIRED)
+ find_package(libavif 1.0 QUIET)
+ if(NOT libavif_FOUND)
+ message(STATUS "libavif-1.0 or compatible not found")
+ find_package(libavif 0.11 QUIET)
+ endif()
+ if(NOT libavif_FOUND)
+ message(STATUS "libavif-0.11 or compatible not found")
+ find_package(libavif 0.10 QUIET)
+ endif()
+ if(NOT libavif_FOUND)
+ message(STATUS "libavif-0.10 or compatible not found")
+ find_package(libavif 0.9.3 QUIET)
+ endif()
+ if(libavif_FOUND)
+ message(STATUS "libavif-${libavif_VERSION} found")
+ else()
+ message(STATUS "libavif-${LIBAVIF_MINIMUM_VERSION} or compatible not found")
+ message(FATAL_ERROR "libavif NOT found")
+ endif()
diff --git a/configure.ac b/configure.ac
index 2080596..b0a7bd2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,17 +306,22 @@
load_avif=0
if test x$enable_avif = xyes; then
- PKG_CHECK_MODULES([LIBAVIF], [libavif], [dnl
+ PKG_CHECK_MODULES([LIBAVIF], [libavif >= 0.9.3], [dnl
have_avif_hdr=yes
have_avif_lib=yes
have_avif_pc=yes
], [dnl
- AC_CHECK_HEADER([avif/avif.h], [
+ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
+ #include <avif/avif.h>
+ ]],, [[
+ #if (AVIF_VERSION < 90300)
+ #error libavif too old.
+ #endif
+ return !!avifVersion();
+ ]])], [
have_avif_hdr=yes
- LIBAVIF_CFLAGS=""
- ])
- AC_CHECK_LIB([avif], [avifVersion], [
have_avif_lib=yes
+ LIBAVIF_CFLAGS=""
LIBAVIF_LIBS="-lavif"
])
]) |
Looks good, apart from we only need to check 0.9.3 and 1.0.0. The |
OK, pushed the following: Leaving any polishing + extra work to you. (The ticket is still open because the actual title subject hasn't happened yet.) |
Off-topic to this ticket (at least some of it), but noticed some weirdness
|
This is already resolved, but wow, you're right about that! |
@sezero EDIT: pushed to SDL2 |
With CMake, libavif on SDL2 is only tested on mingw64: it needs an explicit |
Thanks. Other satellite libs (SDL_mixer especially) may need similar treatment.
CI looks fine so far.
Does |
I will do a sync this weekend.
Yes, see 0c34e8c |
OK
Missed that, sorry. P.S.: Autotools P.S./2: CI logs in SDL2 cmake runs still seem to output weird. (see above) |
This is output of |
I see this with SDL2_image's autotools on msys2:
https://packages.msys2.org/package/mingw-w64-x86_64-libjxl |
Sigh... We do |
@slouken, @icculus, @madebr: Is the following correct? diff --git a/configure.ac b/configure.ac
index 021e2a7..0cc7bcc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -339,10 +339,16 @@ if test x$enable_avif = xyes; then
case "$host" in
*-*-darwin*)
- avif_lib=[`find_lib libavif.dylib`]
+ avif_lib=[`find_lib "libavif.[0-9]*.dylib"`]
+ if test x$avif_lib = x; then
+ avif_lib=[`find_lib libavif.dylib`]
+ fi
;;
*-*-cygwin* | *-*-mingw*)
- avif_lib=[`find_lib "libavif*.dll"`]
+ avif_lib=[`find_lib "libavif-[0-9]*.dll"`]
+ if test x$avif_lib = x; then
+ avif_lib=[`find_lib libavif.dll`]
+ fi
;;
*)
avif_lib=[`find_lib "libavif[0-9]*.so.*"`] |
Is the vendored libavif compatible with the vendored libdav1d?
|
I recently built libavif-1.0.1 against dav1d-1.2.1 locally without any issues at all. It may be that the current vendored dav1d (1.0.0) is old and incompatible? |
It was a bug in my |
I think this issue is fixed by #384 |
(1) 5f4a94a and c88caa5 need porting to SDL3_mixer SDL2_mixer. (Hopefully not missed anything among the commits.) (2) SDL2, autotools: c88caa5 couldn't be ported to SDL2 autotools and the libjxl.dll issue was band-aided. Therefore, I suggest the following. NOTE: Even though the issue is general, I only touched libavif and libjxl discoveries. Please review. (CC: @slouken). diff --git a/configure.ac b/configure.ac
index 72b3eee..2f9b49c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -339,10 +339,16 @@ if test x$enable_avif = xyes; then
case "$host" in
*-*-darwin*)
- avif_lib=[`find_lib libavif.dylib`]
+ avif_lib=[`find_lib "libavif.[0-9]*.dylib"`]
+ if test x$avif_lib = x; then
+ avif_lib=[`find_lib "libavif*.dylib"`]
+ fi
;;
*-*-cygwin* | *-*-mingw*)
- avif_lib=[`find_lib "libavif*.dll"`]
+ avif_lib=[`find_lib "libavif-[0-9]*.dll"`]
+ if test x$avif_lib = x; then
+ avif_lib=[`find_lib "libavif*.dll"`]
+ fi
;;
*)
avif_lib=[`find_lib "libavif[0-9]*.so.*"`]
@@ -435,10 +441,16 @@ if test x$enable_jxl = xyes; then
case "$host" in
*-*-darwin*)
- jxl_lib=[`find_lib libjxl.dylib`]
+ jxl_lib=[`find_lib libjxl.[0-9]*.dylib`]
+ if test x$jxl_lib = x; then
+ jxl_lib=[`find_lib libjxl.dylib`]
+ fi
;;
*-*-cygwin* | *-*-mingw*)
- jxl_lib=[`find_lib "libjxl.dll"`]
+ jxl_lib=[`find_lib "libjxl-[0-9]*.dll"`]
+ if test x$jxl_lib = x; then
+ jxl_lib=[`find_lib "libjxl.dll"`]
+ fi
;;
*)
jxl_lib=[`find_lib "libjxl[0-9]*.so.*"`] |
For both SDL2 and SDL3.
For libavif, the new source going from 0.10.1 to 1.0.1 seems to be exif.c
For dav1d, things may be more complex.
The text was updated successfully, but these errors were encountered: