-
Notifications
You must be signed in to change notification settings - Fork 38
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
Context 0.1: Huffman tables #454
base: master
Are you sure you want to change the base?
Conversation
This table provides best performance but may only be used reasonably for small bit lengths due to its memory cost. We'll implement a more space-efficient (but not as fast) MultiLookupHuffmanTable in a followup patch.
I think it LGTM. I'm wondering if you couldn't use an existing implementation on crates.io? Looking at the results https://crates.io/search?q=huffman, they doesn't seem commonly used tho. |
I've looked at a few crates and they all seemed to use actual trees to represent Huffman codes, which makes them very slow for our use case. |
Happy to review... could you look at the failing test first? |
9d85128
to
1cf92cf
Compare
I've reworked the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback.
/// | ||
/// If `bit_len` is larger than the number of bits, the prefix is padded with | ||
/// lower-weight bits into `bit_len` bits. | ||
pub fn split_bits(&self, bit_len: BitLen) -> (u32, u32) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure of the naming of split_bits versus split... Does this need both these APIs? Can the names be made more distinct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it into split_raw_bits
, is that better?
This is a first step towards decompressing Huffman content.