Conversation
ericonr
left a comment
There was a problem hiding this comment.
Isn't this missing the changes to
ADCore/ADApp/pluginSrc/NDFileHDF5Dataset.cpp
Line 299 in 6c53844
|
|
||
| NDArrayInfo_t info; | ||
| input->getInfo(&info); | ||
| int outputSize = LZ4_compressBound((int)info.totalBytes); |
There was a problem hiding this comment.
Should we add our own compress_bound_lz4hdf5 function in ADSupport? The checking performed by compress_lz4hdf5 is reasonably complex and I don't think it should be repeated across the code base.
As is, I believe this size would cause compress_lz4hdf5 below to error out.
| field(FRST, "BSLZ4") | ||
| field(FRST, "LZ4HDF5") | ||
| field(FRVL, "4") | ||
| field(FVST, "BSLZ4") | ||
| field(FVVL, "5") |
There was a problem hiding this comment.
I think this change in order would be nice to at least document in release notes, since I believe it could break someone's autosave.
There was a problem hiding this comment.
I have added this to RELEASE.md
…ressLZ4HDF5 amd decompressLZ4HDF5
…ressLZ4HDF5 amd decompressLZ4HDF5
No. The codecName for lz4hdf5 is NDCODEC_LZ4HDF5, not NDCODEC_LZ4, and there is no special treatment required for that codec in NDFileHDF5Dataset.cpp. I have tested this. Here is the output of h5dump --header -p on a file saving NDArrays compressed with lz4hdf5 with a block size of 65000. This was generated with ADSimDetector and NDPluginCodec. |
This adds support for the lz4hdf5 codec to NDPluginCodec and NDFileHDF5. The codec itself is in ADSupport. There are also changes to ADEiger to use this new codec for Stream2 LZ4 compression, and to ADViewers to display arrays with this codec in ImageJ.