image-rs / image-png

PNG decoding and encoding library in pure Rust
https://docs.rs/png
Apache License 2.0
357 stars 140 forks source link

Support for encoding metadata chunks #116

Open birktj opened 5 years ago

birktj commented 5 years ago

Currently the encoder lacks built-in support for writing metadata chunks, you can do it manually with Writer::write_chunk. However I believe that there should be a nicer high-level way of doing this.

Not all of these are equally important, however it would be nice to have support for all of them. See https://www.w3.org/TR/2003/REC-PNG-20031110/#11Ancillary-chunks for specifications on what the chunks should look like.

See also https://github.com/image-rs/image/issues/911

jansol commented 4 years ago

There is an addition to the spec that adds support for PQ-encoded HDR images: https://www.w3.org/TR/png-hdr-pq/ . This relies on iCCP, with fallbacks to cHRM and gAMA. Krita reportedly supports this format for HDR exports.

However adding support for HDR images is probably going to require some API redesign...

zlstringham commented 4 years ago

+1 for encoder AND decoder support of text chunks. I'm currently working on a library to support a proprietary file format for a 2d game engine with the following specifications:

Unfortunately, without access to read/write the zTXt chunk, I currently can't use this crate for my needs. (Image format is the DMI image format used by BYOND.)

de-vri-es commented 3 years ago

gAMA and cHRM have been merged with #244 :tada: I opened a PR #260 for the sRGB chunk. I can't tick the boxes, but it would be nice to update the list for potential contributors :)

mainrs commented 3 years ago

Hey!

Since I am going to use the library (especially the metadata part of it), I'd like to contribute back by working on some of the chunks here. Maybe some guidance on how to approach a PR would be nice, for example for the tEXT chunk.

Is the overall goal to add some new struct and a field to Info so that decoder/encoder can populate it? If so would the appropriate type for tEXT be Vec<NewtEXTChunkStruct>?

de-vri-es commented 3 years ago

Is the overall goal to add some new struct and a field to Info so that decoder/encoder can populate it? If so would the appropriate type for tEXT be Vec<NewtEXTChunkStruct>?

That sounds logical to me (but note that I'm not a maintainer).

You could still wonder if you want to provide the chunk data without processing, or if you want to decompress the text during decoding. Getting decompressed data would be nice, but might allow malicious PNGs to contain small compressed text that expands to large amounts of memory.

For that reason, it might be wise to store the data compressed in the info struct, and provide an easy interface to perform the decompression as needed. That also prevents spending time and memory on decompression when the application doesn't care about the text chunk.