search

darktable-org / rawspeed

fast raw decoding library
GNU Lesser General Public License v2.1
351 stars 113 forks source link

CR2Decoder potentially broken #39

Closed axxel closed 7 years ago

axxel commented 7 years ago

@LebedevRI getting rid of another couple hundred lines of duplicated code inside the CR2Decoder I noticed something that looks suspicious. It is about the selection of the predictors on a line change. What would help me, is some kind of ground truth data. Do you have a CR2 file and an accompanying 16bit raw tiff or PPM/PGM from some other source than rawspeed?

What may also work is a DNG that got created from the CR2. Thanks.

LebedevRI commented 7 years ago

from some other source than rawspeed?

I'm afraid i don't...

You probably want to try dcraw -E -4, which should produce Linear 16-bit PGM/PPM/PAM with no interpolation, which i think is the rawspeed output we are testing.

axxel commented 7 years ago

You probably want to try dcraw -E -4

You probably mean -D -4, right? (There is -E option.)

I tried that already. Result: almost all pixels have different values. Even the height differed by 1 pixel. I tried only one random CR2, though and then thought it might be best to ask for "real" ground truth data.

What about DNGs? Where do the DNGs processed in the dt script, you recently mentioned, come from? I would hope Adobe converter does the right thing.

LebedevRI commented 7 years ago

No, i meant -E https://www.cybercom.net/~dcoffin/dcraw/dcraw.1.html

INTERPOLATION OPTIONS

What about DNGs? Where do the DNGs processed in the dt script, you recently mentioned, come from? I would hope Adobe converter does the right thing.

http://supportdownloads.adobe.com/product.jsp?product=106&platform=Mac

axxel commented 7 years ago

No, i meant -E https://www.cybercom.net/~dcoffin/dcraw/dcraw.1.html

Oh, new cmdline option! I haven't updated my dcraw code for a while, it seems.

http://supportdownloads.adobe.com/product.jsp?product=106&platform=Mac

Thanks. Looks like I do have to boot up my OSX then. I hope it is still working. It's been quite a while ;).

LebedevRI commented 7 years ago

There is of course an http://supportdownloads.adobe.com/product.jsp?product=106&platform=Windows

axxel commented 7 years ago

There is definitely something weird going on with the update of the predictors. There are basically random jumps all over the image depending on the relative sizes of the cr2_slices. I am surprised that the images that come out are not complete garbage. So either I'm missing something important or Canon actually decided to implement it that way which makes no sense from a compression theory point of view, because you'd want to take a pixel that is next to the current one (like one row above the current one) as a best guess, not a pixel that is halfway across the sensor....

Anyway, I pushed a pr with code that still produces the same hashes as the old one. I'll investigate further at a later time.

axxel commented 7 years ago

Back at work I continued with the Cr2 code. Quick question: can you confirm that the current develop branch produces sane results for sRaw and mRaw files in dt?

LebedevRI commented 7 years ago

You'll need to be more specific than that. Which three samples specifically (raw, sraw, mraw)?

axxel commented 7 years ago

You'll need to be more specific than that. Which three samples specifically (raw, sraw, mraw)?

Just any mRaw and sRaw file, e.g. the two in the EOS 6D sample set.

LebedevRI commented 7 years ago

I'd say so: raw eos_6d_raw mraw eos_6d_mraw sraw eos_6d_sraw

Please do note that i'm not done with #46/#47 and a followup for #46. Also, looking at darktable build log, i will soon be taking a second look at #32

axxel commented 7 years ago

Thanks. Looking good, indeed. I just wanted to make sure I was not trying to make sense of how this part works if it does in fact not work...

Please do note that i'm not done with #46/#47 and a followup for #46.

I thought so but thanks for pointing it out. I am currently looking a very localized continuation of the LJpeg/Cr2 cleanup effort.

axxel commented 7 years ago

Arghhhh.... turns out my problem was a bug (or rather missing feature) in my rstest -d code. It simply did not handle 3 component 'raw' data in a meaningful way. After that was fixed. Everything fell into place...

As for my original suspicion that there could be a subtle bug in the Cr2Decoder, I am now convinced that the code is correct and does what needs to be done to undo what the Canon engineers did to the data. I have to say: I was already pretty underwhelmed by the quality of their work after hacking on libgphoto2 to get their stuff working but now after understanding all the hoops this decoder code has to jump though... I'll leave the rest of this thought to the imagination of the reader.

Bottom line: issue was invalid -> closed.