[Patch] Re-geotagging hides pictures that are not covered by gpx track

[skorbut]

There is a somewhat special condition where JOSM/the photo geotagging plugin doesn't behave ideally:

• Load some geotagged images in JOSM
• Try to correlate those images using a gpx track which only covers some, but not all of the images (use the option 'Override position for images with geo location in exif data', otherwise the images wont be repositioned)
• Images that were not covered by the gpx track disappear, although they still have the original geotagging.
• (Writing the coordinates to the files that are covered by the gpx track and reloading all images into JOSM displays everything again in correct fashion. So the situation is not too bad...)

This happened to me in the following situation: I have a camera with an integrated, rather low-quality GPS. To improve the accuracy I also have an external logger whose gpx track is of better quality and that I want to use as a primary source of geotagging. When the battery of the external logger was exhausted, the camera still recorded the (low-quality) position that I would prefer to use if the alternative was to discard the image completely.

[Bjoeni]

Prevent removal of geotags from files that weren't matched to the track:

src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java

@@ -229 +229 @@
                     break;
                 }
                 long tagms = TimeUnit.MINUTES.toMillis(tagTime);
-                if (curTmp.getPos() == null &&
-                        (Math.abs(time - curWpTime) <= tagms
-                        || Math.abs(prevWpTime - time) <= tagms)) {
+                if (Math.abs(time - curWpTime) <= tagms
+                        || Math.abs(prevWpTime - time) <= tagms) {
                     if (prevWp != null && time < curWpTime - half) {
                         curTmp.setPos(prevWp.getCoor());
                     } else {
@@ -255 +254 @@
                 if (imgTime < prevWpTime) {
                     break;
                 }
-                if (curTmp.getPos() == null) {
-                    // The values of timeDiff are between 0 and 1, it is not seconds but a dimensionless variable
-                    double timeDiff = (double) (imgTime - prevWpTime) / Math.abs(curWpTime - prevWpTime);
-                    curTmp.setPos(prevWp.getCoor().interpolate(curWp.getCoor(), timeDiff));
-                    curTmp.setSpeed(speed);
-                    if (curElevation != null && prevElevation != null) {
-                        curTmp.setElevation(prevElevation + (curElevation - prevElevation) * timeDiff);
-                    }
-                    curTmp.setGpsTime(curImg.getExifInstant().minusMillis(offset));
-                    curTmp.flagNewGpsData();
-                    curImg.tmpUpdated();
+                // The values of timeDiff are between 0 and 1, it is not seconds but a dimensionless variable
+                double timeDiff = (double) (imgTime - prevWpTime) / Math.abs(curWpTime - prevWpTime);
+                curTmp.setPos(prevWp.getCoor().interpolate(curWp.getCoor(), timeDiff));
+                curTmp.setSpeed(speed);
+                if (curElevation != null && prevElevation != null) {
+                    curTmp.setElevation(prevElevation + (curElevation - prevElevation) * timeDiff);
+                }
+                curTmp.setGpsTime(curImg.getExifInstant().minusMillis(offset));
+                curTmp.flagNewGpsData();
+                curImg.tmpUpdated();
 
-                    ret++;
-                }
+                ret++;
                 i--;
             }
         }

src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java

@@ -1090 +1090 @@
             // Construct a list of images that have a date, and sort them on the date.
             List<ImageEntry> dateImgLst = getSortedImgList();
             // Create a temporary copy for each image
-            for (ImageEntry ie : dateImgLst) {
-                ie.createTmp();
-                ie.getTmp().setPos(null);
-            }
+            dateImgLst.forEach(ImageEntry::createTmp);
 
             GpxDataWrapper selGpx = selectedGPX(false);
             if (selGpx == null)

See also the changes from #16681 allowing the user to determine what images should be matched.

[Bjoeni]

Ticket #11015 has been marked as a duplicate of this ticket.

[Bjoeni]

@Simon (or someone else)
could you please commit this patch in the near future? #20795 depends on it (or it would at least conflict, so it's easier when this one is committed before I provide a patch for the other one)

[simon04]

This patch breaks the unit test org.openstreetmap.josm.data.gpx.GpxImageCorrelationTest#testMatchGpxTrack

[Bjoeni]

That's interesting, I thought I checked that. I'll have a look.

[Bjoeni]

Fixed that (note to self: shouldn't change a patch after running the tests...). Sorry about that.

I also added another ImageEntry i7 to testMatchGpxTrack (testing the changes of this ticket).

[Bjoeni]

Btw. I just noticed that checkstyle complains that the testMatchGpxTrack() method is now too long (157 executable statements). I'll change that.

[Bjoeni]

Restructured GpxImageCorrelationTest into separate methods

[simon04]

In 17878/josm:

fix #11710 - Re-geotagging hides pictures that are not covered by GPX track (patch by Bjoeni)

[simon04]

Thank you!

[simon04]

In 17882/josm:

see #11710 - Fix MethodOrderer deprecation