Page MenuHomePhabricator

Provide a well-performing API to rotate an image
Open, LowPublic

Assigned To
None
Authored By
RobLa-WMF
Dec 16 2011, 12:31 AM
Referenced Files
F8701: patch33186.txt
Nov 22 2014, 12:07 AM
F8700: ApiImageRotate.php
Nov 22 2014, 12:07 AM
Tokens
"Like" token, awarded by Bugreporter2."Love" token, awarded by Sj."Love" token, awarded by Isochrone."Mountain of Wealth" token, awarded by Frostly."Like" token, awarded by ToBeFree."Like" token, awarded by Ata."Like" token, awarded by Poyekhali."Like" token, awarded by Stemoc."Like" token, awarded by Ankry."Like" token, awarded by Emha."Like" token, awarded by Romaine."Like" token, awarded by Didym."Manufacturing Defect?" token, awarded by Josve05a."Piece of Eight" token, awarded by Nemo_bis."Like" token, awarded by Jianhui67."Like" token, awarded by Luke081515."Like" token, awarded by Jarekt."Like" token, awarded by FDMS."Like" token, awarded by Natuur12."Like" token, awarded by zhuyifei1999."Like" token, awarded by Krd."Like" token, awarded by Reguyla."Like" token, awarded by PierreSelim."Like" token, awarded by revi."Like" token, awarded by Thibaut120094."Like" token, awarded by Steinsplitter.

Description

Original description: As a first step toward creating an interface for rotating an image (see bug 32875), it would be very nice to have an API for rotating an image. This would then make it pretty easy to build a user interface to rotate an image (ala http://commons.wikimedia.org/wiki/Help:RotateLink )

Current progress: There is an API action=imagerotate, however, it is not considered to be performant enough to be enabled on Wikimedia wikis. We need a proper solution in MediaWiki that can be enabled on the WMF production cluster.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

It rather seems like the scope of this is growing into something the community doesn't want or need, when all we really need is the ability to correct simple 90° rotations. Hell, even just the ability to set an orientation code in the EXIF might be enough.

As stated earlier in the comment thread, 90 degree rotations can be performed losslessly on many formats (the vast majority of our originals, when you look at the distribution of file types), including JPEG without having to use EXIF rotation. jpegtran is what allows lossless EXIF-free rotation of JPEGs.

As stated earlier in the comment thread, 90 degree rotations can be performed losslessly on many formats (the vast majority of our originals, when you look at the distribution of file types), including JPEG without having to use EXIF rotation. jpegtran is what allows lossless EXIF-free rotation of JPEGs.

Agreed that jpegtran and the equivalents for other formats are better; just pointing out the EXIF option as a possible fallback if that's unfeasable.

@brion: May i ask what is the status of this? :-)

Change 250291 had a related patch set uploaded (by Ori.livneh):
include mediawiki::multimedia on all application servers

https://gerrit.wikimedia.org/r/250291

Since the migration to HHVM, several things have changed:

  • Request resource limits are now identical across all application servers.
  • CPU utilization on the API cluster is a third of what it was a year ago.

If we simply install multimedia packages on all application servers (as I'm proposing we do in Ib5dee0fce), the end result would be a world in which every application server is in principle capable of serving every kind of request. I think that this would make our software stack substantially simpler to manage and would pave the way for elastic load-balancing and SLAs.

Once Ib5dee0fce is merged, I think the next logical step is to simply enable the imagerotate API (in its current, synchronous form) while monitoring load.

There is a risk that the load generated by image rotation requests will be too high and that we will need to disable image rotation once again, which would no doubt be frustrating to users. But I think there is a very good chance that we will handle the load just fine, and that it is worth a shot.

@ori, just for me getting things right, it is okay now to do multimedia operations synchronously on the application servers? No queue required because the resource limits are now identical across all application servers and there is less load on them and not desired because it would complicate managing the software stack substantially?

That aside, I like Brion V.'s suggested interface "filetransform". It's not so narrow in scope.

hashar subscribed.

From 105877, the imagerotate API module is disabled on beta just like production:

CommonSettings.php has:

// T35186: turn off incomplete feature action=imagerotate
$wgAPIModules['imagerotate'] = 'ApiDisabled';

Disabled because of T35186: Provide a well-performing API to rotate an image.

If one want to enable it for testing purpose, I guess it is all about adding the setting in a -labs.php file.

Because of the recent changes in session handling/tokens the bot is broken. I am unable, even after multiple hours of work (i don't have time to spend moor hours for this), to detect the problem. Which mean rotatebot is no longer rotating files and the backlog will become bigger every day: https://commons.wikimedia.org/wiki/Category:Images_requiring_rotation_by_bot

If the bot is getting logged out, it's probably not handling cookies right. (Use a standard library, don't try to do it by hand.) If it's not getting logged out but getting token errors, it's probably not urlencoding request parameters properly.

Unfortunately, i don't have time to re-write it / porting to a standard library. And i think it is time now to include this function in mw, especially because it is highly used.

Thanks but unfortunately it does not work. The whole code needs a re-write. The best would be to port it to MW (the function in MW api exist yet, but not compatible with WMF cluster yet).

Hmm, I see your bot uploaded rotated versions of a few files today – have you gotten it to work after all?

Hmm, I see your bot uploaded rotated versions of a few files today – have you gotten it to work after all?

I am current merging the bot to a stable library (https://en.wikipedia.org/wiki/Wikipedia:Peachy).

Update: Bot is back again. Now rotation of bigger files and .tif(f) is supported.

Update2: But of course this bug should be fixed contemporary.

Change 250291 abandoned by Ori.livneh:
include mediawiki::multimedia on all application servers

https://gerrit.wikimedia.org/r/250291

The biggest problem with this patch is that it came out of a request to
allow rotations in multiples of 90°. Commons does not need or want anything
besides that, because you don't want eyeballed rotations, and 90° multiples
are about the only ones you can do eyeballing that are worth doing.
Anything else you want people taking the image to proper image editing
software, with guidelines and care.

PLEASE DON'T give us something that you think would be better, when,
practically, all it does is encourage slow degredation of image as people
go "is 1°' enough... no... try another degree..."

Image curation and a free rotation tool are not compatible.

https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail
Virus-free.
www.avast.com
https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail
<#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>

Sometimes, the rotation sought is not around the center point, but around the central vertical or horizontal axis (sometimes called mirroring, flipping, or flopping). This should be doable losslessly. See also c:MediaWiki talk:Gadget-RotateLink.js#Mirroring as Rotation.

Aklapper removed a subscriber: RobLa-WMF.

This task has been assigned to the same task owner for more than two years. Resetting task assignee due to inactivity, to decrease task cookie-licking and to get a slightly more realistic overview of plans. Please feel free to assign this task to yourself again if you still realistically work or plan to work on this task - it would be welcome!

For tips how to manage individual work in Phabricator (noisy notifications, lists of task, etc.), see https://phabricator.wikimedia.org/T228575#6237124 for available options.
(For the records, two emails were sent to assignee addresses before resetting assignees. See T228575 for more info and for potential feedback. Thanks!)

BPirkle lowered the priority of this task from High to Low.Jun 24 2020, 8:32 PM
BPirkle moved this task from Inbox to Volunteer Needed Tasks on the Platform Engineering board.

This seems something that is much more doable now that we have Shellbox. I reviewed the current code in ApiImageRotate and would suggest the following modifications:

  • Only allow a single file to be operated on at once (can skip using ApiPageSet)
  • Initially, set some max size limit of files that can be rotated. This can be raised over time as the API module proves stable.
  • Given this has really never been enabled widely, we should rename it to ApiFileRotate.
  • Add a change tag like mw-file-rotate to all uploads that use this.
  • Make all the various $handler->rotate() calls be Shellbox-compatible by using BoxedCommand.

Happy to review/mentor/guide someone in working on this.

It seems CropTool already supports rotation (maybe this should be advertised better?), I'm slightly hesitant to add more to mediawiki and bloat it. MediaWiki is not really an image editing tool.

@Legoktm @Ladsgroup it seems crop and rotate (by 90/180) are such a common use case they should be "on" by default, whether by closer integration of CropTool into the upload wizard or otherwise.

The existence of CropTool, and its rotation feature (I didn't see it when I first used it!) could both be made more visible.
Agreed with AdamC's old point above that arbitrary-value rotation can be confusing, if the common rotations aren't [more] prominent.

I think simple rotating (let's leave cropping out of it for now) is a must for Commons and something that should be supported at production quality, especially given the recent emphasis on better multimedia and technical support for Commons. Anyways it seems like low hanging fruit.

Whether or not it is implemented as part of MediaWiki or a separate service is up for debate, but I would say that based on our current architecture, getting it deployed via a Shellbox implementation will be the most secure and also simplest to implement.

I also think this is one of those cases where if we just provide 90/180/270 rotation for files under say 10MB, we'll address like 75% of the use case (the remaining being larger files or custom rotation angles). Anyways, no one took me up on my offer to mentor, so I might go ahead and do this (but explicitly not cookie licking yet!), I'll just have to find a +2'er...

Agreed re: just offering 90/180/-90 for under-X-MB files. Raise that to 20MB and you'll cover most good/featured images.

Change 973279 had a related patch set uploaded (by Legoktm; author: Legoktm):

[mediawiki/core@master] WIP: Implement file rotation via Shellbox

https://gerrit.wikimedia.org/r/973279

So I've posted a patch that is basically what I outlined in T35186#7487506. Rough next steps:

  • Find a reviewer, who ideally knows the correct file abstraction APIs to use on the Wikimedia cluster
  • Send notification to wikitech-l on the plan and notification for MW breaking change (removal of ApiImageRotate)
    • Also get SRE buy-in
  • Send notification to API lists on planned API breaking change, removal of API action=imagerotate (maybe people actually use it and we need a deprecation process??)

This makes my week. Thank you :) (: