Index: build.xml =================================================================== --- build.xml (revision 36433) +++ build.xml (working copy) @@ -4,7 +4,7 @@ - + Index: pom.xml =================================================================== --- pom.xml (revision 36433) +++ pom.xml (working copy) @@ -16,7 +16,7 @@ src - 19044 + 19389 Paul Hartmann org.openstreetmap.josm.plugins.photo_geotagging.GeotaggingPlugin Write gps position info to the image file header. Run this feature from the right click menu of the image layer. Index: src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java =================================================================== --- src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java (revision 36433) +++ src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java (working copy) @@ -9,7 +9,6 @@ import java.io.FileOutputStream; import java.io.IOException; import java.text.DecimalFormat; -import java.time.Instant; import java.util.Calendar; import java.util.GregorianCalendar; import java.util.TimeZone; @@ -25,6 +24,7 @@ import org.apache.commons.imaging.formats.tiff.write.TiffImageWriterLossy; import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory; import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet; +import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; /** * Wrapper class for sanselan library @@ -40,19 +40,16 @@ * * @param imageFile A source image file. * @param dst The output file. - * @param lat latitude - * @param lon longitude - * @param gpsTime time - can be null if not available - * @param speed speed in km/h - can be null if not available - * @param ele elevation - can be null if not available - * @param imgDir image direction in degrees (0..360) - can be null if not available + * @param imageEntry the image object from Josm core * @param lossy whether to use lossy approach when writing metadata (overwriting unknown tags) * @throws IOException in case of I/O error + * @since xxx separate image parameters (lat, lon, gpsTime, speed, ele, imgDir), replaced by the whole ImageEntry object. */ - public static void setExifGPSTag(File imageFile, File dst, double lat, double lon, Instant gpsTime, Double speed, - Double ele, Double imgDir, boolean lossy) throws IOException { + + public static void setExifGPSTag(File imageFile, File dst, ImageEntry imageEntry, + boolean lossy) throws IOException { try { - setExifGPSTagWorker(imageFile, dst, lat, lon, gpsTime, speed, ele, imgDir, lossy); + setExifGPSTagWorker(imageFile, dst, imageEntry, lossy); } catch (ImagingException ire) { // This used to be two separate exceptions; ImageReadException and imageWriteException throw new IOException(tr("Read/write error: " + ire.getMessage()), ire); @@ -59,8 +56,8 @@ } } - public static void setExifGPSTagWorker(File imageFile, File dst, double lat, double lon, Instant gpsTime, Double speed, - Double ele, Double imgDir, boolean lossy) throws IOException { + public static void setExifGPSTagWorker(File imageFile, File dst, ImageEntry imageEntry, + boolean lossy) throws IOException { TiffOutputSet outputSet = null; ImageMetadata metadata = Imaging.getMetadata(imageFile); @@ -80,11 +77,11 @@ TiffOutputDirectory gpsDirectory = outputSet.getOrCreateGpsDirectory(); gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_VERSION_ID); - gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_VERSION_ID, (byte)2, (byte)3, (byte)0, (byte)0); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_VERSION_ID, (byte) 2, (byte) 3, (byte) 0, (byte) 0); - if (gpsTime != null) { + if (imageEntry.getGpsInstant() != null) { Calendar calendar = new GregorianCalendar(TimeZone.getTimeZone("UTC")); - calendar.setTimeInMillis(gpsTime.toEpochMilli()); + calendar.setTimeInMillis(imageEntry.getGpsInstant().toEpochMilli()); final int year = calendar.get(Calendar.YEAR); final int month = calendar.get(Calendar.MONTH) + 1; @@ -117,42 +114,78 @@ gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_DATE_STAMP, dateStamp); } - outputSet.setGpsInDegrees(lon, lat); + outputSet.setGpsInDegrees(imageEntry.getPos().lon(), imageEntry.getPos().lat()); - if (speed != null) { + if (imageEntry.getSpeed() != null) { gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_SPEED_REF); gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED_REF, GpsTagConstants.GPS_TAG_GPS_SPEED_REF_VALUE_KMPH); gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_SPEED); - gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED, RationalNumber.valueOf(speed)); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED, RationalNumber.valueOf(imageEntry.getSpeed())); } - if (ele != null) { - byte eleRef = ele >= 0 ? (byte) 0 : (byte) 1; + if (imageEntry.getElevation() != null) { + byte eleRef = imageEntry.getElevation() >= 0 ? (byte) 0 : (byte) 1; gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_ALTITUDE_REF); gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_ALTITUDE_REF, eleRef); gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_ALTITUDE); - gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_ALTITUDE, RationalNumber.valueOf(Math.abs(ele))); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_ALTITUDE, RationalNumber.valueOf(Math.abs(imageEntry.getElevation()))); } - if (imgDir != null) { + if (imageEntry.getExifImgDir() != null) { gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_IMG_DIRECTION_REF); gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_IMG_DIRECTION_REF, GpsTagConstants.GPS_TAG_GPS_IMG_DIRECTION_REF_VALUE_TRUE_NORTH); - // make sure the value is in the range 0.0...<360.0 - if (imgDir < 0.0) { - imgDir %= 360.0; // >-360.0...-0.0 - imgDir += 360.0; // >0.0...360.0 - } - if (imgDir >= 360.0) { - imgDir %= 360.0; - } + Double imgDir = checkAngle(imageEntry.getExifImgDir()); gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_IMG_DIRECTION); gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_IMG_DIRECTION, RationalNumber.valueOf(imgDir)); } + if (imageEntry.getExifGpsTrack() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_TRACK_REF); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_TRACK_REF, + GpsTagConstants.GPS_TAG_GPS_TRACK_REF_VALUE_TRUE_NORTH); + // make sure the value is in the range 0.0...<360.0 + Double gpsTrack = checkAngle(imageEntry.getExifGpsTrack()); + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_TRACK); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_TRACK, RationalNumber.valueOf(gpsTrack)); + } + + if (imageEntry.getGpsDiffMode() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_DIFFERENTIAL); + // make sure the gpsDiffMode value is 0 (no diffential) or 1 (differential) + if (imageEntry.getGpsDiffMode().equals(0) || imageEntry.getGpsDiffMode().equals(1)) { + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_DIFFERENTIAL, imageEntry.getGpsDiffMode().shortValue()); + } + } + + if (imageEntry.getGps2d3dMode() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_MEASURE_MODE); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_MEASURE_MODE, imageEntry.getGps2d3dMode().toString()); + } + + if (imageEntry.getExifGpsProcMethod() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_PROCESSING_METHOD); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_PROCESSING_METHOD, imageEntry.getExifGpsProcMethod()); + } + + if (imageEntry.getExifGpsDatum() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_MAP_DATUM); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_MAP_DATUM, imageEntry.getExifGpsDatum().toString()); + } + + if (imageEntry.getExifHPosErr() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_HOR_POSITIONING_ERROR); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_HOR_POSITIONING_ERROR, RationalNumber.valueOf(imageEntry.getExifHPosErr())); + } + + if (imageEntry.getExifGpsDop() != null) { + gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_DOP); + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_DOP, RationalNumber.valueOf(imageEntry.getExifGpsDop())); + } + try (BufferedOutputStream os = new BufferedOutputStream(new FileOutputStream(dst))) { if (metadata instanceof JpegImageMetadata) { if (lossy) { @@ -165,4 +198,21 @@ } } } + + /** + * Normalizes an angle to the range [0.0, 360.0[ degrees. + * This will fix any angle value <0 and >= 360 + * @param angle the angle to normalize (in degrees) + * @return the equivalent angle value in the range [0.0, 360.0[ + */ + private static Double checkAngle(Double angle) { + if (angle < 0.0) { + angle %= 360.0; // >-360.0...-0.0 + angle += 360.0; // >0.0...360.0 + } + if (angle >= 360.0) { + angle %= 360.0; + } + return angle; + } } Index: src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java =================================================================== --- src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java (revision 36433) +++ src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java (working copy) @@ -85,7 +85,7 @@ DecimalFormat dFormatter = new DecimalFormat("###0.000000"); for (ImageEntry e : layer.getImages()) { - /* Only write lat/lon to the file, if the position is known and + /* Only write lat/lon to the file, if the position is known and the GPS data changed. */ if (e.getPos() != null && e.hasNewGpsData()) { String pth = e.getFile().getAbsolutePath(); @@ -134,13 +134,13 @@ final JPanel settingsPanel = new JPanel(new GridBagLayout()); settingsPanel.setBorder(BorderFactory.createTitledBorder(tr("settings"))); - cont.add(settingsPanel, GBC.eol().insets(3,10,3,0)); + cont.add(settingsPanel, GBC.eol().insets(3, 10, 3, 0)); final JCheckBox backups = new JCheckBox(tr("keep backup files"), Config.getPref().getBoolean(KEEP_BACKUP, true)); - settingsPanel.add(backups, GBC.eol().insets(3,3,0,0)); + settingsPanel.add(backups, GBC.eol().insets(3, 3, 0, 0)); final JCheckBox setMTime = new JCheckBox(tr("change file modification time:"), Config.getPref().getBoolean(CHANGE_MTIME, false)); - settingsPanel.add(setMTime, GBC.std().insets(3,3,5,3)); + settingsPanel.add(setMTime, GBC.std().insets(3, 3, 5, 3)); final String[] mTimeModeArray = {"----", tr("to gps time"), tr("to previous value (unchanged mtime)")}; final JComboBox mTimeMode = new JComboBox<>(mTimeModeArray); @@ -154,7 +154,7 @@ } mTimeMode.setSelectedIndex(setMTime.isSelected() ? mTimeIdx : 0); } - settingsPanel.add(mTimeMode, GBC.eol().insets(3,3,3,3)); + settingsPanel.add(mTimeMode, GBC.eol().insets(3, 3, 3, 3)); setMTime.addActionListener(e -> { if (setMTime.isSelected()) { @@ -225,7 +225,6 @@ this.mTimeMode = mTimeMode; } - @Override protected void realRun() { List failedEntries = processEntries(images, false); @@ -359,7 +358,7 @@ mTime = e.getExifGpsInstant(); } } - if ( mTimeMode == MTIME_MODE_PREVIOUS_VALUE + if (mTimeMode == MTIME_MODE_PREVIOUS_VALUE // this is also the fallback if one of the other // modes failed to determine the modification time || (mTimeMode != 0 && mTime == null)) { @@ -370,8 +369,7 @@ chooseFiles(e.getFile()); if (canceled) return; - ExifGPSTagger.setExifGPSTag(fileFrom, fileTo, e.getPos().lat(), e.getPos().lon(), - e.getGpsInstant(), e.getSpeed(), e.getElevation(), e.getExifImgDir(), lossy); + ExifGPSTagger.setExifGPSTag(fileFrom, fileTo, e, lossy); if (mTime != null) { if (!fileTo.setLastModified(mTime.toEpochMilli())) @@ -390,7 +388,7 @@ return; } - File fileBackup = new File(file.getParentFile(),file.getName()+"_"); + File fileBackup = new File(file.getParentFile(), file.getName()+"_"); if (fileBackup.exists()) { confirm_override(); if (canceled) @@ -510,7 +508,7 @@ } private GeoImageLayer getLayer() { - return (GeoImageLayer)LayerListDialog.getInstance().getModel().getSelectedLayers().get(0); + return (GeoImageLayer) LayerListDialog.getInstance().getModel().getSelectedLayers().get(0); } /** @@ -538,3 +536,4 @@ return layers.size() == 1 && layers.get(0) instanceof GeoImageLayer; } } + Index: test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java =================================================================== --- test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java (revision 36433) +++ test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java (working copy) @@ -18,6 +18,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; class ExifGPSTaggerTest { @@ -24,11 +26,23 @@ @TempDir File tempFolder; + private static ImageEntry newImageEntry(String file, Double lat, Double lon, Instant exifTime, + Double speed, Double elevation, Double imgDir) { + ImageEntry entry = new ImageEntry(new File(file)); + entry.setPos(new LatLon(lat, lon)); + entry.setExifTime(exifTime); + entry.setSpeed(speed); + entry.setElevation(elevation); + entry.setExifImgDir(imgDir); + return entry; + } + @Test void testTicket11757() { final File in = new File(TestUtils.getTestDataRoot(), "_DSC1234.jpg"); final File out = new File(tempFolder, in.getName()); - assertDoesNotThrow(() -> ExifGPSTagger.setExifGPSTag(in, out, 12, 34, Instant.now(), 12.34, Math.E, Math.PI, true)); + final ImageEntry image = newImageEntry("test", 12d, 34d, Instant.now(), 12.34d, Math.E, Math.PI); + assertDoesNotThrow(() -> ExifGPSTagger.setExifGPSTag(in, out, image, true)); } @Test @@ -46,10 +60,27 @@ void testTicket11902() throws Exception { final File in = new File(TestUtils.getTestDataRoot(), "IMG_7250_small.JPG"); final File out = new File(tempFolder, in.getName()); - ExifGPSTagger.setExifGPSTag(in, out, 12, 34, Instant.now(), 12.34, Math.E, Math.PI, false); + final ImageEntry image = newImageEntry("test", 12d, 34d, Instant.now(), 12.34d, Math.E, Math.PI); + ExifGPSTagger.setExifGPSTag(in, out, image, true); final Process jhead = Runtime.getRuntime().exec(new String[]{"jhead", out.getAbsolutePath()}); final String stdout = new Scanner(jhead.getErrorStream()).useDelimiter("\\A").next(); System.out.println(stdout); assertFalse(stdout.contains("Suspicious offset of first Exif IFD value")); } + + @Test + public void testTicket24278() { + final File in = new File(TestUtils.getTestDataRoot(), "_DSC1234.jpg"); + final File out = new File(tempFolder, in.getName()); + final ImageEntry image = newImageEntry("test", 12d, 34d, Instant.now(), 12.34d, Math.E, Math.PI); + image.setExifGpsTrack(Math.PI); + image.setGpsDiffMode(2); + image.setGps2d3dMode(3); + image.setExifGpsProcMethod("GPS"); + image.setExifHPosErr(1.2d); + image.setExifGpsDop(2.5d); + image.setExifGpsDatum("WGS84"); + assertDoesNotThrow(() -> ExifGPSTagger.setExifGPSTag(in, out, image, true)); + /* TODO read temp file and assertEquals EXIF metadata values */ + } } Index: test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java =================================================================== --- test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java (revision 36433) +++ test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java (working copy) @@ -1,57 +1,57 @@ -package org.openstreetmap.josm.plugins.photo_geotagging; - - -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.StandardCopyOption; -import java.util.Arrays; -import java.util.List; - -import org.junit.jupiter.api.Test; -import org.openstreetmap.josm.TestUtils; -import org.openstreetmap.josm.data.coor.LatLon; -import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; -import org.openstreetmap.josm.plugins.photo_geotagging.GeotaggingAction.GeoTaggingRunnable; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -class GeotaggingActionTest { - - @Test - void testProcessEntries() throws IOException { - File original = new File(TestUtils.getTestDataRoot(), "_DSC1234.jpg"); - assertTrue(original.exists()); - - File copy = new File(TestUtils.getTestDataRoot(), "_DSC1234.copy.jpg"); - if (copy.exists()) copy.delete(); - //this method will actually override the file, so use a copy: - Files.copy(original.toPath(), copy.toPath(), StandardCopyOption.REPLACE_EXISTING); - - ImageEntry entry = new ImageEntry(copy); - entry.setPos(new LatLon(1, 2)); - List list = Arrays.asList(entry); - - GeoTaggingRunnable runnable = new GeotaggingAction.GeoTaggingRunnable(list, true, 0); - //this causes some warnings from the PleaseWaitRunnable because not all resources are available - //but that's irrelevant to the test - runnable.getProgressMonitor().beginTask("test"); - runnable.override_backup = true; - assertEquals(1, runnable.processEntries(list, false).size()); - assertEquals(0, runnable.processEntries(list, true).size()); - //test if overriding backup works: - assertEquals(0, runnable.processEntries(list, true).size()); - - runnable = new GeotaggingAction.GeoTaggingRunnable(list, false, 0); - runnable.getProgressMonitor().beginTask("test"); - //file is now "repaired" from operation above and lossless writing should work: - assertEquals(0, runnable.processEntries(list, false).size()); - assertEquals(0, runnable.processEntries(list, true).size()); - - File backup = new File(TestUtils.getTestDataRoot(), "_DSC1234.copy.jpg_"); - assertTrue(backup.exists()); - backup.delete(); - copy.delete(); - } -} +package org.openstreetmap.josm.plugins.photo_geotagging; + + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.Arrays; +import java.util.List; + +import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; +import org.openstreetmap.josm.plugins.photo_geotagging.GeotaggingAction.GeoTaggingRunnable; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class GeotaggingActionTest { + + @Test + void testProcessEntries() throws IOException { + File original = new File(TestUtils.getTestDataRoot(), "_DSC1234.jpg"); + assertTrue(original.exists()); + + File copy = new File(TestUtils.getTestDataRoot(), "_DSC1234.copy.jpg"); + if (copy.exists()) copy.delete(); + //this method will actually override the file, so use a copy: + Files.copy(original.toPath(), copy.toPath(), StandardCopyOption.REPLACE_EXISTING); + + ImageEntry entry = new ImageEntry(copy); + entry.setPos(new LatLon(1, 2)); + List list = Arrays.asList(entry); + + GeoTaggingRunnable runnable = new GeotaggingAction.GeoTaggingRunnable(list, true, 0); + //this causes some warnings from the PleaseWaitRunnable because not all resources are available + //but that's irrelevant to the test + runnable.getProgressMonitor().beginTask("test"); + runnable.override_backup = true; + assertEquals(1, runnable.processEntries(list, false).size()); + assertEquals(0, runnable.processEntries(list, true).size()); + //test if overriding backup works: + assertEquals(0, runnable.processEntries(list, true).size()); + + runnable = new GeotaggingAction.GeoTaggingRunnable(list, false, 0); + runnable.getProgressMonitor().beginTask("test"); + //file is now "repaired" from operation above and lossless writing should work: + assertEquals(0, runnable.processEntries(list, false).size()); + assertEquals(0, runnable.processEntries(list, true).size()); + + File backup = new File(TestUtils.getTestDataRoot(), "_DSC1234.copy.jpg_"); + assertTrue(backup.exists()); + backup.delete(); + copy.delete(); + } +}