search

klauspost / rawspeed

Raw Image Decoder Library
GNU Lesser General Public License v2.1
72 stars 20 forks source link

Bug in NikonDecompressor for "lossy after split" raws #169

Open axxel opened 7 years ago

axxel commented 7 years ago

I am currently looking into reducing code duplication in the 3 Huffman decoder implementations and it seems to me there is a bug in NikonDecompressor. The special Huffman sign extension code in the 'lossy after split' versions is not used in the bigTable lookup.

I'd like to test this but I could not find a suitable NEF file with the 'split' format. Could someone (maybe @LebedevRI ?) please point me to one?

LebedevRI commented 7 years ago

@axxel i'm sorry, are you with some project that uses rawspeed?

there may be some changes wrt upstreaming changes, so if you are just looking for a nice library to hack on, this might or might not be be the wrong place.

axxel commented 7 years ago

@LebedevRI

I am talking about the difference between: https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.cpp#L201-L204 and https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/LJpegDecompressor.cpp#L520-L521. The interesting case here is https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.h#L48-L49, which makes shl != 0.

LebedevRI commented 7 years ago

@axxel looking through the code, i do not think that case is ever triggered, at least not for nef's.

nikon_tree is only used in https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.cpp#L36-L49

And that function is only called from https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.cpp#L92 and https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.cpp#L112 In both cases, huffSelect is no less than two, so with current code, second entry in nikon_tree, / 12-bit lossy after split /` is never used.

axxel commented 7 years ago

@LebedevRI If v0 != 70 then huffSelect is 0. So I am looking for a NEF file where https://github.com/klauspost/rawspeed/blob/develop/RawSpeed/NikonDecompressor.cpp#L78 is true.

LebedevRI commented 7 years ago

I'm unable to find such a file in my sample set. http://rawsamples.ch/index.php/en/all-files-as-7z-archive would be the next place to look, but not today.

axxel commented 7 years ago

I checked all the files from rawsamples.ch. None of them executes the code path in question. I am not surprised, because if this was a common case, someone would have noticed the (presumed) bug.