✨ RFC-HDFG-2026-001: String-based filter configuration API#6470
Open
brtnfld wants to merge 40 commits into
Open
✨ RFC-HDFG-2026-001: String-based filter configuration API#6470brtnfld wants to merge 40 commits into
brtnfld wants to merge 40 commits into
Conversation
Contributor
Review ChecklistThis PR touches the following areas. Each needs a sign-off
|
hyoklee
requested changes
Jun 19, 2026
hyoklee
previously approved these changes
Jun 20, 2026
Adds a human-readable key=value parameter string API for HDF5 filters, alongside the existing integer cd_values arrays. New C API: - H5Pappend_filter(plist, filter_id, flags, params) — appends a filter using either a key=value string or raw cd_values (H5Z_params_t) - H5Pget_filter_params_by_idx(plist, idx, buf, buf_size, content_len) — retrieves the parameter string for a filter by pipeline index - H5Zconfig_get_int/double/bool/str — typed accessors for use inside filter set_config callbacks - H5Z_filter_id_by_name(name) — look up a filter id by registered name - H5Zget_filter_info2(id, info) — extended filter info including v3 fields New H5Z_class3_t fields: name, description, set_config, get_config, and reserved blob-callback placeholders (write_blob/read_blob/close_blob). H5Z_pipeline gains dxpl_id, scaled[], and ndims arguments threaded through from all call sites so v3 filter callbacks have full context. All six built-in filters (deflate, shuffle, fletcher32, nbit, szip, scaleoffset) implement set_config/get_config callbacks. TOML subset parser: tomlc17 (MIT) vendored in src/tomlc17/ and compiled unconditionally into libhdf5. Hex-float literals are transparently rewritten to decimal before parsing. tomlc17 symbols are hidden via -fvisibility=hidden to prevent namespace collisions. On-disk format: no new pipeline version. Parameter strings are converted to cd_values by set_config at H5Pappend_filter time and stored using the existing v2 pipeline message. On read, get_config reconstructs the string. Full backward read compatibility is preserved. Fortran, C++, and Java bindings added. Tests in test/tfilter2.c (~2300 lines) and testpar/t_filters_parallel.c (par-01–par-04). h5dump displays filter parameter strings; h5repack accepts TOML-form UD= filter specs. Code-review fixes included: tomlc17 visibility, H5Pget_filter_params_by_idx arg validation and true-length two-pass contract, flags re-validation after set_config, H5Z_register3 runtime plugin validation, Java two-pass protocol and h5libraryError() consistency, CHANGELOG corrections. Fixes GitHub issue HDFGroup#6153
…ib absent Two bugs caused H5TEST-tfilter2 to fail on BSD and non-zlib CI builds: 1. H5Zconfig.c: The review changed strlen(params) > H5Z_CONFIG_STRING_MAX to >=, which incorrectly rejected strings of exactly H5Z_CONFIG_STRING_MAX bytes. The public contract (and test_config_string_max_boundary) accept strings up to and including H5Z_CONFIG_STRING_MAX characters. Reverted to >. 2. test/tfilter2.c: test_config_string_max_boundary called H5Pappend_filter with H5Z_FILTER_DEFLATE without first checking availability. On CI builds without zlib the filter lookup fails with "filter not found" before the length check runs. Added H5Zfilter_avail guard; the test is now SKIPPED on non-zlib builds.
-p was printing PARAMS_STRING and DESCRIPTION as flat siblings of
FILTERS{} rather than inside the entry of the filter they describe,
which is ambiguous (or outright misleading) once more than one filter
is applied to a dataset. Move them inside each filter's own block,
opening one for SHUFFLE/FLETCHER32/NBIT only when there's something to
attach.
Update the affected h5dump DDL fixtures and add DDLBNF220.dox
documenting the new <filter_extra> nesting, bumping the \ref DDLBNF200
cross-references to DDLBNF220.
Two test failures from the h5dump nesting fix (2604f5d): 1. tools/test/h5repack/expected/deflate_limit.h5repack_layout.h5.ddl and h5repack_layout.h5-plugin_test.ddl had PARAMS_STRING (and DESCRIPTION) as flat siblings of the filter block rather than nested inside it — the h5dump fixture files were updated in 2604f5d but these two h5repack expected files were missed. 2. java/src-jni/jni/h5zImp.c called CALL_CONSTRUCTOR with a 6-arg array and signature "(IILjava/lang/String;Ljava/lang/String;ZZ)V" but H5Z_class_info_t's constructor takes 7 args (adds has_blob_callbacks). GetMethodID failed at runtime with the wrong arity. Add args[6] = JNI_FALSE and update the descriptor to ZZZ)V.
- java/test/TestH5Z.java (FFM): fix wrong method names
- H5Pget_filter2 (private) -> H5Pget_filter (public), pass new int[1]
instead of null for filter_config
- H5Zconfig_get_int/double/bool/str -> H5Zconfig_get_param (overloaded)
- Update assertion messages to match corrected method names
- release_docs/CHANGELOG.md: replace em-dashes with hyphens per style
- tools/src/h5repack/h5repack_parse.c: restructure UD= legacy numeric
loop from for-loop to while-loop to avoid CodeQL "loop counter modified
in body" warning; add bounds guard after comma-skip u++
One bullet in the string-based filter configuration API entry was missed by the earlier em-dash cleanup.
hbool_t is deprecated for new public API functions; every other new accessor in this header already uses bool.
These three params were appended at the very end when added for v3 filter support, after the IN/OUT params (filter_mask, nbytes, buf_size, buf). Since they're IN-only, group them with the other IN params right after flags. Updates the declaration, definition, and all 15 call sites in H5Dchunk.c, H5Dmpio.c, H5HFcache.c, and H5HFhuge.c.
The check had been rewritten to branch on H5Zget_filter_info's own flags and then re-check those same flags against themselves, which validates nothing -- it can't catch flags disagreeing with reality. Restore branching on H5Z_SZIP->encoder_present (the actual state) and checking flags against that, and drop the redundant second DECODE_ENABLED check.
It was only ever set alongside display_dcpl in the same -p branch and never read anywhere except to copy it into ctx.show_filter_params, so it carried no information display_dcpl didn't already have. Source show_filter_params from display_dcpl directly. Also fixes the duplicate dump_opt_t definition in h5dump_extern.h to match, and its now-stale trailing initializer value in h5dump.h.
The trailing "2" follows HDF5's convention for versioned/deprecated symbols (H5Dopen2, H5Gcreate2, etc.), implying this is a successor to H5Zget_filter_info registered in H5vers.txt -- it isn't. The two functions are independent: H5Zget_filter_info returns only encode/decode config flags, H5Zget_filter_class_info additionally exposes name, description, and set_config/get_config presence. Renamed across C, Fortran, and both Java binding variants (JNI + FFM) since it hadn't shipped yet.
…macro family
A separate H5Z_CLASS3_T_VERS macro (=2) alongside H5Z_CLASS_T_VERS (=1)
looked like an independent version counter for H5Z_class3_t rather than
the same version field H5Z_class2_t uses. Remove the public macro;
H5Z_class3_t plugins now set the version field to the literal value 2,
matching the already-documented convention for H5Z_CLASS_T_VERS_MAX
("plugin authors should set the version field to the literal value...
rather than referencing this constant directly"). The library's own
internal dispatch in H5Z.c uses a new package-private constant
(H5Z_CLASS3_T_VERS_INTERNAL in H5Zpkg.h) pinned to 2, so it can't drift
if H5Z_CLASS_T_VERS_MAX is later bumped for a future struct generation.
Nothing in src/tools/hl/fortran/java/c++ calls these 8 functions -- only their own self-test in tfilter2.c did. The cd_values packing convention they implemented is a few lines of memcpy that the RFC already documents as directly implementable by filter authors, so a dedicated helper API wasn't carrying its own weight as a distinct library entry point.
The struct was forward-declared in H5Zprivate.h with its full definition in H5Zpkg.h, but every actual user (H5Z.c, H5Pocpl.c) already includes H5Zpkg.h directly, so the indirection didn't hide anything from anyone. H5Zprivate.h already pulls in H5Z_class2_t/H5Z_class3_t/H5Z_func2_t via H5Zpublic.h/H5Zdevelop.h, so nothing blocks defining it there directly.
…rammar doc DDLBNF*.dox files are versioned snapshots of the DDL grammar as of each release (110/112/114/200), not a single evolving document. This feature is 2.2.0-only, so it doesn't belong in the 2.0.0 snapshot -- and DDLBNF220.dox (added by an earlier commit on this branch) already documents it correctly there.
Comments should describe current behavior, not cite external documents by section number that will drift as the RFC is revised -- and should stick to ASCII. Sweeps beyond this session's own edits since these patterns were already present in this feature's earlier commits: em-dashes, arrows, curly quotes, ellipses, and multiplication signs replaced with ASCII equivalents; "RFC-HDFG-2026-001 SecN" citations replaced with plain descriptions of the behavior itself.
…quotes
PARAMS_STRING's content is TOML, whose string values are themselves
double-quoted (e.g. mode = "rate"), so wrapping the whole thing in double
quotes forced escaping the common case. Single quotes never collide with
TOML's own quoting: neither TOML string form can contain a literal
unescaped single quote, except a TOML double-quoted value may itself
contain one (e.g. "it's") -- so the escape path is kept but now only
fires on that rare case instead of on every filter with a string
parameter. No existing FILTERS{} field was a clean precedent either way
(filter COMMENT is unquoted; group COMMENT is quoted but doesn't escape),
so this isn't a consistency fix, just a cleaner-in-the-common-case format.
Updates all affected h5dump/h5repack .ddl reference files.
Same rationale as H5Zconfig_get_bool: hbool_t is deprecated for new public API, and every other new field/function in this header already uses bool.
tszip.ddl/tdeflate.ddl assume the filter is registered (PARAMS_STRING uses the get_config form). Add tszip_noavail.ddl/tdeflate_noavail.ddl covering the cd_values fallback form, and split the CMakeTests.cmake SZIP/deflate tests to run the matching variant based on filter availability. Also wrap the H5Zget_filter_class_info() call in h5tools_dump.c with H5E_BEGIN_TRY/H5E_END_TRY, since an unregistered filter is an expected case here and shouldn't pollute --enable-error-stack output.
write_blob/read_blob/close_blob and the corresponding H5Z_PARAMS_BLOB enum value and has_blob_callbacks info field were speculative reservations for a not-yet-designed feature. Pre-reserving generic void* slots doesn't avoid the ABI/version bump the feature will need once its callback signatures are actually decided, so there's no benefit to carrying them now. Drop them from the public struct, the internal filter table entry, all built-in filter registrations, the Java bindings (JNI and Panama/FFM), and the test plugin/test suite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cd_valuesarrays:H5Pappend_filter,H5Pget_filter_params_by_idx,H5Zconfig_get_int/double/bool/str,H5Z_filter_id_by_name,H5Zget_filter_info2.H5Z_class3_twithname,description,set_config/get_configcallbacks (plus reserved blob-callback placeholders), and threadsdxpl_id/scaled[]/ndimsthroughH5Z_pipelineso v3 filter callbacks have full context. All six built-in filters (deflate, shuffle, fletcher32, nbit, szip, scaleoffset) implementset_config/get_config.src/tomlc17/, built unconditionally into libhdf5 with hidden visibility to avoid symbol collisions.cd_valuesatH5Pappend_filtertime and stored in the existing v2 pipeline message, so read compatibility with older files/libraries is preserved.h5dump/h5repacksupport for the new parameter strings.Fixes #6153.