WIP: Allow users to use an external zlib-ng #393
Conversation
| #include "zlib.h" | ||
| #else | ||
| #include "zlib-ng.h" | ||
| #endif |
There was a problem hiding this comment.
By selecting "zlib-ng.h" you are introducing yet another format to Blosc2, which is not what we'd like to do. We have chosen zlib-ng as a modern implementation of zlib, but that was always meant to be run in ZLIB_COMPAT mode. What we want is to use the external zlib-ng, but in ZLIB_COMPAT mode. Is that possible?
There was a problem hiding this comment.
To my knowledge, compat mode allows zlibng to export the same symbols as zlib. That is, in addition to the new symbols.
Without compat mode, you have to prefix the functions with zng_
Remains the version identifier. Maybe that one is wrong.
There was a problem hiding this comment.
Honestly, I'm not too sure what the goal of using zlib-ng, but restricting yourself to the zlib compression/decrompression. You are likely considering a use-case I am not.
In my mind, if you do want the speedups of zlib-ng, you might as well use the new form of the function calls??? I might also be mis-interpretting what ZLIB_COMPAT means. My ultimate understanding is that they didn't change the compression format, but rather sped up the implementation.
Is that not true?
Do you think that data compressed with zng_compress2 will not be able to be opened with decompress from vanila zlib?
|
Oops, it looks like I was wrong and zlib-ng is meant to always produce bit streams that are compatible with the original zlib project: zlib-ng/zlib-ng#905. Sorry about the confusion. With that, I am happy with this PR. If it is this ready to be merged, I will gladly do so :-) |
| clibversion = ZLIB_VERSION; | ||
| #elif defined(HAVE_ZLIB_NG) | ||
| clibversion = ZLIBNG_VERSION; |
There was a problem hiding this comment.
Does the return value of clib version get saved in the file?
Zlibng version will be in the 2.x.x while zlib version. 1.x.x which may be an important change
There was a problem hiding this comment.
And potentially b undesirable
There was a problem hiding this comment.
That's a good question. Maybe @nmoinvaz can shed some light here.
There was a problem hiding this comment.
Ups, sorry, clibversion is only used for blosc_get_complib_info() to get the versions of internal codecs, so not stored in the file. With that, I think we are safe merging that.
|
All done. Thanks @hmaarrfk ! |
|
Thanks for this! |
|
#358 might also provide insights as to why things aren't working here |
This supersedes the previous PR that allowed users to use an external ZLIB-NG.
I think it works because it is less invasive.