[Patch] Upgrade metadata-extractor

[ebourg]

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.

[bastiK]

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

[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.

[Don-vip]

@bastiK: Has this bug been reported upstream ?

[ebourg]

I reported the bug:

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

It seems #5220 should be forwarded too.

[ebourg]

Updated patch addressing #5220 and #6162

[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?

[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.

[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.

[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).

[bastiK]

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.

[ebourg]

Updated patch including the metadata-extractor source files

[ebourg]

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

[bastiK]

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

[ebourg]

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

[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)

[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;
    }

[bastiK]

In 6127/josm:

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

[ebourg]

Thank you very much!

[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

[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.

[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?

[bastiK]

In 6128/josm:

see #8895 - add most recent changes

[ebourg]

It looks perfect now, thank you.

[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

[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.

[Don-vip]

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.

[Don-vip]

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

[Don-vip]

work in progress (unit test to finish)

[Don-vip]

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

[Don-vip]

In 6209/josm:

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

[ebourg]

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

[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.

[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.

[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 :)

[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.