Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1/3] Regression: huffmantable is just broken. #127

Open
LebedevRI opened this Issue May 19, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@LebedevRI
Copy link
Member

LebedevRI commented May 19, 2018

Split off from #100
The current HuffmanTable implementation is just completely broken.
Like, i have stopped counting the amount of bugs it has.
In retrospect, #36 should not have been merged.

@LebedevRI LebedevRI referenced this issue May 19, 2018

Open

Regression: huffmantable is broken. #100

2 of 3 tasks complete

@LebedevRI LebedevRI self-assigned this May 19, 2018

LebedevRI added a commit to LebedevRI/rawspeed that referenced this issue May 19, 2018

HuffmanTable unfucking, part 1.
The current `HuffmanTable` implementation is just completely broken.
Like, i have stopped counting the amount of bugs it has.
In retrospect, darktable-org#36
should not have been merged.

The rough gist of the changes is:
1. Implement two new huffmantable implementations, that do the
   same thing via different codepaths. They have to be readable.
2. Cross-fuzz them. They should agree, produce identical results.
3. Split the old huffman table into two, the base variant ("lookup"),
   which only smartly does symbol decoding bit-by-bit,
   and the more advanced variant that also uses LUT first.
4. Fuzz the lookup variant against those two new implementations.
5. Fix it to agree.
6. Fuzz them all.
7. Finally fix the LUT variant to agree.

This has been complicated :/

Fixes darktable-org#127
Or, does not, and the oss-fuzz will complain.

The darktable-org#128 and
    darktable-org#129 remain.

Some further code cleanup will be needed afterwards.

I did not do any benchmarking whatsoever, since it would not be
a meaningful to compare broken code with not broken code.

Merge branch 'huffmantable' into develop

* huffmantable: (33 commits)
  AbstractHuffmanTableTest: sanitize expected msgs in ASSERT_DEATH()'s
  Fix clang <6.0 -Wmissing-braces warning
  Fix gcc and/or assert-less build.
  HuffmanTableLUT: pay special attention to FULL_DECODE && l_diff == 16
  HuffmanTable{Lookup,LUT}: make maxCodeOL 32-bit.
  Duplicate HuffmanTableLUT as HuffmanTableLookup, without the LUT part
  HuffmanTableVector: fix handling of edge cases with no codes at len.
  Reduce number of HuffmanTable fuzzers by restricting the pump list.
  Fuzz: autogenerate huffmantable dual fuzzer lists
  HuffmanTableVector: make use of strict global ordering of code id's
  HuffmanTableVector::getSymbol(): use early return when looking for prefix.
  HuffmanTableVector::peekSymbol(): hmm,the other way around, why did it work?
  Add HuffmanTableTree implementation +fuzzers
  Add basic BinaryHuffmanTree, +basic tests.
  Fuzz: add Dual HuffmanTable fuzzer - cross-compare implementations.
  Fuzz: huffmantable: rename main.cpp to Solo.cpp - it only takes one impl at time
  HuffmanTableVector: adjust to CodeSymbol::HaveCommonPrefix() changes
  AbstractHuffmanTable::CodeSymbol::HaveCommonPrefix(): ok, asymmetrical.
  Fuzz: add moar fuzzers for all the huffmantable implementations
  Add super idiotic reference HuffmanTableVector implementation.
  ...

@LebedevRI LebedevRI reopened this May 20, 2018

LebedevRI added a commit to darktable-org/darktable that referenced this issue Jun 6, 2018

RawSpeed submodule update: Panasonic DC-GH5S, DC-G9 high-res raw support
Those are using new v5 Panasonic raw compression algorithm.
Which is actually dumber than what they used before.
Now, it is basically just uncompressed packed 12/14 bits.

Also, huffman table salvaging part 1.

Fixes darktable-org/rawspeed#126
Refs. #11886
Refs. darktable-org/rawspeed#127
Refs. #12207
Refs. #11581
Fixes #12188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.