AVIF loader disabled by default?

[sezero]

SDL2 branch autotools has it enabled by default, but both SDL2 and SDL3 cmake has it disabled. Reason? (I admit, SDL2 autotools has even jxl loader enabled by default..)

[madebr]

Autotools is lenient when a dependency is missing (it disables the feature). CMake on the contrary is more strict and fails the build.

Because of this, defaults were selected.

Perhaps for SDL3 CMake should become more lenient and get a strict option. Then, we can enable pretty much everything.

[sezero]

Autotools is lenient when a dependency is missing (it disables the feature). CMake on the contrary is more strict and fails the build.

Because of this, defaults were selected.

OK, figured as much, but it's disabled in CI too: why? ubuntu-latest (22.04) has libavif, for e.g.

Perhaps for SDL3 CMake should become more lenient and get a strict option. Then, we can enable pretty much everything.

Would be better, IMO. (SDL_mixer is the same way too, e.g. cmake fails if I don't deliberately disable fluidsynth..)

[slouken]

Given that we print out a summary of the enabled subsystems at the end it seems reasonable for CMake to be more lenient and pick up dependencies that are available. I don't feel super strongly about this though, distribution maintainers and people with strict codec requirements are going to do what is necessary to enable the pieces they need.

[smcv]

There are three things that could make sense for an optional feature:

• enable feature, require its dependencies, fail to build if they are missing (Meson calls this enabled)
• disable feature even if its dependencies are present (Meson disabled)
• automatically enable or disable feature depending on whether its dependencies are found (Meson auto, often called an "automagic dependency")

Obviously this can't map completely neatly to a boolean like CMake's conventional ON/OFF.

OS distributors dislike automagic dependencies if there's no way to turn them off, because it makes the same package source code produce different binaries when compiled on a minimal or non-minimal system. From an OS distributor point of view, these dependencies need at least an off-switch: we want to be able to say -DSDL3IMAGE_AVIF=OFF (or something similar) and have the feature be reliably disabled.

It is also nice if there is a way we can make a feature mandatory (so we can say something like -DSDL3IMAGE_PNG=REQUIRED and have it be a hard failure if the required dependencies aren't found), but that's less important than being able to force-disable it.

[eli-schwartz]

The classic autotools style would be to document acceptable values as enabled/disabled/check, and if the value isn't "disabled" then try to find the feature dependencies, and if it both isn't found and the value wasn't "check" but rather was "enabled", then emit an error message. You don't need booleans here, especially since booleans are just strings with particularly common values.

I don't feel super strongly about this though, distribution maintainers and people with strict codec requirements are going to do what is necessary to enable the pieces they need.

It's particularly problematic for source distributions such as Gentoo, where you cannot enforce strict requirements by building without the unwanted codecs installed -- since it may be installed for some other package where it is wanted.

It's a lot less problematic for binary distros where most software is anyways built on a buildbot from a minimal OS image that is discarded after the build.

[sezero]

Going back to actual topic, i.e. why avif support is disabled by default or: why is only jxl is enabled in CI but avif is not?

Current distros have been including libavif already, have they not been?

Is it because some decoder backend other than dav1d (e.g. aom) is needed on some platforms or something??? (if that is ever the case..)

[madebr]

I think the current cmake script is the limiting factor: it does not allow not having available an enabled option. I'm thinking about adding a SDL3IMAGE_STRICT option to select the behavior.

[sezero]

I think the current cmake script is the limiting factor: it does not allow not having available an enabled option. I'm thinking about adding a SDL3IMAGE_STRICT option to select the behavior.

What are you saying?? PNG is enabled by default already, so are many others.

[madebr]

Currently, when you configure with SDL3IMAGE_AVIF=ON, and avif is not found, cmake fails. My proposal is to add an option to simply disable avif when strict mode is disabled, and fail when strict mode is enabled. (looks like I proposed this already in https://github.com/libsdl-org/SDL_image/issues/343#issuecomment-1454738995)

This allows to enable support by default for file format libraries that are not available on all platforms.

It's also a convenience for users that are less versed in using CMake. Package managers can keep using strict mode.

[sezero]

Is the following OK then?

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 97ca59e..cbd867d 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -18,8 +18,8 @@ jobs:
         - { name: Windows (MSVC),    os: windows-latest, shell: sh,  vendored: true, cmake: '-GNinja', msvc: 1, shared: 1, static: 0 }
         - { name: Windows (mingw64), os: windows-latest, shell: 'msys2 {0}', vendored: false, msystem: mingw64, msys-env: mingw-w64-x86_64, shared: 1, static: 0,
             cmake: '-DSDL3IMAGE_BACKEND_STB=OFF -DSDL3IMAGE_BACKEND_WIC=OFF  -DSDL3IMAGE_AVIF=ON -G "Ninja Multi-Config"' }
-        - { name: Linux,             os: ubuntu-20.04,   shell: sh,  vendored: false, cmake: '-GNinja', shared: 1, static: 0, nojxl: true }
-        - { name: 'Linux (static)',  os: ubuntu-20.04,   shell: sh,  vendored: true, cmake: '-DBUILD_SHARED_LIBS=OFF -GNinja', shared: 0, static: 1 }
+        - { name: Linux,             os: ubuntu-latest,   shell: sh,  vendored: false, cmake: '-GNinja', shared: 1, static: 0, nojxl: true }
+        - { name: 'Linux (static)',  os: ubuntu-latest,   shell: sh,  vendored: true, cmake: '-DBUILD_SHARED_LIBS=OFF -GNinja', shared: 0, static: 1 }
         - { name: Macos,             os: macos-latest,   shell: sh,  vendored: false, cmake: '-GNinja', shared: 1, static: 0 }

     steps:
@@ -75,6 +75,7 @@ jobs:
       run: |
         sudo apt-get update
         sudo apt-get -y install \
+          libavif-dev \
           libjpeg-dev \
           libpng-dev \
           libtiff-dev \
@@ -130,7 +131,7 @@ jobs:
         set -eu

         # Expect to support all formats except AVIF in these builds
-        export SDL_IMAGE_TEST_REQUIRE_LOAD_AVIF=0
+        export SDL_IMAGE_TEST_REQUIRE_LOAD_AVIF=1
         export SDL_IMAGE_TEST_REQUIRE_LOAD_BMP=1
         export SDL_IMAGE_TEST_REQUIRE_LOAD_CUR=1
         export SDL_IMAGE_TEST_REQUIRE_LOAD_GIF=1

[madebr]

That will add ci for avif, and looks good. I was talking about enabling avif by default, while also preserving ubuntu 20.04 support by disabling avf there (because libavif cannot be found).

[sezero]

OK, will push the patches from my fork once they pass CI runs, and then will close this.

[madebr]

You will need to configure cmake with -DSDL3IMAGE_AVIF=ON. It's off by default: https://github.com/libsdl-org/SDL_image/blob/abe699be0f067932f2a95968bb0996bf4dacbf8f/CMakeLists.txt#L78

[sezero]

Pushed 9766476f64e4a1d74d511828bfb866f636a57e2d to SDL3 .. and e4979a232868024d485c3425afc783c7950caa57 to SDL2.

P.S.: This info message from cmake (CI Linux runner) looks ridiculous: -- Dynamic libavif: $<TARGET_FILE_NAME:avif>

P.S./2: We should bump libavif to 0.11.1 and dav1d to 1.2.1. (and also support the vendored libavif option.)

Closing this ticket.

[madebr]

P.S.: This info message from cmake (CI Linux runner) looks ridiculous: -- Dynamic libavif: $<TARGET_FILE_NAME:avif>

Kinda agree. It's only emitted when using vendored libraries. The reason is that the filename of the final shared library is calculated by cmake at a later stage (after configuration, during the generation stage). It's a consequence of using the cmake scripts of the external libraries (I want to avoid forking their cmake build scripts). There are heuristics to calculate the filename, but they remain heuristics.

[sezero]

P.S.: This info message from cmake (CI Linux runner) looks ridiculous: -- Dynamic libavif: $<TARGET_FILE_NAME:avif>

Kinda agree. It's only emitted when using vendored libraries.

No. See https://github.com/libsdl-org/SDL_image/actions/runs/5884310254/job/15958793475

-- SDL3_image: Using system libavif
-- Dynamic libavif: $<TARGET_FILE_NAME:avif>
-- SDL3_image: Using system libtiff
-- Found TIFF: /usr/lib/x86_64-linux-gnu/libtiff.so (found version "4.3.0") 
-- Dynamic libtiff: libtiff.so.5
-- SDL3_image: Using system libwebp
-- Found webp: /usr/lib/x86_64-linux-gnu/libwebp.so  
-- Dynamic libwebpdemux: libwebpdemux.so.2
-- Dynamic libwebp: libwebp.so.7

[madebr]

P.S.: This info message from cmake (CI Linux runner) looks ridiculous: -- Dynamic libavif: $<TARGET_FILE_NAME:avif>

Kinda agree. It's only emitted when using vendored libraries.

No. See https://github.com/libsdl-org/SDL_image/actions/runs/5884310254/job/15958793475

I forget about the configuration where a *-config.cmake is used to locate a dependency (instead of FindXXX.cmake modules). The difference is the first sets IMPORTED_LOCATION_<config>, where the latter sets IMPORTED_LOCATION. The script would need to start using heuristics to find out what config to use.

It can of course echo something, and internally keep using generator expressions. But that's extra work for not a lot of gain.

Alternatively, we can introduce a sdl3_image-summary target that simply echoes a summary to stdout. It will print sensible things guaranteed.