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

add support for jxl thumbnails #825

Merged
merged 6 commits into from
Jul 5, 2023
Merged

Conversation

polak14
Copy link
Contributor

@polak14 polak14 commented Jul 4, 2023

This adds an optional toggle to enable JXL thumbnails.

Why?
Saves a bit of space (~25-30% per file) and quality is superior.

thumbnail & Use high-quality thumbnails for pages enabled
scale

Use high-quality thumbnails for pages disabled
sample

If this is OK, it could also be a good option for the "Resize Images in Reader" function, as there the gains from JXL in size and quality would be most notable.

Caveats:
Enabling the option will regenerate all thumbnails as LRR is now looking for .jxl not .jpg.
Old .jpg (or .jxl if the option is being disabled instead) files will not be deleted automatically.

@Difegue
Copy link
Owner

Difegue commented Jul 5, 2023

I don't have time to look at this PR in detail (reading groups took all of it..), but I have a few shorthand questions:

  • Current JPEG thumbnails are quite space efficient already due to the decoder optimization flags -- Do you have some size comparisons with HQ thumbnails on and off so I can verify that 30% number?
  • I wouldn't mind using JXL in "Resize images in Reader" if Chrome hadn't unceremoniously dumped the format because they can't control it... Currently I don't feel too good about the format's future, sadly.
  • We should probably check for both jpg and jxl when serving thumbnails and deleting them! If users want to go full jxl after toggling the option, they should just fire the Minion job to regen all thumbs.

@polak14
Copy link
Contributor Author

polak14 commented Jul 5, 2023

Current JPEG thumbnails are quite space efficient already due to the decoder optimization flags -- Do you have some size comparisons with HQ thumbnails on and off so I can verify that 30% number?

On a random archive
commands used:

mogrify -sample 500x1000 -format jpg -quality 50 *
mogrify -sample 500x1000 -format jxl -quality 50 *
mogrify -scale 500x1000 -format jpg -quality 50 *
mogrify -scale 500x1000 -format jxl -quality 50 *
du --apparent-size  
1404    ./jpg sample
1152    ./jxl sample
21.88% diff

1180    ./jpg scale
906     ./jxl scale
30.24% diff

More importantly to me at least is that it eliminates the horrible artifacting that jpeg suffers from, as seen in the comparison.

I wouldn't mind using JXL in "Resize images in Reader" if Chrome hadn't unceremoniously dumped the format because they can't control it... Currently I don't feel too good about the format's future, sadly.

Apple has added support on both iOS/OSX and Safari (https://caniuse.com/jpegxl)
Adobe too encourages use of JPEG XL (https://helpx.adobe.com/camera-raw/using/hdr-output.html)
All major, in development, image editors and viewers support JXL as well as most relevant libraries (ffmpeg, graphics/imagemagick, libvips)
All that makes me think that Google will eventually have to budge and add JXL back.
And considering most people use Tachiyomi (supports JXL) and Docker (the more recent alpine image support JXL) the compatibility for most users of LRR should be fairly high actually.

For a bit more convincing of JXL's potential, here is a comparison of my whole archive before and after converting to lossless JXL

609.1 GiB (653,991,251,275)
373,000 files, 9,038 sub-folders
421.7 GiB (452,776,548,367)
373,000 files, 9,038 sub-folders

We should probably check for both jpg and jxl when serving thumbnails and deleting them! If users want to go full jxl after toggling the option, they should just fire the Minion job to regen all thumbs.

sure, that works too

@RePod
Copy link
Contributor

RePod commented Jul 5, 2023

This kicks the can down the road until the next hot format. On a 5600X I also noticed a 50% increase in generation time (13~15 -> 20~22) with the mogrify command to JXL vs JPEG, ignoring any increased memory usage.

Something like this would cater to everybody. Maybe as a plugin/plugin that supports uploaded Perl functions.
image


unlink $thumbname;
my $jpg_thumbname = "$thumbdir/$subfolder/$id.jpg";
unlink $jpg_thumbname;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am assuming unlink() doesn't care if we don't check if the file exists

unlink $jxl_thumbname;

# Delete the thumbpages folder
remove_tree("$thumbdir/$subfolder/$id/");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i hope this is fine, before LRR did not remove the thumbpages folder when deleting an archive

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I believe this was an oversight on my part back when adding thumbpages. Thanks!

@polak14
Copy link
Contributor Author

polak14 commented Jul 5, 2023

This kicks the can down the road until the next hot format. On a 5600X I also noticed a 50% increase in generation time (1315 -> 2022) with the mogrify command to JXL vs JPEG, ignoring any increased memory usage.

Something like this would cater to everybody. Maybe as a plugin/plugin that supports uploaded Perl functions. image

I did note that "for performance reasons it defaults to JPEG", although personally i don't know how much it actually is as my server has high-end recent hardware. From what i know JXL is still very fast even compared to JPEG, and definitely compared to more similar formats like AVIF.

You can change the size threshold and the quality for resizing images, although you are not currently able to change the quality and resolution of thumbails, would be easy to add however.

Copy link
Owner

@Difegue Difegue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small suggestion left from me. Thanks for the size comparisons, I'm sold! Additional GPU load is fine since we're not making JXL the default imo.

I'd forgotten Apple announced JXL support at WWDC -- It's fair to believe google will probably cave in when you look at the related chromium issue still getting comments.

Not sure I'd go as far as exposing imagemagick arguments in the UI though, that feels way overkill.

lib/LANraragi/Model/Archive.pm Outdated Show resolved Hide resolved
unlink $jxl_thumbname;

# Delete the thumbpages folder
remove_tree("$thumbdir/$subfolder/$id/");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I believe this was an oversight on my part back when adding thumbpages. Thanks!

@polak14
Copy link
Contributor Author

polak14 commented Jul 5, 2023

BTW, while i was checking out resize_image i noticed that it serves the resized image in the same format as the original file. So jxl, avif, png, they stay the same format and don't actually use JPEG unless the original file is JPEG.

@Difegue
Copy link
Owner

Difegue commented Jul 5, 2023

huh, I wonder if that's not a magick behavior change in newer versions - The image resizing code is fairly old at this point, I haven't looked at how it behaves in a while. 🤔 Probably worth editing the documentation for that.

Thanks for the thumbnail PR in any case, I'll be merging this!

@holopin-bot
Copy link

holopin-bot bot commented Jul 5, 2023

Congratulations @polak14, you just earned a holobyte! Here it is: https://holopin.io/holobyte/cljq5to9501970fmg24qude7m

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

@Difegue Difegue merged commit 303c151 into Difegue:dev Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants