Index: /applications/editors/josm/plugins/photo_geotagging/build.xml =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/build.xml (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/build.xml (revision 36436) @@ -5,5 +5,5 @@ - + Index: /applications/editors/josm/plugins/photo_geotagging/pom.xml =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/pom.xml (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/pom.xml (revision 36436) @@ -17,5 +17,5 @@ src - 19044 + 19389 Paul Hartmann org.openstreetmap.josm.plugins.photo_geotagging.GeotaggingPlugin @@ -32,4 +32,19 @@ 1.0-SNAPSHOT provided + + + org.junit.jupiter + junit-jupiter + test + + + org.wiremock + wiremock + test + + + org.junit.vintage + junit-vintage-engine + test Index: /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTagger.java (revision 36436) @@ -10,5 +10,4 @@ import java.io.IOException; import java.text.DecimalFormat; -import java.time.Instant; import java.util.Calendar; import java.util.GregorianCalendar; @@ -26,4 +25,5 @@ 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; /** @@ -41,17 +41,14 @@ * @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 36436 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 @@ -60,6 +57,6 @@ } - 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; @@ -81,14 +78,14 @@ 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); - - if (gpsTime != null) { + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_VERSION_ID, (byte) 2, (byte) 3, (byte) 0, (byte) 0); + + if (imageEntry.getGpsInstant() != null) { Calendar calendar = new GregorianCalendar(TimeZone.getTimeZone("UTC")); - calendar.setTimeInMillis(gpsTime.toEpochMilli()); - - final int year = calendar.get(Calendar.YEAR); - final int month = calendar.get(Calendar.MONTH) + 1; - final int day = calendar.get(Calendar.DAY_OF_MONTH); - final int hour = calendar.get(Calendar.HOUR_OF_DAY); + calendar.setTimeInMillis(imageEntry.getGpsInstant().toEpochMilli()); + + final int year = calendar.get(Calendar.YEAR); + final int month = calendar.get(Calendar.MONTH) + 1; + final int day = calendar.get(Calendar.DAY_OF_MONTH); + final int hour = calendar.get(Calendar.HOUR_OF_DAY); final int minute = calendar.get(Calendar.MINUTE); final int second = calendar.get(Calendar.SECOND); @@ -118,7 +115,7 @@ } - outputSet.setGpsInDegrees(lon, lat); - - if (speed != null) { + outputSet.setGpsInDegrees(imageEntry.getPos().lon(), imageEntry.getPos().lat()); + + if (imageEntry.getSpeed() != null) { gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_SPEED_REF); gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED_REF, @@ -126,30 +123,66 @@ gpsDirectory.removeField(GpsTagConstants.GPS_TAG_GPS_SPEED); - gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED, RationalNumber.valueOf(speed)); - } - - if (ele != null) { - byte eleRef = ele >= 0 ? (byte) 0 : (byte) 1; + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_SPEED, RationalNumber.valueOf(imageEntry.getSpeed())); + } + + 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))); - } - - if (imgDir != null) { + gpsDirectory.add(GpsTagConstants.GPS_TAG_GPS_ALTITUDE, RationalNumber.valueOf(Math.abs(imageEntry.getElevation()))); + } + + 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())); } @@ -166,3 +199,20 @@ } } + + /** + * 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: /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/src/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingAction.java (revision 36436) @@ -65,5 +65,5 @@ static final int MTIME_MODE_PREVIOUS_VALUE = 2; - public GeotaggingAction() { + GeotaggingAction() { super(tr("Write coordinates to image header"), ImageProvider.get("geotagging")); } @@ -86,5 +86,5 @@ 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()) { @@ -135,25 +135,25 @@ 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); - { - String mTimeModePref = Config.getPref().get(MTIME_MODE, null); - int mTimeIdx = 0; - if ("gps".equals(mTimeModePref)) { - mTimeIdx = 1; - } else if ("previous".equals(mTimeModePref)) { - mTimeIdx = 2; - } - mTimeMode.setSelectedIndex(setMTime.isSelected() ? mTimeIdx : 0); - } - settingsPanel.add(mTimeMode, GBC.eol().insets(3,3,3,3)); + + String mTimeModePrefName = Config.getPref().get(MTIME_MODE, null); + int mTimeIdx = 0; + if ("gps".equals(mTimeModePrefName)) { + mTimeIdx = 1; + } else if ("previous".equals(mTimeModePrefName)) { + mTimeIdx = 2; + } + mTimeMode.setSelectedIndex(setMTime.isSelected() ? mTimeIdx : 0); + + settingsPanel.add(mTimeMode, GBC.eol().insets(3, 3, 3, 3)); setMTime.addActionListener(e -> { @@ -219,5 +219,5 @@ private int currentIndex; - public GeoTaggingRunnable(List images, boolean keep_backup, int mTimeMode) { + GeoTaggingRunnable(List images, boolean keep_backup, int mTimeMode) { super(tr("Photo Geotagging Plugin")); this.images = images; @@ -225,5 +225,4 @@ this.mTimeMode = mTimeMode; } - @Override @@ -258,5 +257,6 @@ } sb.append("
") - .append(tr("This can likely be fixed by rewriting the entire EXIF section, however some (rare) unknown tags may get lost in the process.
" + .append(tr("This can likely be fixed by rewriting the entire EXIF section, however some" + + " (rare) unknown tags may get lost in the process.
" + "Would you like to proceed anyway?")); @@ -360,5 +360,5 @@ } } - 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 @@ -371,6 +371,5 @@ 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) { @@ -391,5 +390,5 @@ } - File fileBackup = new File(file.getParentFile(),file.getName()+"_"); + File fileBackup = new File(file.getParentFile(), file.getName()+"_"); if (fileBackup.exists()) { confirm_override(); @@ -511,5 +510,5 @@ private GeoImageLayer getLayer() { - return (GeoImageLayer)LayerListDialog.getInstance().getModel().getSelectedLayers().get(0); + return (GeoImageLayer) LayerListDialog.getInstance().getModel().getSelectedLayers().get(0); } @@ -539,2 +538,3 @@ } } + Index: /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/ExifGPSTaggerTest.java (revision 36436) @@ -1,2 +1,4 @@ +// License: GPL. For details, see LICENSE file. +// SPDX-License-Identifier: GPL-2.0-or-later package org.openstreetmap.josm.plugins.photo_geotagging; @@ -19,4 +21,6 @@ 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 { @@ -25,9 +29,21 @@ 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)); } @@ -47,5 +63,6 @@ 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(); @@ -53,3 +70,19 @@ 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: /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java =================================================================== --- /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java (revision 36435) +++ /applications/editors/josm/plugins/photo_geotagging/test/unit/org/openstreetmap/josm/plugins/photo_geotagging/GeotaggingActionTest.java (revision 36436) @@ -1,4 +1,5 @@ +// License: GPL. For details, see LICENSE file. +// SPDX-License-Identifier: GPL-2.0-or-later package org.openstreetmap.josm.plugins.photo_geotagging; - import java.io.File;