From d1aa36719f5349e4e51a692f25858afca0d1b141 Mon Sep 17 00:00:00 2001 From: Tom Birch Date: Sun, 2 Jul 2023 12:02:49 +1000 Subject: [PATCH 01/13] Better test for bytedelta --- plugins/filters/bytedelta/test_bytedelta.c | 210 +++++++++++++++------ 1 file changed, 153 insertions(+), 57 deletions(-) diff --git a/plugins/filters/bytedelta/test_bytedelta.c b/plugins/filters/bytedelta/test_bytedelta.c index a30ecede5..737ea5318 100644 --- a/plugins/filters/bytedelta/test_bytedelta.c +++ b/plugins/filters/bytedelta/test_bytedelta.c @@ -15,17 +15,29 @@ To run: $ ./test_bytedelta + Testing bytedelta with write_correct=1, read_correct=1 + Testing bytedelta with write_correct=0, read_correct=1 + Testing bytedelta with write_correct=1, read_correct=0 + Testing bytedelta with write_correct=0, read_correct=0 Successful roundtrip! - Compression: 41472 -> 4937 (8.4x) - rand: 36535 obtained + Compression: 41472 -> 5599 (7.4x) + rand: saved 35873 bytes + Testing bytedelta with write_correct=1, read_correct=1 + Testing bytedelta with write_correct=0, read_correct=1 + Testing bytedelta with write_correct=1, read_correct=0 + Testing bytedelta with write_correct=0, read_correct=0 Successful roundtrip! - Compression: 1792 -> 1005 (1.8x) - mixed_values: 787 obtained + Compression: 1792 -> 499 (3.6x) + mixed_values: saved 1293 bytes + Testing bytedelta with write_correct=1, read_correct=1 + Testing bytedelta with write_correct=0, read_correct=1 + Testing bytedelta with write_correct=1, read_correct=0 + Testing bytedelta with write_correct=0, read_correct=0 Successful roundtrip! - Compression: 16128 -> 1599 (10.1x) - arange_like: 14529 obtained + Compression: 16128 -> 1473 (10.9x) + arange_like: saved 14655 bytes **********************************************************************/ @@ -37,6 +49,64 @@ #include #include +/* The original implmentation of the bytedelta filter had incorrect + * roundtrip behavior between SIMD and non-SIMD binaries. This filter provides + * the correct behavior regardless of SIMD availability. + * See https://github.com/Blosc/c-blosc2/issues/524 */ +int correct_bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, blosc2_cparams *cparams, uint8_t id) +{ + int typesize = meta; + if (typesize == 0) { + if (cparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(cparams->schunk); + typesize = schunk->typesize; + } + int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + uint8_t _v2 = 0; + for (; ip < stream_len ; ip++) { + uint8_t v = *input; + input++; + *output = v - _v2; + output++; + _v2 = v; + } + } + return BLOSC2_ERROR_SUCCESS; +} + +int correct_bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, + blosc2_dparams *dparams, uint8_t id) { + int typesize = meta; + if (typesize == 0) { + if (dparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(dparams->schunk); + typesize = schunk->typesize; + } + + const int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + uint8_t _v2 = 0; + for (; ip < stream_len; ip++) { + uint8_t v = *input + _v2; + input++; + *output = v; + output++; + _v2 = v; + } + } + + return BLOSC2_ERROR_SUCCESS; +} + static int test_bytedelta(blosc2_schunk *schunk) { int64_t nchunks = schunk->nchunks; @@ -49,75 +119,101 @@ static int test_bytedelta(blosc2_schunk *schunk) { uint8_t *data_out = malloc(chunksize + BLOSC2_MAX_OVERHEAD); uint8_t *data_dest = malloc(chunksize); - /* Create a context for compression */ - blosc2_cparams cparams = BLOSC2_CPARAMS_DEFAULTS; - cparams.typesize = schunk->typesize; - cparams.compcode = BLOSC_LZ4; - cparams.filters[BLOSC2_MAX_FILTERS - 2] = BLOSC_SHUFFLE; - cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; - cparams.filters_meta[BLOSC2_MAX_FILTERS - 1] = 0; // 0 means typesize when using schunks - cparams.clevel = 9; - cparams.nthreads = 1; - cparams.blocksize = schunk->blocksize; - cparams.schunk = schunk; - blosc2_context *cctx; - cctx = blosc2_create_cctx(cparams); - if (cctx == NULL) { - printf("Cannot create compression context!\n"); + blosc2_filter correct_bytedelta = {.id = 250, .name = "bytedelta_correct", .version = 1, + .forward = correct_bytedelta_forward, .backward = correct_bytedelta_backward}; + if (blosc2_register_filter(&correct_bytedelta)) { + printf("Cannot register bytedelta filter!"); return -1; } - blosc2_dparams dparams = BLOSC2_DPARAMS_DEFAULTS; - dparams.nthreads = 1; - dparams.schunk = schunk; - blosc2_context *dctx; - dctx = blosc2_create_dctx(dparams); - if (cctx == NULL) { - printf("Cannot create decompression context!\n"); - return -1; - } - for (int ci = 0; ci < nchunks; ci++) { - - decompressed = blosc2_schunk_decompress_chunk(schunk, ci, data_in, chunksize); - if (decompressed < 0) { - printf("Error decompressing chunk \n"); + // Test each pair of possible forward/backward implementations: + for (int direction = 0; direction < 4; direction++) { + bool write_correct = (direction % 2 == 0); + bool read_correct = (direction / 2 == 0); + printf("Testing bytedelta with write_correct=%d, read_correct=%d\n", write_correct, read_correct); + + /* Create a context for compression */ + blosc2_cparams cparams = BLOSC2_CPARAMS_DEFAULTS; + cparams.typesize = schunk->typesize; + cparams.compcode = BLOSC_LZ4; + cparams.filters[BLOSC2_MAX_FILTERS - 2] = BLOSC_SHUFFLE; + if (write_correct) { + cparams.filters[BLOSC2_MAX_FILTERS - 1] = 250; + } else { + cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + } + cparams.filters_meta[BLOSC2_MAX_FILTERS - 1] = 0; // 0 means typesize when using schunks + cparams.clevel = 9; + cparams.nthreads = 1; + cparams.blocksize = schunk->blocksize; + cparams.schunk = schunk; + blosc2_context *cctx; + cctx = blosc2_create_cctx(cparams); + if (cctx == NULL) { + printf("Cannot create compression context!\n"); return -1; } - /* Compress with clevel=5 and shuffle active */ - csize = blosc2_compress_ctx(cctx, data_in, chunksize, data_out, chunksize + BLOSC2_MAX_OVERHEAD); - if (csize == 0) { - printf("Buffer is incompressible. Giving up.\n"); + blosc2_dparams dparams = BLOSC2_DPARAMS_DEFAULTS; + dparams.nthreads = 1; + dparams.schunk = schunk; + blosc2_context *dctx; + dctx = blosc2_create_dctx(dparams); + if (cctx == NULL) { + printf("Cannot create decompression context!\n"); return -1; - } else if (csize < 0) { - printf("Compression error. Error code: %" PRId64 "\n", csize); - return (int) csize; } - csize_f += csize; - /* Decompress */ - dsize = blosc2_decompress_ctx(dctx, data_out, chunksize + BLOSC2_MAX_OVERHEAD, data_dest, chunksize); - if (dsize <= 0) { - printf("Decompression error. Error code: %" PRId64 "\n", dsize); - return (int) dsize; - } + for (int ci = 0; ci < nchunks; ci++) { + + decompressed = blosc2_schunk_decompress_chunk(schunk, ci, data_in, chunksize); + if (decompressed < 0) { + printf("Error decompressing chunk \n"); + return -1; + } - for (int i = 0; i < chunksize; i++) { - if (data_in[i] != data_dest[i]) { - printf("i: %d, data %u, dest %u", i, data_in[i], data_dest[i]); - printf("\n Decompressed data differs from original!\n"); + /* Compress with clevel=5 and shuffle active */ + csize = blosc2_compress_ctx(cctx, data_in, chunksize, data_out, chunksize + BLOSC2_MAX_OVERHEAD); + if (csize == 0) { + printf("Buffer is incompressible. Giving up.\n"); return -1; + } else if (csize < 0) { + printf("Compression error. Error code: %" PRId64 "\n", csize); + return (int) csize; + } + csize_f += csize; + + // Force usage of alternative decoder + if (read_correct) { + data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = 250; + } else { + data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + } + + /* Decompress */ + dsize = blosc2_decompress_ctx(dctx, data_out, chunksize + BLOSC2_MAX_OVERHEAD, data_dest, chunksize); + if (dsize <= 0) { + printf("Decompression error. Error code: %" PRId64 "\n", dsize); + return (int) dsize; + } + + for (int i = 0; i < chunksize; i++) { + if (data_in[i] != data_dest[i]) { + printf("i: %d, data %u, dest %u", i, data_in[i], data_dest[i]); + printf("\n Decompressed data differs from original!\n"); + return -1; + } } } - } - csize_f = csize_f / nchunks; + csize_f = csize_f / nchunks; + blosc2_free_ctx(cctx); + blosc2_free_ctx(dctx); + } free(data_in); free(data_out); free(data_dest); - blosc2_free_ctx(cctx); - blosc2_free_ctx(dctx); printf("Successful roundtrip!\n"); printf("Compression: %d -> %" PRId64 " (%.1fx)\n", chunksize, csize_f, (1. * chunksize) / (double) csize_f); From 7614d284ed88d97e7fb039f4d754a02dca7d0b32 Mon Sep 17 00:00:00 2001 From: Tom Birch Date: Sat, 1 Jul 2023 19:27:32 +1000 Subject: [PATCH 02/13] Fix discrepancy between SIMD/Non-SIMD versions of bytedelta --- plugins/filters/bytedelta/bytedelta.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/plugins/filters/bytedelta/bytedelta.c b/plugins/filters/bytedelta/bytedelta.c index 70168b653..1afd3ace5 100644 --- a/plugins/filters/bytedelta/bytedelta.c +++ b/plugins/filters/bytedelta/bytedelta.c @@ -46,6 +46,8 @@ bytes16 simd_prefix_sum(bytes16 x) return x; } +uint8_t simd_get_last(bytes16 x) { return (_mm_extract_epi16(x, 7) >> 8) & 0xFF; } + #elif defined(__aarch64__) || defined(_M_ARM64) // ARM v8 NEON code path #define CPU_HAS_SIMD 1 @@ -72,6 +74,8 @@ bytes16 simd_prefix_sum(bytes16 x) return x; } +uint8_t simd_get_last(bytes16 x) { return vgetq_lane_u8(x, 15); } + #endif @@ -93,6 +97,7 @@ int bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uin const int stream_len = length / typesize; for (int ich = 0; ich < typesize; ++ich) { int ip = 0; + uint8_t _v2 = 0; // SIMD delta within each channel, store #if defined(CPU_HAS_SIMD) bytes16 v2 = {0}; @@ -104,9 +109,12 @@ int bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uin output += 16; v2 = v; } + if (stream_len > 15) { + _v2 = simd_get_last(v2); + + } #endif // #if defined(CPU_HAS_SIMD) // scalar leftover - uint8_t _v2 = 0; for (; ip < stream_len ; ip++) { uint8_t v = *input; input++; @@ -137,6 +145,7 @@ int bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, ui const int stream_len = length / typesize; for (int ich = 0; ich < typesize; ++ich) { int ip = 0; + uint8_t _v2 = 0; // SIMD fetch 16 bytes from each channel, prefix-sum un-delta #if defined(CPU_HAS_SIMD) bytes16 v2 = {0}; @@ -148,9 +157,11 @@ int bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, ui simd_store(output, v2); output += 16; } + if (stream_len > 15) { + _v2 = simd_get_last(v2); + } #endif // #if defined(CPU_HAS_SIMD) // scalar leftover - uint8_t _v2 = 0; for (; ip < stream_len; ip++) { uint8_t v = *input + _v2; input++; From bcc86104087e643e78cbcd55953fdc4ec0773a31 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Fri, 30 Jun 2023 14:57:36 -0700 Subject: [PATCH 03/13] CMake: Quote $ENV{CMAKE_OSX_ARCHITECTURES} For some reason, we see build issues in cibuildwheel with this when it is not quoted. Probably when defines as an empty string it throws: ``` ... -- Using ZSTD internal sources. "STREQUAL" "arm64" -- Configuring incomplete, errors occurred! Unknown arguments specified ``` --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index aa846f1e4..85d1e03c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -240,7 +240,7 @@ endif() # Propagate CMAKE_OSX_ARCHITECTURES env variable into CMAKE_SYSTEM_PROCESSOR if(DEFINED ENV{CMAKE_OSX_ARCHITECTURES}) - if($ENV{CMAKE_OSX_ARCHITECTURES} STREQUAL "arm64") + if("$ENV{CMAKE_OSX_ARCHITECTURES}" STREQUAL "arm64") set(CMAKE_SYSTEM_PROCESSOR arm64) endif() endif() From 4b4097723dea1fd3c77558cf0d8e13c3b9463ec6 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Fri, 30 Jun 2023 16:31:24 -0700 Subject: [PATCH 04/13] blosc2.h: Avoid C++20 Designated Init See #526. Designated initializers are in C99 but only in C++20, too. --- include/blosc2.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/blosc2.h b/include/blosc2.h index ecd78670b..13b600be8 100644 --- a/include/blosc2.h +++ b/include/blosc2.h @@ -1053,9 +1053,9 @@ typedef struct { } blosc2_io; static const blosc2_io BLOSC2_IO_DEFAULTS = { - .id = BLOSC2_IO_FILESYSTEM, - .name = "filesystem", - .params = NULL, + /* .id = */ BLOSC2_IO_FILESYSTEM, + /* .name = */ "filesystem", + /* .params = */ NULL, }; From b34c1d276d57d2eb68a55d766c9a1591194601e9 Mon Sep 17 00:00:00 2001 From: Axel Huebl Date: Fri, 30 Jun 2023 22:27:45 -0700 Subject: [PATCH 05/13] Fix: CMake External Zlib I see downstream linkage issues with externally built, static zlib library on Windows. This tries to fix it. Docs: - https://cmake.org/cmake/help/v3.27/module/FindZLIB.html Generally, we should use `ZLIB::ZLIB` as the target to link instead of decluttering these result variables. --- blosc/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blosc/CMakeLists.txt b/blosc/CMakeLists.txt index 9e390309c..a6d566db8 100644 --- a/blosc/CMakeLists.txt +++ b/blosc/CMakeLists.txt @@ -29,7 +29,7 @@ if(NOT DEACTIVATE_ZLIB) if(ZLIB_NG_FOUND) set(BLOSC_INCLUDE_DIRS ${BLOSC_INCLUDE_DIRS} ${ZLIB_NG_INCLUDE_DIR}) elseif(ZLIB_FOUND) - set(BLOSC_INCLUDE_DIRS ${BLOSC_INCLUDE_DIRS} ${ZLIB_INCLUDE_DIR}) + set(BLOSC_INCLUDE_DIRS ${BLOSC_INCLUDE_DIRS} ${ZLIB_INCLUDE_DIRS}) else() set(ZLIB_LOCAL_DIR ${INTERNAL_LIBS}/${ZLIB_NG_DIR}) set(BLOSC_INCLUDE_DIRS ${BLOSC_INCLUDE_DIRS} ${ZLIB_LOCAL_DIR}) @@ -112,7 +112,7 @@ if(NOT DEACTIVATE_ZLIB) if(ZLIB_NG_FOUND) set(LIBS ${LIBS} ${ZLIB_NG_LIBRARY}) elseif(ZLIB_FOUND) - set(LIBS ${LIBS} ${ZLIB_LIBRARY}) + set(LIBS ${LIBS} ${ZLIB_LIBRARIES}) else() set(ZLIB_LOCAL_DIR ${INTERNAL_LIBS}/${ZLIB_NG_DIR}) file(GLOB ZLIB_FILES ${ZLIB_LOCAL_DIR}/*.c) From 9dc3b01e48fa33b1206985a017b1aa9b6c6df730 Mon Sep 17 00:00:00 2001 From: Tom Birch Date: Sat, 1 Jul 2023 16:28:00 +1000 Subject: [PATCH 06/13] Test for filters --- tests/test_filters.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 tests/test_filters.c diff --git a/tests/test_filters.c b/tests/test_filters.c new file mode 100644 index 000000000..5c121e45d --- /dev/null +++ b/tests/test_filters.c @@ -0,0 +1,45 @@ +#include +#include +#include + +#define NCHUNKS 1 +#define TYPESIZE 2 +#define LEN 39 +#define CHUNKSIZE (TYPESIZE * LEN) + +int main(void) { + blosc2_init(); + srandom(0); + uint16_t *ref_data = (uint16_t *)malloc(CHUNKSIZE); + uint16_t *data_dest = (uint16_t *)malloc(CHUNKSIZE); + for (int i = 0; i < LEN; i++) { + ref_data[i] = random() % 118; + } + blosc2_cparams cparams = BLOSC2_CPARAMS_DEFAULTS; + cparams.compcode = BLOSC_ZSTD; + cparams.filters[BLOSC2_MAX_FILTERS - 2] = BLOSC_BITSHUFFLE; + cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_SHUFFLE; + + blosc2_dparams dparams = BLOSC2_DPARAMS_DEFAULTS; + + cparams.typesize = TYPESIZE; + blosc2_storage storage = {.contiguous=false, .urlpath=NULL, .cparams=&cparams, .dparams=&dparams}; + + blosc2_schunk* schunk = blosc2_schunk_new(&storage); + blosc2_schunk_append_buffer(schunk, ref_data, CHUNKSIZE); + + blosc2_schunk_decompress_chunk(schunk, 0, data_dest, CHUNKSIZE); + for (int i = 0; i < LEN; i++) { + if (data_dest[i] != ref_data[i]) { + printf("Decompressed data differs from original %d, %d, %d!\n", + i, ref_data[i], data_dest[i]); + return -1; + } + } + + printf("Successful roundtrip data <-> schunk !\n"); + + blosc2_schunk_free(schunk); + blosc2_destroy(); + return 0; +} From d62e35c2d26d62b0cfb7f4b7e7bf953ef7787aba Mon Sep 17 00:00:00 2001 From: Tom Birch Date: Sat, 1 Jul 2023 16:09:34 +1000 Subject: [PATCH 07/13] Correctly cycle buffers --- blosc/blosc2.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/blosc/blosc2.c b/blosc/blosc2.c index cf98bcfe0..f5876a30f 100644 --- a/blosc/blosc2.c +++ b/blosc/blosc2.c @@ -907,6 +907,12 @@ static int blosc2_intialize_header_from_context(blosc2_context* context, blosc_h return 0; } +void _cycle_buffers(void **src, void **dest, void **tmp) { + void *tmp2 = *src; + *src = *dest; + *dest = *tmp; + *tmp = tmp2; +} uint8_t* pipeline_forward(struct thread_context* thread_context, const int32_t bsize, const uint8_t* src, const int32_t offset, @@ -948,10 +954,7 @@ uint8_t* pipeline_forward(struct thread_context* thread_context, const int32_t b // No more filters are required return _dest; } - // Cycle buffers - _src = _dest; - _dest = _tmp; - _tmp = _src; + _cycle_buffers(&_src, &_dest, &_tmp); } /* Process the filter pipeline */ @@ -964,9 +967,7 @@ uint8_t* pipeline_forward(struct thread_context* thread_context, const int32_t b shuffle(typesize, bsize, _src, _dest); // Cycle filters when required if (j < filters_meta[i]) { - _src = _dest; - _dest = _tmp; - _tmp = _src; + _cycle_buffers(&_src, &_dest, &_tmp); } } break; @@ -1025,9 +1026,7 @@ uint8_t* pipeline_forward(struct thread_context* thread_context, const int32_t b // Cycle buffers when required if (filters[i] != BLOSC_NOFILTER) { - _src = _dest; - _dest = _tmp; - _tmp = _src; + _cycle_buffers(&_src, &_dest, &_tmp); } } return _src; @@ -1344,9 +1343,7 @@ int pipeline_backward(struct thread_context* thread_context, const int32_t bsize unshuffle(typesize, bsize, _src, _dest); // Cycle filters when required if (j < filters_meta[i]) { - _src = _dest; - _dest = _tmp; - _tmp = _src; + _cycle_buffers(&_src, &_dest, &_tmp); } // Check whether we have to copy the intermediate _dest buffer to final destination if (last_copy_filter && (filters_meta[i] % 2) == 1 && j == filters_meta[i]) { @@ -1424,9 +1421,7 @@ int pipeline_backward(struct thread_context* thread_context, const int32_t bsize // Cycle buffers when required if ((filters[i] != BLOSC_NOFILTER) && (filters[i] != BLOSC_TRUNC_PREC)) { - _src = _dest; - _dest = _tmp; - _tmp = _src; + _cycle_buffers(&_src, &_dest, &_tmp); } if (last_filter_index == i) { break; From 49b5dbd94f8fead435da32543259e6e4646aa02a Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Mon, 3 Jul 2023 10:24:46 +0200 Subject: [PATCH 08/13] Fix some warnings --- blosc/blosc2.c | 4 ++-- tests/b2nd/test_b2nd_open_offset.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/blosc/blosc2.c b/blosc/blosc2.c index f5876a30f..f4ab97a5a 100644 --- a/blosc/blosc2.c +++ b/blosc/blosc2.c @@ -907,8 +907,8 @@ static int blosc2_intialize_header_from_context(blosc2_context* context, blosc_h return 0; } -void _cycle_buffers(void **src, void **dest, void **tmp) { - void *tmp2 = *src; +void _cycle_buffers(uint8_t **src, uint8_t **dest, uint8_t **tmp) { + uint8_t *tmp2 = *src; *src = *dest; *dest = *tmp; *tmp = tmp2; diff --git a/tests/b2nd/test_b2nd_open_offset.c b/tests/b2nd/test_b2nd_open_offset.c index 687f2ed7d..ae58fa6a4 100644 --- a/tests/b2nd/test_b2nd_open_offset.c +++ b/tests/b2nd/test_b2nd_open_offset.c @@ -130,7 +130,7 @@ CUTEST_TEST_TEST(open_offset) { blosc_set_timestamp(¤t); ttotal = blosc_elapsed_secs(last, current); printf("Time for fileframe (%s) + offset %ld -> open_offset : %.3g s, %.1f GB/s\n", - arr_read_offset->sc->storage->urlpath, offset, ttotal, (double)arr_read_offset->sc->nbytes / (ttotal * GB)); + arr_read_offset->sc->storage->urlpath, (long)offset, ttotal, (double)arr_read_offset->sc->nbytes / (ttotal * GB)); uint8_t* cframe_read_start, *cframe_read_offset; bool cframe_needs_free2, cframe_needs_free3; From 88c46fd9019f4125b53b70ef1475605a6cde8302 Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Mon, 3 Jul 2023 11:08:58 +0200 Subject: [PATCH 09/13] Fix some issues on win and scenarios where zstd is not there --- tests/test_filters.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_filters.c b/tests/test_filters.c index 5c121e45d..f40277cc1 100644 --- a/tests/test_filters.c +++ b/tests/test_filters.c @@ -10,6 +10,12 @@ int main(void) { blosc2_init(); srandom(0); + char *name = NULL; + char *version = NULL; + if (blosc2_get_complib_info("zstd", &name, &version) < 0) { + // We need ZSTD for the test here... + return 0; + } uint16_t *ref_data = (uint16_t *)malloc(CHUNKSIZE); uint16_t *data_dest = (uint16_t *)malloc(CHUNKSIZE); for (int i = 0; i < LEN; i++) { From 1a26298a132bedc9144b36b88e9f80fca0e316c7 Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Mon, 3 Jul 2023 12:11:15 +0200 Subject: [PATCH 10/13] Use srand and rand for Win compat --- tests/test_filters.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_filters.c b/tests/test_filters.c index f40277cc1..728fe9a4e 100644 --- a/tests/test_filters.c +++ b/tests/test_filters.c @@ -9,17 +9,23 @@ int main(void) { blosc2_init(); - srandom(0); - char *name = NULL; + srand(0); + char *libname = NULL; char *version = NULL; - if (blosc2_get_complib_info("zstd", &name, &version) < 0) { + int ret = blosc2_get_complib_info("zstd", &libname, &version); + if (libname != NULL) { + free(libname); + free(version); + } + if (ret < 0) { // We need ZSTD for the test here... return 0; } + uint16_t *ref_data = (uint16_t *)malloc(CHUNKSIZE); uint16_t *data_dest = (uint16_t *)malloc(CHUNKSIZE); for (int i = 0; i < LEN; i++) { - ref_data[i] = random() % 118; + ref_data[i] = rand() % 118; } blosc2_cparams cparams = BLOSC2_CPARAMS_DEFAULTS; cparams.compcode = BLOSC_ZSTD; From f14aa157ed376b024ff32486aaa7b32664df4005 Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Mon, 3 Jul 2023 12:28:01 +0200 Subject: [PATCH 11/13] Better check for zstd presence --- tests/test_filters.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/test_filters.c b/tests/test_filters.c index 728fe9a4e..c65681bcf 100644 --- a/tests/test_filters.c +++ b/tests/test_filters.c @@ -10,14 +10,7 @@ int main(void) { blosc2_init(); srand(0); - char *libname = NULL; - char *version = NULL; - int ret = blosc2_get_complib_info("zstd", &libname, &version); - if (libname != NULL) { - free(libname); - free(version); - } - if (ret < 0) { + if (blosc2_compname_to_compcode("zstd") < 0) { // We need ZSTD for the test here... return 0; } From e5ff89ecc55caa00e4b2a8f6b459850031493ad9 Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Mon, 3 Jul 2023 19:13:09 +0200 Subject: [PATCH 12/13] Use another filter ID for fixed bytedelta --- include/blosc2.h | 2 +- include/blosc2/filters-registry.h | 3 +- plugins/filters/bytedelta/bytedelta.c | 94 +++++++++++++++++++++- plugins/filters/bytedelta/bytedelta.h | 10 ++- plugins/filters/bytedelta/test_bytedelta.c | 79 ++---------------- plugins/filters/filters-registry.c | 14 +++- 6 files changed, 123 insertions(+), 79 deletions(-) diff --git a/include/blosc2.h b/include/blosc2.h index 13b600be8..128755955 100644 --- a/include/blosc2.h +++ b/include/blosc2.h @@ -219,7 +219,7 @@ enum { BLOSC2_GLOBAL_REGISTERED_FILTERS_START = 32, BLOSC2_GLOBAL_REGISTERED_FILTERS_STOP = 159, //!< Blosc-registered filters must be between 32 - 159. - BLOSC2_GLOBAL_REGISTERED_FILTERS = 3, + BLOSC2_GLOBAL_REGISTERED_FILTERS = 4, //!< Number of Blosc-registered filters at the moment. BLOSC2_USER_REGISTERED_FILTERS_START = 160, BLOSC2_USER_REGISTERED_FILTERS_STOP = 255, diff --git a/include/blosc2/filters-registry.h b/include/blosc2/filters-registry.h index 49a98970c..16effbf8d 100644 --- a/include/blosc2/filters-registry.h +++ b/include/blosc2/filters-registry.h @@ -18,7 +18,8 @@ extern "C" { enum { BLOSC_FILTER_NDCELL = 32, BLOSC_FILTER_NDMEAN = 33, - BLOSC_FILTER_BYTEDELTA = 34, + BLOSC_FILTER_BYTEDELTA_V1 = 34, // buggy version. See #524 + BLOSC_FILTER_BYTEDELTA = 35, // fixed version }; void register_filters(void); diff --git a/plugins/filters/bytedelta/bytedelta.c b/plugins/filters/bytedelta/bytedelta.c index 1afd3ace5..db2631aaf 100644 --- a/plugins/filters/bytedelta/bytedelta.c +++ b/plugins/filters/bytedelta/bytedelta.c @@ -111,7 +111,6 @@ int bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uin } if (stream_len > 15) { _v2 = simd_get_last(v2); - } #endif // #if defined(CPU_HAS_SIMD) // scalar leftover @@ -129,7 +128,7 @@ int bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uin // Fetch 16b from N streams, sum SIMD undelta int bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, - blosc2_dparams *dparams, uint8_t id) { + blosc2_dparams *dparams, uint8_t id) { BLOSC_UNUSED_PARAM(id); int typesize = meta; @@ -173,3 +172,94 @@ int bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, ui return BLOSC2_ERROR_SUCCESS; } + +// This is the original (and buggy) version of bytedelta. It is kept here for backwards compatibility. +// See #524 for details. +int bytedelta_forward_v1(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, + blosc2_cparams *cparams, uint8_t id) { + BLOSC_UNUSED_PARAM(id); + + int typesize = meta; + if (typesize == 0) { + if (cparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(cparams->schunk); + typesize = schunk->typesize; + } + + const int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + // SIMD delta within each channel, store +#if defined(CPU_HAS_SIMD) + bytes16 v2 = {0}; + for (; ip < stream_len - 15; ip += 16) { + bytes16 v = simd_load(input); + input += 16; + bytes16 delta = simd_sub(v, simd_concat(v, v2)); + simd_store(output, delta); + output += 16; + v2 = v; + } +#endif // #if defined(CPU_HAS_SIMD) + // scalar leftover + uint8_t _v2 = 0; + for (; ip < stream_len ; ip++) { + uint8_t v = *input; + input++; + *output = v - _v2; + output++; + _v2 = v; + } + } + + return BLOSC2_ERROR_SUCCESS; +} + + +// This is the original (and buggy) version of bytedelta. It is kept here for backwards compatibility. +// See #524 for details. +int bytedelta_backward_v1(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, + blosc2_dparams *dparams, uint8_t id) { + BLOSC_UNUSED_PARAM(id); + + int typesize = meta; + if (typesize == 0) { + if (dparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(dparams->schunk); + typesize = schunk->typesize; + } + + const int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + // SIMD fetch 16 bytes from each channel, prefix-sum un-delta +#if defined(CPU_HAS_SIMD) + bytes16 v2 = {0}; + for (; ip < stream_len - 15; ip += 16) { + bytes16 v = simd_load(input); + input += 16; + // un-delta via prefix sum + v2 = simd_add(simd_prefix_sum(v), simd_duplane15(v2)); + simd_store(output, v2); + output += 16; + } +#endif // #if defined(CPU_HAS_SIMD) + // scalar leftover + uint8_t _v2 = 0; + for (; ip < stream_len; ip++) { + uint8_t v = *input + _v2; + input++; + *output = v; + output++; + _v2 = v; + } + } + + return BLOSC2_ERROR_SUCCESS; +} diff --git a/plugins/filters/bytedelta/bytedelta.h b/plugins/filters/bytedelta/bytedelta.h index 66e82203d..f4ecb921a 100644 --- a/plugins/filters/bytedelta/bytedelta.h +++ b/plugins/filters/bytedelta/bytedelta.h @@ -19,6 +19,12 @@ int bytedelta_forward(const uint8_t* input, uint8_t* output, int32_t length, uin blosc2_cparams* cparams, uint8_t id); int bytedelta_backward(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, - blosc2_dparams* dparams, uint8_t id); + blosc2_dparams* dparams, uint8_t id); -#endif /* BLOSC_PLUGINS_FILTERS_BYTEDELTA_BYTEDELTA_H*/ +int bytedelta_forward_v1(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, + blosc2_cparams* cparams, uint8_t id); + +int bytedelta_backward_v1(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, + blosc2_dparams* dparams, uint8_t id); + +#endif /* BLOSC_PLUGINS_FILTERS_BYTEDELTA_BYTEDELTA_H */ diff --git a/plugins/filters/bytedelta/test_bytedelta.c b/plugins/filters/bytedelta/test_bytedelta.c index 737ea5318..d39f29218 100644 --- a/plugins/filters/bytedelta/test_bytedelta.c +++ b/plugins/filters/bytedelta/test_bytedelta.c @@ -49,63 +49,6 @@ #include #include -/* The original implmentation of the bytedelta filter had incorrect - * roundtrip behavior between SIMD and non-SIMD binaries. This filter provides - * the correct behavior regardless of SIMD availability. - * See https://github.com/Blosc/c-blosc2/issues/524 */ -int correct_bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, blosc2_cparams *cparams, uint8_t id) -{ - int typesize = meta; - if (typesize == 0) { - if (cparams->schunk == NULL) { - BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); - BLOSC_ERROR(BLOSC2_ERROR_FAILURE); - } - blosc2_schunk* schunk = (blosc2_schunk*)(cparams->schunk); - typesize = schunk->typesize; - } - int stream_len = length / typesize; - for (int ich = 0; ich < typesize; ++ich) { - int ip = 0; - uint8_t _v2 = 0; - for (; ip < stream_len ; ip++) { - uint8_t v = *input; - input++; - *output = v - _v2; - output++; - _v2 = v; - } - } - return BLOSC2_ERROR_SUCCESS; -} - -int correct_bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, - blosc2_dparams *dparams, uint8_t id) { - int typesize = meta; - if (typesize == 0) { - if (dparams->schunk == NULL) { - BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); - BLOSC_ERROR(BLOSC2_ERROR_FAILURE); - } - blosc2_schunk* schunk = (blosc2_schunk*)(dparams->schunk); - typesize = schunk->typesize; - } - - const int stream_len = length / typesize; - for (int ich = 0; ich < typesize; ++ich) { - int ip = 0; - uint8_t _v2 = 0; - for (; ip < stream_len; ip++) { - uint8_t v = *input + _v2; - input++; - *output = v; - output++; - _v2 = v; - } - } - - return BLOSC2_ERROR_SUCCESS; -} static int test_bytedelta(blosc2_schunk *schunk) { @@ -119,14 +62,6 @@ static int test_bytedelta(blosc2_schunk *schunk) { uint8_t *data_out = malloc(chunksize + BLOSC2_MAX_OVERHEAD); uint8_t *data_dest = malloc(chunksize); - blosc2_filter correct_bytedelta = {.id = 250, .name = "bytedelta_correct", .version = 1, - .forward = correct_bytedelta_forward, .backward = correct_bytedelta_backward}; - if (blosc2_register_filter(&correct_bytedelta)) { - printf("Cannot register bytedelta filter!"); - return -1; - } - - // Test each pair of possible forward/backward implementations: for (int direction = 0; direction < 4; direction++) { bool write_correct = (direction % 2 == 0); @@ -139,9 +74,9 @@ static int test_bytedelta(blosc2_schunk *schunk) { cparams.compcode = BLOSC_LZ4; cparams.filters[BLOSC2_MAX_FILTERS - 2] = BLOSC_SHUFFLE; if (write_correct) { - cparams.filters[BLOSC2_MAX_FILTERS - 1] = 250; - } else { cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + } else { + cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA_V1; } cparams.filters_meta[BLOSC2_MAX_FILTERS - 1] = 0; // 0 means typesize when using schunks cparams.clevel = 9; @@ -174,7 +109,8 @@ static int test_bytedelta(blosc2_schunk *schunk) { } /* Compress with clevel=5 and shuffle active */ - csize = blosc2_compress_ctx(cctx, data_in, chunksize, data_out, chunksize + BLOSC2_MAX_OVERHEAD); + csize = blosc2_compress_ctx(cctx, data_in, chunksize, data_out, + chunksize + BLOSC2_MAX_OVERHEAD); if (csize == 0) { printf("Buffer is incompressible. Giving up.\n"); return -1; @@ -186,13 +122,14 @@ static int test_bytedelta(blosc2_schunk *schunk) { // Force usage of alternative decoder if (read_correct) { - data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = 250; - } else { data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + } else { + data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA_V1; } /* Decompress */ - dsize = blosc2_decompress_ctx(dctx, data_out, chunksize + BLOSC2_MAX_OVERHEAD, data_dest, chunksize); + dsize = blosc2_decompress_ctx(dctx, data_out, chunksize + BLOSC2_MAX_OVERHEAD, + data_dest, chunksize); if (dsize <= 0) { printf("Decompression error. Error code: %" PRId64 "\n", dsize); return (int) dsize; diff --git a/plugins/filters/filters-registry.c b/plugins/filters/filters-registry.c index 58ee65dc2..89c08d0b6 100644 --- a/plugins/filters/filters-registry.c +++ b/plugins/filters/filters-registry.c @@ -1,5 +1,5 @@ /* - Copyright (c) 2021 The Blosc Development Team + Copyright (c) 2021 The Blosc Development Team https://blosc.org License: BSD 3-Clause (see LICENSE.txt) */ @@ -29,11 +29,21 @@ void register_filters(void) { ndmean.backward = &ndmean_backward; register_filter_private(&ndmean); + // Buggy version. See #524 + blosc2_filter bytedelta_v1; + bytedelta_v1.id = BLOSC_FILTER_BYTEDELTA_V1; + bytedelta_v1.name = "bytedelta_v1"; + bytedelta_v1.version = 1; + bytedelta_v1.forward = &bytedelta_forward_v1; + bytedelta_v1.backward = &bytedelta_backward_v1; + register_filter_private(&bytedelta_v1); + + // Fixed version. See #524 blosc2_filter bytedelta; bytedelta.id = BLOSC_FILTER_BYTEDELTA; bytedelta.name = "bytedelta"; + bytedelta.version = 1; bytedelta.forward = &bytedelta_forward; bytedelta.backward = &bytedelta_backward; register_filter_private(&bytedelta); - } From 4966a176a81cd7480a82fc5d6acfd1b15d95e7dc Mon Sep 17 00:00:00 2001 From: Francesc Alted Date: Tue, 4 Jul 2023 07:04:00 +0200 Subject: [PATCH 13/13] Fix test and buggy filter renamed to bytedelta_buggy --- include/blosc2/filters-registry.h | 2 +- plugins/filters/bytedelta/bytedelta.c | 13 ++-- plugins/filters/bytedelta/bytedelta.h | 8 +- plugins/filters/bytedelta/test_bytedelta.c | 88 +++++++++++++++++++--- plugins/filters/filters-registry.c | 14 ++-- 5 files changed, 96 insertions(+), 29 deletions(-) diff --git a/include/blosc2/filters-registry.h b/include/blosc2/filters-registry.h index 16effbf8d..b39e2aa7e 100644 --- a/include/blosc2/filters-registry.h +++ b/include/blosc2/filters-registry.h @@ -18,7 +18,7 @@ extern "C" { enum { BLOSC_FILTER_NDCELL = 32, BLOSC_FILTER_NDMEAN = 33, - BLOSC_FILTER_BYTEDELTA_V1 = 34, // buggy version. See #524 + BLOSC_FILTER_BYTEDELTA_BUGGY = 34, // buggy version. See #524 BLOSC_FILTER_BYTEDELTA = 35, // fixed version }; diff --git a/plugins/filters/bytedelta/bytedelta.c b/plugins/filters/bytedelta/bytedelta.c index db2631aaf..32e81f7f3 100644 --- a/plugins/filters/bytedelta/bytedelta.c +++ b/plugins/filters/bytedelta/bytedelta.c @@ -175,8 +175,9 @@ int bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, ui // This is the original (and buggy) version of bytedelta. It is kept here for backwards compatibility. // See #524 for details. -int bytedelta_forward_v1(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, - blosc2_cparams *cparams, uint8_t id) { +// Fetch 16b from N streams, compute SIMD delta +int bytedelta_forward_buggy(const uint8_t *input, uint8_t *output, int32_t length, + uint8_t meta, blosc2_cparams *cparams, uint8_t id) { BLOSC_UNUSED_PARAM(id); int typesize = meta; @@ -218,11 +219,9 @@ int bytedelta_forward_v1(const uint8_t *input, uint8_t *output, int32_t length, return BLOSC2_ERROR_SUCCESS; } - -// This is the original (and buggy) version of bytedelta. It is kept here for backwards compatibility. -// See #524 for details. -int bytedelta_backward_v1(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, - blosc2_dparams *dparams, uint8_t id) { +// Fetch 16b from N streams, sum SIMD undelta +int bytedelta_backward_buggy(const uint8_t *input, uint8_t *output, int32_t length, + uint8_t meta, blosc2_dparams *dparams, uint8_t id) { BLOSC_UNUSED_PARAM(id); int typesize = meta; diff --git a/plugins/filters/bytedelta/bytedelta.h b/plugins/filters/bytedelta/bytedelta.h index f4ecb921a..48c1f3936 100644 --- a/plugins/filters/bytedelta/bytedelta.h +++ b/plugins/filters/bytedelta/bytedelta.h @@ -21,10 +21,10 @@ int bytedelta_forward(const uint8_t* input, uint8_t* output, int32_t length, uin int bytedelta_backward(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, blosc2_dparams* dparams, uint8_t id); -int bytedelta_forward_v1(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, - blosc2_cparams* cparams, uint8_t id); +int bytedelta_forward_buggy(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, + blosc2_cparams* cparams, uint8_t id); -int bytedelta_backward_v1(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, - blosc2_dparams* dparams, uint8_t id); +int bytedelta_backward_buggy(const uint8_t* input, uint8_t* output, int32_t length, uint8_t meta, + blosc2_dparams* dparams, uint8_t id); #endif /* BLOSC_PLUGINS_FILTERS_BYTEDELTA_BYTEDELTA_H */ diff --git a/plugins/filters/bytedelta/test_bytedelta.c b/plugins/filters/bytedelta/test_bytedelta.c index d39f29218..e7142fa35 100644 --- a/plugins/filters/bytedelta/test_bytedelta.c +++ b/plugins/filters/bytedelta/test_bytedelta.c @@ -49,6 +49,66 @@ #include #include +/* The original implmentation of the bytedelta filter had incorrect + * roundtrip behavior between SIMD and non-SIMD binaries. This filter provides + * the correct behavior regardless of SIMD availability. + * See https://github.com/Blosc/c-blosc2/issues/524 */ +int correct_bytedelta_forward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, + blosc2_cparams *cparams, uint8_t id) +{ + BLOSC_UNUSED_PARAM(id); + int typesize = meta; + if (typesize == 0) { + if (cparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(cparams->schunk); + typesize = schunk->typesize; + } + int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + uint8_t _v2 = 0; + for (; ip < stream_len ; ip++) { + uint8_t v = *input; + input++; + *output = v - _v2; + output++; + _v2 = v; + } + } + return BLOSC2_ERROR_SUCCESS; +} + +int correct_bytedelta_backward(const uint8_t *input, uint8_t *output, int32_t length, uint8_t meta, + blosc2_dparams *dparams, uint8_t id) { + BLOSC_UNUSED_PARAM(id); + int typesize = meta; + if (typesize == 0) { + if (dparams->schunk == NULL) { + BLOSC_TRACE_ERROR("When meta is 0, you need to be on a schunk!"); + BLOSC_ERROR(BLOSC2_ERROR_FAILURE); + } + blosc2_schunk* schunk = (blosc2_schunk*)(dparams->schunk); + typesize = schunk->typesize; + } + + const int stream_len = length / typesize; + for (int ich = 0; ich < typesize; ++ich) { + int ip = 0; + uint8_t _v2 = 0; + for (; ip < stream_len; ip++) { + uint8_t v = *input + _v2; + input++; + *output = v; + output++; + _v2 = v; + } + } + + return BLOSC2_ERROR_SUCCESS; +} static int test_bytedelta(blosc2_schunk *schunk) { @@ -62,6 +122,14 @@ static int test_bytedelta(blosc2_schunk *schunk) { uint8_t *data_out = malloc(chunksize + BLOSC2_MAX_OVERHEAD); uint8_t *data_dest = malloc(chunksize); + blosc2_filter correct_bytedelta = {.id = 250, .name = "bytedelta_correct", .version = 1, + .forward = correct_bytedelta_forward, .backward = correct_bytedelta_backward}; + if (blosc2_register_filter(&correct_bytedelta)) { + printf("Cannot register bytedelta filter!"); + return -1; + } + + // Test each pair of possible forward/backward implementations: for (int direction = 0; direction < 4; direction++) { bool write_correct = (direction % 2 == 0); @@ -74,9 +142,9 @@ static int test_bytedelta(blosc2_schunk *schunk) { cparams.compcode = BLOSC_LZ4; cparams.filters[BLOSC2_MAX_FILTERS - 2] = BLOSC_SHUFFLE; if (write_correct) { - cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + cparams.filters[BLOSC2_MAX_FILTERS - 1] = 250; } else { - cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA_V1; + cparams.filters[BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; } cparams.filters_meta[BLOSC2_MAX_FILTERS - 1] = 0; // 0 means typesize when using schunks cparams.clevel = 9; @@ -122,9 +190,9 @@ static int test_bytedelta(blosc2_schunk *schunk) { // Force usage of alternative decoder if (read_correct) { - data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; + data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = 250; } else { - data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA_V1; + data_out[BLOSC2_CHUNK_FILTER_CODES + BLOSC2_MAX_FILTERS - 1] = BLOSC_FILTER_BYTEDELTA; } /* Decompress */ @@ -179,8 +247,8 @@ int rand_() { blosc2_storage b2_storage = {.cparams=&cparams}; b2_storage.contiguous = true; - b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, NULL, 0, - NULL, 0); + b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, NULL, + 0,NULL, 0); b2nd_array_t *arr; BLOSC_ERROR(b2nd_from_cbuffer(ctx, &arr, data, size)); @@ -216,8 +284,8 @@ int mixed_values() { blosc2_storage b2_storage = {.cparams=&cparams}; b2_storage.contiguous = true; - b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, NULL, 0, - NULL, 0); + b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, + NULL, 0, NULL, 0); b2nd_array_t *arr; BLOSC_ERROR(b2nd_from_cbuffer(ctx, &arr, data, size)); @@ -253,8 +321,8 @@ int arange_like() { blosc2_storage b2_storage = {.cparams=&cparams}; b2_storage.contiguous = true; - b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, NULL, 0, - NULL, 0); + b2nd_context_t *ctx = b2nd_create_ctx(&b2_storage, ndim, shape, chunkshape, blockshape, + NULL, 0,NULL, 0); b2nd_array_t *arr; BLOSC_ERROR(b2nd_from_cbuffer(ctx, &arr, data, size)); diff --git a/plugins/filters/filters-registry.c b/plugins/filters/filters-registry.c index 89c08d0b6..04b69ac93 100644 --- a/plugins/filters/filters-registry.c +++ b/plugins/filters/filters-registry.c @@ -30,13 +30,13 @@ void register_filters(void) { register_filter_private(&ndmean); // Buggy version. See #524 - blosc2_filter bytedelta_v1; - bytedelta_v1.id = BLOSC_FILTER_BYTEDELTA_V1; - bytedelta_v1.name = "bytedelta_v1"; - bytedelta_v1.version = 1; - bytedelta_v1.forward = &bytedelta_forward_v1; - bytedelta_v1.backward = &bytedelta_backward_v1; - register_filter_private(&bytedelta_v1); + blosc2_filter bytedelta_buggy; + bytedelta_buggy.id = BLOSC_FILTER_BYTEDELTA_BUGGY; + bytedelta_buggy.name = "bytedelta_buggy"; + bytedelta_buggy.version = 1; + bytedelta_buggy.forward = &bytedelta_forward_buggy; + bytedelta_buggy.backward = &bytedelta_backward_buggy; + register_filter_private(&bytedelta_buggy); // Fixed version. See #524 blosc2_filter bytedelta;