Last modified: 2013-07-25 07:04:29 UTC
Installed the extension on my local test wiki running from master. Got the extension through a git clone of the PagedTiffHandler master. After I had set it up, I was no longer able to upload tiff files. Some debugging revealed that PagedTiffHandler.image.php assumed that the page numbers as returned by ImageMagick's identify command started at 1. However, my ImageMagick 6.7.7-2 returns zero-based page numbers (i.e., the first page got "page=0"). As a result, the extension always complained about "inconsistent page numbering in TIFF directory" in PagedTiffInfoParserState::finishPage() on the first page since $this->prevPage is also initialized to zero. I don't know whether other versions of ImageMagick return one-based page numbers. I fixed this locally by hacking in mapping zero-based page numbers to one-based numbers in parseIdentifyOutput(), and then the extension worked as advertized.
Hi Lupo, need to investigate this. In our tests, it worked with page number one. Maybe it is a ImageMagic version issue, but that does not sound right ;) Can you post your workaround here? I would then incorporate this into the code. I think we need to normalize the starting page number according to MediaWiki needs, either to 0 or 1. That should not be a major issue.
Created attachment 10769 [details] ImageMagick identify output on two test files I'd rather not; I really just hacked it in, and I don't really have the time to do this right. I didn't analyze whether this could be fixed simply by initializing $prevPage to -1 instead of zero. I don't know if it's some ImageMagick version thing. I got my ImageMagick through MacPorts; maybe their version does something wrong (or at least differently from other distributions). In any case, I've attached the output of my ImageMagick on two test files from the Commons: http://commons.wikimedia.org/wiki/File:Lehndorf_(Saara)_-_Zentrum.tif http://commons.wikimedia.org/wiki/File:Tochon_d%27Annecy.tiff The first is a single page, the second has two pages.
BTW, note the that the alpha2 values also do not correspond to what the extension tests for.
Lupo, can you add your 'convert -version' info ?
Sure, though I don't see what convert has got to do with it, as the extension uses identify to get the info it wants. Anyway, it's identical to the version of identify: $ convert -version Version: ImageMagick 6.7.7-2 2012-05-28 Q16 http://www.imagemagick.org Copyright: Copyright (C) 1999-2012 ImageMagick Studio LLC Features: OpenCL $
Confirmed, i see the same problem. The issue is in PagedTiffInfoParserState, which has prevPage, which starts at 0, which only works if the first page starts with 1.
Path in https://gerrit.wikimedia.org/r/30621
Created attachment 11272 [details] Patch showing my quick hack mentioned in comment 2 Hi DJ, I've left you some comments on that patch in Gerrit. Since apparently there are quite a few other places that assume 1-based indexing (and no gaps), I have now attached here my quick hack to map 0-based IM output to 1-based internal page numbers. Maybe that's easier than doing it your way, which did look cleaner at first sight, but which might require adapting several other parts. Maybe you can use it, and maybe not. I'm not sure what would be the best way to handle this page numbering cleanly. And perhaps you see a simple way to fix the problems I mentioned in Gerrit.
Interestingly: http://www.awaresystems.be/imaging/tiff/tifftags/pagenumber.html "The first page is numbered 0 (zero)."
I'd like to figure out the exact imagemagick version where this is switched, if anyone has any clues, I'd welcome it.
I think it was this change: http://trac.imagemagick.org/changeset/4014/ImageMagick/trunk/magick/property.c It should have a +1 in line 2815. (Unless the change from 1-based to zero-based indexing was intentional, of course). That would place the change betweeen IM 6.6.8-8 (1-based) and 6.6.8-10 (zero-based) according to http://www.imagemagick.org/script/changelog.php.
New patch in Gerrit (basically attachment 11272 [details]): https://gerrit.wikimedia.org/r/#/c/37654/
