Opened 11 months ago

Closed 11 months ago

Last modified 11 months ago

#13668 closed enhancement (fixed)

[Patch] Simplify ImageEntry#extractExif?

Reported by: simon04 Owned by: holgermappt
Priority: normal Milestone: 16.10
Component: Core image mapping Version:
Keywords: Cc:


While looking into #13376, I stumbled over a possible simplification at ImageEntry#extractExif (relating to #12255):

  • src/org/openstreetmap/josm/gui/layer/geoimage/

    diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ b/src/org/openstreetmap/josm/gui/layer/geoimage/
    index 1c3a4d3..5acb2c1 100644
    a b public void extractExif() { 
    523523            Main.debug(ex);
    524524        }
    526         // Time and date. We can have these cases:
    527         // 1) GPS_TIME_STAMP not set -> date/time will be null
    528         // 2) GPS_DATE_STAMP not set -> use EXIF date or set to default
    529         // 3) GPS_TIME_STAMP and GPS_DATE_STAMP are set
    530         int[] timeStampComps = dirGps.getIntArray(GpsDirectory.TAG_TIME_STAMP);
    531         if (timeStampComps != null) {
    532             int gpsHour = timeStampComps[0];
    533             int gpsMin = timeStampComps[1];
    534             int gpsSec = timeStampComps[2];
    535             Calendar cal = new GregorianCalendar(TimeZone.getTimeZone("UTC"));
    537             // We have the time. Next step is to check if the GPS date stamp is set.
    538             // dirGps.getString() always succeeds, but the return value might be null.
    539             String dateStampStr = dirGps.getString(GpsDirectory.TAG_DATE_STAMP);
    540             if (dateStampStr != null && dateStampStr.matches("^\\d+:\\d+:\\d+$")) {
    541                 String[] dateStampComps = dateStampStr.split(":");
    542                 cal.set(Calendar.YEAR, Integer.parseInt(dateStampComps[0]));
    543                 cal.set(Calendar.MONTH, Integer.parseInt(dateStampComps[1]) - 1);
    544                 cal.set(Calendar.DAY_OF_MONTH, Integer.parseInt(dateStampComps[2]));
    545             } else {
    546                 // No GPS date stamp in EXIF data. Copy it from EXIF time.
    547                 // Date is not set if EXIF time is not available.
    548                 if (hasExifTime()) {
    549                     // Time not set yet, so we can copy everything, not just date.
    550                     cal.setTime(getExifTime());
    551                 }
    552             }
    554             cal.set(Calendar.HOUR_OF_DAY, gpsHour);
    555             cal.set(Calendar.MINUTE, gpsMin);
    556             cal.set(Calendar.SECOND, gpsSec);
    558             setExifGpsTime(cal.getTime());
    559         }
     526        setExifGpsTime(dirGps.getGpsDate());
    560527    }

Are there some aspects against this simplification?

Attachments (0)

Change History (3)

comment:1 Changed 11 months ago by holgermappt

One difference is that no GPS time is extracted if GPS_TIME_STAMP is set but GPS_DATE_STAMP is not. I think the photo_geotagging plugin used to look at the exifGpsTime to decide what photo to update. It now uses hasNewGpsData() instead, so it is not that important anymore to have an EXIF GPS time. I don't know if there are photos that have that combination. The current code will set a wrong GPS date if GPS_DATE_STAMP is not set, exifTime is one second before midnight and GPS_TIME_STAMP is one second after midnight. But at the time the code was written is was more important to have an exifGpsTime than to have it right.

The other difference is that the current code does not change exifGpsTime if it is not set in the EXIF data. That should be changed in the patch to align it with the other code in extractExif, i.e. call setExifGpsTime() only if dirGps.getGpsDate() is not null. There might be a case where exifGpsTime is set during GPX correlation and then extractExif() is called (e.g. in the photoadjust plugin property editor). In that case the exifGpsTime should not be set to null.

For me the simplification is fine with the mentioned modification (set only if not null).

comment:2 Changed 11 months ago by simon04

Resolution: fixed
Status: newclosed

In 11037/josm:

fix #13668 - Simplify ImageEntry#extractExif

comment:3 Changed 11 months ago by simon04

Milestone: 16.0916.10

Milestone renamed

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain holgermappt.
as The resolution will be set. Next status will be 'closed'.
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.