24-bit uncompressed FP DNGs may be being read incorrectly

[LebedevRI]

The problem: 24-bit DNGs created by Adobe (tools and software that uses DNG SDK) will not read correctly by RawSpeed.

Originally posted by @LibRaw in https://github.com/darktable-org/rawspeed/issues/251#issuecomment-732006581

[kmilos]

In the TIFF Tech Note 3 that introduced floating point support to DNG, I do not see anything about treating 24-bit floats differently to 16-bit (or 32-bit) ones. Furthermore:

This means that the predictor reorders the bytes into a semi-BigEndian order, and that the TIFF reader and writer should not change the byte order of the image data outside of the predictor.

To me this sounds like 24-bit float image data should match the endiannes of the file/machine, just like the 16-bit and 32-bit case...

And no mention of FillOrder in this document...

[parafin]

I would like to ask how do you interpret FillOrder TIFF tag meaning? As @kmilos mentioned in original ticket TIFF specification is confusing in this part, it covers only bitdepth of 1 examples. It talks about packing bits in bytes, not byte order. DNG specification mentioning big-endian is IMHO a red-herring, it just means that bits should be packed in "big-endian" order, not that byte order is big-endian.

[LibRaw]

Adobe DNG SDK reorders 24-bit FP input based on input file endiannes and predictor used /if compressed/ (dng_read_image.cpp)

Note: DNG_FP24ToFloat inputs array of bytes, not (32-bit) integer, so internal machine byte order does not matter here.

[parafin]

So my question is - in case of let's say 10-bit image in little-endian DNG file with FillOrder of 1 in what order bits go in the file? And in case of big-endian DNG file?

[kmilos]

Note: DNG_FP24ToFloat inputs array of bytes, not (32-bit) integer, so internal machine byte order does not matter here.

Of course it does, just like if you passed your 16-bit or 32-bit values as arrays of bytes. Knowing the order matters.

[LibRaw]

the TIFF Tech Note 3 that introduced floating point support to DNG

TIFF tech note was published years before FP support in DNG (it is dated 2005 or so, not 2015). I'm not sure that real DNG FP implementation (as implemented in Adobe DNG SDK) is exactly the same.

I do not have much time today to dig into this, but simple experiment with FP TIFF (not DNG!) exported from current Photoshop CC as 24-bit uncompressed FP TIFF:

• same file, the only difference is exported TIFF byte order (II or MM)
  • payload is INDEED byteswapped: FF 1F 37 for II vs 37 1F FF in MM file(s)

Right now I do not have ready-to-use app to create different bit/different byteorder DNG via DNG SDK. sorry. Need some time to patch one of our apps for that. Latest versions of Adobe Camera Raw does not allow to select output bit/byte order for FP DNG export (HDRs). Not sure previous versions did.

[LibRaw]

Of course it does, just like if you passed your 16-bit or 32-bit values as arrays of bytes. Knowing the order matters.

Again: DNG SDK's operates with input bytes: int32 sign = (input [0] >> 7) & 0x01; but not on something like (input & 0xff)

[LibRaw]

* payload is INDEED byteswapped: FF 1F 37 for II* vs 37 1F FF in MM* file(s)

And NOT byteswapped if deflate compression is used (without predictor as I can see from absent predictor tag)

[kmilos]

Thanks, that byteswapped TIFF output seems to confirm the Tech Note (of course pre-dates the DNG, but DNG seems to reference it and hopefully follow it.)

Again: DNG SDK's operates with input bytes: int32 sign = (input [0] >> 7) & 0x01; but not on something like (input & 0xff)

Interesting... so at this point it already assumes a big-endian byte stream (sign & exponent are always assumed to be in input[0]). Is there a step beforehand that does the byteswapping?

[LibRaw]

Is there a step beforehand that does the byteswapping?

Yes, and it depends on both input file byte ordering and predictor used (or not). For example, for compressed input: if (stream.BigEndian () || ifd.fPredictor == cpFloatingPoint || ifd.fPredictor == cpFloatingPointX2 || ifd.fPredictor == cpFloatingPointX4) => no byteswapping

UPD: the same for uncompressed stream, no predictors, so always byteswapped for LE input.

[kmilos]

Great, I think that is consistent with the Tech Note comment above...

In any case, I have uploaded the all little-endian 24-bit uncompressed sample to RPU just now to replace the existing one using the wrong combo of big-endian stream in II* file with non-sensical FillOrder=1.

[LibRaw]

, I have uploaded the all little-endian 24-bit uncompressed sample

I'm not sure I download it correctly, new file named Canon - EOS 5D Mark III - 24bit RAW (3:2).dng is exactly the same (same MD5) as yesterday dated file (downloaded yesterday) with same name (and results in same noisy image in Adobe Camera RAW)

May be some web-side caching, I do not know.

[LebedevRI]

, I have uploaded the all little-endian 24-bit uncompressed sample

I'm not sure I download it correctly, new file named Canon - EOS 5D Mark III - 24bit RAW (3:2).dng is exactly the same (same MD5) as yesterday dated file (downloaded yesterday) with same name (and results in same noisy image in Adobe Camera RAW)

May be some web-side caching, I do not know.

There's premoderation for new uploads.

[kmilos]

Here's a link to my google drive just in case.

[LibRaw]

a link

This one looks normal in Adobe Camera RAW (and in other apps that use Adobe DNG SDK)

[parafin]

@LebedevRI, could you answer my question about FillOrder? It's not related to this particular issue, I'm just interested for myself. Thanx.

[LebedevRI]

@parafin Uh, well. For packed integers, compare https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/decompressors/UncompressedDecompressor.cpp#L212-L223 ("big-endian") with https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/decompressors/UncompressedDecompressor.cpp#L260-L270 ("little-endian").

All that is due for a refactoring, but their only difference is they are using https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/io/BitPumpMSB.h#L35 or https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/io/BitPumpLSB.h#L35 where BitStreamCacheRightInLeftOut/BitStreamCacheLeftInRightOut is the magic: https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/io/BitStream.h#L59-L75 vs https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/src/librawspeed/io/BitStream.h#L77-L92

Not sure if that answered the question.

[kmilos]

I wonder if it was more appropriate to just add the "case 24:", I really don't think there is a difference between 16/24/32b storage in a TIFF. This would make the code more readable, no exceptions, etc.?

[LebedevRI]

I wonder if it was more appropriate to just add the "case 24:", I really don't think there is a difference between 16/24/32b storage in a TIFF. This would make the code more readable, no exceptions, etc.?

@kmilos you mean in a9243b18677e26f8333e4e9458edff86fecde000? I guess that would be an NFC change, yes, since non-fp data there is 16 bit max. But i would also say it would make code less readable.

[kmilos]

Yes. The outcome is the indeed same, but given what we learned today from interpreting the various specs more carefully, perhaps more "logical" for someone reading the code 10 years from now. But I concur that's highly subjective, it's your prerogative ;)

[parafin]

@LebedevRI, sorry, I'm not too familiar with rawspeed codebase, please check if I understood correctly what BitPump does.

Let's say there is uncompressed integer TIFF file with BitsPerSample == 10. Re-reading the TIFF spec right now I see that byte-order of TIFF (II or MM) itself doesn't matter in this case (that was my main confusion), so only FillOrder determines how image data is stored. So am I correct with the following diagram (drawn from the point of view of little endian machine)?

FillOrder = 1
| bytes     |            0                  |             1                 |             2                 |             3                 |
| bits      | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
| pixel #0  | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |                       | 0 | 1 |
| pixel #1  |                               | 4 | 5 | 6 | 7 | 8 | 9 |                       | 0 | 1 | 2 | 3 |
| pixel #2  |                                                               | 6 | 7 | 8 | 9 |                       | 0 | 1 | 2 | 3 | 4 | 5 |

FillOrder = 2
| bytes     |            0                  |             1                 |             2                 |             3                 |
| bits      | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
| pixel #0  | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |
| pixel #1  |                                       | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |
| pixel #2  |                                                                               | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9 |

And follow-up question - does rawspeed ignore FillOrder tag for DNG (because by spec it only can be 1)?

[LebedevRI]

Re-reading the TIFF spec right now I see that byte-order of TIFF (II or MM) itself doesn't matter in this case (that was my main confusion) And follow-up question - does rawspeed ignore FillOrder tag for DNG (because by spec it only can be 1)?

That's not what it says. It says that the byte order of the packed data is NOT necessarily the same as for the DNG itself (which could be either FillOrder=1 (big-endian) or FillOrder=2 (little-endian)), but is always FillOrder=1 (big-endian). Well, i guess we don't actually read FillOrder, but rely on the endianness marker on the file itself (first two bytes - II or MM).

So am I correct with the following diagram (drawn from the point of view of little endian machine)?

The following two tests could be educational to answer that question: https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/test/librawspeed/io/BitPumpMSBTest.cpp#L34-L42 https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/test/librawspeed/io/BitPumpLSBTest.cpp#L34-L42

there we ask for 1 bit and get 1, for next 2 bits and get 01, for next 3 bits and get 001 and so on. Let me know if that is still not understandable.

[parafin]

Re-reading the TIFF spec right now I see that byte-order of TIFF (II or MM) itself doesn't matter in this case (that was my main confusion) And follow-up question - does rawspeed ignore FillOrder tag for DNG (because by spec it only can be 1)?

That's not what it says. It says that the byte order of the packed data is NOT necessarily the same as for the DNG itself (which could be either FillOrder=1 (big-endian) or FillOrder=2 (little-endian)), but is always FillOrder=1 (big-endian). Well, i guess we don't actually read FillOrder, but rely on the endianness marker on the file itself (first two bytes - II or MM).

Sorry, didn't parse this. In TIFF spec in the description of Compression tag it is stated:

1 = No compression, but pack data into bytes as tightly as possible leaving no unused
bits except at the end of a row.
If Then the sample values are stored as an array of type:
BitsPerSample = 16 for all samples SHORT
BitsPerSample = 32 for all samples LONG
Otherwise BYTE
Each row is padded to the next BYTE/SHORT/LONG boundary, consistent with
the preceding BitsPerSample rule.
If the image data is stored as an array of SHORTs or LONGs, the byte ordering
must be consistent with that specified in bytes 0 and 1 of the TIFF file header.
Therefore, little-endian format files will have the least significant bytes preceding
the most significant bytes, while big-endian format files will have the opposite
order.

We are in the "Otherwise" case, meaning that byte ordering doesn't depend on TIFF file header.

Then we have FillOrder tag description in TIFF spec:

1 = pixels are arranged within a byte such that pixels with lower column values are
stored in the higher-order bits of the byte.
1-bit uncompressed data example: Pixel 0 of a row is stored in the high-order bit
of byte 0, pixel 1 is stored in the next-highest bit, ..., pixel 7 is stored in the low-
order bit of byte 0, pixel 8 is stored in the high-order bit of byte 1, and so on.
<...>
Default is FillOrder = 1

That's somewhat fuzzy on the case when BitsPerSample > 8, but the diagram from my previous post is IMHO the only logical way to interpret it.

In DNG spec we have:

BitsPerSample
Supported values are from 8 to 32 bits/sample. The depth must be the same for each sample if
SamplesPerPixel is not equal to 1. 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), even if the TIFF
file itself uses little-endian byte order.

To me it sounds like a re-iteration of what TIFF spec says, except for the fact that it apparently forbids specifying non-default FillOrder of 2.

So when you say that you rely on TIFF marker to decide on FillOrder it seems wrong to me twice:

1. Default value of FillOrder doesn't depend on TIFF marker, it's always 1 per specification.
2. DNG spec also says that it's 1 independent of TIFF marker.

[kmilos]

That's not what it says. It says that the byte order of the packed data is NOT necessarily the same as for the DNG itself (which could be either FillOrder=1 (big-endian) or FillOrder=2 (little-endian)), but is always FillOrder=1 (big-endian).

I don't think the DNG spec mentions anywhere the endianess of the file being defined by FillOrder. This is where the confusion lies, and we should stop using this tag when talking about endianess. FillOrder only defines bit order in a packed bitstream and that's it; this is not endianness. The bitsteram payload itself is stored as bytes/words in the file according to II or MM which is the only definition of the endiannes of the DNG file. Once you read the bytes/words according to II or MM, then you can use FIllOrder to get to the bits you want.

Well, i guess we don't actually read FillOrder, but rely on the endianness marker on the file itself (first two bytes - II or MM).

This part is correct and is expected. FillOrder hardly plays a role in most real cases (assumption of default value of 1 is totally sane), and as the original TIFF 6.0 spec mentions, a non-default value is almost never used and has nothing to do with a usable DNG file as far as I can tell:

We recommend that FillOrder=2 be used only in special-purpose applications. It is easy and inexpensive for writers to reverse bit order by using a 256-byte lookup table. FillOrder = 2 should be used only when BitsPerSample = 1 and the data is either uncompressed or compressed using CCITT 1D or 2D compression, to avoid potentially ambigous situations.

[LebedevRI]

Ok, i give up. I've lost the track of thought here. If there is some image that doesn't decode properly, please file a bug.

[kmilos]

I think we're just saying FillOrder sent us in the wrong direction ;)

All good on the floating point support now!

[parafin]

I think original @LebedevRI interpretation of specs that 24-bit data should always be big-endian is correct, because nowhere it's mentioned that 24-bit case should follow TIFF byte order. But I'm not arguing in reverting that commit, since it's about compatibility with other software. But I think FillOrder is indeed what matters here.

As to your request about files that aren't decoded correctly - I have DNG files generated by our custom software at work (can't release it publicly, at least for now) with FillOrder = 2. Motivation for this format is that most industrial cameras use little-endian packed format (see GenICam Pixel Format Naming Convention (PFNC), e.g. Mono10p format) and it's not always possible to transform the data on-the-fly. Right now these files aren't rendered correctly in darktable (as expected, since they don't conform to DNG specification). Will you be interested in implementing support for FillOrder tag in DNG? I don't think it can break support for normal DNG files (ignoring this 24 bit special case). I can of course provide sample files, both monochrome and bayer. If you're interested, I will create a feature request.

[kmilos]

I think original @LebedevRI interpretation of specs that 24-bit data should always be big-endian is correct, because nowhere it's mentioned that 24-bit case should follow TIFF byte order. But I'm not arguing in reverting that commit, since it's about compatibility with other software. But I think FillOrder is indeed what matters here.

And yet nowhere does it say that it shouldn't. Big-endian 24-bit is a matter of internal implementation/assumption/convention after you've done the unpredict step, and matters only for compressed files. This is a diferent level of abstraction to the data stored in the file. For uncompressed data, we have established 24-bit float follows TIFF/DNG file endianess just like any other bit depth. FillOrder has nothing to do with either of these cases, there is no bit packing involved whatsoever.

[parafin]

@kmilos, so how should data be presented in uncompressed 10bit packed little-endian TIFF with FillOrder = 1 (big-endian) according to you? Can you draw a diagram like I did before? Or are you saying it's undefined by a standard?

[LebedevRI]

so how should data be presented in uncompressed 10bit packed little-endian TIFF with FillOrder = 1 (big-endian) according to you? Can you draw a diagram like I did before? Or are you saying it's undefined by a standard?

https://github.com/darktable-org/rawspeed/blob/374b49fc5646c927815e7467fe206ff6a87c3193/test/librawspeed/io/BitPumpLSBTest.cpp#L34-L42

I.e.

| Byte 0          | Byte 1                | Byte 2            | Byte 3                | Byte 4          |
| Pixel 0         | Pixel 2     | Pixel 1 | Pixel 3 | Pixel 2 | Pixel 4 | Pixel 3     | Pixel 4         |
| 7 6 5 4 3 2 1 0 | 5 4 3 2 1 0 |     9 8 | 3 2 1 0 | 9 8 7 6 |     1 0 | 9 8 7 6 5 4 | 9 8 7 6 5 4 3 2 |