Modify

Opened 17 months ago

Closed 15 months ago

Last modified 15 months ago

#20341 closed enhancement (fixed)

[Patch] Support more image formats

Reported by: Bjoeni Owned by: team
Priority: normal Milestone: 21.02
Component: Core image mapping Version:
Keywords: java11 Cc: stoecker

Description

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.

Attachments (5)

20341-core.diff (34.1 KB) - added by Bjoeni 17 months ago.
20341-photo_geotagging.diff (10.3 KB) - added by Bjoeni 17 months ago.
20341-core-V2.diff (35.2 KB) - added by Bjoeni 17 months ago.
20341-core-V3.diff (8.5 KB) - added by Bjoeni 15 months ago.
fix Jenkins warnings and NPE caused by malformed images
20341-core-V4.diff (8.8 KB) - added by Bjoeni 15 months ago.

Download all attachments as: .zip

Change History (34)

Changed 17 months ago by Bjoeni

Attachment: 20341-core.diff added

Changed 17 months ago by Bjoeni

Attachment: 20341-photo_geotagging.diff added

comment:1 Changed 17 months ago by 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.

comment:2 Changed 17 months ago by Don-vip

Keywords: java11 added

comment:3 Changed 17 months ago by 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.

comment:4 Changed 17 months ago by 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?

comment:5 Changed 17 months ago by 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.

Last edited 17 months ago by Bjoeni (previous) (diff)

Changed 17 months ago by Bjoeni

Attachment: 20341-core-V2.diff added

comment:6 Changed 16 months ago by Don-vip

I wonder if even PNG are used for this purpose?

comment:7 Changed 16 months ago by 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.

comment:8 Changed 16 months ago by stoecker

Milestone: 21.0121.02

Milestone renamed

comment:9 Changed 15 months ago by Don-vip

Resolution: fixed
Status: newclosed

In 17548/josm:

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

comment:10 Changed 15 months ago by 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.

Last edited 15 months ago by Bjoeni (previous) (diff)

comment:11 in reply to:  10 ; Changed 15 months ago by Don-vip

Cc: stoecker added

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?

comment:12 in reply to:  11 Changed 15 months ago by 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.

comment:13 Changed 15 months ago by Bjoeni

Can you please insert a name in the Trac account settings

done!

comment:14 in reply to:  13 ; Changed 15 months ago by stoecker

Replying to Bjoeni:

Can you please insert a name in the Trac account settings

done!

You should have access to plugin-svn now.

comment:15 in reply to:  9 ; Changed 15 months ago by 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.

comment:16 Changed 15 months ago by Bjoeni

In 35715/osm:

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

comment:17 in reply to:  14 Changed 15 months ago by 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!

comment:18 in reply to:  15 Changed 15 months ago by 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

Changed 15 months ago by Bjoeni

Attachment: 20341-core-V3.diff added

fix Jenkins warnings and NPE caused by malformed images

comment:19 Changed 15 months ago by Bjoeni

Resolution: fixed
Status: closedreopened

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

comment:20 Changed 15 months ago by Don-vip

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

comment:21 Changed 15 months ago by 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...

Changed 15 months ago by Bjoeni

Attachment: 20341-core-V4.diff added

comment:22 in reply to:  21 Changed 15 months ago by 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.

comment:23 Changed 15 months ago by Don-vip

Resolution: fixed
Status: reopenedclosed

In 17553/josm:

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

comment:24 Changed 15 months ago by Don-vip

Thanks a lot!

comment:25 Changed 15 months ago by Don-vip

In 35725/osm:

see #20341 - see #20046 - fix i18n string

comment:26 in reply to:  25 ; Changed 15 months ago by 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()!

Last edited 15 months ago by stoecker (previous) (diff)

comment:27 in reply to:  26 Changed 15 months ago by 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.

comment:28 Changed 15 months ago by stoecker

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

comment:29 Changed 15 months ago by Don-vip

In 35726/osm:

see #20341 - see #20046 - fix i18n string

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.