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 memories.image.highres/convert_all_images_formarts/format/quality/_max_x/_max_y/ #653

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

add memories.image.highres/convert_all_images_formarts/format/quality/_max_x/_max_y/ #653

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e613076
add memories.preview.format, memories.preview.quality, memories.previ…
JanisPlayer May 17, 2023
21b8a28
Remove AVIF Benchmark | Understandable settings
JanisPlayer May 18, 2023
437e00b
remove user input checks, change getSystemValueString and use getImag…
JanisPlayer May 18, 2023
267a0e6
Remove a comment
JanisPlayer Aug 27, 2023
3e6ffcc
Merge branch 'master' into master
JanisPlayer Aug 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 66 additions & 4 deletions lib/Controller/ImageController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ public function decodable(string $id): Http\Response
$blob = $file->getContent();

// Convert image to JPEG if required
if (!\in_array($mimetype, ['image/png', 'image/webp', 'image/jpeg', 'image/gif'], true)) {
$highres_enabled = $this->config->getSystemValueString('memories.image.highres_convert_all_images_formarts_enabled', 'false');
Copy link
Owner

Choose a reason for hiding this comment

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

This is not desirable. What we really want is a cap on the resolution, so the original image should be used to determine if we want to scale it.

E.g. if the cap is set to 12MP, then scaling a 4MP image is meaningless (even decoding it is a resource waste)

Copy link
Owner

Choose a reason for hiding this comment

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

Also need to always exclude GIF, since webp animation depends on library support

Copy link
Author

@JanisPlayer JanisPlayer May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I just noticed it when testing that the webp image wasn't really animated.
Then I wrote code for each case.
But it was so terribly big that I deleted it and the best was this:

            // Convert image to JPEG if required
            //You might want to lower the maximum execution time here.
            //And increase again, to the default value, when the image is finished.
            //And set a maximum number of concurrent executions, which might prevent thrashing.
            //JSON(tmp) Database where you just save the entries as a number and delete them when they are done or the entries are 5 minutes old.
            $highres_enabled = $this->config->getSystemValueString('memories.image.highres.convert_all_images_formarts_enabled', 'false');
            $format = $this->config->getSystemValueString('memories.image.highres.format', 'jpeg');
            if ($highres_enabled == 'true') {
              switch ($format) {
                case 'jpeg':
                  if (!\in_array($mimetype, ['image/png', 'image/webp', 'image/gif'], true)) {
                      [$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
                  }
                  break;
                case 'webp':
                  if (!\in_array($mimetype, ['image/gif'], true)) {
                      [$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
                  }
                  break;
              }
            } else {
              if (!\in_array($mimetype, ['image/png', 'image/webp', 'image/jpeg', 'image/gif'], true)) {
                [$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
              }
              // why did i do that? :D
              /*Set maximum width and height
              $maxWidth = (int)$this->config->getSystemValue('memories.image.highres_max_x', '0');
              $maxHeight = (int)$this->config->getSystemValue('memories.image.highres_max_y', '0');

              if (!\in_array($mimetype, ['image/png', 'image/webp', 'image/jpeg', 'image/gif'], true)) {
                // Check if the image exceeds the maximum resolution
                //$imageInfo = getimagesize($blob); //Dont work with a Blob need Imagick
                //$width = $imageInfo[0];
                //$height = $imageInfo[1];
                
                $img = imagecreatefromstring($blob); //Better als Imagick?
                $width = imagesx($img);
                $width = imagesy($img);

                if ($maxWidth > 0 && $maxHeight > 0) {
                  if ($width > $maxWidth || $height > $maxHeight) {
                    [$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
                  }
                } else {
                  [$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
                }
              }*/
            }

I think I should go to sleep. :D

Have to admit, if a user loads this with always always full screen, he has a PC from the future.
PHP makes an effort to let the script run as long as possible as it is in the config and thus allows an infinite number of calls.
So always full you should be very careful.
But I think it's more of an experimental feature anyway.

Thank you for your support. :)
Learned some new things, using the bitmap to divide the image into parts, which google does, was very interesting.
I try to finish adapting the code after sleeping.
I think you've tried what I do before, heard the experience? :D

Edit: I added the funny code, so you should always sleep enough, otherwise you will do such funny things. :D

Update:
https://imagemagick.org/script/resources.php
https://www.php.net/manual/de/imagick.setresourcelimit.php
putenv("MAGICK_THREAD_LIMIT=maxcores");
$image->setResourceLimit($image::RESOURCETYPE_THREAD, maxcores);
putenv("MAGICK_THROTTLE_LIMIT=limit");
putenv("MAGICK_TIME_LIMIT=sec");
These parameters can be used to speed up the process and set a timeout.
However, a limit must be set for conversions, database entries, or a JSON file.
This stores the current number of running conversions and the timestamp of the last one, deleting the value after the timeout period. Resetting after a certain time helps avoid errors during server restarts.
This was the only way I could prevent server overload due to multiple users.
However, it does require a few lines of code and is only useful in environments that need protection against such attacks or have a high number of users using this function.
The resolutions are stored in the database for each image, for whatever purpose I needed them back then.

if (!\in_array($mimetype, ['image/png', 'image/webp', 'image/jpeg', 'image/gif'], true) || $highres_enabled == 'true') {
[$blob, $mimetype] = $this->getImageJPEG($blob, $mimetype);
}

Expand Down Expand Up @@ -377,14 +378,75 @@ private function getImageJPEG($blob, $mimetype): array
} catch (\ImagickException $e) {
throw Exceptions::Forbidden('Imagick failed to read image: '.$e->getMessage());
}

// Convert to JPEG
try {
$image->autoOrient();
$image->setImageFormat('jpeg');
$image->setImageCompressionQuality(95);
$format = $this->config->getSystemValueString('memories.highres_format', 'jpeg');
Copy link
Owner

Choose a reason for hiding this comment

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

Util::getSystemConfig. Also see Util::systemConfigDefaults.

The config values need to be correctly scoped, e.g. memories.image.highres.format

switch ($format) {
case 'jpeg':
$format = 'jpeg';
break;
case 'webp':
$format = 'webp';
break;
/*case 'avif': //CPU Benchmark
//$format = 'avif';
break*/
default:
$format = 'jpeg';
}
Copy link
Owner

Choose a reason for hiding this comment

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

Don't need this check. If the admin sets the value to something invalid, it's not our problem.

$image->setImageFormat($format);

$quality = (int)$this->config->getSystemValue('memories.image.highres_quality', '95');
if ($quality < 0 || $quality > 100) {
//throw Exceptions::Forbidden('Warning: You have set an invalid quality value for image conversion');
}
$image->setImageCompressionQuality($quality);

// Set maximum width and height
$maxWidth = (int)$this->config->getSystemValue('memories.image.highres_max_x', '0');
$maxHeight = (int)$this->config->getSystemValue('memories.image.highres_max_y', '0');

// Get current dimensions
$width = (int)$image->getImageWidth();
$height = (int)$image->getImageHeight();

// Calculate new dimensions while maintaining aspect ratio
if ($maxWidth > 0 && $maxHeight > 0) {
if ($width > $maxWidth || $height > $maxHeight) {
$aspectRatio = $width / $height;
if ($width > $height) {
$newWidth = $maxWidth;
$newHeight = $maxWidth / $aspectRatio;
} else {
$newHeight = $maxHeight;
$newWidth = $maxHeight * $aspectRatio;
}
Copy link
Owner

Choose a reason for hiding this comment

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

$image->scaleImage(maxh, maxw, true); does the same thing.

// Resize the image
$image->scaleImage((int)$newWidth, (int)$newHeight);
}
}

$blob = $image->getImageBlob();
$mimetype = $image->getImageMimeType();

//getImageMimeType() dont work for webp you can use pathinfo() and strtolower() but i make it shorter
Copy link
Owner

Choose a reason for hiding this comment

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

getImageMimeType seems to work fine for me. What does it return for you? Imagick version?

Copy link
Author

Choose a reason for hiding this comment

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

ImageMagick 6.9.11-60 Q16 x86_64 2021-01-25 https://imagemagick.org
is already the newest version (8:6.9.11.60+dfsg-1.3ubuntu0.22.04.3)
imagemagick/jammy-updates,jammy-security 8:6.9.11.60+dfsg-1.3ubuntu0.22.04.3 i386
imagemagick/jammy 8:6.9.11.60+dfsg-1.3build2 i386
It doesn't work, I don't know why.
But actually that's not bad, even if it's missing which format it is, the browser still shows it.
I've also tried installing 7.1, but it won't work with PHP, nor have I been able to test it.
But when I save the images in the browser, they do so without formatting.
But he still knows that it is an image.

$extension = pathinfo("file.".$format, PATHINFO_EXTENSION);
$extension = strtolower($extension);

$mimeTypes = [
'jpeg' => 'image/jpeg',
'jpg' => 'image/jpeg',
'webp' => 'image/webp',
'avif' => 'image/avif',
];

if (isset($mimeTypes[$extension])) {
$mimetype = $mimeTypes[$extension];
}

//$mimetype = 'image/'.$format;

} catch (\ImagickException $e) {
throw Exceptions::Forbidden('Imagick failed to convert image: '.$e->getMessage());
}
Expand Down