Description
Status | Subtype | Assigned | Task | ||
---|---|---|---|---|---|
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 |
Event Timeline
More examples from different sources:
https://commons.wikimedia.org/wiki/File:Recycling_Center_reduces_tons_of_waste_output_130626-M-ZH183-010.jpg
https://commons.wikimedia.org/wiki/File:Bald_eagle_vocalizing_at_Eastern_Neck_National_Wildlife_Refuge._(21818868859).jpg
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.
6.8.0 | |
6.9.0 | |
7.0.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.
6.8.4 is the first buggy version:
6.8.3 | |
6.8.4 | |
7.0.0 is the first fixed version:
6.9.6 | |
7.0.0 | |
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).
If we can pinpoint this to a specific commit in 7, which is backportable to 6.9.6.2 from jessie, we can work with the Debian maintainer to backport the fix into a Debian point release.
It appears that the vast majority of commits to ImageMagick have no commit message at all… I suppose I could try to bisect it to find the fix. I wonder how long it takes to compile.
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).
The Thumbor servers are on Debian Jessie, which currently uses IM version 8:6.8.9.9-5+deb8u5: https://packages.debian.org/jessie/imagemagick
Which means 6.8.9.9 with patches applied in the Debian package.
I see nothing in http://metadata.ftp-master.debian.org/changelogs/main/i/imagemagick/imagemagick_6.8.9.9-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 :)
Yes, I was doing sharpening-only as a "minimal test case" of sorts. Thanks. So I guess just waiting on Thumbor is also an option to solve this.
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.
OK, upstream have released ImageMagick 6.9.6-5 with a fix for the issue. https://github.com/ImageMagick/ImageMagick/issues/299#issuecomment-260824152
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.
A build with the backported patch is now at https://people.wikimedia.org/~jmm/imagemagick/
That fixes the test cases from https://github.com/ImageMagick/ImageMagick/issues/299 in my tests
Mentioned in SAL (#wikimedia-operations) [2016-11-17T07:57:54Z] <moritzm> uploaded imagemagick 8:6.8.9.9-5+deb8u5+wmf1 to carbon (T141739)
Mentioned in SAL (#wikimedia-operations) [2016-11-17T10:20:16Z] <moritzm> upgrading imagemagick on mw1293 (T141739)
Mentioned in SAL (#wikimedia-operations) [2016-11-17T12:50:41Z] <moritzm> upgrading imagemagick on remaining image scalers (T141739)
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.
@Gilles @MoritzMuehlenhoff Thank you! Everything looks fine on the wikis now. I purged the files linked on this task that were still affected, and they thumbnail correctly now.
imagescalers and thumbors upgraded from imagemagick 8:6.8.9.9-5+deb8u5+wmf1 to imagemagick 8:6.8.9.9-5+deb8u6+wmf1
security upgrade https://www.debian.org/security/2016/dsa-3726 and our patch here for this on top of it again
I've filed https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=856639 to get this integrated into Debian jessie as well.
The update has been accepted by the Debian stable release managers, so starting with the next jessie update we can use the stock Debian packages again.
Have we reverted, or do we have a regression on our hand?
https://commons.wikimedia.org/wiki/File:Logo_Tver.jpg
Full version | Commons thumbnail |
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.
Restoring previous task status per previous comment.
@Josve05a: Please create a separate task - thanks!
I don't actually see this problem on that file. Looks like it was uploaded before this task was resolved, so it might have just been an old broken thumbnail that no one purged.