strukturag / libheif

libheif is an HEIF and AVIF file format decoder and encoder.
Other
1.74k stars 301 forks source link

libheif.pc shows builtin encoders/decoders even if built as plugins #758

Closed fancycode closed 10 months ago

fancycode commented 1 year ago

The libheif.pc has fields like builtin_h265_decoder=yes even if the libde265 plugin was built as plugin, i.e. is not builtin.

Are these builtin_* fields really used given that there are more possible decoders / encoders than what is currently checked?

kmilos commented 1 year ago

TBH, I've rarely (if ever?) seen (private?) library features being recorded and queried like this via pkg-config, so not sure if anyone is actually using these...

farindk commented 1 year ago

We can find out by removing these fields and waiting if somebody will complain. I also doubt that these are used, and even if they are, people should move away from them, now that codecs can be loaded dynamically.

kleisauke commented 1 year ago

libvips depends on these pkg-config fields, I just opened PR https://github.com/libvips/libvips/pull/3299 for that.

kleisauke commented 1 year ago

GIMP also depends on these pkg-config fields, see: https://gitlab.gnome.org/GNOME/gimp/-/blob/4b6ff68094abd7bd3dd9a074d0ee1dacd3b9ff00/meson.build#L755-770 (they do not provide the default_value argument, so I think there will be an error raised if these fields cannot be found)

farindk commented 1 year ago

@kleisauke Right. Maybe instead of removing these fields completely, it would be better to always set them to 'true'. In that case, there might be an "unknown codec" error during runtime if the corresponding plugin is mussing, but at least it will still compile correctly everywhere. What do you think?

kleisauke commented 1 year ago

@farindk Sounds good to me. For GIMP specifically, these fields should be set to yes instead of true, i.e.:

builtin_h265_decoder=yes
builtin_h265_encoder=yes
builtin_avif_decoder=yes
builtin_avif_encoder=yes

(to satisfy that == 'yes' check)

farindk commented 1 year ago

Reported to gimp here: https://gitlab.gnome.org/GNOME/gimp/-/issues/9080

kmilos commented 1 year ago

Just 2c: why not leave this open until the GIMP and libvips drop using these so they can be removed eventually?

farindk commented 1 year ago

@kmilos Also fine with me.

Leaving this open as a reminder to remove the variables completely when other software does not depend on it anymore.

farindk commented 10 months ago

I checked the current GIMP build script. It does not depend on these variables anymore. Therefore, I would support removing these variables with release v1.18.0. Any objections?

farindk commented 10 months ago

1062 removes the variables (v1.18.0 branch).

hornschorsch commented 1 month ago

Well, sorry for posting into this closed issue, but the current stable gimp version 2.10.38 still does the check for builtin_h265_decoder and builtin_h265_encoder. Only in the 2.99 versions they now try to compile a C program and check if heif_have_decoder_for_format (heif_compression_HEVC); returns true.

Possibly this removal was a bit early?

farindk commented 1 month ago

@hornschorsch These flags do not make any sense anymore with current libheif versions. If there are old build scripts that use them, you can use libheif v1.17.6 (even though they were already disfunctional at that time).

fancycode commented 1 month ago

Also checking for available codecs at build-time with heif_have_decoder_for_format doesn't make much sense as with the plugin system, there might be codecs available at runtime that were not present while building (or vice versa).

hornschorsch commented 1 month ago

Well, thats what the gimp people are doing. Above you stated that you would keep these variables as long as gimp and vips use them, and now you removed them just because the development versions do not use them any more. Took me several hours to understand what was happening and finally compile the current stable version - not an "old" build script! - of gimp with heic support... but let's leave it at that...