Closed Yoric closed 5 years ago
20 bits is part of the spec; how you compute the number 0 ≤ x ≤ 20 is an implementation detail.
On Fri, Sep 13, 2019, 6:38 PM David Teller notifications@github.com wrote:
@Yoric commented on this pull request. In crates/binjs_io/src/context/huffman.rs https://github.com/binast/binjs-ref/pull/441#discussion_r324113159: // Take the two rarest nodes, merge them behind a prefix, // turn them into a single node with combined number of // instances. Repeat. while heap.len() > 1 { let left = heap.pop().unwrap(); let right = heap.pop().unwrap(); heap.push(Reverse(Node { instances: left.0.instances + right.0.instances, content: NodeContent::Internal(Rc::new((left.0.content, right.0.content))), })); } // Convert tree into bit lengths let root = heap.pop().unwrap(); // We have checked above that there is at least one value. let mut bit_lengths = Vec::with_capacity(len); fn aux(bit_lengths: &mut Vec<(T, BitLen)>, depth: u8, node: NodeContent) { I thought the idea was to not make the 20 bits part of the spec? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/binast/binjs-ref/pull/441?email_source=notifications&email_token=AAANOUDJT4VCG6XUPCX4QKDQJNNQDA5CNFSM4IVS5T2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEUSZAY#discussion_r324113159, or mute the thread https://github.com/notifications/unsubscribe-auth/AAANOUE7DOCLSQ7LGXIN2YLQJNNQDANCNFSM4IVS5T2A .
In crates/binjs_io/src/context/huffman.rs https://github.com/binast/binjs-ref/pull/441#discussion_r324113159:
// Take the two rarest nodes, merge them behind a prefix, // turn them into a single node with combined number of // instances. Repeat. while heap.len() > 1 { let left = heap.pop().unwrap(); let right = heap.pop().unwrap(); heap.push(Reverse(Node { instances: left.0.instances + right.0.instances, content: NodeContent::Internal(Rc::new((left.0.content, right.0.content))), })); } // Convert tree into bit lengths let root = heap.pop().unwrap(); // We have checked above that there is at least one value. let mut bit_lengths = Vec::with_capacity(len); fn aux(bit_lengths: &mut Vec<(T, BitLen)>, depth: u8, node: NodeContent) {
I thought the idea was to not make the 20 bits part of the spec?
— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/binast/binjs-ref/pull/441?email_source=notifications&email_token=AAANOUDJT4VCG6XUPCX4QKDQJNNQDA5CNFSM4IVS5T2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEUSZAY#discussion_r324113159, or mute the thread https://github.com/notifications/unsubscribe-auth/AAANOUE7DOCLSQ7LGXIN2YLQJNNQDANCNFSM4IVS5T2A .
20 bits is part of the spec; how you compute the number 0 ≤ x ≤ 20 is an implementation detail.
On Fri, Sep 13, 2019, 6:38 PM David Teller notifications@github.com wrote: