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.

Sign up for GitHub

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

Jump to bottom

Add support for JPEG XL image format #41058

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Add support for JPEG XL image format #41058

wants to merge 15 commits into from

Conversation

1div0
Copy link

@1div0 1div0 commented Oct 23, 2023

JPEG XL support

This PR is intended for adding support of the JPEG XL image format in the Nextcloud.

solracsf
solracsf reviewed Oct 23, 2023
View reviewed changes
apps/dav/lib/Avatars/AvatarNode.php Outdated
@@ -61,6 +61,8 @@ public function get() {
ob_start();
if ($this->ext === 'png') {
imagepng($res);
} else if ($this->ext === 'jxl') {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else if ($this->ext === 'jxl') {
} elseif ($this->ext === 'jxl') {

@miaulalala
Copy link
Contributor

Looks like browser support is really not there for jxl: https://caniuse.com/jpegxl

So this would likely lead to broken images for everyone but Safar users if we support it.

@1div0
Copy link
Author

1div0 commented Oct 23, 2023

Thorium supports JPEG XL OOB.

I am waiting for the day when Google will reverse that mistaken decision.

https://bugs.chromium.org/p/chromium/issues/detail?id=1451807

@1div0 1div0 marked this pull request as draft October 25, 2023 10:50
1div0 added 3 commits October 26, 2023 10:45
@1div0
Add support for JPEG XL image format
f5c4c14 
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
@1div0
Changed according to review comment
77bca06 
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
@1div0
Added JPEG XL MIME type
7b99332 
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
@1div0
Copy link
Author

1div0 commented Oct 26, 2023

BTW JPEG XL could be transparently converted to JPEG on the fly in order to save server storage space.

@solracsf solracsf added this to the Nextcloud 28 milestone Oct 27, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
@1div0
Add JPEG XL to MIME type list
9e0839c 
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
@tcitworld
Copy link
Member

Looks like browser support is really not there for jxl: https://caniuse.com/jpegxl

So this would likely lead to broken images for everyone but Safar users if we support it.

We can just add support for previewing JXL files (all previews are png in the browser) for now, but not support handling it in other places as this PR tries to do. Just like Krita files for instance.

1div0 and others added 2 commits November 2, 2023 17:17
@1div0
Trying to resolve merge conflicts
a243d3b 
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
@1div0
Merge branch 'master' into JPEG XL
05f6ab9
github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 2, 2023
View reviewed changes
apps/dav/lib/Avatars/AvatarNode.php
@@ -61,6 +61,8 @@
ob_start();
if ($this->ext === 'png') {
imagepng($res);
} elseif ($this->ext === 'jxl') {
imagejxl($res);

Check failure

Code scanning / Psalm

UndefinedFunction

Function OCA\DAV\Avatars\imagejxl does not exist
apps/theming/lib/ImageManager.php
@@ -254,6 +254,10 @@
if (!imagejpeg($outputImage, $newTmpFile, 90)) {
throw new \Exception('Could not recompress background image as JPEG');
}
} else if (str_contains($detectedMimeType, 'image/jxl')) {
if (!imagejxl($outputImage, $newTmpFile, 90)) {

Check failure

Code scanning / Psalm

UndefinedFunction

Function OCA\Theming\imagejxl does not exist
lib/private/legacy/OC_Image.php
@@ -273,6 +273,9 @@
case 'image/jpeg':
$imageType = IMAGETYPE_JPEG;
break;
case 'image/jxl':
$imageType = IMAGETYPE_JXL;

Check failure

Code scanning / Psalm

UndefinedConstant

Const IMAGETYPE_JXL is not defined
lib/private/legacy/OC_Image.php
@@ -297,6 +300,9 @@
imageinterlace($this->resource, (PHP_VERSION_ID >= 80000 ? true : 1));
$retVal = imagejpeg($this->resource, $filePath, $this->getJpegQuality());
break;
case IMAGETYPE_JXL:

Check failure

Code scanning / Psalm

UndefinedConstant

Const IMAGETYPE_JXL is not defined
lib/private/legacy/OC_Image.php
@@ -297,6 +300,9 @@
imageinterlace($this->resource, (PHP_VERSION_ID >= 80000 ? true : 1));
$retVal = imagejpeg($this->resource, $filePath, $this->getJpegQuality());
break;
case IMAGETYPE_JXL:
$retVal = imagejxl($this->resource, $filePath, $this->getJxlQuality());

Check failure

Code scanning / Psalm

UndefinedFunction

Function imagejxl does not exist
lib/private/legacy/OC_Image.php
@@ -297,6 +300,9 @@
imageinterlace($this->resource, (PHP_VERSION_ID >= 80000 ? true : 1));
$retVal = imagejpeg($this->resource, $filePath, $this->getJpegQuality());
break;
case IMAGETYPE_JXL:
$retVal = imagejxl($this->resource, $filePath, $this->getJxlQuality());

Check failure

Code scanning / Psalm

UndefinedMethod

Method OC_Image::getJxlQuality does not exist
lib/private/legacy/OC_Image.php
@@ -388,6 +395,10 @@
$quality = $this->getJpegQuality();
$res = imagejpeg($this->resource, null, $quality);
break;
case "image/jxl":
$quality = $this->getJxlQuality();

Check failure

Code scanning / Psalm

UndefinedMethod

Method OC_Image::getJxlQuality does not exist
lib/private/legacy/OC_Image.php
@@ -388,6 +395,10 @@
$quality = $this->getJpegQuality();
$res = imagejpeg($this->resource, null, $quality);
break;
case "image/jxl":
$quality = $this->getJxlQuality();
$res = imagejxl($this->resource, null, $quality);

Check failure

Code scanning / Psalm

UndefinedFunction

Function imagejxl does not exist
@1div0
Copy link
Author

1div0 commented Nov 5, 2023

I will check and try to add JPEG XL preview if the browser advertizes image/jxl support ASAP.

@ChristophWurst ChristophWurst added the 2. developing Work in progress label Nov 6, 2023
This was referenced Nov 6, 2023
Merged
Merged
kesselb
kesselb reviewed Nov 12, 2023
View reviewed changes
lib/private/legacy/OC_Image.php
@@ -297,6 +300,9 @@ private function _output(?string $filePath = null, ?string $mimeType = null): bo
imageinterlace($this->resource, (PHP_VERSION_ID >= 80000 ? true : 1));
$retVal = imagejpeg($this->resource, $filePath, $this->getJpegQuality());
break;
case IMAGETYPE_JXL:
Copy link
Contributor

Choose a reason for hiding this comment

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

Our OC_Image class is a wrapper for php's gd extension. The constant IMAGETYPE_JXL or the function imagejxl can only exist if the gd library get's support for jxl. libgd -> php-gd -> OC_Image.

Upstream feature request: libgd/libgd#699

@kesselb kesselb removed this from the Nextcloud 28 milestone Nov 12, 2023
@solracsf solracsf added this to the Nextcloud 28 milestone Nov 21, 2023
@blizzz blizzz mentioned this pull request Nov 22, 2023
Merged
5 tasks
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@kesselb kesselb removed this from the Nextcloud 29 milestone Nov 23, 2023
@kesselb
Copy link
Contributor

kesselb commented Nov 23, 2023

@solracsf Please don't assign a milestone right now. In the current state, the pull request is far from ready.

@1div0 Is there a reason that you rebase this branch every few days without changing anything? This will always trigger a notification for subscribed people and trigger a CI run.

@1div0
Copy link
Author

1div0 commented Nov 23, 2023

I am still working locally on the GD library JPEG XL support. Will commit ASAP.

I apologize for the noise.

@kesselb
Copy link
Contributor

kesselb commented Nov 23, 2023

Thank you for your efforts in moving this topic forward 👍

@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Dec 3, 2024

@1div0 any news regarding the GD library JPEG XL support ? :)

@1div0
Copy link
Author

1div0 commented Dec 3, 2024

0 ATM, maybe next year++

@skjnldsv skjnldsv added this to the Nextcloud 32 milestone Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kesselb kesselb kesselb left review comments

@solracsf solracsf solracsf left review comments

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@miaulalala miaulalala Awaiting requested review from miaulalala

@tcitworld tcitworld Awaiting requested review from tcitworld

@hamza221 hamza221 Awaiting requested review from hamza221 hamza221 will be requested when the pull request is marked ready for review hamza221 is a code owner

@SebastianKrupinski SebastianKrupinski Awaiting requested review from SebastianKrupinski SebastianKrupinski will be requested when the pull request is marked ready for review SebastianKrupinski is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
2. developing Work in progress
Projects
None yet
Milestone
Nextcloud 32
Development

Successfully merging this pull request may close these issues.
9 participants
@1div0 @miaulalala @tcitworld @kesselb @skjnldsv @solracsf @ChristophWurst @susnux @blizzz