jcelaya / hdrmerge

HDR exposure merging
http://jcelaya.github.io/hdrmerge/
Other
363 stars 78 forks source link

Remove FillOrder tag #207

Open kmilos opened 3 years ago

kmilos commented 3 years ago

The default FillOrder is 1 anyway if absent. More details:

Upon closer inspection of TIFF technical Note 3 which is the basis for floating point DNGs, it appears there is no difference whatsoever in treatment of 16b vs 24b vs 32b floats.

The spurious paragraph in the DNG spec saying

"If BitsPerSample is not equal to 8 or 16 or 32, then the bits must be packed into bytes using the TIFF default FillOrder of 1 (big-endian)"

was there even before DNG 1.4 that introduced floating point (and IMHO should've been updated to include 24 bits), and the mentioned FillOrder tag is relevant only for bit packing order within a byte (not applicable at all in this case), and not byte endiannes.

HDRMerge is writing a compressed bytestream in the file anyway so the endiannes doesn't matter, but any confusion should be best avoided. Any assumption of big-endian for the float->fp24->float conversion is purely internal (defined by the predictor) and has nothing to do with how the bytes are actually stored in the TIFF/DNG file (a compressed bytestream looks the same for all floating point bit depths).

kmilos commented 3 years ago

Further evidence from the TIFF spec:

LZW compression codes are stored into bytes in high-to-loworder fashion, i.e., FillOrder is assumed to be 1. The compressed codes are written as bytes (not words) so that the compressed data will be identical whether it is an ‘II’ or ‘MM’ file.

NB: the spec was written before ZIP/Deflate was introduced as an extension, but it doesn't matter because both produce a lossless byte stream output. Whether you feed 16, 24, or 32-bits into the compressor is irrelevant because they are whole bytes, and both the appropriate predictors and the compressor treat them as whole bytes input.

Endianness only matters to perform the prediction step correctly, and FillOrder must be left at default value of 1.

kmilos commented 1 year ago

See also https://gitlab.com/libtiff/libtiff/-/issues/558