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

Added AVIF support #691

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Added AVIF support #691

wants to merge 11 commits into from

Conversation

Photon89
Copy link
Member

Added AVIF support as requested in #690.

@Photon89 Photon89 linked an issue Aug 16, 2024 that may be closed by this pull request
@Photon89 Photon89 mentioned this pull request Aug 16, 2024
@DarthGandalf
Copy link
Member

Does this need any new dependency?

Looking at the code, why do we need so much custom code if the only thing different between different formats is the name of the setting where the quality is stored?

@Photon89
Copy link
Member Author

Indeed, it requires libavif as optional dependency for AVIF support.

I just did a copy&paste from the recently merged PR where I implemented the WEBP support, but actually you are right, I will try to simplify the code!

@Photon89
Copy link
Member Author

I'm out of ideas how to further simplify the code. It's still a bit ugly in some places, but I think, the biggest chunks should be taken care of now.

@Photon89
Copy link
Member Author

Please test and review, ready to merge from my end!

@Photon89
Copy link
Member Author

Photon89 commented Aug 20, 2024

One remark: After getting rid of the int_png, int_jpg etc we don't have control over the order of the supported file types in the file type drop down list and also when choosing the default file type (for an empty settings file).

On my machine, the order is png, bmp, jpg, avif, webp, that is, when the settings file is empty, png is taken as default if available (second priority is bmp and so on). I'm not sure if this order is the same on all machines, it is the order of the list which is returned by https://docs.gtk.org/gdk-pixbuf/type_func.Pixbuf.get_formats.html. Saving and loading the settings also relies on the fact that this order doesn't change, because the number that is saved in the settings file is referring to this order.

Before the refactoring the order for default file type priority was png, jpg, bmp, webp, avif (fixed like that in

foreach my $cformat (@{[$int_png, $int_jpeg, $int_bmp, $int_webp]}) {
). The numbers 0 and 1 have been fixed to jpg and png respectively, but the other file types relied on the order in the Pixbuf.get_formats list as well, as far as I understand.

We get a problem if a lib supporting a new format is installed and then the number in the settings file doesn't suit any more. Say, without libavif, we have png, bmp, jpg, webp, so webp has number 3 in the settings file. But after installing libavif, we have png, bmp, jpg, avif, webp and thus number 3 in the settings file corresponds to avif while webp shifts to number 4.

I'm not sure if this is acceptable or we should try to assign a number to each supported file format globally (which will probably make the code a bit uglier again, but more stable).

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

Successfully merging this pull request may close these issues.

Request AVIF Format
2 participants