[Patch] Support more image formats

[Bjoeni]

I think JOSM should support more image formats than just JPEG.

The attached patch determines the image file formats that are supported by the client system / the java version and the metadata library. It allows opening them through javax.ImageIO.

E.g. on Java 8 JOSM then allows opening *.bmp,*.gif,*.jpg,*.jpeg,*.png, on Java 11 also *.tif,*.tiff. I'm unsure if there are any other formats available / unavailable in other versions/systems but the script should be able to deal with that accordingly.

I had to remove some compiler exclusions in the build.xml of the metadata-extractor library (for the FileTypeDetector) which results in a ~120kb larger dist (.jar). I can try to strip that further down, but I would have to implement each format manually which could result in further problems and would also not be flexible in case future Java versions (or plugins) support more image formats. So I think it makes sense to rely on the existing library at the cost of a few kb.

--

Also attached is a patch for the plugin photo_geotagging which enables writing metadata to both JPEG and TIFF files and warns in case other unsupported formats are present.

[Don-vip]

I don't think it is worth to be fully dynamic here. Just keep it simple: hardcode *.bmp,*.gif,*.jpg,*.jpeg,*.png on Java < 11, *.bmp,*.gif,*.jpg,*.jpeg,*.png,*.tif,*.tiff on Java >= 11.

It's very rare to see new image formats to be shipped in the JDK. We can update the list manually if it ever happens. 120k for just this functionality is too much.

[Bjoeni]

It's not about which formats are supported (these are a couple lines of code), it's the metadata library that has a format determination (based on the first bytes of the file, not the extension).

I'll see what I can actually strip away from that library, it won't be bulletproof but should still work in most cases.

[Bjoeni]

Ok so the minimum I'm getting would be ~50kb more, that is by implementing all the formats mentioned above individually. Would that be fine?
Otherwise it's not possible to read the metadata in any format other than JPEG and TIFF (which includes the date taken). I could possibly use the last modified date instead but that's not always gonna be the same.

Or I could only implement it for some formats. The additional cost would be:

bmp: 18 kb
gif: 13 kb
png: 18 kb

What do you think? Worth 50kb or not?

[Bjoeni]

Actually, I think I could get rid of BMP metadata support since it ​doesn't contain the date taken anyways. And ​while GIFs can technically contain XMP data (and therefore exif data) I think it's neither commonly used nor does it really make much sense.

So we'd be down to an additional 18kb for PNGs, that should be fine I guess? For the other formats I'll just fall back to the lastModifiedDate.

[Don-vip]

I wonder if even PNG are used for this purpose?

[Bjoeni]

Well I'm using PNG for this purpose sometimes, because it supports alpha channels, lossless compression (somewhat better than TIFF) and is compatible with web browsers (unlike TIFF again).

Can't speak for others though.

[stoecker]

Milestone renamed

[Don-vip]

In 17548/josm:

fix #20341 - Support metadata from more image formats (patch by Bjoeni)

[Bjoeni]

Thanks!

Support metadata from more image formats

Just for the changelog: It's not just supporting the metadata but supporting the format, because before the patch JOSM was unable to open these formats completely :)

The photo_geotagging plugin should also be updated before the next stable as it will crash when the user attempts to write metadata to a non-JPEG image (see the second attachment above).
Could you please give me access to the plugin repo? Then I can try to commit it myself.

[Don-vip]

Replying to Bjoeni:

Could you please give me access to the plugin repo? Then I can try to commit it myself.

@Dirk is it possible to give an OSM svn account to Bjoeni?

[stoecker]

Replying to Don-vip:

Replying to Bjoeni:

Could you please give me access to the plugin repo? Then I can try to commit it myself.

@Dirk is it possible to give an OSM svn account to Bjoeni?

Yes sure.

@Bjoerni: Can you please insert a name in the Trac account settings.

[Bjoeni]

Can you please insert a name in the Trac account settings

done!

[stoecker]

Replying to Bjoeni:

Can you please insert a name in the Trac account settings

done!

You should have access to plugin-svn now.

[skyper]

Replying to Don-vip:

In 17548/josm:

fix #20341 - Support metadata from more image formats (patch by Bjoeni)

Jenkins reports two PMDs and some lower warnings.

[Bjoeni]

In 35715/osm:

fix #20341 - support more image formats and writing metadata to TIFF files

[Bjoeni]

Replying to stoecker:

Replying to Bjoeni:

Can you please insert a name in the Trac account settings

done!

You should have access to plugin-svn now.

Yep that worked, thank you Dirk!

[Bjoeni]

Replying to skyper:

Replying to Don-vip:

In 17548/josm:

fix #20341 - Support metadata from more image formats (patch by Bjoeni)

Jenkins reports two PMDs and some lower warnings.

I'll have a look

[Bjoeni]

fix Jenkins warnings and NPE caused by malformed images

[Bjoeni]

I'm gonna reopen this ticket since I guess it makes sense to fix the potential NPE before the next stable

[Don-vip]

toUnmodifiableList has been introduced in Java 10, we still need Java 8 compatibility.

[Bjoeni]

Damned, I always forget that. One second, I'll change it.

Let's hope that the number of Java 8 users actually goes down in the foreseeable future...

[Don-vip]

Replying to Bjoeni:

Let's hope that the number of Java 8 users actually goes down in the foreseeable future...

See #17858. Until I find enough time to make all the required changes it's unrealistic to see a Java 11 transition before the end of the year at best.

[Don-vip]

In 17553/josm:

fix #20341 - fix Jenkins warnings and NPE caused by malformed images (patch by Bjoeni)

[Don-vip]

Thanks a lot!

[Don-vip]

In 35725/osm:

see #20341 - see #20046 - fix i18n string

[stoecker]

Replying to Don-vip:

In 35725/osm:

see #20341 - see #20046 - fix i18n string

I don't consider replacing the correct trn() usage by handcoded stuff a fix. This will break a lot of languages! Extracting the single case is ok, but the other one still needs trn()!

[Don-vip]

Replying to stoecker:

I don't consider replacing the correct trn() usage by handcoded stuff a fix. This will break a lot of languages! Extracting the single case is ok, but the other one still needs trn()!

The old string was completely broken and was untranslatable on Launchpad. I didn't see any better solution.

[stoecker]

As said. Make the second one trn() again.

[Don-vip]

In 35726/osm:

see #20341 - see #20046 - fix i18n string