-
Notifications
You must be signed in to change notification settings - Fork 27
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
Expose decoder/encoder primitives as public #72
Comments
It surely would be feasible! Could you send a pull request for review? I don't think a feature flag is needed here - just make sure that the newly exposed API is documented and that the GitHub workflows pass. |
I did some internal refactoring to expose the raw decoder as a primitive where the LZMA compression parameters are encoded aas const generics. I also did a similar wrapper for LZMA2 although I'm not too sure about the usefulness of that in my fork. I believe this is a better approach than just exposing If you could take a quick look to see if you're comfortable with those changes and the shape of the |
Some comments:
Feel free to break down your changes into multiple PRs (exposing |
With that said I'd be happy to split decoupling |
I maintain a Rust implementation of MAME's Compressed Hunks of Data format, which is essentially chunks of data compressed using various compression algorithms, LZMA included. One of the quirks of this format is that because they use a very old LZMA 19.0, the encoding parameters are not saved into the output stream, and therefore every CHD decoder has to essentially mimic the defaults of LZMA 19.0.
Thankfully lzma-rs allows this but since the
encode
anddecode
modules aren't public, I had to fork and vendor lzma-rs to access those primitives likeLzmaParams
in my crate. Additionally I addedLzmaParams::new
to construct an instance manually for this purpose.Ideally I would like to just be able to use mainline
lzma-rs
. Sincedecoder
is mostly documented already, would it be feasible to expose those primitives as public without much work, possibly behind a feature flag?The text was updated successfully, but these errors were encountered: