Bump vendored libraries

[madebr]

• libwebp: 1.0.3 -> 1.3.2
• zlib: 1.2.12 -> 1.3
• libavif: 0.10.1 -> 1.0.1
• dav1d: 1.0.0 -> 1.2.1

And add support for building a vendored libavif library with cmake.

[sezero]

Haven't read any details, but

libwebp: 1.0.3 -> 1.3.1

I hope libsharpyuv in new libwebp won't be a problem, i.e. libwebp and libwebpdemux libs hopefully not rely on it?

EDIT: .. and the webp bump is not that necessary IMO, but that's just me.

[sezero]

Should this not be cover under the if(SDL3IMAGE_AVIF_VENDORED) block?

[madebr]

You're right it would be equivalent to put it under the if(SDL3IMAGE_AVIF_VENDORED), but I didn't do it for cosmatic reasons and consistency. The script also does this for zib.

[sezero]

The script also does this for zib.

Then zlib handling should be changed too, IMO

[sezero]

dav1d: 1.0.0 -> 1.2.1

Looked very briefly at the new v1.2.1-SDL branch: You added cmake support to it, but I couldn't see how you handle nasm requirement?

EDIT: Is Android.mk in new branch tested?

[sezero]

libwebp: 1.0.3 -> 1.3.1

zlib: 1.2.12 -> 1.3

libavif: 0.10.1 -> 1.0.1

dav1d: 1.0.0 -> 1.2.1

The .gitmodules file should reflect the new branches

[sezero]

This PR is also a good opportunity for us to audit symbol visibility from 3rd party libs. I.e.: If we statically link to them, their symbols should not be exported at all by our library.

[sezero]

Great branch name btw :)

[madebr]

Haven't read any details, but

libwebp: 1.0.3 -> 1.3.1

I hope libsharpyuv in new libwebp won't be a problem, i.e. libwebp and libwebpdemux libs hopefully not rely on it?

Looks like libwebp.dll depends on it, but libwebpdemux.dll don't:

>dumpbin /imports libwebp.dll | findstr dll

Dump of file libwebp.dll
    libsharpyuv.dll
    KERNEL32.dll
    VCRUNTIME140D.dll
    ucrtbased.dll
>dumpbin /imports libwebpdemux.dll | findstr dll
Dump of file libwebpdemux.dll
    libwebp.dll
    VCRUNTIME140D.dll
    ucrtbased.dll
    KERNEL32.dll

I don't see a configure option to disable it in the cmake script, and neither in the configure.ac script.

EDIT: .. and the webp bump is not that necessary IMO, but that's just me.

Let's treat this as a yearly update.

The .gitmodules file should reflect the new branches

Fixed. Thanks!

dav1d: 1.0.0 -> 1.2.1

Looked very briefly at the new v1.2.1-SDL branch: You added cmake support to it, but I couldn't see how you handle nasm requirement?

There is no asm support. ENABLE_ASM is always disabled. This was also done by the android project.
I think this is the only core feature/test I didn't port from the meson project.

EDIT: Is Android.mk in new branch tested

No. With the libsdl-org/setup-sdl action, we can also test non-standard configurations.

[sezero]

Haven't read any details, but

libwebp: 1.0.3 -> 1.3.1

I hope libsharpyuv in new libwebp won't be a problem, i.e. libwebp and libwebpdemux libs hopefully not rely on it?

Looks like libwebp.dll depends on it,

That's bad, actually. If we are to build vendored webp as shared lib, we should
somehow change sharpyuv to build only as static: I don't think that we wanta a
3rd *.so dependency.

EDIT: .. and the webp bump is not that necessary IMO, but that's just me.

Let's treat this as a yearly update.

I'm not against it. But here you have additional issues to solve and the
new version nothing new to offer specifically to us.

dav1d: 1.0.0 -> 1.2.1

Looked very briefly at the new v1.2.1-SDL branch: You added cmake support to it, but I couldn't see how you handle nasm requirement?

There is no asm support. ENABLE_ASM is always disabled. This was also done by the android project. I think this is the only core feature/test I didn't port from the meson project.

Well, we won't expect much from dav1d side then with asm disabled..

EDIT: Is Android.mk in new branch tested

No. With the libsdl-org/setup-sdl action, we can also test non-standard configurations.

Its asm file lists / configs most possibly need updating, but after seeing the no-asm above, well...

[sezero]

The .gitmodules file should reflect the new branches

Fixed. Thanks!

You seem to have reverted it..

EDIT: Seems fixed now.

EDIT/2: Just deleted the obsolete v1.2.1 branch of dav1d in favor of new 1.2.1

[sezero]

Ragarding dav1d and cmake portage issues and no asm: If it's a big hassle, there is the option to use libaom ?

[madebr]

Ragarding dav1d and cmake portage issues and no asm: If it's a big hassle, there is the option to use libaom ?

There are 4 options:

• AOM codec
• dav1d codec
• libgav1 codec
• AVM (AV2) codec

[sezero]

Ragarding dav1d and cmake portage issues and no asm: If it's a big hassle, there is the option to use libaom ?

There are 4 options:

* AOM codec
* dav1d codec
* libgav1 codec
* AVM (AV2) codec

• AV2 is of no interest, at least for now.

• libgav1 is c++ only (ew) - let's not. (and it also has a dependency on an abseil thing, whatever that is. no, no..)

• dav1d is fast with its x86 / x86_64 / arm / neon asm.

• aom has asm optimizations too for those cpus but in compiler intrinsics as far as I remember. (its x86 32 bit detection is broken in cmake - you have to tell it to it manually)

[madebr]

I added assembly support to dav1d, but it's not going to work for arm and msvc: it needs gas-preprocess.pl.

https://github.com/libsdl-org/dav1d/blob/769bd1457a9567da5e50b8c670e49234b5bea8c0/meson.build#L445-L466

I'm not 100% sure, but I think it's this: https://github.com/libav/gas-preprocessor

[sezero]

Re: libwebp: If we are to bump to 1.3.1, please rebase 1.3.1-SDL branch to current 1.3.1, or cherry-pick libsdl-org/libwebp@2af2626 into it

EDIT: If we are to keep going with 1.0.3, I cherry-picked the relevant commit into 1.0.3-SDL so SDL_image side needs updating

[sezero]

Anything else left?

Maybe a few things noted at #383 but they are mostly SDL2-related and can
be put in a new pull request.

(I just rebuilt the libavif dlls for the msvc project and uploaded, btw)

[slouken]

Well, if it works and outputs libavif-16.dll instead of libavif.dll then it's fine. (Although I wonder whether we should/could've changed libavif for it..)

You're right. I patched libavif's cmake script.

We should link with whatever the default DLL name is for each project. That way if someone builds it independently, they can drop in a new DLL.

[sezero]

Well, if it works and outputs libavif-16.dll instead of libavif.dll then it's fine. (Although I wonder whether we should/could've changed libavif for it..)

You're right. I patched libavif's cmake script.

We should link with whatever the default DLL name is for each project. That way if someone builds it independently, they can drop in a new DLL.

I disagree: We shouldn't. At the very least, the default windows builds of libavif have no versioning in their names, only libavif.dll, and 0.9.3, 0.10, 0.11 and 1.0 are abi-incompatible.

[slouken]

Well, if it works and outputs libavif-16.dll instead of libavif.dll then it's fine. (Although I wonder whether we should/could've changed libavif for it..)

You're right. I patched libavif's cmake script.

We should link with whatever the default DLL name is for each project. That way if someone builds it independently, they can drop in a new DLL.

I disagree: We shouldn't. At the very least, the default windows builds of libavif have no versioning in their names, only libavif.dll, and 0.9.3, 0.10, 0.11 and 1.0 are abi-incompatible.

Fair enough.

[madebr]

msys2 is not consistent in versioning their libraries.
dav1d's dll contains no versioning.

[sezero]

zlib tagged new 1.3.1 release: Do we want to upgrade?

[slouken]

Sure. In general we want to follow library public releases.

[sezero]

OK, I created v1.3.1-SDL branch in our zlib fork and applied two of our patches.
Intentionally not applied libsdl-org/zlib@8d8abbc:
zlib 1.3.1 has a new ZLIB_BUILD_EXAMPLES option, which we can disable in
our own cmake'ry: https://github.com/libsdl-org/zlib/blob/master/CMakeLists.txt#L8

@madebr: Will you do the upgrade in SDL_image or should I attempt it?

@slouken: Should we upgrade in 2.8.x release branch too?

[madebr]

Feel free to do it, you probably need to add set(ZLIB_BUILD_EXAMPLES OFF CACHE BOOL "zlib examples").

Do set(ZLIB_BUILD_EXAMPLES OFF CACHE BOOL "zlib examples" FORCE) so a SDL_image user is not able to enable it manually.

[slouken]

What are the changes in 1.3.1? If it's security fixes, we should definitely add it to the release branch. If not, it's probably fine to leave it in SDL2 for now.

[sezero]

What are the changes in 1.3.1? If it's security fixes, we should definitely add it to the release branch. If not, it's probably fine to leave it in SDL2 for now.

Changelog doesn't look critical to me: https://github.com/madler/zlib/releases/tag/v1.3.1
Leave release branch as is?

[slouken]

Yep, sounds good.

[sezero]

I don't know what else is left, but one thing about libavif: We want to build it with CMAKE_DLL_NAME_WITH_SOVERSION=ON -- see: #383 (comment)

That property is CMake 3.27+ only. I rolled my own versioning with:

@madebr: Can we not emulate it with something like the following regardless of cmake version?

    cmake_push_check_state()
    if(WIN32)
        set (CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION 1)
    endif()
    add_library (avif ...
    stuff...
    cmake_pop_check_state()

[madebr]

Older CMake versions will just ignore CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION.
Sure, we could do the following, but I argue it looks uglier.

if(CMAKE_VERSION VERSION_LESS "3.27")
    set_property(TARGET avif PROPERTY OUTPUT_NAME "avif-$<TARGET_PROPERTY:avif,SOVERSION>")
else()
    set_property(TARGET avif PROPERTY DLL_NAME_WITH_SOVERSION "1")
endif()

[sezero]

Older CMake versions will just ignore CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION.

Are you sure? It works for me with cmake 3.1.0 (at least with mingw) ??

[madebr]

Older CMake versions will just ignore CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION.

Are you sure? It works for me with cmake 3.1.0 (at least with mingw) ??

No, I'm not.
I mixed DLL_NAME_WITH_SOVERSION with CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION.
But the lack of documentation of this variable makes me wary.
Its only usage is in Modules/Platform/CYGWIN.cmake.

[sezero]

Looked at the sources:

In cmGeneratorTarget.cxx : GetFullNameInternalComponents() of 3.26.6
there is this:

  // Name shared libraries with their version number on some platforms.
  if (cmValue soversion = this->GetProperty("SOVERSION")) {
    if (this->GetType() == cmStateEnums::SHARED_LIBRARY &&
        !isImportedLibraryArtifact &&
        this->Makefile->IsOn("CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION")) {
      outBase += "-";
      outBase += *soversion;
    }
  }

In 3.1.0, it is like this in cmTarget.cxx : GetFullNameInternal() :

  // Name shared libraries with their version number on some platforms.
  if(const char* soversion = this->GetProperty("SOVERSION"))
    {
    if(this->GetType() == cmTarget::SHARED_LIBRARY && !implib &&
       this->Makefile->IsOn("CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION"))
      {
      outBase += "-";
      outBase += soversion;
      }
    }

[madebr]

Just tried it out in a test project, looks like it works.
Feel free to apply this in libsdl-org/libavif and remove my workaround in SDL_image's cmake script.

[sezero]

So, the libavif patch should look like the following ?

diff --git a/CMakeLists.txt b/CMakeLists.txt
index dbdcd01..4b7c791 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -533,6 +533,11 @@ if(NOT AVIF_CODEC_AOM
     message(WARNING "libavif: No decoding library is enabled.")
 endif()
 
+include(CMakePushCheckState)
+cmake_push_check_state()
+if(BUILD_SHARED_LIBS AND WIN32)
+    set(CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION 1)
+endif()
 add_library(avif ${AVIF_SRCS})
 set_target_properties(avif PROPERTIES VERSION ${LIBRARY_VERSION} SOVERSION ${LIBRARY_SOVERSION} C_VISIBILITY_PRESET hidden)
 target_compile_definitions(avif PRIVATE ${AVIF_PLATFORM_DEFINITIONS} ${AVIF_CODEC_DEFINITIONS})
@@ -541,10 +546,7 @@ target_include_directories(
     avif PUBLIC $<BUILD_INTERFACE:${libavif_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include> PRIVATE ${AVIF_PLATFORM_INCLUDES}
                                                                                                       ${AVIF_CODEC_INCLUDES}
 )
-if(BUILD_SHARED_LIBS AND WIN32 AND NOT CMAKE_PLATFORM_NO_VERSIONED_SONAME)
-    set_property(TARGET avif PROPERTY OUTPUT_NAME "avif-${LIBRARY_SOVERSION}")
-    set_property(TARGET avif PROPERTY DLL_NAME_WITH_SOVERSION OFF)
-endif()
+cmake_pop_check_state()
 target_include_directories(avif PRIVATE "${libavif_SOURCE_DIR}/third_party/libyuv/include/")
 set(AVIF_PKG_CONFIG_EXTRA_CFLAGS "")
 if(BUILD_SHARED_LIBS)

Am I supposed to set DLL_NAME_WITH_SOVERSION to 0 to be safe??

NOTE: Built libavif patched as above using Cmake+MSVC, it gave me an avif-16.dll not libavif-16.dll, so should we set CMAKE_SHARED_LIBRARY_PREFIX to lib or something? (Your original workaround doesn't set prefix either...) Or do like that as is?

Not sure what to change in SDL_image's cmake'ry, though?? The only commit about DLL_NAME_WITH_SOVERSION is 69ba38c which seems irrelevant, and I can't see what to change in the avif block.

[madebr]

You need to do CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION in the cmake script of libavif.
It looks like it is a variable that only takes affect at the end of the current cmake script (like CMAKE_C_FLAGS etc).
cmake_push_check_state/cmake_pop_check_state does nothing in this regard.

Don't disable the DLL_NAME_WITH_SOVERSION property, or just unconditionally enable it.
In a dummy project, CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION and DLL_NAME_WITH_SOVERSION don't stack.
Disabling DLL_NAME_WITH_SOVERSION will disable the SOVERSION prefix.

[sezero]

OK, got it. So the final libavif patch is the following yes? (I'm keeping MSVC case as is)

And there is nothing to do in SDL_image cmake'ry?

diff --git a/CMakeLists.txt b/CMakeLists.txt
index dbdcd01..28a67a5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -531,22 +531,21 @@ if(NOT AVIF_CODEC_AOM
    AND NOT AVIF_CODEC_AVM
 )
     message(WARNING "libavif: No decoding library is enabled.")
 endif()
 
+if(BUILD_SHARED_LIBS AND WIN32)
+    set(CMAKE_SHARED_LIBRARY_NAME_WITH_VERSION 1)
+endif()
 add_library(avif ${AVIF_SRCS})
 set_target_properties(avif PROPERTIES VERSION ${LIBRARY_VERSION} SOVERSION ${LIBRARY_SOVERSION} C_VISIBILITY_PRESET hidden)
 target_compile_definitions(avif PRIVATE ${AVIF_PLATFORM_DEFINITIONS} ${AVIF_CODEC_DEFINITIONS})
 target_link_libraries(avif PRIVATE ${AVIF_CODEC_LIBRARIES} ${AVIF_PLATFORM_LIBRARIES})
 target_include_directories(
     avif PUBLIC $<BUILD_INTERFACE:${libavif_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include> PRIVATE ${AVIF_PLATFORM_INCLUDES}
                                                                                                       ${AVIF_CODEC_INCLUDES}
 )
-if(BUILD_SHARED_LIBS AND WIN32 AND NOT CMAKE_PLATFORM_NO_VERSIONED_SONAME)
-    set_property(TARGET avif PROPERTY OUTPUT_NAME "avif-${LIBRARY_SOVERSION}")
-    set_property(TARGET avif PROPERTY DLL_NAME_WITH_SOVERSION OFF)
-endif()
 target_include_directories(avif PRIVATE "${libavif_SOURCE_DIR}/third_party/libyuv/include/")
 set(AVIF_PKG_CONFIG_EXTRA_CFLAGS "")
 if(BUILD_SHARED_LIBS)
     target_compile_definitions(avif PUBLIC AVIF_DLL PRIVATE AVIF_BUILDING_SHARED_LIBS)
     set(AVIF_PKG_CONFIG_EXTRA_CFLAGS " -DAVIF_DLL")

[madebr]

OK, got it. So the final libavif patch is the following yes? (I'm keeping MSVC case as is)

Looks fine to me (if CI generates the same set of files).
What MSVC case are you referring to?

And there is nothing to do in SDL_image cmake'ry?

I don't think so: $<TARGET_SONAME_FILE:avif> is used there, which should just pick up the final name.

[sezero]

OK, got it. So the final libavif patch is the following yes? (I'm keeping MSVC case as is)

Looks fine to me (if CI generates the same set of files).

OK, will be testing patches in my fork soon

What MSVC case are you referring to?

That MSVC is outputting avif-16.dll, not libavif-16.dll as I mentioned above

And there is nothing to do in SDL_image cmake'ry?

I don't think so: $<TARGET_SONAME_FILE:avif> is used there, which should just pick up the final name.

OK, got it, Not touching SDL_image.

[madebr]

What MSVC case are you referring to?

That MSVC is outputting avif-16.dll, not libavif-16.dll as I mentioned above

This can be fixed by doing set_property(TARGET avif PROPERTY PREFIX "lib") in the avif project.

(Sorry, I didn't read that part of your original post)

[sezero]

OK, pushed libsdl-org/libavif@5bcd7d0 and updated vendored libavif in SDL_image, both main and SDL2 branches.

libavif has a CI failure, seems unrelated to the patch.

SDL_image, both main SDL2 branches, has test failures in macos runs (for jxl I think), seems unrelated to the patch.

Downloaded the SDL3 MSVC artifact from CI runs: avif dll is named libavif-16.dll and SDL3_image.dll is looking for it with the correct name judging by the strings in the dll.

[madebr]

OK, pushed libsdl-org/libavif@5bcd7d0 and updated vendored libavif in SDL_image, both main and SDL2 branches.

Great!

libavif has a CI failure, seems unrelated to the patch.

I think you're seeing an issue with libjxl that's too recent.
I also encountered it in #443 (comment).
It only shows up on Macos.
My distro's libjxl is older, and our vendored libjxl is even older.