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

Non-geotagged photos are ignored #160

Closed
andylolz opened this issue Jun 28, 2018 · 5 comments
Closed

Non-geotagged photos are ignored #160

andylolz opened this issue Jun 28, 2018 · 5 comments

Comments

@andylolz
Copy link
Contributor

andylolz commented Jun 28, 2018

For https://openbenches.org/bench/7498 (now deleted) one of my photos was geotagged; the other two were not (my phone messed up, I guess).

When I hit send, the non-geotagged photos were excluded. This was surprising! I thought one geotagged photo would be sufficient. The bench remained stationary throughout the photoshoot, which I guess is usually the case. So the geotags for all three photos of the bench would have been roughly similar.


More generally: Is the requirement that photos are geotagged necessary at all? AFAICT it’s just used for an initial map location; the interface allows the user to adjust the location later anyway. So surely if I don’t submit a geotagged photo, I could just provide a location without an initial suggested location derived from metadata.

@arizonagroovejet
Copy link
Contributor

Looks like the save_image function doesn't save images that don't contain GPS data and it returns a message to that effect,
https://github.com/edent/openbenches/blob/c2d1b9e063a813d7b144479d958fceeb4f8f63e0/www/functions.php#L288
but nothing is done with that message. E.g.
https://github.com/edent/openbenches/blob/c2d1b9e063a813d7b144479d958fceeb4f8f63e0/www/add.php#L53
There output of the function call is not assigned to anything so the image is silently discarded. Which is unhelpful.

There's Javascript which shouts at you if the inscription image doesn't have GPS data
https://github.com/edent/openbenches/blob/c2d1b9e063a813d7b144479d958fceeb4f8f63e0/www/add.php#L224
but it's only called for the inscription image, there's a separate chuck of Javascript called when each photo is selected.

If there were a vote on all photos needing to have GPS data or just the inscription image, I'd vote for the latter. Sadly GPS tagging simply isn't reliable, not just in terms of accuracy, but in working at all. Trees, buildings, who knows what else can all mess it up. I would not bet on the GPS data in photos of the same bench all being "similar" (#51 ) In my experience the reliability of GPS varies between different apps on the same device. Lots of times I've had Google Maps zoom in on my location within seconds and minutes later OpenCamera, set to not take a photo unless it has location data, still has no idea where I am.

A pro of not requiring GPS data in photos at all would be that it may make it easier for people to submit photos from devices which lack GPS capability. Such as pretty much every camera anyone owns.

A con of not having GPS data in the inscription image is that it would make setting location harder. With the current set up you hopefully at worst have to zoom out a step, drag the pointer several hundred meters, zoom in again. Would the current set up of the map on the Add page work for the scenario of no GPS data at all? What's would the default pin position be? Would it help if there were some sort of location search, like offered by https://www.geoimgr.com/

@edent
Copy link
Collaborator

edent commented Jul 3, 2018

I agree that other photos don't need GPS. Happy if you want to make the change - I won't have time until the weekend.

I'm of the opinion that only the first image needs a geotag. I'd rather have a slightly innacurate tag than rely on a person's memory.

arizonagroovejet pushed a commit to arizonagroovejet/openbenches that referenced this issue Jul 4, 2018
@arizonagroovejet
Copy link
Contributor

Well I just did a fix for this and was about hit the button to create a PR when I realised it's bad in at least one way. My brain is starting to melt in the heat so maybe I'll have another go tomorrow. Or when the heatwave is over.

FWIW
https://github.com/arizonagroovejet/openbenches/tree/issue160
It's certainly bad because it contains the short-circuiting of get_twitter_details() I put in to make it work without having to provide Twitter API credentials. As I was looking at the PR I decided making the call to get_image_location() conditional on value of $GPSrequired was bad, but now I think that's actually OK because the call isn't used to get any location information to do anything with, merely to check if it's present or not.

@edent edent closed this as completed in 35db935 Jul 5, 2018
@edent
Copy link
Collaborator

edent commented Jul 5, 2018

Now fixed. First image must contain a geotag. Subsequent ones can be blank. Shout if there are any problems.

@andylolz
Copy link
Contributor Author

andylolz commented Jul 6, 2018

Ace – thanks both!

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

No branches or pull requests

3 participants