rawspeed
rawspeed copied to clipboard
klauspost
the zero_is_bad flag should perhaps be reversed
I just noticed that the Panasonic FZ1000 doesn't have zero_is_bad set so isn't properly handling broken pixels. It would probably make sense to reverse the flag to zero_is_ok so it's only set for whatever files actually need it, which in the RW2 format should be few if any of them.
You are welcome to add the hint to new cameras, even if you cannot confirm it, but I don't like changing the default.
Do you know of any cases where we definitely do not want to add zero_is_bad? Because if this is consistent across the format making it the default shouldn't really break compatibility
I've just ran a full regression test on all the RW2 images I have, forcing zero_is_bad to true. They all worked fine. dcraw seems to agree as it uses zero_is_bad for all Leica and Panasonic images:
https://github.com/pedrocr/dcraw/blob/master/dcraw.c#L9012-L9019
Given this I'd much rather just drop the option entirely and just make it the default. dcraw also uses zero_is_bad for the Canon chdk cameras:
https://github.com/pedrocr/dcraw/blob/master/dcraw.c#L8480
So adding that hint to NakedDecoder could make sense.
Ok, you have convinced me.
Unless @LibRaw has any objections, that change can be made.
LibRaw uses RawSpeed only as data decoder. Bad pixels processing is applied later, on postprocessing stage. So, if it is still possible to obtain unaltered RAW data with RawSpeed (without any DNG opcodes applied, without bad pixels filtering, without data dithering, etc), it does not matter what defaults are used.
Fortunately, interpolateBadPixel can be turned off in decodeRaw(), so we'll use it when we'll switch to some fork of 'develop' branch.
BTW, we're still using 'master' branch of RawSpeed for our end-user software (esp. RawDigger ) because we're very unhappy with random dithering added to RawSpeed: it might be useful for image presentation, but unacceptable if one needs repeatable results. This feature should be configurable, but it is not (at least, for Sony ARW2 format)
@pedrocr ok - go ahead!
@LibRaw - you should just have said so! Adding a "applyDither" to the RawDecoder will just be a few lines.
edit: btw, the dither should be deterministic. It will likely change when I get to #108, but once fixed it should remain stable.
@klauspost, dithering (even deterministic) is not good is one want to determine 'camera bit count' by histogram holes (easiest way to do it for average user). For precise raw inspection tool (RawDigger) it is better to have RAW data unaltered. Sure, for RAW data display (esp. in Sony 11+7 bit case) it is better to add some noise.
I have not asked for it, because I'll able to patch RawSpeed myself, but switching to actual RawSpeed is in the middle of our TODO list, not on top (in 'Technical Debt' section, of course).
BTW, if this thread is a good place for feature requests (for use RawSpeed in our RawDigger and, also, in general LibRaw based applications which uses RawSpeed only as raw data decoder, and LibRaw as metadata parser), we need two modes for RawSpeed:
- mostly unaltered RAW values: linearization curve applied, but no other changes made (no zero_is_bad filtering, no white balance, no data scaling, no black subtraction, no DNG opcodes)
- same as above, but without linearization data too (this mode is needed only for RawDigger 'no tone curve' mode).
@LibRaw - "RawDecoder->uncorrectedRawValues" should satisfy the latter. A dither option should satisfy the former. But please open an issue, so I don't forget when this is closed.
One last doubt @klauspost, currently the zero_is_bad code only works for the Rw2 encoding when it should actually apply to all of them and can even be used for other formats. Wouldn't it be cleaner to just do that with an extra pass over the final array and thus push that code into RawDecoder? I doubt it would make things much slower. But maybe you'd prefer the Rw2 code to stay the same and the extra pass code to just be used for other formats?
@LibRaw there's only one piece of code to do the interpolation itself but the zero_is_bad code is the one in the RW2 decoder that actually detects the pixels based on them being 0. DNG doesn't need this as the pixels are identified by the format metadata.
Any decoder can insert known bad pixels into RawImage->mBadPixelPositions, but only RW2 has zero is bad.
The "bad" pixels are inserted while the image is being decoded. That way we avoid an additional pass over the image to check all pixel values.
DNG can have opcodes that defines a pixel value as "bad". These are also added into RawImage->mBadPixelPositions, but in a separate pass.
but only RW2 has zero is bad
In reality, all of Panasonic/Leica has it (.RW2 or .RAW) and apparently the CHDK cameras also have it.
That way we avoid an additional pass over the image to check all pixel values.
What I was proposing was adding just such an additional pass to make it simpler to support other formats. I'll keep the RW2 code the same if you feel the added complexity pays off in performance.