Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warning should be raised (or error?) on trying to handle images larger than 4096 #14

Open
jabooth opened this issue May 14, 2015 · 4 comments
Labels

Comments

@jabooth
Copy link
Member

jabooth commented May 14, 2015

We current scale users images which have a width or height greater than 4096 and don't even tell them, which leads to scale errors on annotations. This is a bug and we need a strategy to solve it. In order of increasing difficulty:

  1. Require a flag (--resize-images) to be explicitly provided by the user to allow the downscale to take place. Else, fail on importing large images. Very easy to do but means users can't practically use the landmarker with large textures.
  2. Track the downscale that was applied on import, and on the server side correct landmarks that are being saved. This means the client is blissfully unaware of the scaling, which is good (KISS) but bad as we ultimately want to make the client a standalone tool that is less dependent on server magic.
  3. Adapt the client so that it is able to do the resizing itself. This is ideally what we would like to do as it is the best solution for a future where the client is standalone - working on for instance local assets.
@lirsacc
Copy link
Contributor

lirsacc commented May 21, 2015

Is that issue still valid ?

Couldn't find an issue for scaling up the landmarks when server detects a scaled down image (which should handle the case of reusing landmarks with the original image) did that evolve from this issue ?

@jabooth jabooth added the bug label May 21, 2015
@jabooth
Copy link
Member Author

jabooth commented May 21, 2015

@lirsacc I've fleshed out the issue to cover the potential options. I think that we are best going forwards with option 2 for now. It fully solves the problem for the only current supported use case of the landmarker (client + server). We just need to be careful if we do this as the exact values reported by the client may now be incorrect as it will not be aware of scaling issues (as all this duty is taken care of my the server).

@jabooth
Copy link
Member Author

jabooth commented May 21, 2015

@lirsacc In order to make this easier, I'm going to work today on exposing the transforms that menpo uses internally when performing image scaling. This means you will have an object you can save out (menpo.io.export_pickle) on image caching in the case that a rescale was required. Then, on landmark saving in the server code you can check for the presence of this file for this asset. If it exists, load the file, apply the transform to the landmarks, and export the result. If no file, then we know no scaling needs to be done.
We may have to think through what this means for loading incomplete landmarks (as we saw the other day menpo may have issues with them) but this is a bridge we need to cross anyway.

@jabooth
Copy link
Member Author

jabooth commented Oct 29, 2015

the necessary code is now in menpo to implement this: menpo/menpo#631
a release of menpo is scheduled in a few weeks, after that is out this issue can be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants