christophlehmann / imageoptimizer

TYPO3 Extension for lossless image optimization with binaries of your choice.
43 stars 22 forks source link

Integrating jpegoptim as an alternative? #3

Closed slemke76 closed 7 years ago

slemke76 commented 7 years ago

Hi!

very nice tool! I have started playing around with this and mentioned that google pagespeed still complains about my (jpeg) filesize. After some investigation it turned out that reducing the image quality would do the trick (see http://webmasters.stackexchange.com/questions/102094/google-pagespeed-new-image-compression-rules there is a 2017-01 update). Would it be an option to integrate jpegoptim as an alternative with a parameter for the -m switch? Secound point is an option to use the quality switch only on images processed by Typo3 and not on images which are referenced directly.

Regards, Sebastian

christophlehmann commented 7 years ago

Hi Sebastian,

i found out that some perfectly compressed jpgs become a little bit bigger with jpegtran. Switching to jpegoptim will prevent it.

Also the quality switch will be a nice new feature.

Sebobo commented 7 years ago

For the Neos CMS cousin of this package we recently made the libraries and options fully configurable. See https://github.com/mocdk/MOC.ImageOptimizer/blob/master/Configuration/Settings.yaml Maybe that's something this package could also target for at some point in the future. Of course it needs to be a bit different as TYPO3 doesn't have so much yaml configuration ;)

Sebobo commented 7 years ago

@slemke76 can you check of #6 solved this problem for you too?

slemke76 commented 7 years ago

Good evening,

I will test the new version tomorrow, thanks! After a quick look - is there a typo error? In OptimizeImageService.php -> --all-progressiv -> --all-progressive ?

Sebastian

Sebobo commented 7 years ago

Yeah, thanks for spotting! I fixed it.

slemke76 commented 7 years ago

Hi!

I have just tested your #ea6f0db commit.

Some points:

--all-progressive switch: In v1.4.4 the switch exists, on v1.2.3 the switch does not exists (Ubuntu 12 LTS, under support to Apr-2017) It seems to work nevertheless.

-m switch: There must not be a space between the m and the quality: /usr/bin/jpegoptim -m 80 --strip-all --all-progressive /var/www2/114/www/krabbenkorb-sl-live/fileadmin/processed/preview_fruehjahr-loewenzahn_ca98a60784.jpg 2>&1 exited with 0. Output was: /usr/bin/jpegoptim: unrecognized option '--all-progressive' jpegoptim: can't open 80 /var/www2/114/www/krabbenkorb-sl-live/fileadmin/processed/preview_fruehjahr-loewenzahn_ca98a60784.jpg 64x43 24bit JFIF [OK] 3078 --> 1626 bytes (47.17, optimized. (msg#4.0.255) take a look at "jpegoptim: can't open 80" It should be: /usr/bin/jpegoptim -m80 [...]

If both - jpegtran and jpegoptim - are activated, jpegoptim will never being used.

When the external program is not being found a protocol entry would be nice. When the program is not installed there is no feedback like "could not found [...]" (Should I open a new issue for this?)

Perhaps it´s a bit "dangerous" - using jpegoptim on upload would also modify the original image. Perhaps it would be a good idea to use on upload only jpegtran (or better jpegoptim with quality = 100?) and on processing images jpegoptim with the desired level of quality. But I have no idea how to make this behaviour tansparent in the configuration parameters. Perhaps two quality settings? One for uploads, one for processed files?

Thanks, Sebastian

Sebobo commented 7 years ago

Thanks for testing.

Regarding the progressive, I would then just leave it as is if it's only a warning. But maybe it should be configurable if someone doesn't want progressive jpeg and then you could this way also get rid of the warning with older jpegoptim versions.

Regarding the quality setting this is strange. I don't get an error and I tried this on a few systems. But we can just replace it with --max=XYZ then it's better readable and save. And for now we could just have a note in the configuration that you should only have one jpeg lib active. Or maybe it's possible to use a selection instead of two booleans. Not sure.

If a binary is not there we should log a warning, but that's another issue. Please open a new one.

In my opinion on upload we shouldn't modify images at all. Not sure why it's even possible. Space shouldn't be an issue there on todays servers. And FAL might want to read some metadata or another extension.

christophlehmann commented 7 years ago

Hi together,

the progressive switch should be an option, not a default.

I'm not sure about the quality switch for jpegoptim since we can reduce the image quality also with graphicsmagick/imagemagick: $TYPO3_CONF_VARS['GFX']['jpg_quality'] = '80'; Is there a benefit using jpegoptim's quality switch and not gm's? Does someone made tests and can share the results? My tests showed me, that around 1% of jpgs processed with jpegoptim (lossless) after gm were improved with a lower filesize so it's just a small deal.

@Sebobo Not every website uses file meta data so we can drop them on upload. Definitely this logic can be improved to strip the marker after reading them with FAL and other extensions.

christophlehmann commented 7 years ago

As mentioned before jpegtran may makes perfectly compressed image bigger so i think we can drop jpegtran completely, or does anybody wants to keep it?

christophlehmann commented 7 years ago

Would someone check/have a short look at https://github.com/christophlehmann/imageoptimizer/tree/next-release ?

I hope all wishes are done with it:

See https://raw.githubusercontent.com/christophlehmann/imageoptimizer/next-release/Documentation/configuration.png

Sebobo commented 7 years ago

@christophlehmann great thx. I'll ask @dgrammlich to try it in our projects early next week.

Btw. we have it now running in production and it's really a huge improvement 👍