Add support of decoding legacy frames#66
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 87.65% 87.67% +0.02%
==========================================
Files 11 11
Lines 2292 2321 +29
==========================================
+ Hits 2009 2035 +26
- Misses 283 286 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
Awesome thank you! |
|
I fixed the broken gh action |
Sure, let me try to add one. |
|
I noticed you force pushed against the branch, but there's no test added |
|
Hi, thanks for following up. I was force pushing the existing commit
rebased onto your github action fix and no new commits added.
I had a quick look at the test cases and they highly depend on both
compression and decompression, so I was thinking to add support of
compression of legacy frame as well and then add test cases for them.
What do you think?
…On Sat, 28 Jan 2023 at 7:48 PM, PSeitz ***@***.***> wrote:
I noticed you force pushed against the branch, but there's no test added
—
Reply to this email directly, view it on GitHub
<#66 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM6ZKFREFENRSK2MNWE4UTWUS6LDANCNFSM6AAAAAAUINL7JI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Since the legacy format is used in the linux kernel, that may be a big enough use case. You can also add a special test only for legacy decompression out site the test suite. |
`benches/dickens.lz4` was generated by cli utility lz4: lz4 -l benches/dickens.txt benches/dickens.lz4
|
Thanks for the suggestion. As described in the commit message, I created and decompress and compare with |
|
A 6MB binary is a little big for git, can you pick something smaller? |
|
Thanks for the review. Since legacy format uses fixed 8MB block size, I think 10MB dickens.txt is a good source to test multiple blocks and more similar to the size of a Linux kernel image. Does it make sense? I'm open to choose a smaller one (like compression_66k_JSON.txt) if you insist. Cheers. |
|
Yes, good point. Thanks for the PR! |
|
Sweet! By the way, any clue when will be the next release publish to crates.io? |
|
Published a new version on crates.io https://crates.io/crates/lz4_flex |
|
Thank you very much! |
Hi @PSeitz ,
While working on an utility ikconfig extracting Linux kernel images, I chose lz4_flex as a dependency to do lz4 decompression. However, Linux kernel is using legacy frame and lz4_flex doesn't support it yet. I added the support in my fork and thinking it'd be good to have it upstream so here's the pull request.
Cheers,
Philip