Last modified: 2014-08-09 14:13:13 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T30135, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 28135 - Review and deploy VipsScaler extension
Review and deploy VipsScaler extension
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Extension setup (Other open bugs)
unspecified
All All
: Normal enhancement with 2 votes (vote)
: ---
Assigned To: Jan Gerber
: need-unittest
Depends on: 25990 32721 51149 51370
Blocks: 31235 18826 48003
  Show dependency treegraph
 
Reported: 2011-03-20 15:49 UTC by Bryan Tong Minh
Modified: 2014-08-09 14:13 UTC (History)
15 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Bryan Tong Minh 2011-03-20 15:49:15 UTC
Review and deploy the VipsScaler extension on Wikimedia so that large PNGs can be properly scaled. The VipsScaler supports conditional scaling, so it is possible to configure it to scale only images that match a certain condition (i.e. area > 12.5M)

Such a condition would look like:

$wgVipsConditions[] = array( 'mimeType' => 'image/png', 'minArea' => 12.5e6 );

Note that a the development version 7.25 needs to be installed on the image scalers because older versions do not handle certain PNGs properly. 

Unfortunately there are a lot of dependent phase3 revisions in BitmapHandler, so this can probably not be done until deployment of 1.18.
Comment 1 Tim Starling 2011-03-21 23:43:11 UTC
I added some initial comments to bug 25990.
Comment 2 Tim Starling 2011-10-11 22:15:57 UTC
Outstanding issues:

* $wgMaxImageArea should only be used for scalers which decompress the source image. This is essential if VIPS deployment is going to allow larger PNGs to be uploaded. Needs a little bit of refactoring.
* The "comment" parameter in $scalerParams is not used, so that feature would disappear entirely. Maybe shell out to exiv2 to add comments. $wgExiv2Command is available in core and is used by PagedTiffHandler already.
* A testing feature of some sort would be nice, to allow for some sort of beta testing.
* Consider having PagedTiffHandler call VipsScaler to do the scaling, to reduce code duplication and to get sharpening support in PagedTiffHandler.
Comment 3 Tim Starling 2011-10-13 17:32:39 UTC
The testing feature I'm imagining would have a special page, say Special:VipsTest, which allows you to specify an image name and a destination width. The image would be scaled with VIPS and saved to a temporary extension-specific directory. Then both the ImageMagick and VIPS thumbnails would be displayed to the user by overlaying them with CSS position:absolute, with the visible image toggled by JavaScript to allow for easy visual comparison.
Comment 4 Bryan Tong Minh 2011-10-14 21:02:03 UTC
To make the test feature without hacking into the 404-handler I would need to stream the vips thumbnail through the Special page. Is vips available on all mediawiki servers or only on the image scalers?
Comment 5 Bryan Tong Minh 2011-10-15 12:04:00 UTC
(In reply to comment #2)
> * The "comment" parameter in $scalerParams is not used, so that feature would
> disappear entirely. Maybe shell out to exiv2 to add comments. $wgExiv2Command
> is available in core and is used by PagedTiffHandler already.
>

I've looked into this, but I can't find out what IM does with the comment parameter. I used "exiv2 print" to print all exif information from a Commons thumbnail, but it does not list any exif data.
Comment 6 Tim Starling 2011-10-15 16:43:15 UTC
(In reply to comment #4)
> To make the test feature without hacking into the 404-handler I would need to
> stream the vips thumbnail through the Special page. Is vips available on all
> mediawiki servers or only on the image scalers?

It looks like VIPS will be just on the scalers for now. I think the easiest way to do it is to hit the image scalers using Http::request() with proxy = rendering.svc.pmtpa.wmnet in $options. The URL could just be a secret subpage of the special page, which would cause a thumbnail to be generated with specified parameters. 

(In reply to comment #5)
> I've looked into this, but I can't find out what IM does with the comment
> parameter. I used "exiv2 print" to print all exif information from a Commons
> thumbnail, but it does not list any exif data.

For a JPEG try exiv2 pr -p c <filename>. It's the JPEG image comment rather than the EXIF data. To set it use exiv2 mo -c.
Comment 7 Bryan Tong Minh 2011-10-15 21:52:17 UTC
(In reply to comment #2)
> Outstanding issues:
> 
> * $wgMaxImageArea should only be used for scalers which decompress the source
> image. This is essential if VIPS deployment is going to allow larger PNGs to be
> uploaded. Needs a little bit of refactoring.
r99911 

> * The "comment" parameter in $scalerParams is not used, so that feature would
> disappear entirely. Maybe shell out to exiv2 to add comments. $wgExiv2Command
> is available in core and is used by PagedTiffHandler already.
r99912 

> * Consider having PagedTiffHandler call VipsScaler to do the scaling, to reduce
> code duplication and to get sharpening support in PagedTiffHandler.
Probably not going to have time for this the coming weeks.


(In reply to comment #6)
> (In reply to comment #4)
> > To make the test feature without hacking into the 404-handler I would need to
> > stream the vips thumbnail through the Special page. Is vips available on all
> > mediawiki servers or only on the image scalers?
> 
> It looks like VIPS will be just on the scalers for now. I think the easiest way
> to do it is to hit the image scalers using Http::request() with proxy =
> rendering.svc.pmtpa.wmnet in $options. The URL could just be a secret subpage
> of the special page, which would cause a thumbnail to be generated with
> specified parameters. 
>
If we need to fetch it over HTTP from the scalers, can't the browser just request it directly?
Comment 8 Tim Starling 2011-10-15 22:56:46 UTC
(In reply to comment #7)
> If we need to fetch it over HTTP from the scalers, can't the browser just
> request it directly?

There's no way for the browser to request that a special page request should be handled by the image scalers. They are on an internal network (10.0.0.0/8), they have no public IP. Squid routes thumb.php requests through to the image scalers, and we could add an extra URL regex to it to allow some particular special page request to be routed in the same way, but that seems like the wrong way to handle a one-off test page.
Comment 9 Bryan Tong Minh 2011-10-20 12:32:37 UTC
I started on the VipsTest special page some days ago, but I'm not sure if I have time this weekend to finish this.

In any case we could already enable VipsScaler for large PNGs; those can't be compared to ImageMagick thumbnails anyway. I believe that there are no outstanding issues for this.

Config:

$wgVipsOptions = array(
 array( 
  'conditions' => array(
   'mimeType' => 'image/png',
   'minArea' => 1.25e7,
  )
 )
);
Comment 10 Antoine "hashar" Musso (WMF) 2011-11-02 22:14:15 UTC
FYI, I have created the VipsScaler component in Bugzilla.
Comment 11 Bryan Tong Minh 2011-11-03 08:33:25 UTC
(In reply to comment #10)
> FYI, I have created the VipsScaler component in Bugzilla.

Can you add me as default CC?
Comment 12 Antoine "hashar" Musso (WMF) 2011-11-03 18:12:30 UTC
(In reply to comment #11)
> Can you add me as default CC?
Already did :-)  I have added myself in as well.
Comment 13 Bryan Tong Minh 2011-11-12 17:02:20 UTC
The JavaScript magic requested in comment #3 should be mostly done now.
Comment 14 Antoine "hashar" Musso (WMF) 2011-12-01 14:24:27 UTC
Available on : http://test2.wikipedia.org/wiki/Special:VipsTest
Comment 15 Derk-Jan Hartman 2011-12-04 13:04:17 UTC
question, what does vips do with metadata ?
Comment 16 Bryan Tong Minh 2011-12-04 20:07:06 UTC
(In reply to comment #15)
> question, what does vips do with metadata ?

Nothing.
Comment 17 Derk-Jan Hartman 2011-12-07 10:47:27 UTC
Which m(In reply to comment #16)
> (In reply to comment #15)
> > question, what does vips do with metadata ?
> 
> Nothing.

Does that mean it will strip any existing metadata ? or will it preserve ? I see it does apparently support the comment field, which we use to reference from thumb to the original image page ?
Comment 18 Bryan Tong Minh 2011-12-07 10:56:09 UTC
(In reply to comment #17)
> Which m(In reply to comment #16)
> > (In reply to comment #15)
> > > question, what does vips do with metadata ?
> > 
> > Nothing.
> 
> Does that mean it will strip any existing metadata ? or will it preserve ? I
> see it does apparently support the comment field, which we use to reference
> from thumb to the original image page ?

It is purely a scaler, all the metadata stuff will still be handled by BitmapHandler.
Comment 19 Rainer Rillke @commons.wikimedia 2012-01-13 10:19:31 UTC
Vips is at least able to handle CYMK color model jpgs correctly.

See:
https://test2.wikipedia.org/wiki/Special:VipsTest?file=Jan+hoet.jpg&width=640&sharpen=0.8
Comment 20 MZMcBride 2012-07-10 00:32:15 UTC
What's the status of this bug?
Comment 21 Bryan Tong Minh 2012-07-10 07:38:05 UTC
(In reply to comment #20)
> What's the status of this bug?

We're waiting for somebody to invent a machine that creates free-time out of air for me. 

On a serious note, the last problem we encountered iirc, was that reading PNG images is a WIO operation instead of PIO. I believe that this was fixed in some of the later releases of VIPS, but somebody should benchmark it to check whether that is the case.
Comment 22 Adam Cuerden 2012-12-07 14:41:50 UTC
And, yet again, Wikipedia decides not to do anything to fix pre-made solutions. Really, I do think Wikimedia should hire coders if they can't get by. This is basic competence-to-run-a-website stuff.
Comment 23 Andre Klapper 2012-12-07 17:40:12 UTC
Adam: Please refrain from Bugzilla comments that have nothing specifically to do with the issue that you comment on. Feel free to discuss project prioritities, WMF behavior and high-level problems on https://meta.wikimedia.org/wiki/Wikimedia_Forum . Thanks.
Comment 24 MZMcBride 2012-12-08 03:40:23 UTC
(In reply to comment #22)
> This is basic competence-to-run-a-website stuff.

So basic that you're incapable of resolving this bug yourself. :-)

Bryan is apparently the only person interested in this bug/extension and he's a volunteer with limited free time currently. Until you can find another volunteer (or organization!) to pick up maintenance of this extension, it's going to sit, as it isn't a priority for the Wikimedia Foundation.
Comment 25 Bryan Tong Minh 2012-12-08 13:31:32 UTC
Also, I should note the incredible constructive feedback on this bug...

Just for your information, I can spend my free time fixing bugs for people that are aggressive and impolite, or I can spend my free time on doing things that does not involve dealing with these kind of people. I have decided already some time ago that it is going to be the latter.  So, if you wonder why your comments are being ignored and bugs you report are not fixed, it might not be coincidence.
Comment 27 Adam Cuerden 2012-12-15 04:32:39 UTC
If you seriously think that 7 years waiting for something that's a basic part of my work on Wikipedia to be fixed so that it works isn't going to result in frustration, then I don't know what planet you live on. 

I have never complained that you worked on it. But if you can't do it in about 7 years, with two or three solutions proposed then never fnished up, then clearly this isn't something that can be done by volunteers, and Wikipedia needs to just put funds towards a coder. Hiring a part-time coder would likely be a tiny fraction of the fundraising that gets splashed all over this site, and would mean thatt things didn't stay broken for years.

Seriously, after over half a decade, expect complaints.
Comment 28 Brylie Oxley 2012-12-15 07:05:29 UTC
Brian, it seems like you need more time. What are some blocking issues preventing this bug from getting resolved?

Adam, does your frustration stem from your need to be heard? Would timeliness or resolution help you to feel at ease?

I appreciate both of your honesty and cooperation, working together on this issue.
Comment 29 Sumana Harihareswara 2012-12-22 17:58:56 UTC
I've updated https://www.mediawiki.org/wiki/VipsScaler so it links to the test page, bugs that this might fix, wikitech-l communications, etc.

People in bug 9497 (thumbnails of large PNGs aren't being generated, affecting maybe 1600 PNGs on Commons?) have said that VipsScaler is the real solution, but a commenter recently wrote of VipsScaler: "I think it can be considered dead now, implementing pngscale as an extension (like wikidiff2) or whatever seems definitely the way to go."  So if you think VipsScaler is or isn't dead, you may want to mention that in bug 9497 so we have a better idea of how to move forward on that.
Comment 30 Greg Grossmeier 2013-06-24 19:51:11 UTC
Jan/Tim/Reedy:

Right now VipsScaler (through the wmgUseVipsTest option) is only enabled on Mediawiki.org, test, and test2.

From my understanding, VipsScaler is ready for deployment across all wikis, yes?

Performance Review: done (by Tim I assume?)
Security Review: any substantial needs here?
Design Review: not needed, from my understanding.

Help clarifying this, please.
Comment 31 MZMcBride 2013-06-25 02:08:43 UTC
(In reply to comment #30)
> From my understanding, VipsScaler is ready for deployment across all wikis,
> yes?

Read comment 21.

It would be helpful to figure out exactly which version of VIPS you want to deploy. (Or rather, which version is installed on the relevant image scalers.)

Comment 9 suggests there wouldn't be much issue enabling this for now and improving it later. My guess is that this could be enabled on nearly every wiki except en.wikipedia.org and commons.wikimedia.org and would be fine. But this is a largely uneducated guess.
Comment 32 Jan Gerber 2013-06-25 11:12:31 UTC
(In reply to comment #30)
> Jan/Tim/Reedy:
> 
> Right now VipsScaler (through the wmgUseVipsTest option) is only enabled on
> Mediawiki.org, test, and test2.
> 
> From my understanding, VipsScaler is ready for deployment across all wikis,
> yes?
> 

new vips packages are available on apt.wikimedia.org and installed on the scalers for some time and the extension should be ready for deployment on more wikis.
Comment 33 Greg Grossmeier 2013-06-25 16:01:05 UTC
(In reply to comment #32)
> (In reply to comment #30)
> > Jan/Tim/Reedy:
> > 
> > Right now VipsScaler (through the wmgUseVipsTest option) is only enabled on
> > Mediawiki.org, test, and test2.
> > 
> > From my understanding, VipsScaler is ready for deployment across all wikis,
> > yes?
> > 
> 
> new vips packages are available on apt.wikimedia.org and installed on the
> scalers for some time and the extension should be ready for deployment on
> more
> wikis.

Thanks much Jan, that's what I was thinking (that the image scalers already had the package, it just needed to be used).

Jan: There's only one question left that I have before we push it out everywhere: The wmgUseVipsTest setting variable implies, well, that it is only for a test instance.

I haven't looked at the code in any detail, but does this do anything that we don't want for our production wikis going forward? If not and it is just fine to deploy Vips via this config setting, then let's do that.

Can we rename the config variable, too? It might confuse third-parties :-)
Comment 34 Sam Reed (reedy) 2013-06-25 16:12:25 UTC
reedy@mw1153:~$ dpkg -l | grep -i vips
ii  libvips-doc                     7.32.3-1wmf1                        image processing system good for very large images (doc)
ii  libvips-tools                   7.32.3-1wmf1                        image processing system good for very large images (tools)
ii  libvips15                       7.26.3-1build1                      image processing system good for very large images
ii  libvips31                       7.32.3-1wmf1                        image processing system good for very large images
reedy@mw1153:~$ apt-get install libvips31 -s
NOTE: This is only a simulation!
      apt-get needs root privileges for real execution.
      Keep also in mind that locking is deactivated,
      so don't depend on the relevance to the real current situation!
Reading package lists... Done
Building dependency tree
Reading state information... Done
libvips31 is already the newest version.
libvips31 set to manually installed.
The following packages were automatically installed and are no longer required:
  libpgm-5.1-0 libzmq3
Use 'apt-get autoremove' to remove them.
0 upgraded, 0 newly installed, 0 to remove and 47 not upgraded.
reedy@mw1153:~$



Is there a newer version still?
Comment 35 Sam Reed (reedy) 2013-06-25 16:16:02 UTC
(In reply to comment #33)
> Jan: There's only one question left that I have before we push it out
> everywhere: The wmgUseVipsTest setting variable implies, well, that it is
> only
> for a test instance.
> 
> I haven't looked at the code in any detail, but does this do anything that we
> don't want for our production wikis going forward? If not and it is just fine
> to deploy Vips via this config setting, then let's do that.
> 
> Can we rename the config variable, too? It might confuse third-parties :-)

It's only a configuration variable used by WMF, so we can do what we want. Dropping the "Test" part probably makes sense though
Comment 36 Jan Gerber 2013-06-25 16:19:50 UTC
@Greg yes that variable should be renamed to $wmgUseVips (https://gerrit.wikimedia.org/r/70439)

@Sam 7.32.3 is the latest version in debian and 7.32.3-1wmf1 is a backport of that (http://packages.debian.org/experimental/libvips31)
Comment 37 Greg Grossmeier 2013-07-12 21:48:55 UTC
Update on the bug: We plan to push this out next Thursday (July 18th) pending the completion of bug 51149.
Comment 38 Sumana Harihareswara 2013-07-19 17:40:35 UTC
Update: launch delayed pending the completion of bug 51370.
Comment 39 Greg Grossmeier 2013-07-25 18:41:29 UTC
Deployed! Sorry about the delay!
Comment 40 Derk-Jan Hartman 2014-08-09 13:21:40 UTC
Bug 69336

It seems we never added the 'set comment' option for the configuration ?

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links