Modify

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#11059 closed defect (fixed)

[patch] To pictures are saving all coordinates, this saved earlier too

Reported by: anonymous Owned by: bastiK
Priority: normal Milestone:
Component: Plugin photo_geotagging Version: latest
Keywords: template_report Cc: holgermappt

Description

What steps will reproduce the problem?

  1. Add pictures 1,2,3 on map using photoadjust plugin
  2. Save coordinates for 1,2,3 to EXIF using option from right click on photo layer (photo_geotagging plugin)
  3. Step 1 for pictures 4,5,6
  4. Step 2, but now are saving all 1,2,3,4,5,6

What is the expected result?

In step 4 should be saved only 4,5,6

What happens instead?

In step 4 are saving all pictures, this saved earlier too

Please provide any additional information below. Attach a screenshot if possible.

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2015-01-27 20:36:55
Last Changed Author: Don-vip
Revision: 7989
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Relative URL: ^/trunk
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2015-01-27 21:33:31 +0100 (Tue, 27 Jan 2015)
Last Changed Rev: 7989

Identification: JOSM/1.5 (7989 pl) Linux Ubuntu 14.04.1 LTS
Memory Usage: 204 MB / 869 MB (45 MB allocated, but free)
Java version: 1.7.0_75, Oracle Corporation, OpenJDK 64-Bit Server VM
Java package: openjdk-7-jre:amd64-7u75-2.5.4-1~trusty1
Dataset consistency test: No problems found

Plugins:
- CADTools (1002)
- InfoMode (30892)
- OpeningHoursEditor (30892)
- PicLayer (30892)
- editgpx (30892)
- photo_geotagging (30892)
- photoadjust (30905)
- reverter (30892)

Last errors/warnings:
- E: java.net.SocketTimeoutException: Read timed out. Przyczyna: java.net.SocketTimeoutException: Read timed out
- E: java.net.SocketTimeoutException: Read timed out. Przyczyna: java.net.SocketTimeoutException: Read timed out
- E: java.net.SocketTimeoutException: Read timed out. Przyczyna: java.net.SocketTimeoutException: Read timed out
- E: java.net.SocketTimeoutException: Read timed out. Przyczyna: java.net.SocketTimeoutException: Read timed out
- E: java.net.SocketTimeoutException: Read timed out. Przyczyna: java.net.SocketTimeoutException: Read timed out

Attachments (2)

GeoImageLayer_unflagNewGpsData.patch (3.7 KB ) - added by holgermappt 9 years ago.
New method unflagNewGpsData(), modified infoText.
photo_geotagging_unflagNewGpsData.patch (1.9 KB ) - added by holgermappt 9 years ago.
Remove new GPS data flag, do not use gpsTime.

Download all attachments as: .zip

Change History (16)

comment:1 by anonymous, 9 years ago

Summary: To pictures are saving all pictures, this saved earlier tooTo pictures are saving all coordinates, this saved earlier too

comment:2 by bastiK, 9 years ago

Cc: holgermappt added

comment:3 by holgermappt, 9 years ago

OK, we do not clear the new-GPS-data flag, so it is saved again and again. There was never a concept to clear it, but it's probably a good idea to add that. I can look at this. JOSM core needs to be changed (ImageEntry, unflagNewGpsData()), then photo_geotagging needs to call that after the GPS data was written. Problem is that other components might still need that information.

Is there a reason why you want to save the GPS information after three pictures? I position all photos first and save the coordinates when I'm done with that. Works fine up to 100 photos. You can even save the session and continue later, JOSM will remember what pictures you touched in the previous session.

in reply to:  3 comment:4 by bastiK, 9 years ago

Replying to holgermappt:

OK, we do not clear the new-GPS-data flag, so it is saved again and again. There was never a concept to clear it, but it's probably a good idea to add that. I can look at this. JOSM core needs to be changed (ImageEntry, unflagNewGpsData()), then photo_geotagging needs to call that after the GPS data was written.

Sounds good.

Problem is that other components might still need that information.

Not at the moment, as far as I'm aware.

in reply to:  3 comment:5 by anonymous, 9 years ago

Replying to holgermappt:

Is there a reason why you want to save the GPS information after three pictures? I position all photos first and save the coordinates when I'm done with that. Works fine up to 100 photos. You can even save the session and continue later, JOSM will remember what pictures you touched in the previous session.

I have photos from many places in country taken few years ago, is not easy to find his place. I position 100 photos, save (don't want lost when system crash), position next 100 photos, etc. I saw this (too many pictures are saving), so I reported. I don't have another reason for this.

by holgermappt, 9 years ago

New method unflagNewGpsData(), modified infoText.

by holgermappt, 9 years ago

Remove new GPS data flag, do not use gpsTime.

comment:6 by holgermappt, 9 years ago

Here are two patches, one for JOSM core (GeoImageLayer_unflagNewGpsData.patch) and one for the photo_geotagging plugin (photo_geotagging_unflagNewGpsData.patch). The core patch must be applied first and then the minimum JOSM version needs to be set in photo_geotagging/build.xml (now set to 9999).

I had to change GeoImageLayer.getImages() to return a list with the actual images instead of a list of image clones. Is there a reason to clone the images?

I also added the number of images with modified GPS data to the GeoImageLayer infoText.

photo_geotagging shows a list of files with modified GPS data that will be written to the image files. I can select one or more of the file paths, but all are written. Was the idea to write only the files that were selected? If not it would be good if it is not possible to select files. Otherwise the user will wonder why the unselected files were modified too.

comment:7 by skyper, 9 years ago

Summary: To pictures are saving all coordinates, this saved earlier too[patch] To pictures are saving all coordinates, this saved earlier too

@holgermappt:
Thanks for the patches. Please always add [patch] at the beginning of the ticket's summary when attaching a patch. Otherwise it will not be listed under report/8 and might get forgotten.

Last edited 9 years ago by skyper (previous) (diff)

comment:8 by holgermappt, 9 years ago

@skyper: I didn't add [patch] because there is an open question. For me patch means something that is ready to be used. Right now the patch files are more work in progress with pending review and test.

comment:9 by bastiK, 9 years ago

Resolution: fixed
Status: newclosed

In 8041/josm:

applied #11059 - New method unflagNewGpsData(), modified infoText (patch by holgermappt)

comment:10 by bastiK, 9 years ago

In [o30967]: applied ​#josm11059 - Remove new GPS data flag, do not use gpsTime (patch by holgermappt)

in reply to:  6 ; comment:11 by bastiK, 9 years ago

Replying to holgermappt:

Here are two patches, one for JOSM core (GeoImageLayer_unflagNewGpsData.patch) and one for the photo_geotagging plugin (photo_geotagging_unflagNewGpsData.patch). The core patch must be applied first and then the minimum JOSM version needs to be set in photo_geotagging/build.xml (now set to 9999).

Thanks for the patches!

I had to change GeoImageLayer.getImages() to return a list with the actual images instead of a list of image clones. Is there a reason to clone the images?

I think it is general defensive programming style. But if you need access to the actual images, this has to be adapted of course.

I also added the number of images with modified GPS data to the GeoImageLayer infoText.

photo_geotagging shows a list of files with modified GPS data that will be written to the image files. I can select one or more of the file paths, but all are written. Was the idea to write only the files that were selected? If not it would be good if it is not possible to select files. Otherwise the user will wonder why the unselected files were modified too.

Yes, this is a problem and should be fixed. Either not allow to select files or change it to a table with checkboxes in front (like in a browser email website).

in reply to:  8 comment:12 by bastiK, 9 years ago

Replying to holgermappt:

@skyper: I didn't add [patch] because there is an open question. For me patch means something that is ready to be used. Right now the patch files are more work in progress with pending review and test.

For patches of this scope, I will only do a review by reading the diff and expect you have done some basic tests.

Anyway, from my perspective, there was nothing wrong with the patches so the marker would have been correct.

in reply to:  11 ; comment:13 by bastiK, 9 years ago

Replying to bastiK:

Replying to holgermappt:

photo_geotagging shows a list of files with modified GPS data that will be written to the image files. I can select one or more of the file paths, but all are written. Was the idea to write only the files that were selected? If not it would be good if it is not possible to select files. Otherwise the user will wonder why the unselected files were modified too.

Yes, this is a problem and should be fixed. Either not allow to select files or change it to a table with checkboxes in front (like in a browser email website).

Now that I think about it, checkboxes aren't so great for block selection with Shift & Ctrl. Maybe a dynamic text below the list would be better, showing the selected vs. total images. Or a confirmation checkbox.

in reply to:  13 comment:14 by holgermappt, 9 years ago

Replying to bastiK:

Now that I think about it, checkboxes aren't so great for block selection with Shift & Ctrl. Maybe a dynamic text below the list would be better, showing the selected vs. total images. Or a confirmation checkbox.

I think the user interface is okay as it is. Just the function to update only the selected files is missing. All should be selected by default. For me this is a nice-to-have feature with low priority.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain bastiK.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.