Added Dynamic Conversion of JPEG-XL, JPEG 2000, HEIF and AVIF to JPG when the browser does not support them. #3141

maxpiva · 2024-08-29T19:00:33Z

Added

Support for JPEG-XL, JPEG 2000, HEIF and AVIF.
Dynamic transcoding to JPEG in the cache, when the browser does not have support for the image types.
Direct serving when the browser supports the image type.
Transcoding of the thumbnails.
Added ImageConverterServices, with ImageConverterProviders to easy expansion.
Edited Dockefile, adding required library support for reading the new image formats in Linux versions. [Bumped ubuntu to oracular, because of the JPEG-XL image support]
Added Magick.NET nuget for reencoding support [Toyed with the idea, of using vips, but netvips binding does not support the 'ALL version', so netvips binding need to be recompiled, mantained, etc, dropped in favor of Magick.NET, until netvips support the above image types]
[Not Related] Update nugets to latest, Flurl Http changed the way its configured, edited accordingly.

Missing

Additional Testing methods.

Fixed test and benchmark DI. Point docker-build to another hub Edit dockerfile and include jxl package.

1. Supported Image Formats came in the accept flag from the browser. 2. If jxl (or future formats, ie browser not supported AVIF, HEIF, etc) are not supported, Kavita will convert it to a suitable display format via conversion providers. 3. if it supported it will return the image as is, no conversion needed. 4. Thumbnails are always converted.

This reverts commit 7f6db24.

This reverts commit 9d48664.

1. Supported Image Formats came in the accept flag from the browser. 2. If jxl (or future formats, ie browser not supported AVIF, HEIF, etc) are not supported, Kavita will convert it to a suitable display format via conversion providers. 3. if it supported it will return the image as is, no conversion needed. 4. Thumbnails are always converted.

…gick. Rollback IOsInfo.cs

majora2007

I looked over the code and provided comments.

I'm not ecstatic about ImageMagick. I'm not sure if it supports ARM deployments as well, so I would need confirmation. Likewise, changing the docker file base image.

I would also need to test this locally to see if it's worth it to add support for transcoding on the fly and if the performance difference is worth it or not.

I personally haven't added JXL because it's DOA with support from the browsers. Jpeg2000 is also quite rare. Owning the maintenance of this code for flavors of formats that aren't common doesn't make much sense to me.

Since you're opening the PR out of nowhere, please provide justification or any supporting Feature Requests.

majora2007 · 2024-08-29T19:11:19Z

API/Controllers/ReaderController.cs

@@ -23,6 +23,7 @@
 using Microsoft.Extensions.Logging;
 using Microsoft.IdentityModel.Tokens;
 using MimeTypes;
+using Org.BouncyCastle.Ocsp;


Remove this

I can only blame Resharper on this ;)
Wil do

majora2007 · 2024-08-29T19:12:08Z

API/Extensions/FileTypeGroupExtensions.cs

+    {
+        // Add jxl format
+        format = format.ToLowerInvariant();
+        if (format == ".jxl")


return on the same line please

API/Extensions/FileTypeGroupExtensions.cs

majora2007 · 2024-08-29T19:13:19Z

API/Extensions/HttpExtensions.cs

+        if (string.IsNullOrEmpty(extension))
+            return;
+        if (!extensions.Contains(extension))
+            extensions.Add(extension);


Please always use {} whenever using any if statement that isn't a direct control flow (continue/break/return).

For any control flow jump logic, they need to be on the same line as the if else use {}

majora2007 · 2024-08-29T19:13:34Z

API/Extensions/HttpExtensions.cs

+    {
+        if (string.IsNullOrEmpty(extension))
+            return;
+        if (!extensions.Contains(extension))


Newline before the if statement to help keep the code clear

API/Extensions/ApplicationServiceExtensions.cs

majora2007 · 2024-08-29T19:18:56Z

API/Extensions/HttpExtensions.cs

+        List<string> supportedExtensions = new List<string>();
+
+        //Add default extensions supported by all browsers.
+        supportedExtensions.Add("jpeg");


Can we use the extensions in Parser so this is all colocated in case we ever add more formats in the future?

For sure, will do, also all the coding styles referenced.

majora2007 · 2024-08-29T19:26:10Z

API/Services/ImageConverterService.cs

+        /// <inheritdoc/>
+        public Stream ConvertStream(string filename, Stream source)
+        {
+            IImageConverterProvider provider = _converters.FirstOrDefault(a => a.IsSupported(filename));


How do you control the order of these?

Since, two Converters cannot support the same extension, there is no ordering needed.

API/Services/Tasks/Scanner/Parser/Parser.cs

majora2007 · 2024-08-29T21:20:38Z

Dockerfile

@@ -14,15 +14,15 @@ RUN /copy_runtime.sh
 RUN chmod +x /Kavita/Kavita

 #Production image
-FROM ubuntu:focal
+FROM ubuntu:oracular


@DieselTech

move to oracular because the include JPEG-XL in there, focal, requires custom build the support. Docker worked really fine. But didn't test ARM.

Yes, adding MagicK was the last thing I wanted to do, problem is, while vips supports all the image formats, netvips binding and libraries are boundled with the standard version, not the ALL version.

Will test on ARM, tested on X64 docker, and windows without an issue. git said, every flavor is supported, but, yes, testing will be needed for ARM.

I'm not going to enter the browser war, or format war, just adding the capability of supporting multiple formats and be dynamic about it, independent of the browser, seems the way to go. And since, from my own testing JPEG-XL seems to be a techincal marvel.

DieselTech · 2024-08-29T21:41:44Z

[Kavita] [2024-08-29 17:32:53.271 -04:00  60] [Warning] API.Services.ArchiveService [GetCoverImage] There was an exception when reading archive stream: Y:/Projects/Image Format Tests/JXL/Max Compression Test.cbz. Defaulting to no cover image
ImageMagick.MagickBlobErrorException: unable to open image 'Terminator - One Shot - 00 - FC.jpg': Permission denied @ error/blob.c/OpenBlob/3596
   at ImageMagick.MagickExceptionHelper.Check(IntPtr exception) in /_/src/Magick.NET/Exceptions/MagickExceptionHelper.cs:line 18
   at ImageMagick.NativeHelper.CheckException(IntPtr exception) in /_/src/Magick.NET/Native/NativeHelper.cs:line 20
   at ImageMagick.MagickImage.NativeMagickImage.WriteFile(IMagickSettings`1 settings) in /_/src/Magick.NET/Generated/Magick.NET.SourceGenerator/ImageMagick.SourceGenerator.NativeInteropGenerator/MagickImage.g.cs:line 8851
   at ImageMagick.MagickImage.Write(String fileName) in /_/src/Magick.NET/MagickImage.cs:line 7047
   at API.Services.ImageConversion.ImageMagickConverterProvider.Convert(String filename) in Z:\Projects\Kavita\API\Services\ImageConversion\JpegXLImageConverterProvider.cs:line 57
   at API.Services.ImageConverterService.ConvertStream(String filename, Stream source) in Z:\Projects\Kavita\API\Services\ImageConverterService.cs:line 73
   at API.Services.ImageService.WriteCoverThumbnail(String sourceFile, Stream stream, String fileName, String outputDirectory, EncodeFormat encodeFormat, CoverImageSize size) in Z:\Projects\Kavita\API\Services\ImageService.cs:line 252
   at API.Services.ArchiveService.GetCoverImage(String archivePath, String fileName, String outputDirectory, EncodeFormat format, CoverImageSize size) in Z:\Projects\Kavita\API\Services\ArchiveService.cs:line 236

The filename it is trying to open doesn't exist in the archive. The file inside Max Compression Test.cbz is Terminator - One Shot - 00 - FC.jxl

maxpiva · 2024-08-29T21:50:38Z

@DieselTech WIll fix. In that case, the file need to be created in a temp directory.

Viewer works fine in your machine?

Fix GetCoverImage, trying to create a temporary image in current directory, and rename the parameter field name, that generates a wrong asumption.

maxpiva · 2024-08-29T22:35:01Z

@DieselTech Please retry, thanks in advance. Seems fixed, but the devil never rests.

For completion's sake I'll add two TODO to my list.

using the list of images in the Parser, in the Accept -> SupportedImageTypes, as suggested by @majora2007
Currently when generating covers, the converter uses a temporary file to create a jpg, that can be future converted into a thumbnail with netvips. This is subpar, it will be better to directly use MagicK in that case to generate the thumb instead netvips, to avoid the extra conversion.

DieselTech · 2024-08-29T22:51:02Z

Preface: These comments are before your latest commit. I am seeing the images in the browser. However it seems like it's trying to call the image endpoint twice, once with the proper image number and a second time that is using page # 1000000

api/reader/image?chapterId=1&apiKey=6e2885d7-e627-466b-b365-56863ad98995&page=1000000

One thing I would like to see is a log entry for the conversion. Not everyone runs these servers on powerful hardware, and it will become necessary when troubleshooting with them why their page loads are taking 5 seconds. That way we can point directly to the conversion task.

maxpiva · 2024-08-29T22:58:00Z

Totally agree on slow servers,

on/off flag?

The delay exists, mostly, because there is frontend caching, client will try to get multiple images, and Kavita will trigger the conversion of them all.
What I see from my end, is, initial delay, 1-2 secs, then normal behavior. since the frontend caching mechanism, hides the additional delay after the initial load, docker with 1 core assigned.

Also, the transcoding only happens once.
Normal Kavita behavior seems to be ARCHIVE -> CACHE -> BROWSER
The conversion happens in the cache, so after the initial conversion, always the converted image will be used going forward. until you clear the read cache.

IDK about the page 1000000.

I can add a stopwatch.

DieselTech · 2024-08-29T23:14:13Z

Totally agree on slow servers,

on/off flag?

Joe will have to answer this one. There are some things that we do give the user a choice in, but otherwise it's opinionated in order to make the compromise of not layering complexity over and over. I would likely say that IF the image does get converted, to output its duration. If it doesn't get converted, then do not log it.

The delay exists, mostly, because there is frontend caching, client will try to get multiple images, and Kavita will trigger the conversion of them all. What I see from my end, is, initial delay, 1-2 secs, then normal behavior. since the frontend caching mechanism, hides the additional delay after the initial load, docker with 1 core assigned.

Honestly I couldn't see a delay at all. The image API responded within 10ms and the page loads felt instant to me. I had to double check to see if it was even working correctly. But using my workstation as a testbed is an unfair comparison with how much power it has. That is why I thought about the log entry so we can know the time taken.

Also, the transcoding only happens once. Normal Kavita behavior seems to be ARCHIVE -> CACHE -> BROWSER The conversion happens in the cache, so after the initial conversion, always the converted image will be used going forward. until you clear the read cache.

Okay so they should stay around until the users cleanup tasks kick in, which defaults to daily.

That said, you sent this PR right as the lead dev is going away for a little bit. This will need to sit until he gets back and looks more in-depth into it. If you have discord you are welcome to join our server and we can communicate in real time there.

maxpiva · 2024-08-30T00:50:04Z

No problemo, will resolve the remaining issues and add some timings to the logging. Will join you guys in discord, tomorrow.

Optimize Cover Creation

majora2007 · 2024-09-23T14:15:38Z

Is this ready for a proper review? If so, I can get it scheduled. I'm still not 100% sure I want to merge this in, but i do want to give it a fair test and also see if there is potential to remove NetVips and ImageSharp altogether, because that could allow for Kavita to run on CPUs that don't have SSE4.2.

maxpiva · 2024-09-23T14:26:09Z

Define proper ;)
I did some research on netvips usage, and one of the main issues, is the way that covers are made, it supports that center on 'subject' on cover creation, and I think is triggered when the aspect ratio does not match. If you can live without it. I think is feasible to remove both. The other is backporting that functionality from netvips code base.

majora2007 · 2024-09-23T14:34:28Z

Haha proper means, me pulling everything down, testing, polishing the code for actual inclusion.

Yeah, I recently added that attention type of scaling, which helps when users need covers with webtoons, which are long strip format. That is a feature we might see if we can build with ImageMagick.NET.

maxpiva · 2024-09-23T14:37:23Z

maxpiva/kavita:nightly

if you want to test the docker. x64/armv7/arm64 (It has your lastest changes). (Release version).

Build and debug from windows works as expected.

On my machines :)

Let'me sit a little with the code, I'll see if we can remove both dependencies.... After that you can take the decision or not to merge.
IOS 18 did a big push on JPEG-XL, so the codec is in a better place today.

maxpiva · 2024-09-29T02:53:41Z

@majora2007 I have a new branch on my fork.

https://github.com/maxpiva/Kavita/tree/RemoveSixLaborsAndNetVips

This one, removes all dependencies on NetVips and ImageSharp and replaces it with ImageMagick.

Only differences so far:

Colorspaces colors are kinda more vibrant, probably related to the resizing interpolation algorithm.
The Attention (Focus on webtoons) is not ported over, it uses a lots of matrix transformation, so, it might take some time.

Of course, the above fork includes the (JP2, JPX, HEIF, AVIF) transcoding depending on what the browser supports.

On iOS per example, all is served directly.

The docker image is updated to this fork.

majora2007 · 2024-09-30T14:28:42Z

Awesome, I'm wrapping up some work on the scanner and while I collect testing from the community, I'm planning to give this a run, especially the new fork.

If needed, we could leave ImageSharp to help with the matrix math. NetVips is what brings the CPU dependency on the project.

I'm hoping for favorable results to bring more support to the project and to more people.

maxpiva · 2024-10-02T01:12:02Z

Updated, instead of using netvips attention, using a direct replacement ported to C#.

from https://github.com/muesli/smartcrop

So far so good.

Docker is also updated.

majora2007 · 2024-10-03T15:37:15Z

I have officially started testing the full NetVips removal branch.

maxpiva · 2024-10-03T15:43:35Z

Nice, probably a refactor will be needed afterwards. We have salted image conversion, thumbnails, and image processing, maybe we can join everything into a service/interface, and use ImageMagick as the first implementation, in that way, if you want to swap motors, another implementation can be created.

majora2007 · 2024-10-03T15:59:43Z

Yeah that's a good idea. I was also looking that jpg was selected as default image format. We might want to think about making it a server setting or even choosing png/webp. Given that Kavita supports major 2 browsers and webp is now supported across the browsers nowadays.

I def think we can likely remove the conversion service and move it within the Image service and refactor quite a bit to streamline the code.

majora2007 · 2024-10-03T16:33:03Z

Since we don't have a PR for the other branch and in reality if I'm merging this, I want to use that other branch, here are some results from testing. So far, it's pretty close and looks fine from my perspective. I included the unit test results as well.

From the other branch, attention port:
Old

New

Old:

New:

Existing Code.zip

New Code.zip

majora2007 · 2024-10-03T16:41:18Z

Okay I'm good to move forward with your NetVips removal branch. Let's clean up the code then I'll do another pass on it myself and we can move this into the nightly. We will need more documentation on the methods as well to cover our bases in case something breaks down the line.

maxpiva · 2024-10-03T16:47:50Z

You want me to refactor the Image Service or, and create documentation about the new methods before merging, I think Image Service is a little bloated, maybe a secondary service, that handles image operations.

Yes currently, if the format is not supported, it'll be converted into jpg with 99 quality. Same for covers.
We could change that from the config, but we can actually do that in a next iteration from fixed to be driven from setting?

I could do a PR from the other branch.

majora2007 · 2024-10-03T17:33:32Z

Yeah, let's close this PR and recreate a new one from the other branch. Let's add documentation and refactor the code to be more streamlined.

Let's make the format a const in ImageService (jpg or png) so we don't have magic strings. I think png/webp could be good targets or even having some sort of tier like webp then png depending accept response headers could be good.

Once that's done, I'll merge into a branch here, then do a round of cleanup to make sure the style is just as I like it and we can finally merge into develop to get into the hands of nightly testers.

kleisauke · 2024-10-23T14:25:02Z

[...] but netvips binding does not support the 'ALL version', so netvips binding need to be recompiled, mantained, etc, dropped in favor of Magick.NET, until netvips support the above image types

This is not correct. The NetVips bindings does support JPEG XL, JPEG 2000, HEIC and AVIF. However, except for AVIF, the pre-built binaries (i.e. NetVips.Native) doesn't support these image formats.

With the update of ubuntu:oracular, you can just remove NetVips.Native and install libvips with all these formats enabled via:

$ sudo apt-get install libvips --no-install-recommends

maxpiva · 2024-10-23T15:40:18Z

Good call @kleisauke. I was unable to make it work on Windows. Since we need to create a custom native and maintain it, correct?
But docker is another beast.

@majora2007 has some secondary concerns related to Netvips, and some CPU limitation that brings to the table on docker.

Anyway, since the current image processing implementation is decoupled into an interface and a ImageMagick is an implementation, we could create another implementation supporting NetVips. The question is, we want to?

This PR, got overseeded by this one:

#3250

kleisauke · 2024-10-23T16:08:12Z

I was unable to make it work on Windows. Since we need to create a custom native and maintain it, correct?

For Windows, you could use the vips-dev-w64-all-x.y.z.zip variant, but that contains ALL the file format loaders that libvips supports, including some very minor ones, and a complete copy of ImageMagick. It can process many different file types, but it is also rather vulnerable to hacking.

[...] and some CPU limitation that brings to the table [...]

The CPU concerns only applies to the pre-built Linux binaries provided by NetVips, see: #1423 (comment).

majora2007 · 2024-10-23T16:30:28Z

My 2 cents is if we can keep NetVips without any CPU dependency, I would like that. NetVips is MUCH faster than ImageMagik.

maxpiva · 2024-10-23T16:36:39Z

@kleisauke I was unable to make it work on Windows, do you have a way to use the net bindings with that vips-dev, i was unable to make it work without the netvips native nugets. Including copying the libraries manually.

I agree that netvips is faster than ImageMagick, but mantaning our custom netvips.native will be probably a lots of pain.

I could also make some defines what is enabled with #define at build time. if we solve the issue with netvips.

for netvips to work. We need to:

Have a NonDocker version working with all the image types. (netvips.native like but working).
Have a docker image created that supports all the image types, but do not have specific CPU limitations. IE: Only working with AVX2. etc.,

Addtional, I'l add the capability of having netvips implementation ready inside kavita. In that way, we can actually by putting a conditional define in the build, we can create with one or another....

This will open the possibility to create a docker image/layer using netvips, and other using ImageMagick. If the CPU limitations are still an issue.

kleisauke · 2024-10-29T19:06:16Z

Hmm, that should work without issues; I regularly test the -all variant on the NetVips test suite. Did you add the directory containing the DLLs to your PATH environment variable?

I won't distribute the -all variant on NuGet, as it contains GPL-licensed libraries. Regarding the other image formats not supported by the -web variant and mentioned here:

JPEG XL: support within -web is being tracked in Move JXL from ALL to WEB group libvips/build-win64-mxe#40.
JPEG 2000: excluded from -web as it offers (in my opinion) minimal advantages. It primarily serves as a compatibility solution for legacy software that still uses it. Also note that Safari was the only browser to provide support for this format, and has removed support for this in version 18, see:
https://webkit.org/blog/15443/news-from-wwdc24-webkit-in-safari-18-beta/#images
HEIC: excluded due to its reliance on HEVC, which is patent-encumbered.

maxpiva · 2024-10-30T12:54:59Z

@kleisauke Thanks. Will start testing again. @majora2007. Could you ask @kleisauke about the docker CPU issues, how we can bypass that?

@majora2007 Leme try this out, initially since the native nuget couldn't be used in this case, will try to add a download/unzip or include a custom tool in the normal build pipeline (no docker), and for the docker pipeline do, what @kleisauke recommended.

maxpiva and others added 15 commits August 15, 2024 08:25

Add JXL Support.

e2a86c6

Make sure if djxl do not exists, continue as usual.

cb647ad

Fixed test and benchmark DI. Point docker-build to another hub Edit dockerfile and include jxl package.

Missing file

dd0021a

Merge branch 'Kareadita:develop' into develop

efc14ff

Revert "Changed the way conversion happens."

9d48664

This reverts commit 7f6db24.

Reapply "Changed the way conversion happens."

cf05e5f

This reverts commit 9d48664.

Merge with Karedita

21b4c1b

Add Documentation

7d48f6e

FIX BOMS

a4a12a1

Recode SupportedImageTypesFromRequest removing possible errors.

0f98d75

Remove own docker build

937f89c

Shell Call no longer used, since swapped image conversion to Image Ma…

a08a795

…gick. Rollback IOsInfo.cs

Merge branch 'Kareadita:develop' into JPEG-XL_JPG2000_HEIF

bc8481c

majora2007 reviewed Aug 29, 2024

View reviewed changes

maxpiva added 2 commits August 29, 2024 19:22

Edit CodeStyle

e7f021a

Fix GetCoverImage, trying to create a temporary image in current directory, and rename the parameter field name, that generates a wrong asumption.

Remove custom docker builder

c624287

maxpiva and others added 2 commits August 30, 2024 17:59

Fixed discovered issues.

6dbeaf1

Optimize Cover Creation

Merge branch 'Kareadita:develop' into JPEG-XL_JPG2000_HEIF

fead3f6

Merge branch 'Kareadita:develop' into JPEG-XL_JPG2000_HEIF

bf30989

majora2007 added the accepted A feature request that is accepted label Oct 3, 2024

maxpiva closed this Oct 4, 2024

maxpiva mentioned this pull request Oct 29, 2024

[Canary] Transcoding Support + More Image Format Support (JPEGXL) #3250

Closed

Added Dynamic Conversion of JPEG-XL, JPEG 2000, HEIF and AVIF to JPG when the browser does not support them. #3141

Added Dynamic Conversion of JPEG-XL, JPEG 2000, HEIF and AVIF to JPG when the browser does not support them. #3141

Conversation

maxpiva commented Aug 29, 2024

Added

Missing

majora2007 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DieselTech commented Aug 29, 2024

maxpiva commented Aug 29, 2024

maxpiva commented Aug 29, 2024 • edited Loading

DieselTech commented Aug 29, 2024

maxpiva commented Aug 29, 2024 • edited Loading

DieselTech commented Aug 29, 2024 • edited Loading

maxpiva commented Aug 30, 2024

majora2007 commented Sep 23, 2024

maxpiva commented Sep 23, 2024 • edited Loading

majora2007 commented Sep 23, 2024

maxpiva commented Sep 23, 2024 • edited Loading

maxpiva commented Sep 29, 2024 • edited Loading

majora2007 commented Sep 30, 2024

maxpiva commented Oct 2, 2024

majora2007 commented Oct 3, 2024

maxpiva commented Oct 3, 2024

majora2007 commented Oct 3, 2024

majora2007 commented Oct 3, 2024

majora2007 commented Oct 3, 2024

maxpiva commented Oct 3, 2024 • edited Loading

majora2007 commented Oct 3, 2024

kleisauke commented Oct 23, 2024 • edited Loading

maxpiva commented Oct 23, 2024

kleisauke commented Oct 23, 2024

majora2007 commented Oct 23, 2024

maxpiva commented Oct 23, 2024 • edited Loading

kleisauke commented Oct 29, 2024

maxpiva commented Oct 30, 2024

maxpiva commented Aug 29, 2024 •

edited

Loading

maxpiva commented Aug 29, 2024 •

edited

Loading

DieselTech commented Aug 29, 2024 •

edited

Loading

maxpiva commented Sep 23, 2024 •

edited

Loading

maxpiva commented Sep 23, 2024 •

edited

Loading

maxpiva commented Sep 29, 2024 •

edited

Loading

maxpiva commented Oct 3, 2024 •

edited

Loading

kleisauke commented Oct 23, 2024 •

edited

Loading

maxpiva commented Oct 23, 2024 •

edited

Loading