Modify

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#8895 closed enhancement (fixed)

[Patch] Upgrade metadata-extractor

Reported by: ebourg Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: exif Cc:

Description

JOSM uses the version 2.3 of metadata-extractor but the version 2.6 is now available. Here is a patch that updates the code to use the latest version.

Attachments (6)

upgrade-metadata-extractor.patch (8.1 KB) - added by ebourg 6 years ago.
upgrade-metadata-extractor2.patch (8.7 KB) - added by ebourg 6 years ago.
Updated patch addressing #5220 and #6162
upgrade-metadata-extractor3.patch (970.1 KB) - added by ebourg 6 years ago.
Updated patch including the metadata-extractor source files
src.tar.gz (103.7 KB) - added by ebourg 6 years ago.
exif-direction-example.jpg (227.1 KB) - added by Don-vip 6 years ago.
regression test file (works fine with r6115, not after library upgrade)
8895.patch (11.9 KB) - added by Don-vip 6 years ago.
work in progress (unit test to finish)

Download all attachments as: .zip

Change History (42)

Changed 6 years ago by ebourg

comment:1 Changed 6 years ago by stoecker

Summary: Upgrade metadata-extractor[PATCH] Upgrade metadata-extractor

comment:2 Changed 6 years ago by bastiK

Have you verified, that the example from #6162 is still working?

comment:3 Changed 6 years ago by Don-vip

Keywords: exif added

comment:4 Changed 6 years ago by ebourg

Thank you for pointing me to #6162, the patch fails to work with this image. I'll rework the patch and forward the issue to metadata-extractor.

comment:5 Changed 6 years ago by Don-vip

@bastiK: Has this bug been reported upstream ?

Last edited 6 years ago by Don-vip (previous) (diff)

comment:6 Changed 6 years ago by ebourg

I reported the bug:

http://code.google.com/p/metadata-extractor/issues/detail?id=84

It seems #5220 should be forwarded too.

Changed 6 years ago by ebourg

Updated patch addressing #5220 and #6162

comment:7 Changed 6 years ago by bastiK

When I download the source distribution metadata-extractor 2.6.4, it doesn't compile. Classes com.adobe.xmp.XMPMeta and so on seem to be missing. How do you get these?

comment:8 in reply to:  6 Changed 6 years ago by bastiK

Replying to ebourg:

I reported the bug:

http://code.google.com/p/metadata-extractor/issues/detail?id=84

Thanks for reporting this!

It seems #5220 should be forwarded too.

Yes, would be nice.

comment:9 Changed 6 years ago by ebourg

I haven't tried to compile metadata-extractor, I just dropped the jar in the lib directory and removed the classes in the src/com/drew directory.

Do you absolutely need to compile this dependency? Using the jar directly would be easier.

comment:10 Changed 6 years ago by bastiK

Yes, that's how we do it at the moment. One reason is that unused classes are dropped automatically, which reduces the filesize of the binary. Another is security/transparency (so we and others can verify what is shipped).

comment:11 Changed 6 years ago by bastiK

Owner: changed from team to ebourg
Status: newneedinfo

This library seems to be fairly stable, so an update doesn't have high priority. I will still do it, if you provide a patch for the lib update, as well.

Changed 6 years ago by ebourg

Updated patch including the metadata-extractor source files

comment:12 Changed 6 years ago by ebourg

I updated the patch, it now includes the subset of the metadata-extractor source files used by JOSM.

comment:13 Changed 6 years ago by ebourg

Owner: changed from ebourg to team
Status: needinfonew

comment:14 Changed 6 years ago by bastiK

I cannot apply your patch. This is probably related to svn:eol-style and automated changes of newline format.

comment:15 Changed 6 years ago by ebourg

Ok, let's try something else then. I'll attach an archive containing the metadata-extractor source files and the modified JOSM files.

Changed 6 years ago by ebourg

Attachment: src.tar.gz added

comment:16 Changed 6 years ago by bastiK

I get NPE when opening an image that is not geotagged:

ava.lang.NullPointerException
	at org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer.extractExif(GeoImageLayer.java:536)
	at org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer.access$000(GeoImageLayer.java:75)
	at org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer$Loader.realRun(GeoImageLayer.java:157)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.doRealRun(PleaseWaitRunnable.java:82)
	at org.openstreetmap.josm.gui.PleaseWaitRunnable.run(PleaseWaitRunnable.java:150)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1146)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:679)

comment:17 Changed 6 years ago by ebourg

Good catch. A simple null check just before the try/catch block starting at line 535 will fix it:

    if (dirGps == null) {
        e.setExifCoor(null);
        e.setPos(null);
        return;
    }

comment:18 Changed 6 years ago by bastiK

Resolution: fixed
Status: newclosed

In 6127/josm:

applied #8895 - Upgrade metadata-extractor to v. 2.6.4 (patch by ebourg)

comment:19 Changed 6 years ago by ebourg

Thank you very much!

comment:20 Changed 6 years ago by ebourg

I just updated my code and got some conflicts. Some classes no longer exist in metadata-extractor 2.6.4 and should be removed:

src/com/drew/metadata/SampleUsage.java
src/com/drew/metadata/exif/ExifDescriptor.java
src/com/drew/metadata/exif/SonyMakernoteDescriptor.java
src/com/drew/metadata/exif/SonyMakernoteDirectory.java
src/com/drew/metadata/exif/ExifProcessingException.java
src/com/drew/metadata/exif/ExifDirectory.java
src/com/drew/metadata/iptc/IptcProcessingException.java

comment:21 Changed 6 years ago by ebourg

Also it looks like the version of GeoImageLayer that was committed in r6127/josm is the one from my first patch. The archive has the most recent version (minus the null check) that adresses #6162.

comment:22 in reply to:  20 Changed 6 years ago by bastiK

Replying to ebourg:

I just updated my code and got some conflicts. Some classes no longer exist in metadata-extractor 2.6.4 and should be removed:

src/com/drew/metadata/SampleUsage.java
src/com/drew/metadata/exif/ExifDescriptor.java
src/com/drew/metadata/exif/SonyMakernoteDescriptor.java
src/com/drew/metadata/exif/SonyMakernoteDirectory.java
src/com/drew/metadata/exif/ExifProcessingException.java
src/com/drew/metadata/exif/ExifDirectory.java
src/com/drew/metadata/iptc/IptcProcessingException.java

These files should be gone, could you check again?

comment:23 Changed 6 years ago by bastiK

In 6128/josm:

see #8895 - add most recent changes

comment:24 Changed 6 years ago by ebourg

It looks perfect now, thank you.

comment:25 Changed 6 years ago by AlfonZ

Introduced findbugs warning:

<BugInstance type="REC_CATCH_EXCEPTION" priority="2" abbrev="REC" category="STYLE">
  <Class classname="org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer">
    <SourceLine classname="org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer" start="74" end="860" sourcefile="GeoImageLayer.java" sourcepath="org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java"/>
  </Class>
  <Method classname="org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer" name="extractExif" signature="(Lorg/openstreetmap/josm/gui/layer/geoimage/ImageEntry;)V" isStatic="true">
    <SourceLine classname="org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer" start="518" end="621" startBytecode="0" endBytecode="1618" sourcefile="GeoImageLayer.java" sourcepath="org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java"/>
  </Method>
  <SourceLine classname="org.openstreetmap.josm.gui.layer.geoimage.GeoImageLayer" start="603" end="603" startBytecode="486" endBytecode="486" sourcefile="GeoImageLayer.java" sourcepath="org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java"/>
</BugInstance>

http://findbugs.sourceforge.net/bugDescriptions.html#REC_CATCH_EXCEPTION

comment:26 Changed 6 years ago by ebourg

I'm not sure to understand why Findbugs reports this one. That may be caused by the JpegProcessingException thrown by JpegMetadataReader.readMetadata(), we catch its super type CompoundException.

comment:27 Changed 6 years ago by Don-vip

Resolution: fixed
Status: closedreopened

comment:28 Changed 6 years ago by Don-vip

Priority: minornormal
Summary: [PATCH] Upgrade metadata-extractorUpgrade metadata-extractor

Seen on French mailing list, this patch did broke support of some geottaged pictures. Here's one taken with a Sony DSC-HX5V that is correctly loaded with r6115 but not with r6207.

Changed 6 years ago by Don-vip

Attachment: exif-direction-example.jpg added

regression test file (works fine with r6115, not after library upgrade)

Changed 6 years ago by Don-vip

Attachment: 8895.patch added

work in progress (unit test to finish)

comment:29 Changed 6 years ago by Don-vip

Summary: Upgrade metadata-extractor[Patch] Upgrade metadata-extractor

comment:30 Changed 6 years ago by Don-vip

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

comment:31 Changed 6 years ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 6209/josm:

fix #8895, fix #9030 - fix regression in geottaged pictures support caused by metadata-extractor upgrade, enhance ExifReader, add JUnit tests + javadoc

comment:32 Changed 6 years ago by ebourg

Would that make sense to forward the ExifReader changes to the metadata-extractor developers, or is this too specific to josm?

comment:33 Changed 6 years ago by cedric.olivier@…

It is a problem, if there are changes done by Josm wich are not reported to upstream.

When packaging JOSM we have to use depedencies, and actually we can't update JOSM because metadata-extractor used by JOSM is not the same as metadata-extractor provided by upstream.

comment:34 Changed 6 years ago by bastiK

@Don-vip: Could you please explain, how this change (http://josm.openstreetmap.de/changeset/6209/josm/trunk/src/com/drew/metadata/exif/ExifReader.java) fixes the problem in #9030? The original code aborts reading of meta data in case of an error. As far as I'm aware, the image exif-direction-example.jpg is valid exif and should not contain an error.

comment:35 Changed 6 years ago by Don-vip

Honestly I don't remember :) I think I found it was this error that prevented the image to be parsed, and when I saw ignoring it resolved the issue I didn't investigate much further :) I you find a proper patch or a solution inside JOSM code, feel free to change it :)

comment:36 Changed 5 years ago by ebourg

FYI I forwarded the issue upstream: https://code.google.com/p/metadata-extractor/issues/detail?id=94

The modification made by @Don-vip partially reverts a change in metadata-extractor, so it's as bad/good as the previous version of metadata-extractor used by JOSM.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.