Modify

Opened 11 years ago

Closed 11 years ago

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

Download all attachments as: .zip

Change History (42)

by ebourg, 11 years ago

comment:1 by stoecker, 11 years ago

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

comment:2 by bastiK, 11 years ago

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

comment:3 by Don-vip, 11 years ago

Keywords: exif added

comment:4 by ebourg, 11 years ago

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 by Don-vip, 11 years ago

@bastiK: Has this bug been reported upstream ?

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

comment:6 by ebourg, 11 years ago

I reported the bug:

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

It seems #5220 should be forwarded too.

by ebourg, 11 years ago

Updated patch addressing #5220 and #6162

comment:7 by bastiK, 11 years ago

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?

in reply to:  6 comment:8 by bastiK, 11 years ago

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 by ebourg, 11 years ago

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 by bastiK, 11 years ago

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 by bastiK, 11 years ago

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.

by ebourg, 11 years ago

Updated patch including the metadata-extractor source files

comment:12 by ebourg, 11 years ago

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

comment:13 by ebourg, 11 years ago

Owner: changed from ebourg to team
Status: needinfonew

comment:14 by bastiK, 11 years ago

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

comment:15 by ebourg, 11 years ago

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

by ebourg, 11 years ago

Attachment: src.tar.gz added

comment:16 by bastiK, 11 years ago

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 by ebourg, 11 years ago

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 by bastiK, 11 years ago

Resolution: fixed
Status: newclosed

In 6127/josm:

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

comment:19 by ebourg, 11 years ago

Thank you very much!

comment:20 by ebourg, 11 years ago

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 by ebourg, 11 years ago

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.

in reply to:  20 comment:22 by bastiK, 11 years ago

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 by bastiK, 11 years ago

In 6128/josm:

see #8895 - add most recent changes

comment:24 by ebourg, 11 years ago

It looks perfect now, thank you.

comment:25 by AlfonZ, 11 years ago

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 by ebourg, 11 years ago

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 by Don-vip, 11 years ago

Resolution: fixed
Status: closedreopened

comment:28 by Don-vip, 11 years ago

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.

by Don-vip, 11 years ago

Attachment: exif-direction-example.jpg added

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

by Don-vip, 11 years ago

Attachment: 8895.patch added

work in progress (unit test to finish)

comment:29 by Don-vip, 11 years ago

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

comment:30 by Don-vip, 11 years ago

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

comment:31 by Don-vip, 11 years ago

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 by ebourg, 10 years ago

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

comment:33 by cedric.olivier@…, 10 years ago

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 by bastiK, 10 years ago

@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 by Don-vip, 10 years ago

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 by ebourg, 10 years ago

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