cuslide2: add pyramidal OME-TIFF semantics and nvImageCodec 0.9 readiness by cdinea · Pull Request #1122 · rapidsai/cucim

cdinea · 2026-07-22T05:18:36Z

What this PR changes

Add decode correctness updates for multiplex OME-TIFF (dynamic GRAY/RGB and UINT8/UINT16 selection)
Add dedicated OME-XML parser module (ome_xml.h/.cpp)
Add multi-file OME fileset handling using TiffData companion-file references
Use OME channel names and physical spacing metadata where available
Relax RGB-only optimization guards in TIFF/IFD read paths
Add OME parser unit tests
Update nvImageCodec integration for 0.9.0 compatibility (version header + SONAME discovery order)

Scope notes

Explicit multiplex selector API expansion is intentionally out of scope for this PR.

Validation

Built cucim.kit.cuslide2 successfully in the prepared build environment
Added OME parser tests and integrated into cuslide2_tests

copy-pr-bot · 2026-07-22T05:18:39Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Update nvImageCodec dependency constraints to 0.9.x, add parser/decoder compatibility fallbacks for 0.9 runtime behavior, and relax legacy SubfileType assumptions so stacked OME-style TIFF pages load successfully in cuslide2 validation.

jakirkham · 2026-07-28T18:15:06Z

/ok to test 57aa615

gigony

Thanks @cdinea for adding ome-tiff support!
Overall, it looks good to me.

gigony · 2026-07-29T04:55:32Z

        exec_params.device_allocator = nullptr;
        exec_params.pinned_allocator = nullptr;
-        exec_params.max_num_cpu_threads = compute_max_decoder_threads();
+        exec_params.max_num_cpu_threads = 0;


I'm wondering why we're setting this to 0 now.

Yeah this would recover the overcontention issues which were fixed in the last release.

Good catch — unintentional, and Lev is right that it reintroduces the over-contention regression.

nvimgcodec.h documents max_num_cpu_threads = 0 as "default value equal to number of cpu cores", so this decoder was silently taking a full-core CPU threadpool. That was harmless when it was CPU-only and metadata-only, but it now backs the primary decode path (NVIMGCODEC_DEVICE_CURRENT, all backends), so with N dataloader workers you get N x cores threads.

The 0 is a leftover from the pre-promotion shape of this code; the rebase dropped compute_max_decoder_threads() along with it, which is the ~76-line shift in this hunk.

Fixed in 9bd44b8 — heuristic is back to min(max(1, hardware_concurrency / 4), 8), still overridable via CUCIM_MAX_DECODER_THREADS (>0 = exact count, 0 = explicit opt-in to the nvImageCodec default).

gigony · 2026-07-29T05:10:58Z

+        // Current limitation (nvImageCodec v0.6.0):
+        // - codec_name returns "tiff" (container format) not compression type
+        // - Individual TIFF tags not exposed through metadata API
+        // - Only vendor-specific metadata blobs available (MED_APERIO, MED_PHILIPS, etc.)
+        //


I am wondering if nvImageCodec v0.9 still has these limitation.

We do support generic tags in the API, here is python example: https://docs.nvidia.com/cuda/nvimagecodec/samples/metadata.html#Generic-tiff-tag-reading
iirc this was already supported in 0.8? Lev can know more details on this and how to querry the tags

This has been supported since 0.8, but only with a GPU decoder.

You're right, and thanks @mkepa-nv for the pointer — the comment was stale. Individual TIFF tags are queried unconditionally via NVIMGCODEC_METADATA_KIND_TIFF_TAG in extract_tiff_tags(), which runs on the line directly above, so the claim contradicted the adjacent code.

Removed in 9bd44b8. I kept only the part that's still true: codec_name reports the container format rather than the compression type, so compression is inferred from the tags below.

gigony · 2026-07-29T05:11:44Z

+    // WORKAROUND for nvImageCodec 0.6.0: Philips TIFF metadata limitation
+    // ========================================================================
+    // nvImageCodec 0.6.0 does NOT expose:
+    // 1. Individual TIFF tags (SOFTWARE, ImageDescription, etc.)
+    // 2. Philips format detection for some files
+    //
+


Is this still specific to nvImageCodec v0.9.0, or does the code also work with nvImageCodec v0.6.0?

That block was a stale v0.6.0 workaround note describing limitations that no longer apply — removed entirely in 9bd44b8. The surrounding code path is not v0.6.0-compatible anyway, since it depends on the 0.7.0+ TIFF tag metadata API.

gigony · 2026-07-29T05:23:05Z

@@ -1,5 +1,5 @@
 /*
- * SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION.
+ * SPDX-FileCopyrightText: Copyright (c) 2020-2021, NVIDIA CORPORATION.


Suggested change

* SPDX-FileCopyrightText: Copyright (c) 2020-2021, NVIDIA CORPORATION.

* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION.

Fixed. This one had actually gone backwards — the base was already 2020-2026 and the branch reverted it to 2021

mkepa-nv · 2026-07-29T10:50:02Z

 if (NOT TARGET deps::nvimgcodec)
+    set(NVIMGCODEC_SONAME_CANDIDATES
+        "libnvimgcodec.so.0.9.0"
+        "libnvimgcodec.so.0.8.0"


In the conda yaml file you set requirements >= 0.9 for nvimagecodec. Why do we need 0.8 here then?

You're right, that 0.8.0 line shouldn't be there — it's left over from when this branch still supported 0.8. Everything else already asks for 0.9: dependencies.yaml, the conda environment files, conda_build_config.yaml and pyproject.toml are all >=0.9.0,<0.10.0, and the vendored headers say NVIMGCODEC_VER_MINOR 9. Dropped it from this file and the cuslide2 copy.

Digging into it though, removing that line isn't quite enough. The list still ends with libnvimgcodec.so.0 and libnvimgcodec.so, and those symlinks point at whatever major-0 build happens to be installed — so on a machine with only 0.8 we'd still pick it up without noticing. Combined with the two things you spotted elsewhere (limit_images gone in 0.9, no compile-time version checks), that would blow up at runtime instead of failing the build, which is a much worse way to find out.

So I added a check at configure time: it reads NVIMGCODEC_VER_MAJOR/MINOR out of the nvimgcodec_version.h we discover, errors below 0.9, warns if it's outside the pinned range, and skips quietly when the header isn't there so wheel and system installs keep working. The find_package branch now looks at nvimgcodec_VERSION too — it wasn't checking anything at all before.

On consolidating the two nvimgcodec.cmake files: fair point, and the check I just added is now duplicated as well. I'd rather do that separately than grow this PR — the two files get picked up through different superbuild_depend() search paths (cuslide2 via SUPERBUILD_ADDITIONAL_DEPS_DIRS, the other via CMAKE_SUPERBUILD_DEPS_ROOT_DIR, which points at the cuslide plugin's cmake dir and has no nvimgcodec entry), so merging them takes a little care not to bbreak standalone plugin builds.

mkepa-nv · 2026-07-29T10:53:13Z


+    set(NVIMGCODEC_SONAME_CANDIDATES
+        "libnvimgcodec.so.0.9.0"
+        "libnvimgcodec.so.0.8.0"


Similar question as above. Also maybe the cmake structure can be simplified so nvimagecodec search is done in a single place? But this is a question for a future pull request

mkepa-nv · 2026-07-29T11:05:55Z

        nvimgcodecCodeStreamView_t view{};
        view.struct_type = NVIMGCODEC_STRUCTURE_TYPE_CODE_STREAM_VIEW;
-        view.struct_size = sizeof(nvimgcodecCodeStreamView_t);
+        view.struct_size = offsetof(nvimgcodecCodeStreamView_t, limit_images);


This doesn't look right. Why would we ask for offset of struct member instead of the whole struct size? And also limit_images was removed in 0.9 so I have no idea how is this working right now

mkepa-nv · 2026-07-29T11:08:25Z

-        uint32_t num_channels = ifd_info.num_channels > 0 ? ifd_info.num_channels : 3;
-        size_t row_stride = width * num_channels;
+        uint32_t num_channels = decode_spec.num_channels;
+        size_t row_stride = width * num_channels * decode_spec.bytes_per_sample;


This can overflow for large width, I would static cast widt to size_t if you store it in that type anyway

mkepa-nv · 2026-07-29T11:09:11Z

-            size_t row_stride = region.width * num_channels;
+            const DecodePixelSpec decode_spec = resolve_decode_pixel_spec(ifd_info);
+            uint32_t num_channels = decode_spec.num_channels;
+            size_t row_stride = region.width * num_channels * decode_spec.bytes_per_sample;


The same overflow can happen here

mkepa-nv · 2026-07-29T12:48:32Z

-                    ifd->height_ = height_l0 / downsample;
-                }
+                // Fix width and height of IFD
+                ifd->width_ = width_l0 / downsample;


what if downsample is 0?

mkepa-nv · 2026-07-29T12:49:30Z

+                        //
+
+                        // 34 is from `Attribute Name="PIM_DP_IMAGE_DATA"`
+                        char* data_ptr = const_cast<char*>(image_desc_cstr) + node_offset + 34;


What if this is outside of the image_desc_cstr?

mkepa-nv · 2026-07-29T12:55:29Z

+    auto add_owned_channel_name = [&resource, &channel_names](const std::string& name) {
+        void* raw = resource.allocate(name.size() + 1, alignof(char));
+        auto* buf = static_cast<char*>(raw);
+        memcpy(buf, name.c_str(), name.size() + 1);


I would just make the channel_names to store string instead of the string_view. That vector is the owner of the names, so this even sounds like a more correct approach.

mkepa-nv · 2026-07-29T12:59:56Z

+            gpu_time = time.time() - start
+            gpu_np = _to_numpy(gpu_region)
+            print(f"  GPU level {level}: {gpu_np.shape}, {gpu_np.dtype}, {gpu_time:.4f}s")
+            if gpu_np.shape != cpu_np.shape:


shape check is not enough. I would check the pixels too. And maybe you should check that each level have a correct dimensions (that they decrease with correct ratio for each level done) - otherwise you will not be sure that parsing is working as expected.

mkepa-nv · 2026-07-29T13:00:54Z

+        print(f"    ⚠️  GPU plane validation skipped/failed: {e}")
+
+
+def _validate_batch_decode(img, level_dims):


You added a bunch of code for higher resolution decode (with dtype uint 16) - I would add test for such case as well

max_num_cpu_threads = 0 requests one CPU thread per core from nvImageCodec's default executor. That was harmless when this decoder was CPU-only and used solely for metadata extraction, but it now backs the primary decode path (NVIMGCODEC_DEVICE_CURRENT, all backends enabled), so every worker process allocated a full-core threadpool and multi-worker runs regressed into the thread over-contention this heuristic was originally added to prevent. Restore compute_max_decoder_threads(), which bounds the pool to min(max(1, hardware_concurrency / 4), 8) and stays overridable via CUCIM_MAX_DECODER_THREADS. Also drop the stale nvImageCodec v0.6.0 limitation comments: individual TIFF tags are queried unconditionally through NVIMGCODEC_METADATA_KIND_TIFF_TAG by extract_tiff_tags(), so the claim that they are unavailable contradicted the adjacent code.

cdinea requested review from a team as code owners July 22, 2026 05:18

grlee77 assigned cdinea Jul 24, 2026

grlee77 added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Jul 24, 2026

grlee77 added this to cucim Jul 24, 2026

grlee77 added this to the v26.08.00 milestone Jul 24, 2026

grlee77 changed the base branch from main to release/26.08 July 24, 2026 20:07

grlee77 requested review from a team as code owners July 24, 2026 20:07

grlee77 requested a review from KyleFromNVIDIA July 24, 2026 20:07

KyleFromNVIDIA requested changes Jul 24, 2026

View reviewed changes

Comment thread VERSION Outdated

cuslide2: add OME-TIFF semantics and nvImageCodec 0.9 support

70e0e73

cdinea force-pushed the feature/cuslide2-ome-nvimgcodec09 branch from 854582c to 70e0e73 Compare July 27, 2026 16:51

cuslide2: harden nvimgcodec 0.9 OME parsing path

57aa615

Update nvImageCodec dependency constraints to 0.9.x, add parser/decoder compatibility fallbacks for 0.9 runtime behavior, and relax legacy SubfileType assumptions so stacked OME-style TIFF pages load successfully in cuslide2 validation.

KyleFromNVIDIA approved these changes Jul 28, 2026

View reviewed changes

gigony approved these changes Jul 29, 2026

View reviewed changes

mkepa-nv suggested changes Jul 29, 2026

View reviewed changes

cdinea added 2 commits July 29, 2026 12:26

fix(cuslide2): require nvImageCodec 0.9 in dependency discovery

0912636

	* SPDX-FileCopyrightText: Copyright (c) 2020-2021, NVIDIA CORPORATION.
	* SPDX-FileCopyrightText: Copyright (c) 2020-2026, NVIDIA CORPORATION.

		print(f" ⚠️ GPU plane validation skipped/failed: {e}")


		def _validate_batch_decode(img, level_dims):

Uh oh!

Conversation

cdinea commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR changes

Scope notes

Validation

Uh oh!

copy-pr-bot Bot commented Jul 22, 2026

Uh oh!

Uh oh!

jakirkham commented Jul 28, 2026

Uh oh!

gigony left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

cdinea commented Jul 22, 2026 •

edited

Loading