Skip to content
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

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

Closed
fancycode opened this issue Jan 10, 2023 · 15 comments
Closed

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

fancycode opened this issue Jan 10, 2023 · 15 comments

Comments

@fancycode
Copy link
Member

fancycode commented Jan 10, 2023

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
Copy link
Contributor

kmilos commented Jan 11, 2023

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
Copy link
Contributor

farindk commented Jan 11, 2023

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
Copy link
Contributor

libvips depends on these pkg-config fields, I just opened PR libvips/libvips#3299 for that.

@kleisauke
Copy link
Contributor

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
Copy link
Contributor

farindk commented Jan 22, 2023

@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 added a commit to libvips/libvips that referenced this issue Jan 22, 2023
@kleisauke
Copy link
Contributor

@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 added a commit that referenced this issue Jan 23, 2023
@farindk farindk closed this as completed Jan 23, 2023
@farindk
Copy link
Contributor

farindk commented Jan 23, 2023

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

@kmilos
Copy link
Contributor

kmilos commented Jan 23, 2023

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

@farindk
Copy link
Contributor

farindk commented Jan 23, 2023

@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 farindk reopened this Jan 23, 2023
bradh added a commit to bradh/libheif that referenced this issue Dec 10, 2023
@farindk
Copy link
Contributor

farindk commented Dec 14, 2023

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
Copy link
Contributor

farindk commented Dec 18, 2023

#1062 removes the variables (v1.18.0 branch).

@farindk farindk closed this as completed Dec 18, 2023
@hornschorsch
Copy link

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
Copy link
Contributor

farindk commented Sep 4, 2024

@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
Copy link
Member Author

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
Copy link

hornschorsch commented Sep 5, 2024

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants