tannerhelland / PhotoDemon

A free portable photo editor focused on pro-grade features, high performance, and maximum usability.
https://photodemon.org
Other
1.36k stars 200 forks source link

Request: Add "Preserve Metadata" option to preferences #59

Closed audioglider closed 11 years ago

audioglider commented 11 years ago

I noticed the capability to retain metadata is missing when saving to a new file. This is pretty easy to implement since Freeimage already has the built-in functionality. Just before calling FreeImage_SaveEx in the save functions I inserted:

Dim fi_DIBSrc As Long fi_DIBSrc = FreeImage_LoadEx(srcPDImage.LocationOnDisk, FILO_LOAD_NOPIXELS) FreeImage_CloneMetadataEx fi_DIBSrc, fi_DIB FreeImage_Unload fi_DIBSrc

This is just a rough outline but it works. I've already tested and it successfully saved IPTC/XMP tags. Unfortfunately, for some reason FreeImage doesn't support the writing of EXIF tags. This should not be difficult to add using additional class. I believe I have an EXIF class I wrote in VB6 many years ago.

tannerhelland commented 11 years ago

Thanks, audioglider. EXIF metadata is on the roadmap for 5.6 (the next release). Unfortunately implementation needs to be fairly complex. A few comments:

FreeImage writes EXIF data by default. (See Metadata section in official documentation: http://downloads.sourceforge.net/freeimage/FreeImage3154.pdf).

IPTC/XMP tags are a subset of the full metadata implementation, specifically types 6 and 7 of FreeImage's metadata model. (See page 72 of documentation.)

Here's the problem with cloning metadata - say you load an image, then resize it, then save it to file. If you clone metadata, any size tags need to be rewritten or there will be conflicting data. Same thing happens for the "Orientation" tag which PhotoDemon reads and processes automatically. Smartphones use accelerometers to write the Orientation tag, so even if you hold the camera upside-down, the Orientation tag allows the image to be displayed properly. (Try it with PhotoDemon - it's actually pretty neat! Even Windows photo viewer doesn't do this.)

However, once PhotoDemon reads the tag and properly orients the image, it needs to rewrite or erase the original Orientation tag. Cloning it leads to problems.

So there are some wrinkles to consider. That's why I've put off metadata support for so long.

If you have a metadata class I'm happy to take a look, but right now the plan is rely completely on FreeImage. There are too many manufacturer-specific tags that I can't hope to process on my own, but FreeImage is pretty good about handling all major ones.

audioglider commented 11 years ago

I didn't realize this was already in your queue. I was already planning on taking into account resize changes, but I'm glad you mentioned orientation. I didn't even consider that.

As for the Freeimage documentation, if you go to Table 13 it shows that only read support is supported for standard EXIF tags. I will do some further testing to see if this is really the case.

tannerhelland commented 11 years ago

No worries mate, no way you could have known I had EXIF support planned.

So FreeImage's metadata support is a bit weird, but here's my understanding - FreeImage divides various types of metadata into individual categories for ease of use. (Table 13 that you mention.) It even subdivides EXIF into sub-categories if you only want to read a single type.

But these EXIF subcategories only work when reading tags. You can still write EXIF tags, you just have to write them as type 11 - FIMD_EXIF_RAW. This makes sense as categorization is pointless while writing (all tags get written as standard EXIF tags), but for reading you may only want to deal with a specific type.

The one exception to this is generic EXIF "comments", which you can write out as type 0 if you want.

I could be misunderstanding, but from prior discussions in the FreeImage forums (e.g. http://sourceforge.net/p/freeimage/discussion/36111/thread/2d087f91/) I think this is correct. Additional testing would be extremely welcome.

Note to self: a preference is also needed for maintaining non-EXIF file metadata (date, time) during batch convert.

tannerhelland commented 11 years ago

NOTE: if it proves impossible to write out full EXIF tags in EXIF format with FreeImage, a possible solution may be to map EXIF tags to XMP format, then write those out. Here's a link to a document with mapping instructions between EXIF and XMP for major tags:

http://www.cipa.jp/english/hyoujunka/kikaku/pdf/DC-010-2012_E.pdf

tannerhelland commented 11 years ago

ADDITIONAL NOTE: after additional research, I'm still unclear on what FreeImage actually supports when it comes to EXIF writing. From page 117 of the official documentation:

"FIMD_EXIF_RAW

This model stores Exif data as a single undecoded raw buffer. FIMD EXIF_RAW represents a single Exif buffer and is indexed using the tag field name ExifRaw.

The FIMD_EXIF_RAW metadata model does not replace the other EXIF models. This is an additional metadata used to store Exif data as a raw binary buffer. Saving Exif data can be very complex, but saving Exif raw data is quite easy (at least with the JPEG format). Thus if it is possible to preserve Exif information connected with the JPEG files if such file is e.g. loaded, resized then saved."

Emphasis mine.

tannerhelland commented 11 years ago

Okay, after some testing I have confirmation on a few key issues.

1) FIMD_EXIF_RAW is only available upon loading a JPEG file with valid EXIF data. It contains the entire EXIF chunk of the file as a single byte array. It cannot (easily) be modified. The data is not human-readable. If stored, the entire chunk CAN be written out later to a new file, but note that there is no way to modify the raw data, for example to change a tag that may no longer be correct.

2) FIMD_EXIF_RAW can only be used once with a single buffer. Thus, I cannot write out a bunch of tags as FIMD_EXIF_RAW - each newly placed tag will overwrite the last, and they won't be in the proper format anyway. So that strategy is out.

3) I have complete code written for writing out metadata at save time, but obviously most EXIF tags cannot be written this way via FreeImage. I am currently investigating other alternatives.

tannerhelland commented 11 years ago

More bad news - it is certainly possible to map EXIF tags to XMP format, and I've already got export code working (for a few basic tags, anyway). But this is not a viable solution, because FreeImage doesn't parse XMP tags at all. It just returns the entire XMP packet as one huge string, and you have to parse out any values yourself. I'm not interested in that, so there's little point pursuing a "translate EXIF to XMP equivalent" strategy.

For now, I may have to go with a "preserve" or "don't preserve" metadata option. Short of coding my own EXIF-writing class (not likely), alternative strategies include:

1) Write out metadata with GDI+. Not my favorite solution, as I still need to use FreeImage for writing files so I can support things like custom JPEG subsampling. It's possible to fool GDI+ into writing new metadata by using a lossless JPEG transform (see http://www.nullskull.com/articles/20030706.asp), but I'd have to manually translate EXIF IDs into their GDI+ equivalents. Alternatively, I could pursue a new strategy of loading metadata via GDI+ and storing it in a compatible GDI+ format to make saving it later easier, but then I'd have to rewrite my present metadata code from scratch. GDI+ also supports a smaller subset of tags than other tools.

2) Use an external tool for metadata handling. Exiftool seems to be the go-to (http://www.sno.phy.queensu.ca/~phil/exiftool/) but it's another 5mb dependency, and it adds a lot of overhead to image load/save (as each file will have to be processed twice). It's also difficult to do complex tag operations because you have to request all changes via command line, and working with the tag data is far less convenient than my current FreeImage model. That said, Exiftool supports multiple languages (a great fit for PD's translation feature) and many other serious photo editors rely on it, so I know it's reliable. It's also kept very up-to-date, and it supports obscure things like maker notes.

At present, exiftool as a new dependency is probably my preferred strategy - but I am open to other suggestions.

tannerhelland commented 11 years ago

ExifTool plugin support is the official decision.

ExifTool was added as a formally supported plugin by 2f7493493b32c674d2a8b58f28ab8c4d326db22b

audioglider commented 11 years ago

I'm glad you went with the EXIFTool route. It really has the most extensive support out there. And it has a built in option for cloning tags to any other file.

tannerhelland commented 11 years ago

I'm glad I went with ExifTool too. What a great project - all kinds of options, and very well-documented. My only lingering concern is processing speed, as full metadata parsing noticeably slows both loading and saving of images, but this shouldn't be a problem since the user can just disable metadata exporting from the Preferences menu, or disable ExifTool completely from the Plugin Manager to restore PD's original behavior.

Now, a few comments for future reference. I've created 4 possible options for metadata handling. Three of these are complete, and the fourth should be done by the end of this week.

The four options are:

1) Preserve all metadata, regardless of relevance. This is not the default option, because full metadata can be enormous, especially when the source is a RAW-type format. Note that this option will also copy over padding tags and empty tags, with the goal of preserving the original metadata as closely as possible.

This option requires the existence of the original file (e.g. if you load a raw image into PD, then delete the raw image, then attempt to save a JPEG, metadata cannot be preserved).

2) Preserve all relevant metadata. This is the default setting, and it does not require the existence of the original file. (PD stores relevant metadata internally.)

This option relies on ExifTool's XML export ability, which is acquired via stdout when an image is loaded. PD then parses the XML and builds an internal array of custom metadata objects, including the name, group, sub-group, description, human-readable value, and actual value of each tag. Extremely long binary tags are ignored (e.g. thumbnails) to improve performance.

This parsed XML data is used to populate the Browse Metadata dialog. In the future, when the option is provided to add, remove, or edit individual tags, we can edit these values in the original XML ourselves (which is not possible with option 1).

When an image is saved, full metadata XML is passed back to ExifTool, which it will then use to write the saved file's metadata. Tags are automatically validated and re-sorted into ideal categories per the Metadata Working Group spec (http://www.metadataworkinggroup.org/specs/). Maker notes are sorted into compatible standard category tags (EXIF, XMP, etc) if available, and non-compatible tags are dropped.

In my opinion, this option strikes the best balance for typical users, which is why I have set it as default. File size implications are minimal as all binary tags are dropped, and only relevant tags are kept.

3) Preserve all relevant metadata, but remove tags with privacy concerns, such as GPS data, serial numbers, etc. I HAVE NOT implemented this yet, as I am still compiling a list of privacy-relevant tags. Input welcome.

Effectively, this will be the same as option 2, but with certain tags parsed out of the XML before passing it to ExifTool.

4) Do not preserve metadata. This was PhotoDemon's previous behavior, and it offers the best performance. Metadata is still parsed at import time so that the Browse Metadata tool can be used.

FINAL NOTE: For options 1 through 3, some tags are always rewritten for compatibility reasons. At present, these include: image dimensions (all tag variations) and image orientation (as PD automatically corrects orientation at import-time).

tannerhelland commented 11 years ago

I had some free time this afternoon, so I knocked out the privacy option. Pretty nice, I think.

With that commit, I believe this issue is functionally resolved. I'll leave it open for a few days more while I do additional testing, but barring any surprises this can be considered "Closed".

tannerhelland commented 11 years ago

Closing this bug today!

Thanks again for your help, audioglider. I've added you to the README acknowledgments and About dialog as a small thanks.