Regression: huffmantable is broken. about rawspeed HOT 43 CLOSED

I just uploaded a D7000 14-bit "lossy after split" image to RPU

Thanks!

(though it's not showing in the repo search yet -- looks like that's updated once a day?

There is a pre-review.

Would there be any chance that the patch from above
-bs.fill(32);
+bs.fill(maxCodePlusDiffLength());

It's quite broken beyond that i'm afraid.
Now that i finally bothered to push the NFC test changes (f5eeb2d...9a52017),
i just need one more push to finish the more major part of the fix
(develop...LebedevRI:huffmantable), that should bring it closer to life..

from rawspeed.

FYI i have finally (sigh) restored then nikon lossy-after-split support (#129).
So the main remaining problem is #128, the last decode.
Not quite sure what is up with #127, might be a semi-duplicate of #132.

from rawspeed.

Yeah, J1. I've already put a sample from that to RPU. If I find any other camera doing the same I'll post it.

from rawspeed.

Apparently happens for Nikon D3400 too.
Really not cool.

from rawspeed.

So basically all nikon raws "lossy after split" are broken.

from rawspeed.

Based on your comments above you seem to make the assumption that my "fucked" HuffmanTable code broke the "lossy after split" files. For that assumption to be true, it would have needed to work before my changes. Have you tried that?

I have the my doubts: The thing is, when working on the rewrite, I was noticing a (presumed) bug in the old code and wanted to investigate further but there was no sample available triggering that bug. In fact, I asked you personally for one, but you did not have one either.

Now that there seems to be sample available, I'm pretty sure you'll manage to "completely unfuck" my HuffmanTable implementation. Maybe my mentioned analysis from last year can be of some help, too, who knows.

from rawspeed.

Based on your comments above you seem to make the assumption that my "fucked" HuffmanTable code broke the "lossy after split" files. For that assumption to be true, it would have needed to work before my changes. Have you tried that?

I absolutely did. darktable-2.2.5, that still had the old rawspeed, loads these files flawlessly.
Feel free to try yourself and catch me on a lie.

I have the my doubts: The thing is, when working on the rewrite, I was noticing a (presumed) bug in the old code and wanted to investigate further but there was no sample available triggering that bug. In fact, I asked you personally for one, but you did not have one either.

b213eaf#diff-98d884ed1d3efccaf22be005567a6726R106

+  // 2. This is the actual huffman encoded data, i.e. the 'alphabet'. Each value
+  // is the number of bits following the code that encode the difference to the
+  // last pixel. Valid values are in the range 0..16.

Now, what is the maximal code value (the second array) for 12-bit lossy after split

Now that there seems to be sample available, I'm pretty sure you'll manage to "completely unfuck" my HuffmanTable implementation. Maybe my mentioned analysis from last year can be of some help, too, who knows.

Yes, i do hope so. Hopefully even without sacrificing performance.

from rawspeed.

https://discuss.pixls.us/t/nikon-a-specific-raw-sample-wanted/5483

from rawspeed.

Feel free to try yourself and catch me on a lie.

?!? Why would I have reason to question the truthfulness of your statements?

Given that it worked for the old code, I must have made a mistake in my analysis that lead to me suspecting the code was not working for that special case. As you might have inferred from my awareness of the fact that the "lossy after split" stuff seemed fishy, I was deliberately ignoring the existence of those invalid lossy after split numbers. It made no sense to me to try to retain some functionality that could not be tested and I suspected to be not working anyway. Much the same reasoning that presumably led you to drop a whole decoder recently.

I had another quick look at the old code to see if I was able to find the fault in my earlier reasoning. Now I'm thinking, the fault might have been that I thought the "big table" was used for decoding the whole image (which could not have worked) but now it seems to me that simply every lookup in the big table for the image part after the split would simply "fail" (which is supposed to happen if the code is too big for the "big table") and then the special/custom shl != 0 stuff would kick in.

from rawspeed.

And one more broken piece, from https://discuss.pixls.us/t/darktable-for-windows/4966/264
DSC_0143.dng.gz
HuffmanTable::decode() { ... bs.fill(32); is the culprit, but even then something else is broken still.
Probably related to #94

@LibRaw i really do hope you weren't planning on upgrading anytime soon :)

from rawspeed.

Yes, it looks like we'll skip RawSpeed(v2) support for LibRaw 0.19

We'll try to implement some kind of 'flexible' support. May be via a whitelist of cameras w/o known problems and with sensor megapixel large enough for significant speedup (so, Canon 5DS*, Nikon D8xx, may be Canon 5Dmk4).

from rawspeed.

RawSpeed(v2)

I'm not aware which commit was the v2 release btw.
Current tag is v3.0, and i'm following the same scheme as with darktable - odd tag versions mean development version, even - release.

from rawspeed.

v1/v2 was used as 'conventional name' for 'Klaus master' and 'current devel' branches.

Is (some) current branch already used in some darktable release?

from rawspeed.

Is (some) current branch already used in some darktable release?

Yeah, darktable git master is using rawspeed git develop since

commit 3b4c49d16b859758ddeccdf8a261719ac3f0c5f9 (upstream/pr/1402)
Author: Roman Lebedev <[email protected]>
Date:   Mon Jan 2 17:03:41 2017 +0300

    Track rawspeed as submodule

commit b82d189e00a8741cd1fce5207f60814c1959cc7f
Author: Roman Lebedev <[email protected]>
Date:   Mon Jan 2 17:02:04 2017 +0300

    Purge bundled rawspeed

So 2.4.0 series will be using (and current 2.4.0rc0 is using) this code.

from rawspeed.

And one more broken piece, from https://discuss.pixls.us/t/darktable-for-windows/4966/264
DSC_0143.dng.gz

I don't use darktable and my rstest has no problem extracting some whitebalance information out of that file. Can you provide some steps to reproduce the issue with using only librawspeed tools?

HuffmanTable::decode() { ... bs.fill(32); is the culprit, but even then something else is broken still.
Probably related to #94

I can decode that dng but it contains a few apparently wrongly decoded tiles. Do you see any connection between the nikon lossy-after-split behavior and the issues with that DNG file? Can you elaborate what the problem with the bs.fill(32); line is in your opinion?

from rawspeed.

@axxel i'm sorry for bothering you, only the original comment in this issue was directed at you, the rest of the comments i'm simply using to record my findings.

from rawspeed.

No worries, I'm not bothered. As I said earlier this year, I'd be happy to look into any issues with my code, in case something shows up later.

from rawspeed.

I was trying to process a .NEF RAW photo from my Nikon D5600 and I got this message:

failed to read camera white balance information from `DSC_0371.NEF

The console error I got was:

[rawspeed] (DSC_0371.NEF) void rawspeed::HuffmanTable::setCodeValues(const rawspeed::Buffer&), line 176: Corrupt Huffman. Code value 92 is bigger than 16
[temperature] failed to read camera white balance information from `DSC_0371.NEF'!
[temperature] `NIKON CORPORATION NIKON D5600' color matrix not found for image
[temperature] failed to read camera white balance information from `DSC_0371.NEF'!
[rawspeed] (DSC_0372.NEF) void rawspeed::HuffmanTable::setCodeValues(const rawspeed::Buffer&), line 176: Corrupt Huffman. Code value 92 is bigger than 16

It seems that here is the issue I'm looking for. I'll wait for support or upgrade for this.

PS. I would be glad to share with you (if needed) photos that triggers this error.

from rawspeed.

Sorry for the trouble :/

It seems that here is the issue I'm looking for.

I think so. Though there is more than one issue here.
Some part of it (not the nikon part you are seeing) is fixed in my WIP branch

PS. I would be glad to share with you (if needed) photos that triggers this error.

Thanks, but for D5600, all the needed samples are already present on RPU.

from rawspeed.

+1 also waiting for the fix.

Btw @LebedevRI some months ago we've found a workaround to this issue for Nikon J1's (a one-liner patch that I accidentally lost), would you happen to remember what the problem was?

from rawspeed.

Uhh, if it's one-liner i think it would be changing

-bs.fill(32);
+bs.fill(maxCodePlusDiffLength());

from rawspeed.

@LebedevRI Fixed the issue. Thanks a lot. 👍

(btw -- if that helps someone -- the point where this breaks for me is in BitStream.h, the error I receive is from this IOE:

from GDB I could see that pos was larger than size (exactly pos was size+4) when the exception was thrown.)

from rawspeed.

I just uploaded a D7000 14-bit "lossy after split" image to RPU (though it's not showing in the repo search yet -- looks like that's updated once a day?). I discovered that even last night's git master still breaks on them when I went back to revisit some old photos.

from rawspeed.

Would there be any chance that the patch from above

-bs.fill(32);
+bs.fill(maxCodePlusDiffLength());

gets dropped into develop / into some darktable release? It's difficult for me to set up a build environment (on Windows)...

from rawspeed.

I was wondering... Is this bug fixed, yet? and added to release candidate?

from rawspeed.

Fully - no, not yet.
I've fixed the base glaring issues in #130, but as cross-fuzzers (between two different huffman table implementations) tell me, there are still issues.
i'm going to re-look into the nikon issue (#129) next.

from rawspeed.

regarding nikon, please ask if you'd need NEFs that fail to decode; I can generate them on demand.

from rawspeed.

Which camera? There are ~6 such raws on RPU already, so i think now * the samples are not an issue

EDIT: right, J1

• If only those samples were in rawsamples.ch back then :/
  transition of rawspeed, and a fork fo rawsamples.ch -> RPU happened at the same time

from rawspeed.

Is there a way to test rawspeed on a raw file using command line?

My idea is to convert affected files to DNG. I would be better than converting all my files.

Also: I have a lot of affected D5500 samples with no people in, if needed I can provide those.

from rawspeed.

Sure. darktable-rs-identify tool if you are using darktable.

from rawspeed.

How do I use it? It always shows this message:
WARNING: Couldn't find cameras.xml in 'C:/darktable/darktable-install/share/darktable/rawspeed/cameras.xml'
WARNING: Couldn't find cameras.xml in 'darktable-rs-identify.exe/../share/darktable/rawspeed/cameras.xml'
ERROR: Couldn't find cameras.xml in 'darktable-rs-identify.exe/../share/darktable/rawspeed/cameras.xml'
terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_M_construct null not valid

OK --> all hard coded paths are not valid, creating C:\Program Files\darktable\share\darktable\rawspeed myself and copying cameras.xml to it.

from rawspeed.

No idea, i don't use windows, i can't help here.

from rawspeed.

Is there any type of workaround possible for this error until the bug gets fixed?
Is it possible to use the white balance info from another RAW NEF file which isn't broken?
I see that Darktable is able to generate a thumbnail image for the file.
Right now I'm dead in the water and unable to edit some files from the camera.

from rawspeed.

Is it possible to use the white balance info from another RAW NEF file which isn't broken?

It's not about white balance. That dt's message is uninformative/misleading i'd say.
It quite literally fails to decode the whole image.

I see that Darktable is able to generate a thumbnail image for the file.

No. Those are exif2 thumbnails, which are shown by default unless you opt into raw preview.

Is there any type of workaround possible for this error until the bug gets fixed?

Which bug are you seeing? I.e. can you show the error console output?

But,

• use older darktable (2.2.x should be last good i think?)
• convert those specific nef's to dng via adobe dng converter/etc

Right now I'm dead in the water and unable to edit some files from the camera.

I'm sorry.

from rawspeed.

from rawspeed.

from rawspeed.

Did you read my mail

Uh, nothing recent comes to mind..

about this script which can mass-test files with rawspeed? I'd like to share it but I'm not sure how to properly do that.

In principle, you can just pass -DRAWSPEED_ENABLE_SAMPLE_BASED_TESTING=ON -DRAWSPEED_REFERENCE_SAMPLE_ARCHIVE=<raw.pixls.us checkout> when building,
and use ninja rstest-{create,recreate,test,check,clean} build targets.
(see sample-based-testing.cmake)
Although locally i have an ugly contraption of a shell script to do that.

from rawspeed.

from rawspeed.

I sent this to the dev mailing list:

Ah, that explains why i have not seen it.
I would have guessed you could simply attach it to the mail to the mailing list.

from rawspeed.

from rawspeed.

from rawspeed.

Ok, I attached my script.

I meant the dt-dev list.
github does not deal with mail attachments, it just silently drops them.

from rawspeed.

4c74963

from rawspeed.