|Resolved||matmarex||T141739 Issues with displaying thumbnails for CMYK JPG images due to buggy version of ImageMagick (black horizontal stripes, black color missing)|
|Resolved||None||T150432 Deploy some fixed version of ImageMagick from apt.wikimedia.org|
- Mentioned In
- T150198: Rendering thumbs of big JPG file fails
T38346: JPEG with CMYK colourspace not thumbnailed correctly
T150432: Deploy some fixed version of ImageMagick from apt.wikimedia.org
- Mentioned Here
- P4555 imagemagick DSA-3726
T26857: New thumbnails are not sharp
T26854: Black/striped thumbnails of CMYK JPEGs
T38346: JPEG with CMYK colourspace not thumbnailed correctly
More examples from different sources:
For batch uploads this presents a bit of a nightmare as the badly thumbnailed files are hard to detect without eating up a lot of bandwith.
https://commons.wikimedia.org/wiki/File:NRCSIL99001_-_Illinois_(4171)(NRCS_Photo_Gallery).jpg might have the same issue. Full image is fine, some thumbnails are fine but https://upload.wikimedia.org/wikipedia/commons/thumb/1/1b/NRCSIL99001_-_Illinois_%284171%29%28NRCS_Photo_Gallery%29.jpg/1536px-NRCSIL99001_-_Illinois_%284171%29%28NRCS_Photo_Gallery%29.jpg is wrong.
Our current version of Thumbor+plugins in production handles those fine:
Since the plan is to have Thumbor handle all thumbnail production traffic at some point this quarter, this task can probably wait for that to happen. If you want to fix this in Mediawiki's thumbnailing code, the first thing I'd check is ImageMagick versions and trying to run the same underlying IM command locally. This feels like an IM bug at first glance and it might just be a case of the Thumbor boxes having a more recent IM than the image scalers.
OK, so I'm going to start by finding out which versions are affected. For a start, I reproduced locally, so this is not specific to the production configuration.
MediaWiki uses a command like this for thumbnailing (enabled $wgDebugLog to have it printed out there):
convert '-quality' '80' '-background' 'white' '-define' 'jpeg:size=480x321' 'NRCSIL99001_-_Illinois_(4171)(NRCS_Photo_Gallery).jpg' '-thumbnail' '480x321!' '-set' 'comment' 'File source: http://localhost:3080/wiki/Plik:NRCSIL99001_-_Illinois_(4171)(NRCS_Photo_Gallery).jpg' '+set' 'Thumb::URI' '-depth' '8' '-sharpen' '0x0.4' '-rotate' '-0' '-sampling-factor' '2x2,1x1,1x1' 'out.jpg'
These versions are definitely buggy:
- ImageMagick 6.8.9-9 (default in Ubuntu 16, and also what we're running in production)
- ImageMagick 6.9.0-0 (random one I had installed locally)
ImageMagick 6.8.0 and ImageMagick 7.0.0 both thumbnail the files correctly, so this was broken in ImageMagick somewhere between 6.8 and 6.9, and fixed between 6.9 and 7.0.
I supposed the solution is to upgrade or downgrade the version we're using. I'll try out some more versions and pinpoint when it was broken/fixed. @Gilles Is Thumbor using the older, or the newer non-broken version? I have no idea where to check this.
To summarize a discussion on wikimedia operations IRC channel:
- ImageMagick 7 API evolved, A porting guide is available.
- A locally maintained package would offer more coherence when we use several OSes to deploy the same version everywhere, at the cost to require package maintenance
- To fix this issue, pending ImageMagick 7 testing, per T141739#2785739, the more careful path should be to use 6.8.3, a version we should recommend on mediawiki.org documentation, and deploy on wmf cluster
Just for fun, here are the thumbnails (120x120px square) that MediaWiki produces for all of the problematic files reported here and on related tasks (except the ones that were since deleted) with each of the four interesting ImageMagick versions (6.7.7 which we used previously and to which we could roll back; 6.8.3 which is the last good one; 6.8.4 which is broken; and 7.0.3 which is the latest).
I tried to bisect between IM 6.8.3-10 and 6.8.4-0 first, hoping I could find the problematic patch and revert it, but the commit introducing this issue is d9f6715841cdad02c37efd64308e85282da580b8, which is somewhat unhelpful for understanding the problem, and doesn't clearly revert on 6.9.6-2.
Looks like this is a part of a chain of commits that leads to 524dfaa16d659a2d5c2ac46c4d4c45f5e61ae23c, adding a release note: "The -blur, -guassian-blur, and -sharpen are now convenience methods for -morphology convolve." So perhaps it was just code cleanup… maybe I'll try a bit harder to revert it.
It also seems that removing '-sharpen' '0x0.4' from the command line clears up the issue. So the minimal test case is:
convert -colorspace CMYK logo: '-sharpen' '0x0.4' logo.jpg
Just sharpening, or just working with CMYK images, is fine:
|convert logo: '-sharpen' '0x0.4'|
|convert -colorspace CMYK logo:|
IM 7 seems to have diverged from IM 6 in 2011 (!), and from what I've seen in the history so far they like to rewrite something important in every other point release, so I doubt that the fix will be backportable even if I do manage to find one. Honestly, I suspect the commits which broke this were just never forwardported to IM 7, since they were done in 2013.
$ git merge-base master ImageMagick-6 1b5421762851bf1649cefc8f5afc4e684f6b68a2 $ git show --stat 1b5421762851bf1649cefc8f5afc4e684f6b68a2 | grep Date Date: Sat Jul 9 16:04:35 2011 +0000 $ git log --oneline 1b5421762851bf1649cefc8f5afc4e684f6b68a2..master | wc -l 7882 $ git log --oneline 1b5421762851bf1649cefc8f5afc4e684f6b68a2..ImageMagick-6 | wc -l 7695
For future reference, because there are no tags for older releases in their Git repo:
- 6.8.3-10 = 8016577473b0eb6bd88c59e7412ed9291e19d671
- 6.8.4-0 = d9bbeb02f3613640092aef9e2277d997ed9986c6
By the way, looking for alternatives: -convolve is broken in exactly the same way (syntax is weird, but try the example from http://www.imagemagick.org/Usage/convolve/#unsharpen). -unsharp is broken in a different but also hopeless way, I'm not even going to look since when. So patching/upgrading/downgrading looks like the only solution.
|Original||-sharpen 0x0.4||-unsharp 0x0.4|
@MoritzMuehlenhoff Here's the best patch I can produce:
This change reverts the functions ConvolveImage(), ConvolveImageChannel(), SharpenImage(), SharpenImageChannel() to exactly how they looked in ImageMagick 6.8.3-10, which is the last version not exhibiting the issue. It applies on top of 6.9.6-2, compiles and as far as I can tell works as expected.
Do you think this would be okay?
Extended table from T141739#2785991, with thumbnails with the patch in the last column:
We should probably report this upstream too. I couldn't find any recent cases on the internet of anyone complaining about sharpening CMYK files with ImageMagick. There are some old cases (including our own tasks), but they indicate the issues were fixed at the time (affecting IM 6.5 and older, mostly).
I see nothing in http://metadata.ftp-master.debian.org/changelogs/main/i/imagemagick/imagemagick_18.104.22.168-5+deb8u5_changelog that would mention this. Perhaps Thumbor just doesn't sharpen the thumbnails?
It does sharpen, but not using the same method as Mediawiki. It currently uses the Python bindings' unsharp_mask() method. Which should be the same as -unsharp in the IM command line.
Specifically, for thumbnails that match the resize ratio condition that triggers sharpening (not all thumbnails get sharpened), it uses the equivalent of -unsharp 0.0x0.8+1.0+0.0 which using DSSIM I found the be the closest visually to -sharpen 0x0.8 which is what's currently used in production.
Hmm, I found -unsharp when looking for alternatives, but I found it to also produce artifacts (different kind though):
Can you put the original image here (generated with convert -colorspace CMYK logo: logo.jpg) through Thumbor and see if the colors of the result look right? Perhaps this is specific to some version, or some values for -unsharp.
-unsharp was also tried in the past, then swiftly reverted: T26857.
I would advise against subjective judgment regarding visual artifacts. This is what algorithms like DSSIM are for. We look closely at status quo thumbnails all the time, which makes us "used" to the kind of artifacts it generates and a new kind of visual artifact looks more wrong to our eyes than it should. While it might actually preserve more - or as much - visual information.
DSSIM is a much more objective way to compare visual variants of the same image.
I'll generate what you've requested in a sec.
Generated in production.
300px coming from Mediawiki:
300px coming from Thumbor:
The softness looks out of place, but the colors look accurate.
Generating an original-sized image with sharpening applied would be kind of irrelevant, because not only we don't currently allow original-sized thumbnails, sharpening is only applied when the resize ratio is greater than a defined minimum. Images whose size is close to the original aren't sharpened.
Um, perhaps you're seeing something else than I do (you never know with these things…), but I'm seeing completely different colors in the processed image than the original. I'm not talking small details :)
In Chromium the colors are identical between the original you've linked to and the thumbor-generated thumbnail, but then again I'm looking at this inside a vmware VM on an external monitor, so yeah, who knows. At least make sure that you're looking at images on the same monitor.
In the 3 images you've provided, the -sharpen one has the same colors and the production Mediawiki example I've provided. And your -unsharp one looks like it has very high contrast, the wizard's suit is almost black instead of blue.
Note that Thumbor, just like Mediawiki currently, leaves color profiles alone when they're not sRGB. That might play a role in having a different experience looking at the same images. And phabricator might mess with that stuff in the embedded previews, make sure to open the actual attachments.
I'll look into a backport to our jessie package currently used on the image scalers, if the patch is self-contained and sane we might be able to get that into the next imagemagick update in jessie.
The backported patch is now deployed on the image scalers, I'll also report this in the Debian bug tracker, ideally we can merge that into the next jessie point release so that we can use unmodified imagemagick builds in the future.
No, that seems like a new/unrelated issue which simply exhibits a similar visual distortion. We're still using the same imagemagick version and the original bug is still fixed, so let's make this a new Phab task.