search

menpo / landmarkerio-server

The Menpo landmarker.io server
BSD 3-Clause "New" or "Revised" License
10 stars 14 forks source link

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

Open jabooth opened 9 years ago

jabooth commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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