Page MenuHomePhabricator

Add support for using graphicsmagick instead of imagemagick for thumbnailing
Open, LowPublic

Description

Author: buzz

Description:
Graphicsmagick (http://www.graphicsmagick.org/) is a fork of imagemagick, that is smaller, faster and has more api stability. It also seems to have improved documentation (or at least it has examples and descriptions in the man file that imagemagick does not)

example performance chart:

http://rmagick.rubyforge.org/resizing-charts.html

it comes with a compatibility package to provide the imagemagick convert/mogrify etc commands but has dropped some of the imagemagick shortcuts like "-thumbnail"

to thumbnail on graphicsmagick you would do

gm convert -size 120x120 cockatoo.jpg -resize 120x120 +profile "*" thumbnail.jpg

the -thumbnail on imagemagick is a shortcut to "-resize 120x120 -strip". GraphicsMagick integrates the -strip with the profile option that already is there to manage image profiles

It is quite easy to add support for this. I made a quick patch for my system, however I would think it might be better to actually replace imagemagick with graphicsmagick (for the reasons listed above). If it was to be added as an option, would you prefer a new set of config options or to modify how the current ones are used? also would you prefer it was handled in a separate "if" block or merged with the current imagemagick support?

I vote for switching to graphicsmagick anyway.


Version: unspecified
Severity: enhancement

Details

Reference
bz19073

Event Timeline

bzimport raised the priority of this task from to Low.
bzimport set Reference to bz19073.
bzimport added a subscriber: Unknown Object (MLST).
bzimport created this task.Jun 3 2009, 11:04 PM

buzz wrote:

I have just seen that in defaultsettings it refers to using customconvert command for example with graphicsmagick. However, this doesn't make it as easy as not everyone will know about the various workaround in imagemagick (such as white background for older browsers on transparent images), that would also apply to graphicsmagick.

buzz wrote:

patch to implement a graphicsmagick option

Here is my patch against 1.14.0 that implements a new variable $wgUseGraphicsMagick.
It uses the config values from the imagemagick config like

$wgUseGraphicsMagick = true;
$wgImageMagickConvertCommand = "/usr/bin/gm";

This is just one way to implement it, and I'm not suggesting it is the best way.
it is more useful than using the custom command as it handles the differing compression for png/jpeg quality as well as other workarounds for animated gifs etc.

attachment diff.txt ignored as obsolete

buzz wrote:

add graphicsmagick option

I had accidentally replaced thumbnail with resize for imagemagick. oops

attachment diff.txt ignored as obsolete

Patches should be against trunk (most recent development version). 1.14.0 is based on r45489, while trunk is at r51464 (1.16alpha). You patch does not apply in a clean way. Please provide an updated patch.

buzz wrote:

Sorry. I didn't consider that it would be accepted anyway, but for reference so get any feedback on what would be the "right" way to implement this (and to get comments on whether it should be the default)

buzz wrote:

add graphicsmagick option

Updated patch to apply against trunk

attachment diff.txt ignored as obsolete

Thanks. The patch applies cleanly now. Added keyword 'need-review'.

buzz wrote:

add option for graphicsmagick support

cleanup. was missing a tab

Attached:

Hm, it'd probably be wise to break up the thumbnailing code to make it easier to extend. We've got a loooot of things in this big giant if/switch block. :)

Bryan.TongMinh wrote:

(In reply to comment #9)

Hm, it'd probably be wise to break up the thumbnailing code to make it easier
to extend. We've got a loooot of things in this big giant if/switch block. :)

Do you suggest not applying this patch until somebody rewrites that code or apply it and hope somebody will clean it up someday?

buzz wrote:

I'm still happy to rework the patch. just got a notification as this bug was tweaked and reverted perhaps by accident, but I think this is still a good issue. for my and my wiki personally, the switch was a significant performance increase.

Bryan.TongMinh wrote:

(In reply to comment #10)

(In reply to comment #9)
> Hm, it'd probably be wise to break up the thumbnailing code to make it easier
> to extend. We've got a loooot of things in this big giant if/switch block. :)
>
Do you suggest not applying this patch until somebody rewrites that code or
apply it and hope somebody will clean it up someday?

Note, did this some months ago. So the patch won't apply without significant rework effort.

sumanah wrote:

CC'ing a few people to ask whether this question/issue something we should follow up on.

(In reply to comment #13)

CC'ing a few people to ask whether this question/issue something we should
follow up on.

You mean supporting graphikmagic? I would call it a low priority issue. If someone wants to write support/adapt the patch on this bug, by all means, but we already support several thumbnailers, so not something to spend large effort on, unless someone is specifically independently interested

buzz wrote:

it's very similar to imagemagick so I don't think it should be a massive issue to integrate it. I already made a patch before (here) which I realise is no longer good with the current codebase. I may well update it, but then, it would be good to know if it would be accepted so I'm not wasting any time. It's worth noting, when using graphicsmagick before, thumbnailing was significantly faster on my hardware at the time, however this was some time ago.

I think it would be worth changing the focus of any patch to the definition of a global with a string input rather than multiple booleans, similar to the way we handle SVG conversion at the moment. Then, $wgUseImageMagick could be deprecated and the whole thing would be more future proof.

If that were the case, I can't see any reason not to merge a patch that added support for an alternative renderer but didn't even seek to make it the default.

Gilles added a subscriber: Gilles.
Restricted Application added subscribers: Matanya, Aklapper. · View Herald TranscriptJul 23 2015, 12:22 AM
Restricted Application added a subscriber: Steinsplitter. · View Herald TranscriptAug 4 2015, 4:37 PM
Gilles claimed this task.Aug 4 2015, 4:39 PM

As per some quick tests, it seems like GM is vastly superior than IM for our PNG thumbnailing command, but significantly worse than IM for JPGs (at least for the stable versions in our environment). Therefore it would make sense to start using GM in lieu of IM piecemeal, for specific formats.

Krinkle: is the plan to use testing to make sure this definitely is a perf benefit?

@Jarry1250 As @Gilles has already shown in the comment before you, Yes we're doing our own comparisons and benchmarks to see what the benefits are of GM for Wikimedia. And unless there are reasons not to, we'll gradually start using it where it makes sense.

@Jarry1250 As @Gilles has already shown in the comment before you, Yes we're doing our own comparisons and benchmarks to see what the benefits are of GM for Wikimedia. ...

Yup (my previous msg was a little more brief than I intended, sorry). Gilles' linked task, T107885, describes a test run on two images (a large PNG and a large JPG). I don't think it would be too tricky to run against a representative sample of hundreds or low thousands of PNGs to improve confidence in the general result. Of course one could do so in the wild by implementing the change and watching the stats, but --

...And unless there are reasons not to, we'll gradually start using it where it makes sense.

`-- per my comment above, I think some work would be necessary to rationalise the way shelled-out-to image handlers are defined in the config before one could usefully implement the distinction between JPG and PNG handling Gilles' research implies. At the moment we have a confusing mixture of boolean switches and command line strings for general images and SVGs, which IMHO isn't particularly scalable. So I think it might be better to do some large scale PNG testing before investing time in changing the config structure.

I'd argue that the messy way the code is organized is actually an argument for cleaning things up regardless and having a saner per-format logic, and that time is better invested doing so than setting up a larger scale test. The worst that can happen is that GM doesn't perform better and we've made our code more flexible and future-proof.

As you said, we can "do it live" to figure out if it performs better. It's the only way to be really sure, in fact, because regardless of how cleverly you pick your test images, or how large the sample is, it will always be an approximation of reality, and the real deal is what matters.

Most of the work required here is code cleanup and reorganization. If our code was better organized, testing GraphicsMagick would only amount to switching from a "convert" command to "gm convert" with exactly the same parameters. Unfortunately, as you know, we're far from that.

Testing new scaler software for a particular format in our code should be easier. Then we wouldn't ask ourselves about creating synthetic tests, we'd just switch some wikis to the new software and measure the impact.

Most of the work required here is code cleanup and reorganization. If our code was better organized, testing GraphicsMagick would only amount to switching from a "convert" command to "gm convert" with exactly the same parameters. Unfortunately, as you know, we're far from that.

Perhaps I missed something - I agree that the scalar code is spaghetti, but if GM is compatible with all IM switches, why can't we just do that?

Gilles added a comment.EditedAug 26 2015, 7:53 AM

Most of the work required here is code cleanup and reorganization. If our code was better organized, testing GraphicsMagick would only amount to switching from a "convert" command to "gm convert" with exactly the same parameters. Unfortunately, as you know, we're far from that.

Perhaps I missed something - I agree that the scalar code is spaghetti, but if GM is compatible with all IM switches, why can't we just do that?

Because it only seems to benefit PNGs. At equal performance, I'd rather stick with IM, which is likely to have better support and faster bugfixes, for JPGs. Currently switching the IM command to GM would affect all formats, afaik.

Oh I got you. I was thinking as a drop in replacement for im over everything.

Gilles added a comment.EditedAug 26 2015, 7:57 AM

We could do that as a temporary test, like running GM on Commons for a day. It would confirm on a large scale whether it's indeed not better than IM for JPGs and better than IM for PNGs. If what I've seen is confirmed, though, we're back to square 1 with the same amount of work to do.

Well there is a hook one could use to handle it all as an extension... but it would be nice to eventually reduce that mess instead of just keep piling on alternate paths.

Jdforrester-WMF moved this task from Untriaged to Backlog on the Multimedia board.Sep 4 2015, 6:15 PM
Gilles removed Gilles as the assignee of this task.Sep 7 2015, 7:47 PM

I'm putting this back on the back-burner since I'm now more inclined to explore a thumbnailing solution that would completely bypass Mediawiki (and would happen to have GraphicsMagick as one of its possible engines, the other ones being OpenCV and PIL): T111718: Service-based thumbnailing re-architecture on Vagrant

Gilles set Security to None.
Restricted Application added a project: Commons. · View Herald TranscriptDec 4 2015, 2:15 PM

Is GraphicsMagick supported? This task indicates that it is not, but this mediawiki.org page indicates it is.

Restricted Application added a subscriber: Poyekhali. · View Herald TranscriptNov 2 2016, 2:03 AM

There's no test coverage for it, I don't think there's any guarantee that everything will work with it. Mediawiki.org states that you can use it, but offers no guarantees about what will work. GM tried to be command-compatible with IM but there can be some syntax differences. I think I recall that it worked by just pointing to the GM commands in the mediawiki configuration, but I didn't try all the thumbnailing options and scenarios.

gpaumier removed a subscriber: gpaumier.Jul 18 2018, 6:02 PM