CMake misc fixes

[kmilos]

"OpenJPEG" is redundant , and doesn't exist anyway as a pkgconf file.

[kmilos]

Actually, I now realize there was a special case for AOM_DECODER_FOUND and AOM_ENCODER_FOUND variables and if (${variableName}_FOUND) above... However, that doesn't work for system supplied JPEG and OpenJPEG CMake find modules (hence the change to ${packageName}_FOUND), so we now have a bit of a conflict to be solved somehow...

[kmilos]

@farindk This is the minimal snippet that is obviously incorrect and really needs to go in before the next release, the rest can wait to be fleshed out more...

[farindk]

I have rewritten the codec plugin configuration. The macro plugin_option' has been split in two because there were more cases that did not fit in. I think I also corrected the Requires.private` list generation. Please have a look whether you think it is ok now.

[kmilos]

Thanks, the recent commits ee976b6 and 64100f2 do address all the points in this PR, so closing.

[farindk]

Thanks for reviewing them.

[kmilos]

@farindk There is still something perhaps a bit off w/ the plugin detection and configuration... W/ the default options (just cmake -S . -B build) I'm getting:

Dav1d AV1 decoder: disabled
SVT AV1 encoder: disabled
Rav1e AV1 encoder: disabled
JPEG decoder: disabled
JPEG encoder: disabled
OpenJPEG J2K decoder: disabled
OpenJPEG J2K encoder: disabled

I have dav1d, svt-av1, rav1e, and openjpeg2 installed, so I'm expecting to see those as plugins?

[kmilos]

Also, you don't want to have -lstdc++ in libheif.pc.in - the modern toolchains should take care of this when static linking, and this can differ from GCC (stdc++) to LLVM (c++).

[farindk]

@farindk There is still something perhaps a bit off w/ the plugin detection and configuration... W/ the default options (just cmake -S . -B build) I'm getting:
...
I have dav1d, svt-av1, rav1e, and openjpeg2 installed, so I'm expecting to see those as plugins?

These plugins are switched off by default. These additional codecs first have to be enabled, as for most users, they will just make the library larger without much benefit.
You can enable them e.g. with ccmake . in the build directory. That's easier than remembering all the options for the command line.

[kmilos]

These plugins are switched off by default.

I see. The the plugin option is ON by default for these, but you're saying it's gated by the first "enabled" option anyway? Perhaps the plugin_option macro warrants a bit of explaining please?

You can enable them e.g. with ccmake . in the build directory. That's easier than remembering all the options for the command line.

Not really a possibility for distro packagers...

[farindk]

There are usually two options for each codec. E.g. "WITH_DAV1D" and "WITH_DAV1D_PLUGIN".
The first selects whether you want Dav1d support at all.
The second chooses whether you want to compile this into libheif directly or as a separate plugin file.

Thus, distributors that support separate plugins can enable all codecs and all WITH_{codec}_PLUGIN so that the user can choose what to install. If the distribution does not support separate plugins, the packager has to choose which codecs to include with WITH_{codec} and disable all WITH_{codec}_PLUGIN.

I think I'll have to write this into the README.

[kmilos]

I think I'll have to write this into the README.

And/or into the CMakeLists.txt as well please. You can perhaps even express your intent explicitly in code as

option(WITH_${optionVariableName}_PLUGIN "Build ${displayName} as a plugin" ${defaultPlugin} AND WITH_${optionVariableName})

Indeed, adding -DWITH_DAV1D=ON -DWITH_RAV1E=ON -DWITH_SvtEnc=ON -DWITH_OpenJPEG_ENCODER=ON -DWITH_OpenJPEG_DECODER=ON worked.

[farindk]

You can perhaps even express your intent explicitly in code as ...

Thanks, I'll try that.

BTW: you can also use cmake --preset release to include everything as plugins.
Also have a look at the new "ffmpeg hevc decoder" plugin. That is hardware accelerated HEIC decoding.

[kmilos]

Also have a look at the new "ffmpeg hevc decoder" plugin. That is hardware accelerated HEIC decoding.

Thanks, but ffmpeg brings a huge dependency chain w/ it (check out its list of required packages e.g. for MSYS2; other distros are probably similar...), so probably won't be enabling it just yet...

[farindk]

option(WITH_${optionVariableName}_PLUGIN "Build ${displayName} as a plugin" ${defaultPlugin} AND WITH_${optionVariableName})

I think this does not help much because the last parameter is only used when the cmake cache variable is not set yet. I.e. it does not create any constraint on the WITH_{codec} variable.

[kleisauke]

FWIW, the removal of -lstdc++ caused a minor issue for libvips' OSS-Fuzz builds, see: libvips/libvips#3713.

Nevertheless, the change is probably correct as hardcoding the C++ runtime library isn't a good idea. It could also causes issues with -static-libstdc++.
https://github.com/kleisauke/libvips-packaging/blob/23c5227c0b00a6611558ad1532b8b07b67d52d23/build/lin.sh#L278-L281

(libjxl has a similar problem in this regard - libjxl/libjxl#1648)

[pszemus]

My GraphicsMagick build also failed after the -lstdc++ removal :-(

[kmilos]

@farindk We can try to bring back -lstdc++ or -lc++ .pc flag depending on toolchain used to restore the old behaviour (for apps that don't use e.g. CMake or Meson), but that's still not ideal - there might be cases where an app might want to build w/ -static-libstdc++ instead and the existence of the flag was making it difficult to do that... Mind you, there were no libheif bugs filed on that problem so far, so it's an option. The alternative is to leave it now w/o a flag and say it's the responsibility of the client app to link libstdc++/libc++ in a way they want (add to release notes)?

[kleisauke]

libvips resolved this via PR libvips/libvips#3715 instead. For autotools, you could use the AX_CXX_CHECK_LIB macro, although that macro is not part of the standard autoconf library.

[kmilos]

libvips resolved this via PR libvips/libvips#3715 instead.

Indeed, things work out automagically if using a C++ compiler/linker. However, I guess there is still the corner case of some apps that would use a C one through an older build system, where that custom macro would come in useful.

@pszemus For example, GM hard codes '-lstdc++' also for libjxl, which is not exactly correct (see relevant libjxl line in https://sourceforge.net/p/graphicsmagick/code/ci/default/tree/configure.ac) and it look like they would need to sort this out eventually anyway (please feel free to report it there).

Still, the question remains: libheif reverts to old behaviour (w/ correct C++ library) to help some apps, or insist on new one?

[pszemus]

Just for academic purposes, how can a library client (e.g. C application) that uses pkg-config to include dependencies, know if it should link the standard C++ library, other than through pkg-config .pc files?

[silverbacknet]

The library or application handles its own standard C/C++ library during its own build. There should be no need for anything higher in the food chain to know about that.

Unless you're asking what if a pure C environment has no C++ installed, and suddenly needs one to satisfy the build dependency. I'm... not sure that's even possible in pkg-config, let alone cmake. I suppose there are scenarios where it must be, I've just never tried or seen it.